fix: Deleting Stage with invalid cluster configuration (#10)

Change-Id: Ic2d0fe7c09510b2e92f702fdbdf39493e06eb29f

.golangci.yaml

@@ -114,6 +114,7 @@ issues:
     # we need refactoring to enable this linter
     - linters:
         - cyclop
+        - funlen
       text: "Reconcile"
 
   # Independently of option `exclude` we use default exclude patterns,

controllers/cdpipeline/cdpipeline_controller.go

@@ -6,7 +6,6 @@ import (
 	"reflect"
 	"time"
 
-	"github.com/go-logr/logr"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -22,18 +21,16 @@ import (
 	"github.com/epam/edp-cd-pipeline-operator/v2/pkg/util/consts"
 )
 
-func NewReconcileCDPipeline(c client.Client, scheme *runtime.Scheme, log logr.Logger) *ReconcileCDPipeline {
+func NewReconcileCDPipeline(c client.Client, scheme *runtime.Scheme) *ReconcileCDPipeline {
 	return &ReconcileCDPipeline{
 		client: c,
 		scheme: scheme,
-		log:    log.WithName("cd-pipeline"),
 	}
 }
 
 type ReconcileCDPipeline struct {
 	client client.Client
 	scheme *runtime.Scheme
-	log    logr.Logger
 }
 
 const (

controllers/cdpipeline/cdpipeline_controller_test.go

@@ -54,15 +54,13 @@ func createScheme(t *testing.T) *runtime.Scheme {
 func TestNewReconcileCDPipeline_Success(t *testing.T) {
 	scheme := runtime.NewScheme()
 	client := fake.NewClientBuilder().Build()
-	log := logr.Discard()
 
 	expectedReconcileCdPipeline := &ReconcileCDPipeline{
 		client: client,
 		scheme: scheme,
-		log:    log.WithName("cd-pipeline"),
 	}
 
-	reconciledCdPipeline := NewReconcileCDPipeline(client, scheme, log)
+	reconciledCdPipeline := NewReconcileCDPipeline(client, scheme)
 	assert.Equal(t, expectedReconcileCdPipeline, reconciledCdPipeline)
 }
 
@@ -71,7 +69,7 @@ func TestReconcile_Success(t *testing.T) {
 	scheme := createScheme(t)
 	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(emptyCdPipeline).Build()
 
-	reconcileCDPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard())
+	reconcileCDPipeline := NewReconcileCDPipeline(client, scheme)
 
 	_, err := reconcileCDPipeline.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{
 		Namespace: namespace,
@@ -95,7 +93,7 @@ func TestReconcile_PipelineIsNotFound(t *testing.T) {
 	scheme := createScheme(t)
 	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&cdPipeline).Build()
 
-	reconcileCDPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard())
+	reconcileCDPipeline := NewReconcileCDPipeline(client, scheme)
 
 	_, err := reconcileCDPipeline.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{
 		Namespace: namespace,
@@ -109,7 +107,7 @@ func TestReconcile_GetCdPipelineError(t *testing.T) {
 	scheme := runtime.NewScheme()
 	client := fake.NewClientBuilder().WithScheme(scheme).Build()
 
-	reconcileCDPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard())
+	reconcileCDPipeline := NewReconcileCDPipeline(client, scheme)
 
 	_, err := reconcileCDPipeline.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{
 		Namespace: namespace,
@@ -133,7 +131,7 @@ func TestAddFinalizer_DeletionTimestampNotZero(t *testing.T) {
 	scheme := createScheme(t)
 	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&cdPipeline).Build()
 
-	reconcileCdPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard())
+	reconcileCdPipeline := NewReconcileCDPipeline(client, scheme)
 
 	res, err := reconcileCdPipeline.tryToDeletePipeline(ctrl.LoggerInto(context.Background(), logr.Discard()), &cdPipeline)
 	assert.NoError(t, err)
@@ -172,7 +170,7 @@ func TestAddFinalizer_PostponeDeletion(t *testing.T) {
 	scheme := createScheme(t)
 	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&cdPipeline, stage).Build()
 
-	reconcileCdPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard())
+	reconcileCdPipeline := NewReconcileCDPipeline(client, scheme)
 
 	res, err := reconcileCdPipeline.tryToDeletePipeline(ctrl.LoggerInto(context.Background(), logr.Discard()), &cdPipeline)
 	assert.NoError(t, err)
@@ -192,7 +190,7 @@ func TestAddFinalizer_DeletionTimestampIsZero(t *testing.T) {
 	scheme := createScheme(t)
 	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cdPipeline).Build()
 
-	reconcileCdPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard())
+	reconcileCdPipeline := NewReconcileCDPipeline(client, scheme)
 
 	res, err := reconcileCdPipeline.tryToDeletePipeline(ctrl.LoggerInto(context.Background(), logr.Discard()), cdPipeline)
 	assert.NoError(t, err)
@@ -212,7 +210,7 @@ func TestSetFinishStatus_Success(t *testing.T) {
 	scheme := createScheme(t)
 	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cdPipeline).Build()
 
-	reconcileCdPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard())
+	reconcileCdPipeline := NewReconcileCDPipeline(client, scheme)
 
 	err := reconcileCdPipeline.setFinishStatus(context.Background(), cdPipeline)
 	assert.NoError(t, err)

controllers/stage/chain/chain.go

@@ -0,0 +1,38 @@
+package chain
+
+import (
+	"context"
+	"fmt"
+
+	ctrl "sigs.k8s.io/controller-runtime"
+
+	cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1"
+	"github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler"
+)
+
+type chain struct {
+	handlers []handler.CdStageHandler
+}
+
+func (ch *chain) Use(handlers ...handler.CdStageHandler) {
+	ch.handlers = append(ch.handlers, handlers...)
+}
+
+func (ch *chain) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error {
+	log := ctrl.LoggerFrom(ctx)
+
+	log.Info("Starting Stage chain")
+
+	for i := 0; i < len(ch.handlers); i++ {
+		h := ch.handlers[i]
+
+		err := h.ServeRequest(ctx, stage)
+		if err != nil {
+			return fmt.Errorf("failed to serve handler: %w", err)
+		}
+	}
+
+	log.Info("Handling of Stage has been finished")
+
+	return nil
+}

controllers/stage/chain/check_namespace_exist.go

@@ -4,45 +4,43 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/go-logr/logr"
 	projectApi "github.com/openshift/api/project/v1"
 	v1 "k8s.io/api/core/v1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
+	ctrl "sigs.k8s.io/controller-runtime"
 
 	cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1"
-	"github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler"
 	"github.com/epam/edp-cd-pipeline-operator/v2/pkg/platform"
 )
 
 // CheckNamespaceExist checks if namespace exists.
 type CheckNamespaceExist struct {
-	next   handler.CdStageHandler
 	client multiClusterClient
-	log    logr.Logger
 }
 
 // ServeRequest serves request to check if namespace/project exists.
-func (h CheckNamespaceExist) ServeRequest(stage *cdPipeApi.Stage) error {
+func (h CheckNamespaceExist) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error {
 	name := stage.Spec.Namespace
 
 	if platform.IsOpenshift() {
-		if err := h.projectExist(context.Background(), name); err != nil {
+		if err := h.projectExist(ctx, name); err != nil {
 			return err
 		}
 	}
 
 	if platform.IsKubernetes() {
-		if err := h.namespaceExist(context.Background(), name); err != nil {
+		if err := h.namespaceExist(ctx, name); err != nil {
 			return err
 		}
 	}
 
-	return nextServeOrNil(h.next, stage)
+	return nil
 }
 
 func (h CheckNamespaceExist) namespaceExist(ctx context.Context, name string) error {
-	h.log.Info("Checking existence of namespace", "name", name)
+	log := ctrl.LoggerFrom(ctx)
+	log.Info("Checking existence of namespace", "name", name)
 
 	if err := h.client.Get(ctx, types.NamespacedName{
 		Name: name,
@@ -58,7 +56,8 @@ func (h CheckNamespaceExist) namespaceExist(ctx context.Context, name string) er
 }
 
 func (h CheckNamespaceExist) projectExist(ctx context.Context, name string) error {
-	h.log.Info("Checking existence of project", "name", name)
+	log := ctrl.LoggerFrom(ctx)
+	log.Info("Checking existence of project", "name", name)
 
 	if err := h.client.Get(ctx, types.NamespacedName{
 		Name: name,

controllers/stage/chain/check_namespace_exist_test.go

@@ -1,6 +1,7 @@
 package chain
 
 import (
+	"context"
 	"testing"
 
 	"github.com/go-logr/logr"
@@ -10,6 +11,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
@@ -122,10 +124,9 @@ func TestCheckNamespaceExist_ServeRequest(t *testing.T) {
 
 			h := CheckNamespaceExist{
 				client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build(),
-				log:    logr.Discard(),
 			}
 
-			tt.wantErr(t, h.ServeRequest(tt.stage))
+			tt.wantErr(t, h.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage))
 		})
 	}
 }

controllers/stage/chain/configure_registryviewer_rbac.go

@@ -4,36 +4,33 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/go-logr/logr"
 	rbacApi "k8s.io/api/rbac/v1"
+	ctrl "sigs.k8s.io/controller-runtime"
 
 	cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1"
-	"github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler"
 	"github.com/epam/edp-cd-pipeline-operator/v2/pkg/platform"
 	"github.com/epam/edp-cd-pipeline-operator/v2/pkg/rbac"
 )
 
 type ConfigureRegistryViewerRbac struct {
-	next handler.CdStageHandler
-	log  logr.Logger
 	rbac rbac.Manager
 }
 
-func (h ConfigureRegistryViewerRbac) ServeRequest(stage *cdPipeApi.Stage) error {
+func (h ConfigureRegistryViewerRbac) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error {
 	targetNamespace := stage.Spec.Namespace
 	roleBindingName := generateSaRegistryViewerRoleBindingName(stage)
-	logger := h.log.WithValues("stage", stage.Name, "targetNamespace", targetNamespace, "roleBindingName", roleBindingName)
+	logger := ctrl.LoggerFrom(ctx).WithValues("targetNamespace", targetNamespace, "roleBindingName", roleBindingName)
 
 	logger.Info("Configuring RoleBinding sa-registry-viewer")
 
 	if !platform.IsOpenshift() {
 		logger.Info("Skip configuring RoleBinding sa-registry-viewer for non-openshift platform")
 
-		return nextServeOrNil(h.next, stage)
+		return nil
 	}
 
 	if err := h.rbac.CreateRoleBindingIfNotExists(
-		context.TODO(),
+		ctx,
 		roleBindingName,
 		stage.Namespace,
 		[]rbacApi.Subject{
@@ -54,7 +51,7 @@ func (h ConfigureRegistryViewerRbac) ServeRequest(stage *cdPipeApi.Stage) error
 
 	logger.Info("RoleBinding sa-registry-viewer has been configured")
 
-	return nextServeOrNil(h.next, stage)
+	return nil
 }
 
 // generateSaRegistryViewerRoleBindingName generates name for RoleBinding for registry-viewer role.

controllers/stage/chain/configure_registryviewer_rbac_test.go

@@ -10,6 +10,7 @@ import (
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
@@ -114,11 +115,10 @@ func TestConfigureRegistryViewerRbac_ServeRequest(t *testing.T) {
 			k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tt.objects...).Build()
 
 			h := ConfigureRegistryViewerRbac{
-				log:  logr.Discard(),
 				rbac: rbac.NewRbacManager(k8sClient, logr.Discard()),
 			}
 
-			err := h.ServeRequest(tt.stage)
+			err := h.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage)
 			tt.wantErr(t, err)
 			tt.wantCheck(t, tt.stage, k8sClient)
 		})

controllers/stage/chain/configure_secret_manager.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	rbacApi "k8s.io/api/rbac/v1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
@@ -15,7 +14,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1"
-	"github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler"
 	"github.com/epam/edp-cd-pipeline-operator/v2/pkg/externalsecrets"
 	"github.com/epam/edp-cd-pipeline-operator/v2/pkg/rbac"
 )
@@ -32,18 +30,14 @@ const (
 
 // ConfigureSecretManager is a stage chain element that configures secret management.
 type ConfigureSecretManager struct {
-	next               handler.CdStageHandler
 	multiClusterClient multiClusterClient
 	internalClient     client.Client
-	log                logr.Logger
 }
 
 // ServeRequest implements the logic to configure secret management.
-func (h ConfigureSecretManager) ServeRequest(stage *cdPipeApi.Stage) error {
-	ctx := context.Background() // TODO: pass ctx from the caller
+func (h ConfigureSecretManager) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error {
 	secretManager := os.Getenv(secretManagerEnv)
-	logger := h.log.WithValues(
-		"stage", stage.Name,
+	logger := ctrl.LoggerFrom(ctx).WithValues(
 		"target-ns", stage.Spec.Namespace,
 		"secret-manager", secretManager,
 	)
@@ -59,10 +53,10 @@ func (h ConfigureSecretManager) ServeRequest(stage *cdPipeApi.Stage) error {
 		}
 	default:
 		logger.Info("Secrets management is disabled, skipping")
-		return nextServeOrNil(h.next, stage)
+		return nil
 	}
 
-	return nextServeOrNil(h.next, stage)
+	return nil
 }
 
 func (h ConfigureSecretManager) configureEso(ctx context.Context, stage *cdPipeApi.Stage) error {

controllers/stage/chain/configure_secret_manager_test.go

@@ -11,6 +11,7 @@ import (
 	rbacApi "k8s.io/api/rbac/v1"
 	metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
@@ -304,10 +305,9 @@ func TestConfigureManageSecretsRBAC_ServeRequest(t *testing.T) {
 			h := ConfigureSecretManager{
 				multiClusterClient: cl,
 				internalClient:     cl,
-				log:                logr.Discard(),
 			}
 
-			tt.wantErr(t, h.ServeRequest(tt.stage))
+			tt.wantErr(t, h.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage))
 			tt.want(t, h.multiClusterClient, tt.stage)
 		})
 	}