From 6d912038256bcbf594b02b4d76c031c4d7d43c92 Mon Sep 17 00:00:00 2001 From: Aline Abler Date: Mon, 22 Jul 2024 17:34:23 +0200 Subject: [PATCH 1/5] Reconcile `enableCompilePipeline` in Cluster objects Add new fields to Tenant and Cluster spec --- api/v1alpha1/cluster_types.go | 2 + api/v1alpha1/tenant_types.go | 15 +- api/v1alpha1/zz_generated.deepcopy.go | 54 +++- config/crd/bases/syn.tools_clusters.yaml | 4 + config/crd/bases/syn.tools_tenants.yaml | 28 ++ .../crd/bases/syn.tools_tenanttemplates.yaml | 16 + .../cluster_compile_pipeline_controller.go | 78 +++++ ...luster_compile_pipeline_controller_test.go | 280 ++++++++++++++++++ controllers/cluster_controller.go | 1 + .../ROOT/pages/references/api-reference.adoc | 36 +++ main.go | 7 + 11 files changed, 519 insertions(+), 2 deletions(-) create mode 100644 controllers/cluster_compile_pipeline_controller.go create mode 100644 controllers/cluster_compile_pipeline_controller_test.go diff --git a/api/v1alpha1/cluster_types.go b/api/v1alpha1/cluster_types.go index 19c165ba..63338e00 100644 --- a/api/v1alpha1/cluster_types.go +++ b/api/v1alpha1/cluster_types.go @@ -39,6 +39,8 @@ type ClusterSpec struct { // Adopt: will create a new external resource or will adopt and manage an already existing resource // +kubebuilder:validation:Enum=Create;Adopt CreationPolicy CreationPolicy `json:"creationPolicy,omitempty"` + // EnableCompilePipeline determines whether the gitops compile pipeline should be set up for this cluster + EnableCompilePipeline bool `json:"enableCompilePipeline,omitempty"` } // BootstrapToken this key is used only once for Steward to register. diff --git a/api/v1alpha1/tenant_types.go b/api/v1alpha1/tenant_types.go index a7832b13..1b3278e3 100644 --- a/api/v1alpha1/tenant_types.go +++ b/api/v1alpha1/tenant_types.go @@ -37,11 +37,14 @@ type TenantSpec struct { // The fields within this can use Go templating. // See https://syn.tools/lieutenant-operator/explanations/templating.html for details. ClusterTemplate *ClusterSpec `json:"clusterTemplate,omitempty"` + // CompilePipeline contains the configuration for the automatically configured compile pipelines on this tenant + CompilePipeline *CompilePipelineSpec `json:"compilePipeline,omitempty"` } // TenantStatus defines the observed state of Tenant type TenantStatus struct { - // TBD + // CompilePipeline contains the status of the automatically configured compile pipelines on this tenant + CompilePipeline *CompilePipelineStatus `json:"compilePipeline,omitempty"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -68,6 +71,16 @@ type TenantList struct { Items []Tenant `json:"items"` } +type CompilePipelineSpec struct { + // Pipelines contains a map of filenames and file contents, specifying files which are added to the GitRepoTemplate in order to set up the automatically configured compile pipeline + PipelineFiles map[string]string `json:"pipelineFiles,omitempty"` +} + +type CompilePipelineStatus struct { + // Clusters contains the list of all clusters for which the automatically configured compile pipeline is enabled + Clusters []string `json:"clusters,omitempty"` +} + func init() { SchemeBuilder.Register(&Tenant{}, &TenantList{}) } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index a47fc4ed..2ea14521 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -225,6 +225,48 @@ func (in *CompileMetaVersionInfo) DeepCopy() *CompileMetaVersionInfo { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CompilePipelineSpec) DeepCopyInto(out *CompilePipelineSpec) { + *out = *in + if in.PipelineFiles != nil { + in, out := &in.PipelineFiles, &out.PipelineFiles + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CompilePipelineSpec. +func (in *CompilePipelineSpec) DeepCopy() *CompilePipelineSpec { + if in == nil { + return nil + } + out := new(CompilePipelineSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CompilePipelineStatus) DeepCopyInto(out *CompilePipelineStatus) { + *out = *in + if in.Clusters != nil { + in, out := &in.Clusters, &out.Clusters + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CompilePipelineStatus. +func (in *CompilePipelineStatus) DeepCopy() *CompilePipelineStatus { + if in == nil { + return nil + } + out := new(CompilePipelineStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DeployKey) DeepCopyInto(out *DeployKey) { *out = *in @@ -457,7 +499,7 @@ func (in *Tenant) DeepCopyInto(out *Tenant) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Tenant. @@ -523,6 +565,11 @@ func (in *TenantSpec) DeepCopyInto(out *TenantSpec) { *out = new(ClusterSpec) (*in).DeepCopyInto(*out) } + if in.CompilePipeline != nil { + in, out := &in.CompilePipeline, &out.CompilePipeline + *out = new(CompilePipelineSpec) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TenantSpec. @@ -538,6 +585,11 @@ func (in *TenantSpec) DeepCopy() *TenantSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TenantStatus) DeepCopyInto(out *TenantStatus) { *out = *in + if in.CompilePipeline != nil { + in, out := &in.CompilePipeline, &out.CompilePipeline + *out = new(CompilePipelineStatus) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TenantStatus. diff --git a/config/crd/bases/syn.tools_clusters.yaml b/config/crd/bases/syn.tools_clusters.yaml index 7535b9d6..c331c503 100644 --- a/config/crd/bases/syn.tools_clusters.yaml +++ b/config/crd/bases/syn.tools_clusters.yaml @@ -74,6 +74,10 @@ spec: description: DisplayName of cluster which could be different from metadata.name. Allows cluster renaming should it be needed. type: string + enableCompilePipeline: + description: EnableCompilePipeline determines whether the gitops compile + pipeline should be set up for this cluster + type: boolean facts: additionalProperties: type: string diff --git a/config/crd/bases/syn.tools_tenants.yaml b/config/crd/bases/syn.tools_tenants.yaml index b0987b39..29bffd3e 100644 --- a/config/crd/bases/syn.tools_tenants.yaml +++ b/config/crd/bases/syn.tools_tenants.yaml @@ -77,6 +77,10 @@ spec: description: DisplayName of cluster which could be different from metadata.name. Allows cluster renaming should it be needed. type: string + enableCompilePipeline: + description: EnableCompilePipeline determines whether the gitops + compile pipeline should be set up for this cluster + type: boolean facts: additionalProperties: type: string @@ -291,6 +295,18 @@ spec: description: TokenLifetime set the token lifetime type: string type: object + compilePipeline: + description: CompilePipeline contains the configuration for the automatically + configured compile pipelines on this tenant + properties: + pipelineFiles: + additionalProperties: + type: string + description: Pipelines contains a map of filenames and file contents, + specifying files which are added to the GitRepoTemplate in order + to set up the automatically configured compile pipeline + type: object + type: object creationPolicy: description: |- CreationPolicy defines how the external resources should be treated upon CR creation. @@ -501,6 +517,18 @@ spec: type: object status: description: TenantStatus defines the observed state of Tenant + properties: + compilePipeline: + description: CompilePipeline contains the status of the automatically + configured compile pipelines on this tenant + properties: + clusters: + description: Clusters contains the list of all clusters for which + the automatically configured compile pipeline is enabled + items: + type: string + type: array + type: object type: object type: object served: true diff --git a/config/crd/bases/syn.tools_tenanttemplates.yaml b/config/crd/bases/syn.tools_tenanttemplates.yaml index 56273db8..c6ee4c7b 100644 --- a/config/crd/bases/syn.tools_tenanttemplates.yaml +++ b/config/crd/bases/syn.tools_tenanttemplates.yaml @@ -77,6 +77,10 @@ spec: description: DisplayName of cluster which could be different from metadata.name. Allows cluster renaming should it be needed. type: string + enableCompilePipeline: + description: EnableCompilePipeline determines whether the gitops + compile pipeline should be set up for this cluster + type: boolean facts: additionalProperties: type: string @@ -291,6 +295,18 @@ spec: description: TokenLifetime set the token lifetime type: string type: object + compilePipeline: + description: CompilePipeline contains the configuration for the automatically + configured compile pipelines on this tenant + properties: + pipelineFiles: + additionalProperties: + type: string + description: Pipelines contains a map of filenames and file contents, + specifying files which are added to the GitRepoTemplate in order + to set up the automatically configured compile pipeline + type: object + type: object creationPolicy: description: |- CreationPolicy defines how the external resources should be treated upon CR creation. diff --git a/controllers/cluster_compile_pipeline_controller.go b/controllers/cluster_compile_pipeline_controller.go new file mode 100644 index 00000000..92809112 --- /dev/null +++ b/controllers/cluster_compile_pipeline_controller.go @@ -0,0 +1,78 @@ +package controllers + +import ( + "context" + "fmt" + "slices" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1" +) + +// ClusterCompilePipelineReconciler reconciles a Cluster object, specifically the `Spec.EnableCompilePipeline` field, updating the corresponding tenant's status accordingly. +type ClusterCompilePipelineReconciler struct { + client.Client + Scheme *runtime.Scheme +} + +//+kubebuilder:rbac:groups=syn.tools,resources=clusters,verbs=get;list;watch; +//+kubebuilder:rbac:groups=syn.tools,resources=tenants/status,verbs=get;update;patch + +func (r *ClusterCompilePipelineReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { + reqLogger := log.FromContext(ctx) + reqLogger.Info("Reconciling Cluster Compile Pipeline") + fmt.Println("RECONCILE") + fmt.Println(request.NamespacedName) + + instance := &synv1alpha1.Cluster{} + + err := r.Client.Get(ctx, request.NamespacedName, instance) + if err != nil { + if errors.IsNotFound(err) { + return reconcile.Result{}, nil + } + return reconcile.Result{}, err + } + + nsName := types.NamespacedName{Name: instance.GetTenantRef().Name, Namespace: instance.GetNamespace()} + tenant := &synv1alpha1.Tenant{} + if err := r.Client.Get(ctx, nsName, tenant); err != nil { + return reconcile.Result{}, fmt.Errorf("couldn't find tenant: %w", err) + } + + pipelineStatus := tenant.Status.CompilePipeline + if pipelineStatus == nil { + pipelineStatus = &synv1alpha1.CompilePipelineStatus{} + } + clusterInList := slices.Contains(pipelineStatus.Clusters, instance.Name) + + if instance.Spec.EnableCompilePipeline && !clusterInList { + pipelineStatus.Clusters = append(pipelineStatus.Clusters, instance.Name) + } + if !instance.Spec.EnableCompilePipeline && clusterInList { + ind := slices.Index(pipelineStatus.Clusters, instance.Name) + pipelineStatus.Clusters = slices.Delete(pipelineStatus.Clusters, ind, ind+1) + } + + if instance.Spec.EnableCompilePipeline != clusterInList { + fmt.Println("Updating!") + tenant.Status.CompilePipeline = pipelineStatus + err = r.Client.Status().Update(ctx, tenant) + return reconcile.Result{}, err + } + return reconcile.Result{}, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *ClusterCompilePipelineReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&synv1alpha1.Cluster{}). + Complete(r) +} diff --git a/controllers/cluster_compile_pipeline_controller_test.go b/controllers/cluster_compile_pipeline_controller_test.go new file mode 100644 index 00000000..35ed4353 --- /dev/null +++ b/controllers/cluster_compile_pipeline_controller_test.go @@ -0,0 +1,280 @@ +package controllers + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + + synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func Test_AddClusterToPipelineStatus(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + } + cluster := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + }, + } + c := preparePipelineTestClient(t, tenant, cluster) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(cluster)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Status.CompilePipeline) + assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster") +} + +func Test_RemoveClusterFromPipelineStatus(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster"}, + }, + }, + } + cluster := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: false, + }, + } + c := preparePipelineTestClient(t, tenant, cluster) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(cluster)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Status.CompilePipeline) + assert.NotContains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster") +} + +func Test_RemoveClusterFromPipelineStatus_EnableUnset(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster"}, + }, + }, + } + cluster := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + }, + } + c := preparePipelineTestClient(t, tenant, cluster) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(cluster)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Status.CompilePipeline) + assert.NotContains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster") +} + +func Test_NoChangeIfClusterInList(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster"}, + }, + }, + } + cluster := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + }, + } + c := preparePipelineTestClient(t, tenant, cluster) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(cluster)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Status.CompilePipeline) + assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster") +} + +func Test_LeaveOtherListEntriesBe_WhenRemoving(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster", "c-other-cluster"}, + }, + }, + } + cluster := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: false, + }, + } + c := preparePipelineTestClient(t, tenant, cluster) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(cluster)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Status.CompilePipeline) + assert.NotContains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster") + assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-other-cluster") +} + +func Test_LeaveOtherListEntriesBe_WhenAdding(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-other-cluster"}, + }, + }, + } + cluster := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + }, + } + c := preparePipelineTestClient(t, tenant, cluster) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(cluster)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Status.CompilePipeline) + assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster") + assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-other-cluster") +} + +func preparePipelineTestClient(t *testing.T, initObjs ...client.Object) client.Client { + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(synv1alpha1.AddToScheme(scheme)) + + client := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(initObjs...). + WithStatusSubresource(&synv1alpha1.Tenant{}). + Build() + + return client +} + +func requestFor(obj client.Object) ctrl.Request { + return ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: obj.GetName(), + Namespace: obj.GetNamespace(), + }, + } +} + +func clusterCompilePipelineReconciler(c client.Client) *ClusterCompilePipelineReconciler { + r := ClusterCompilePipelineReconciler{ + Client: c, + Scheme: c.Scheme(), + } + return &r +} diff --git a/controllers/cluster_controller.go b/controllers/cluster_controller.go index 738d25e4..e4720fd4 100644 --- a/controllers/cluster_controller.go +++ b/controllers/cluster_controller.go @@ -30,6 +30,7 @@ type ClusterReconciler struct { //+kubebuilder:rbac:groups=syn.tools,resources=clusters,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=syn.tools,resources=clusters/status,verbs=get;update;patch //+kubebuilder:rbac:groups=syn.tools,resources=clusters/finalizers,verbs=update +//+kubebuilder:rbac:groups=syn.tools,resources=tenants/status,verbs=get;update;patch //+kubebuilder:rbac:groups="",resources=secrets;serviceaccounts,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings;roles,verbs=get;list;watch;create;update;patch;delete diff --git a/docs/modules/ROOT/pages/references/api-reference.adoc b/docs/modules/ROOT/pages/references/api-reference.adoc index 157397a9..8efcebfb 100644 --- a/docs/modules/ROOT/pages/references/api-reference.adoc +++ b/docs/modules/ROOT/pages/references/api-reference.adoc @@ -111,6 +111,7 @@ Archive: will archive the external resources, if it supports that | *`creationPolicy`* __xref:{anchor_prefix}-github-com-projectsyn-lieutenant-operator-api-v1alpha1-creationpolicy[$$CreationPolicy$$]__ | CreationPolicy defines how the external resources should be treated upon CR creation. Create: will only create a new external resource and will not manage already existing resources Adopt: will create a new external resource or will adopt and manage an already existing resource +| *`enableCompilePipeline`* __boolean__ | EnableCompilePipeline determines whether the gitops compile pipeline should be set up for this cluster |=== @@ -183,6 +184,40 @@ Can point to a tag, branch or any other git reference. |=== +[id="{anchor_prefix}-github-com-projectsyn-lieutenant-operator-api-v1alpha1-compilepipelinespec"] +=== CompilePipelineSpec + + + +.Appears In: +**** +- xref:{anchor_prefix}-github-com-projectsyn-lieutenant-operator-api-v1alpha1-tenantspec[$$TenantSpec$$] +**** + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`pipelineFiles`* __object (keys:string, values:string)__ | Pipelines contains a map of filenames and file contents, specifying files which are added to the GitRepoTemplate in order to set up the automatically configured compile pipeline +|=== + + +[id="{anchor_prefix}-github-com-projectsyn-lieutenant-operator-api-v1alpha1-compilepipelinestatus"] +=== CompilePipelineStatus + + + +.Appears In: +**** +- xref:{anchor_prefix}-github-com-projectsyn-lieutenant-operator-api-v1alpha1-tenantstatus[$$TenantStatus$$] +**** + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`clusters`* __string array__ | Clusters contains the list of all clusters for which the automatically configured compile pipeline is enabled +|=== + + [id="{anchor_prefix}-github-com-projectsyn-lieutenant-operator-api-v1alpha1-creationpolicy"] === CreationPolicy (string) @@ -497,6 +532,7 @@ Adopt: will create a new external resource or will adopt and manage an already | *`clusterTemplate`* __xref:{anchor_prefix}-github-com-projectsyn-lieutenant-operator-api-v1alpha1-clusterspec[$$ClusterSpec$$]__ | ClusterTemplate defines a template which will be used to set defaults for the clusters of this tenant. The fields within this can use Go templating. See https://syn.tools/lieutenant-operator/explanations/templating.html for details. +| *`compilePipeline`* __xref:{anchor_prefix}-github-com-projectsyn-lieutenant-operator-api-v1alpha1-compilepipelinespec[$$CompilePipelineSpec$$]__ | CompilePipeline contains the configuration for the automatically configured compile pipelines on this tenant |=== diff --git a/main.go b/main.go index 06bc0696..1ba7e41f 100644 --- a/main.go +++ b/main.go @@ -139,6 +139,13 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "Cluster") os.Exit(1) } + if err = (&controllers.ClusterCompilePipelineReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "ClusterCompilePipeline") + os.Exit(1) + } if err = (&controllers.GitRepoReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), From 2526a24b50ca7f0bc6a7d3b178c04498d09b387c Mon Sep 17 00:00:00 2001 From: Aline Abler Date: Tue, 23 Jul 2024 18:45:11 +0200 Subject: [PATCH 2/5] Reconcile compile pipeline for tenants --- api/v1alpha1/cluster_types.go | 7 + api/v1alpha1/tenant_types.go | 18 + config/crd/bases/syn.tools_tenants.yaml | 4 + .../crd/bases/syn.tools_tenanttemplates.yaml | 4 + .../cluster_compile_pipeline_controller.go | 72 ++- ...luster_compile_pipeline_controller_test.go | 281 ++++++++++- controllers/common_test.go | 46 ++ controllers/tenant/git.go | 7 + controllers/tenant/steps.go | 2 +- .../tenant_compile_pipeline_controller.go | 124 +++++ ...tenant_compile_pipeline_controller_test.go | 435 ++++++++++++++++++ controllers/util.go | 79 ++++ .../ROOT/pages/references/api-reference.adoc | 1 + main.go | 10 + 14 files changed, 1050 insertions(+), 40 deletions(-) create mode 100644 controllers/common_test.go create mode 100644 controllers/tenant_compile_pipeline_controller.go create mode 100644 controllers/tenant_compile_pipeline_controller_test.go create mode 100644 controllers/util.go diff --git a/api/v1alpha1/cluster_types.go b/api/v1alpha1/cluster_types.go index 63338e00..e7cebd0c 100644 --- a/api/v1alpha1/cluster_types.go +++ b/api/v1alpha1/cluster_types.go @@ -133,6 +133,9 @@ func init() { // GetGitTemplate returns the git repository template func (c *Cluster) GetGitTemplate() *GitRepoTemplate { + if c.Spec.GitRepoTemplate == nil { + c.Spec.GitRepoTemplate = &GitRepoTemplate{} + } return c.Spec.GitRepoTemplate } @@ -173,3 +176,7 @@ func (c *Cluster) GetMeta() metav1.ObjectMeta { func (c *Cluster) GetStatus() interface{} { return c.Status } + +func (c *Cluster) GetEnableCompilePipeline() bool { + return c.Spec.EnableCompilePipeline +} diff --git a/api/v1alpha1/tenant_types.go b/api/v1alpha1/tenant_types.go index 1b3278e3..ed44c723 100644 --- a/api/v1alpha1/tenant_types.go +++ b/api/v1alpha1/tenant_types.go @@ -72,6 +72,8 @@ type TenantList struct { } type CompilePipelineSpec struct { + // Enabled enables or disables the compile pipeline for this tenant + Enabled bool `json:"enabled,omitempty"` // Pipelines contains a map of filenames and file contents, specifying files which are added to the GitRepoTemplate in order to set up the automatically configured compile pipeline PipelineFiles map[string]string `json:"pipelineFiles,omitempty"` } @@ -93,6 +95,22 @@ func (t *Tenant) GetGitTemplate() *GitRepoTemplate { return t.Spec.GitRepoTemplate } +// GetCompilePipelineStatus returns the compile pipeline status +func (t *Tenant) GetCompilePipelineStatus() *CompilePipelineStatus { + if t.Status.CompilePipeline == nil { + t.Status.CompilePipeline = &CompilePipelineStatus{} + } + return t.Status.CompilePipeline +} + +// GetCompilePipelineSpec returns the compile pipeline spec +func (t *Tenant) GetCompilePipelineSpec() *CompilePipelineSpec { + if t.Spec.CompilePipeline == nil { + t.Spec.CompilePipeline = &CompilePipelineSpec{} + } + return t.Spec.CompilePipeline +} + // GetTenantRef returns the tenant of this CR func (t *Tenant) GetTenantRef() corev1.LocalObjectReference { return corev1.LocalObjectReference{Name: t.GetName()} diff --git a/config/crd/bases/syn.tools_tenants.yaml b/config/crd/bases/syn.tools_tenants.yaml index 29bffd3e..e3199eb5 100644 --- a/config/crd/bases/syn.tools_tenants.yaml +++ b/config/crd/bases/syn.tools_tenants.yaml @@ -299,6 +299,10 @@ spec: description: CompilePipeline contains the configuration for the automatically configured compile pipelines on this tenant properties: + enabled: + description: Enabled enables or disables the compile pipeline + for this tenant + type: boolean pipelineFiles: additionalProperties: type: string diff --git a/config/crd/bases/syn.tools_tenanttemplates.yaml b/config/crd/bases/syn.tools_tenanttemplates.yaml index c6ee4c7b..116d329b 100644 --- a/config/crd/bases/syn.tools_tenanttemplates.yaml +++ b/config/crd/bases/syn.tools_tenanttemplates.yaml @@ -299,6 +299,10 @@ spec: description: CompilePipeline contains the configuration for the automatically configured compile pipelines on this tenant properties: + enabled: + description: Enabled enables or disables the compile pipeline + for this tenant + type: boolean pipelineFiles: additionalProperties: type: string diff --git a/controllers/cluster_compile_pipeline_controller.go b/controllers/cluster_compile_pipeline_controller.go index 92809112..f1708560 100644 --- a/controllers/cluster_compile_pipeline_controller.go +++ b/controllers/cluster_compile_pipeline_controller.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "slices" + "strings" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -28,8 +29,6 @@ type ClusterCompilePipelineReconciler struct { func (r *ClusterCompilePipelineReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { reqLogger := log.FromContext(ctx) reqLogger.Info("Reconciling Cluster Compile Pipeline") - fmt.Println("RECONCILE") - fmt.Println(request.NamespacedName) instance := &synv1alpha1.Cluster{} @@ -47,27 +46,68 @@ func (r *ClusterCompilePipelineReconciler) Reconcile(ctx context.Context, reques return reconcile.Result{}, fmt.Errorf("couldn't find tenant: %w", err) } - pipelineStatus := tenant.Status.CompilePipeline - if pipelineStatus == nil { - pipelineStatus = &synv1alpha1.CompilePipelineStatus{} + if ensureTenantStatus(tenant, instance) { + err = r.Client.Status().Update(ctx, tenant) + if err != nil { + return reconcile.Result{}, err + } } - clusterInList := slices.Contains(pipelineStatus.Clusters, instance.Name) - if instance.Spec.EnableCompilePipeline && !clusterInList { - pipelineStatus.Clusters = append(pipelineStatus.Clusters, instance.Name) + if ensureClusterCiVariable(tenant, instance) { + err = r.Client.Update(ctx, tenant) + return reconcile.Result{}, err } - if !instance.Spec.EnableCompilePipeline && clusterInList { - ind := slices.Index(pipelineStatus.Clusters, instance.Name) + + return reconcile.Result{}, nil +} + +func ensureTenantStatus(t *synv1alpha1.Tenant, c *synv1alpha1.Cluster) bool { + deleted := !c.GetDeletionTimestamp().IsZero() + pipelineStatus := t.GetCompilePipelineStatus() + clusterInList := slices.Contains(pipelineStatus.Clusters, c.Name) + if deleted && clusterInList { + ind := slices.Index(pipelineStatus.Clusters, c.Name) pipelineStatus.Clusters = slices.Delete(pipelineStatus.Clusters, ind, ind+1) + return true + } + if c.GetEnableCompilePipeline() && !clusterInList { + pipelineStatus.Clusters = append(pipelineStatus.Clusters, c.Name) + slices.Sort(pipelineStatus.Clusters) + return true } + if !c.GetEnableCompilePipeline() && clusterInList { + ind := slices.Index(pipelineStatus.Clusters, c.Name) + pipelineStatus.Clusters = slices.Delete(pipelineStatus.Clusters, ind, ind+1) + return true + } + return false +} - if instance.Spec.EnableCompilePipeline != clusterInList { - fmt.Println("Updating!") - tenant.Status.CompilePipeline = pipelineStatus - err = r.Client.Status().Update(ctx, tenant) - return reconcile.Result{}, err +func ensureClusterCiVariable(t *synv1alpha1.Tenant, c *synv1alpha1.Cluster) bool { + remove := !c.GetDeletionTimestamp().IsZero() || + c.GetGitTemplate().AccessToken.SecretRef == "" || + !t.GetCompilePipelineSpec().Enabled || + !c.GetEnableCompilePipeline() + + template := t.GetGitTemplate() + envVarName := fmt.Sprintf("%s%s", CI_VARIABLE_PREFIX_CLUSTER_ACCESS_TOKEN, strings.Replace(c.GetName(), "-", "_", -1)) + + var list []synv1alpha1.EnvVar + var changed bool + + if remove { + list, changed = removeEnvVar(envVarName, template.CIVariables) + } else { + list, changed = updateEnvVarValueFrom(envVarName, c.Spec.GitRepoTemplate.AccessToken.SecretRef, SECRET_KEY_GITLAB_TOKEN, true, template.CIVariables) } - return reconcile.Result{}, nil + + if changed { + template.CIVariables = list + t.Spec.GitRepoTemplate = template + } + + return changed + } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/cluster_compile_pipeline_controller_test.go b/controllers/cluster_compile_pipeline_controller_test.go index 35ed4353..be1b462c 100644 --- a/controllers/cluster_compile_pipeline_controller_test.go +++ b/controllers/cluster_compile_pipeline_controller_test.go @@ -1,21 +1,18 @@ -package controllers +package controllers_test import ( "context" "testing" + "time" "github.com/stretchr/testify/assert" synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1" + "github.com/projectsyn/lieutenant-operator/controllers" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" - clientgoscheme "k8s.io/client-go/kubernetes/scheme" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func Test_AddClusterToPipelineStatus(t *testing.T) { @@ -91,6 +88,48 @@ func Test_RemoveClusterFromPipelineStatus(t *testing.T) { assert.NotContains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster") } +func Test_RemoveClusterFromPipelineStatus_WhenDeleting(t *testing.T) { + now := metav1.NewTime(time.Now()) + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster"}, + }, + }, + } + cluster := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster", + Namespace: "lieutenant", + DeletionTimestamp: &now, + Finalizers: []string{"foo"}, + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + }, + } + c := preparePipelineTestClient(t, tenant, cluster) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(cluster)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Status.CompilePipeline) + assert.NotContains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster") +} + func Test_RemoveClusterFromPipelineStatus_EnableUnset(t *testing.T) { tenant := &synv1alpha1.Tenant{ ObjectMeta: metav1.ObjectMeta{ @@ -247,32 +286,228 @@ func Test_LeaveOtherListEntriesBe_WhenAdding(t *testing.T) { assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster") assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-other-cluster") } +func Test_CiVariableNotUpdated_IfNotEnabled(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster"}, + }, + }, + } + cluster := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ + AccessToken: synv1alpha1.AccessToken{ + SecretRef: "my-secret", + }, + }, + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + }, + } + c := preparePipelineTestClient(t, tenant, cluster) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(cluster)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.Empty(t, mod_tenant.GetGitTemplate().CIVariables) +} -func preparePipelineTestClient(t *testing.T, initObjs ...client.Object) client.Client { - scheme := runtime.NewScheme() - utilruntime.Must(clientgoscheme.AddToScheme(scheme)) - utilruntime.Must(synv1alpha1.AddToScheme(scheme)) +func Test_CiVariableNotUpdated_IfNoToken(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster"}, + }, + }, + } + cluster := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + }, + } + c := preparePipelineTestClient(t, tenant, cluster) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(cluster)) + assert.NoError(t, err) - client := fake.NewClientBuilder(). - WithScheme(scheme). - WithObjects(initObjs...). - WithStatusSubresource(&synv1alpha1.Tenant{}). - Build() + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) - return client + assert.Empty(t, mod_tenant.GetGitTemplate().CIVariables) } -func requestFor(obj client.Object) ctrl.Request { - return ctrl.Request{ - NamespacedName: types.NamespacedName{ - Name: obj.GetName(), - Namespace: obj.GetNamespace(), +func Test_CiVariableUpdated_IfEnabled(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.TenantSpec{ + CompilePipeline: &synv1alpha1.CompilePipelineSpec{ + Enabled: true, + }, + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster"}, + }, + }, + } + cluster := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ + AccessToken: synv1alpha1.AccessToken{ + SecretRef: "my-secret", + }, + }, + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + }, + } + c := preparePipelineTestClient(t, tenant, cluster) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(cluster)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Spec.GitRepoTemplate) + assert.Equal(t, mod_tenant.Spec.GitRepoTemplate.CIVariables[0].Name, "ACCESS_TOKEN_c_cluster") + assert.Equal(t, mod_tenant.Spec.GitRepoTemplate.CIVariables[0].ValueFrom.SecretKeyRef.Name, "my-secret") + assert.Equal(t, mod_tenant.Spec.GitRepoTemplate.CIVariables[0].ValueFrom.SecretKeyRef.Key, "token") + assert.Equal(t, mod_tenant.Spec.GitRepoTemplate.CIVariables[0].GitlabOptions.Masked, true) + assert.Equal(t, mod_tenant.Spec.GitRepoTemplate.CIVariables[0].GitlabOptions.Protected, true) + assert.Equal(t, mod_tenant.Spec.GitRepoTemplate.CIVariables[0].GitlabOptions.Raw, true) +} +func Test_KeepListInAlphabeticalOrder(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-b", "c-d"}, + }, + }, + } + clusterA := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-a", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + }, + } + clusterB := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-b", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: false, }, } + clusterC := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-c", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + }, + } + c := preparePipelineTestClient(t, tenant, clusterA, clusterB, clusterC) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(clusterA)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Status.CompilePipeline) + assert.Equal(t, mod_tenant.Status.CompilePipeline.Clusters[0], "c-a") + assert.Equal(t, mod_tenant.Status.CompilePipeline.Clusters[1], "c-b") + assert.Equal(t, mod_tenant.Status.CompilePipeline.Clusters[2], "c-d") + + _, err = r.Reconcile(ctx, requestFor(clusterB)) + assert.NoError(t, err) + + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Status.CompilePipeline) + assert.Equal(t, mod_tenant.Status.CompilePipeline.Clusters[0], "c-a") + assert.Equal(t, mod_tenant.Status.CompilePipeline.Clusters[1], "c-d") + + _, err = r.Reconcile(ctx, requestFor(clusterC)) + assert.NoError(t, err) + + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Status.CompilePipeline) + assert.Equal(t, mod_tenant.Status.CompilePipeline.Clusters[0], "c-a") + assert.Equal(t, mod_tenant.Status.CompilePipeline.Clusters[1], "c-c") + assert.Equal(t, mod_tenant.Status.CompilePipeline.Clusters[2], "c-d") } -func clusterCompilePipelineReconciler(c client.Client) *ClusterCompilePipelineReconciler { - r := ClusterCompilePipelineReconciler{ +func clusterCompilePipelineReconciler(c client.Client) *controllers.ClusterCompilePipelineReconciler { + r := controllers.ClusterCompilePipelineReconciler{ Client: c, Scheme: c.Scheme(), } diff --git a/controllers/common_test.go b/controllers/common_test.go new file mode 100644 index 00000000..a055e8ae --- /dev/null +++ b/controllers/common_test.go @@ -0,0 +1,46 @@ +package controllers_test + +import ( + "testing" + + synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func preparePipelineTestClient(t *testing.T, initObjs ...client.Object) client.Client { + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(synv1alpha1.AddToScheme(scheme)) + + client := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(initObjs...). + WithStatusSubresource(&synv1alpha1.Tenant{}). + Build() + + return client +} + +func requestFor(obj client.Object) ctrl.Request { + return ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: obj.GetName(), + Namespace: obj.GetNamespace(), + }, + } +} + +func envVarIndex(name string, list *[]synv1alpha1.EnvVar) int { + for i, envvar := range *list { + if envvar.Name == name { + return i + } + } + return -1 +} diff --git a/controllers/tenant/git.go b/controllers/tenant/git.go index e18e5c38..adc1ef03 100644 --- a/controllers/tenant/git.go +++ b/controllers/tenant/git.go @@ -47,6 +47,13 @@ func updateTenantGitRepo(obj pipeline.Object, data *pipeline.Context) pipeline.R delete(oldFiles, fileName) } + if tenantCR.GetCompilePipelineSpec().Enabled { + for pipelineFile, content := range tenantCR.GetCompilePipelineSpec().PipelineFiles { + tenantCR.Spec.GitRepoTemplate.TemplateFiles[pipelineFile] = content + delete(oldFiles, pipelineFile) + } + } + for fileName := range oldFiles { if fileName == CommonClassName+".yml" { tenantCR.Spec.GitRepoTemplate.TemplateFiles[CommonClassName+".yml"] = "" diff --git a/controllers/tenant/steps.go b/controllers/tenant/steps.go index c10cdb65..ab6f4bca 100644 --- a/controllers/tenant/steps.go +++ b/controllers/tenant/steps.go @@ -8,7 +8,7 @@ func Steps(obj pipeline.Object, data *pipeline.Context) pipeline.Result { steps := []pipeline.Step{ {Name: "apply template from TenantTemplate", F: applyTemplateFromTenantTemplate}, {Name: "add default class file", F: addDefaultClassFile}, - {Name: "uptade tenant git repo", F: updateTenantGitRepo}, + {Name: "update tenant git repo", F: updateTenantGitRepo}, {Name: "set global git repo url", F: setGlobalGitRepoURL}, {Name: "create ServiceAccount", F: createServiceAccount}, {Name: "reconcile Role", F: reconcileRole}, diff --git a/controllers/tenant_compile_pipeline_controller.go b/controllers/tenant_compile_pipeline_controller.go new file mode 100644 index 00000000..0b3d3e62 --- /dev/null +++ b/controllers/tenant_compile_pipeline_controller.go @@ -0,0 +1,124 @@ +package controllers + +import ( + "context" + "fmt" + "strings" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1" +) + +// TenantCompilePipelineReconciler reconciles a Tenant object, specifically the `Spec.CompilePipeline` field, updating the corresponding tenant's git repo accordingly. +type TenantCompilePipelineReconciler struct { + client.Client + Scheme *runtime.Scheme + ApiUrl string +} + +const ( + CI_VARIABLE_API_URL = "COMMODORE_API_URL" + CI_VARIABLE_API_TOKEN = "COMMODORE_API_TOKEN" + CI_VARIABLE_CLUSTERS = "CLUSTERS" + CI_VARIABLE_PREFIX_CLUSTER_ACCESS_TOKEN = "ACCESS_TOKEN_" + SECRET_KEY_API_TOKEN = "token" + SECRET_KEY_GITLAB_TOKEN = "token" +) + +//+kubebuilder:rbac:groups=syn.tools,resources=tenants,verbs=get;list;watch;update;patch; +//+kubebuilder:rbac:groups=syn.tools,resources=clusters,verbs=get;list;watch; +//+kubebuilder:rbac:groups=syn.tools,resources=tenants/status,verbs=get; + +func (r *TenantCompilePipelineReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { + reqLogger := log.FromContext(ctx) + reqLogger.Info("Reconciling Tenant Compile Pipeline") + + tenant := &synv1alpha1.Tenant{} + + err := r.Client.Get(ctx, request.NamespacedName, tenant) + if err != nil { + if errors.IsNotFound(err) { + return reconcile.Result{}, nil + } + return reconcile.Result{}, err + } + + changed := false + + if !tenant.GetCompilePipelineSpec().Enabled { + changed = ensureCiVariablesAbsent(tenant) + } else { + changed = r.ensureCiVariables(tenant) || changed + } + + cluster := &synv1alpha1.Cluster{} + pipelineStatus := tenant.GetCompilePipelineStatus() + for _, clusterName := range pipelineStatus.Clusters { + + nsName := types.NamespacedName{Name: clusterName, Namespace: tenant.GetNamespace()} + err := r.Client.Get(ctx, nsName, cluster) + if err != nil && !errors.IsNotFound(err) { + return reconcile.Result{}, fmt.Errorf("while reconciling CI variables for clusters: %w", err) + } + + if err == nil { + changed = ensureClusterCiVariable(tenant, cluster) || changed + } + } + if changed { + err = r.Client.Update(ctx, tenant) + return reconcile.Result{}, err + } + return reconcile.Result{}, nil + +} + +func ensureCiVariablesAbsent(t *synv1alpha1.Tenant) bool { + vars, changed := removeEnvVar(CI_VARIABLE_API_URL, t.GetGitTemplate().CIVariables) + vars, ch := removeEnvVar(CI_VARIABLE_API_TOKEN, vars) + changed = ch || changed + vars, ch = removeEnvVar(CI_VARIABLE_CLUSTERS, vars) + changed = ch || changed + if changed { + t.GetGitTemplate().CIVariables = vars + } + return changed +} + +func (r *TenantCompilePipelineReconciler) ensureCiVariables(t *synv1alpha1.Tenant) bool { + template := t.GetGitTemplate() + changed := false + + pipelineStatus := t.Status.CompilePipeline + if pipelineStatus == nil { + pipelineStatus = &synv1alpha1.CompilePipelineStatus{} + } + clusterList := strings.Join(pipelineStatus.Clusters, ",") + + list, ch := updateEnvVarValue(CI_VARIABLE_API_URL, r.ApiUrl, template.CIVariables) + changed = ch + list, ch = updateEnvVarValue(CI_VARIABLE_CLUSTERS, clusterList, list) + changed = changed || ch + list, ch = updateEnvVarValueFrom(CI_VARIABLE_API_TOKEN, t.Name, SECRET_KEY_API_TOKEN, false, list) + changed = changed || ch + + if changed { + template.CIVariables = list + } + + return changed +} + +// SetupWithManager sets up the controller with the Manager. +func (r *TenantCompilePipelineReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&synv1alpha1.Tenant{}). + Complete(r) +} diff --git a/controllers/tenant_compile_pipeline_controller_test.go b/controllers/tenant_compile_pipeline_controller_test.go new file mode 100644 index 00000000..0fceeefe --- /dev/null +++ b/controllers/tenant_compile_pipeline_controller_test.go @@ -0,0 +1,435 @@ +package controllers_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + + synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1" + "github.com/projectsyn/lieutenant-operator/controllers" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func Test_AddBasicPipelineStatus(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.TenantSpec{ + CompilePipeline: &synv1alpha1.CompilePipelineSpec{ + Enabled: true, + }, + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster1", "c-cluster2"}, + }, + }, + } + c := preparePipelineTestClient(t, tenant) + r := tenantCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(tenant)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Spec.GitRepoTemplate) + i := envVarIndex("COMMODORE_API_URL", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i >= 0) + assert.Equal(t, "https://api-url", mod_tenant.GetGitTemplate().CIVariables[i].Value) + i = envVarIndex("COMMODORE_API_TOKEN", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i >= 0) + assert.Equal(t, "t-tenant", mod_tenant.GetGitTemplate().CIVariables[i].ValueFrom.SecretKeyRef.Name) + assert.Equal(t, "token", mod_tenant.GetGitTemplate().CIVariables[i].ValueFrom.SecretKeyRef.Key) + i = envVarIndex("CLUSTERS", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i >= 0) + assert.Equal(t, "c-cluster1,c-cluster2", mod_tenant.GetGitTemplate().CIVariables[i].Value) +} +func Test_RemoveBasicPipelineStatus(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.TenantSpec{ + CompilePipeline: &synv1alpha1.CompilePipelineSpec{ + Enabled: false, + }, + GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ + CIVariables: []synv1alpha1.EnvVar{ + { + Name: "COMMODORE_API_URL", + Value: "foo", + }, + { + Name: "COMMODORE_API_TOKEN", + Value: "foo", + }, + { + Name: "CLUSTERS", + Value: "foo", + }, + }, + }, + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster1", "c-cluster2"}, + }, + }, + } + c := preparePipelineTestClient(t, tenant) + r := tenantCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(tenant)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Spec.GitRepoTemplate) + i := envVarIndex("COMMODORE_API_URL", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i < 0) + i = envVarIndex("COMMODORE_API_TOKEN", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i < 0) + i = envVarIndex("CLUSTERS", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i < 0) +} + +func Test_UpdateBasicPipelineStatus(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.TenantSpec{ + CompilePipeline: &synv1alpha1.CompilePipelineSpec{ + Enabled: true, + }, + GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ + CIVariables: []synv1alpha1.EnvVar{ + { + Name: "COMMODORE_API_URL", + Value: "foo", + }, + { + Name: "COMMODORE_API_TOKEN", + ValueFrom: &synv1alpha1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "wrong-secret", + }, + Key: "token", + }, + }, + }, + { + Name: "CLUSTERS", + Value: "foo", + }, + }, + }, + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster1", "c-cluster2"}, + }, + }, + } + c := preparePipelineTestClient(t, tenant) + r := tenantCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(tenant)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Spec.GitRepoTemplate) + i := envVarIndex("COMMODORE_API_URL", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i >= 0) + assert.Equal(t, "https://api-url", mod_tenant.GetGitTemplate().CIVariables[i].Value) + i = envVarIndex("COMMODORE_API_TOKEN", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i >= 0) + assert.Equal(t, "t-tenant", mod_tenant.GetGitTemplate().CIVariables[i].ValueFrom.SecretKeyRef.Name) + assert.Equal(t, "token", mod_tenant.GetGitTemplate().CIVariables[i].ValueFrom.SecretKeyRef.Key) + i = envVarIndex("CLUSTERS", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i >= 0) + assert.Equal(t, "c-cluster1,c-cluster2", mod_tenant.GetGitTemplate().CIVariables[i].Value) +} + +func Test_UpdateBasicPipelineStatus_NoUpdate_IfNoChanges(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.TenantSpec{ + CompilePipeline: &synv1alpha1.CompilePipelineSpec{ + Enabled: true, + }, + GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ + CIVariables: []synv1alpha1.EnvVar{ + { + Name: "COMMODORE_API_URL", + Value: "https://api-url", + }, + { + Name: "COMMODORE_API_TOKEN", + ValueFrom: &synv1alpha1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "t-tenant", + }, + Key: "token", + }, + }, + }, + { + Name: "CLUSTERS", + Value: "c-cluster1,c-cluster2", + }, + }, + }, + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster1", "c-cluster2"}, + }, + }, + } + c := preparePipelineTestClient(t, tenant) + r := tenantCompilePipelineReconciler(c) + ctx := context.Background() + + orig_tenant := &synv1alpha1.Tenant{} + err := c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, orig_tenant) + assert.NoError(t, err) + + _, err = r.Reconcile(ctx, requestFor(tenant)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.Equal(t, orig_tenant.ResourceVersion, mod_tenant.ResourceVersion) +} + +func Test_AddClusterCiVars(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.TenantSpec{ + CompilePipeline: &synv1alpha1.CompilePipelineSpec{ + Enabled: true, + }, + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster1", "c-cluster2", "c-cluster3"}, + }, + }, + } + cluster1 := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster1", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: v1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ + AccessToken: synv1alpha1.AccessToken{ + SecretRef: "cluster1-token", + }, + }, + }, + } + cluster2 := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster2", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: v1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ + AccessToken: synv1alpha1.AccessToken{ + SecretRef: "cluster2-token", + }, + }, + }, + } + cluster3 := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster3", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: v1.LocalObjectReference{ + Name: "t-tenant", + }, + GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ + AccessToken: synv1alpha1.AccessToken{ + SecretRef: "cluster2-token", + }, + }, + }, + } + + c := preparePipelineTestClient(t, tenant, cluster1, cluster2, cluster3) + r := tenantCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(tenant)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.NotNil(t, mod_tenant.Spec.GitRepoTemplate) + i := envVarIndex("ACCESS_TOKEN_c_cluster1", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i >= 0) + assert.Equal(t, "cluster1-token", mod_tenant.GetGitTemplate().CIVariables[i].ValueFrom.SecretKeyRef.Name) + assert.Equal(t, "token", mod_tenant.GetGitTemplate().CIVariables[i].ValueFrom.SecretKeyRef.Key) + i = envVarIndex("ACCESS_TOKEN_c_cluster2", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i >= 0) + assert.Equal(t, "cluster2-token", mod_tenant.GetGitTemplate().CIVariables[i].ValueFrom.SecretKeyRef.Name) + assert.Equal(t, "token", mod_tenant.GetGitTemplate().CIVariables[i].ValueFrom.SecretKeyRef.Key) + i = envVarIndex("ACCESS_TOKEN_c_cluster3", &mod_tenant.GetGitTemplate().CIVariables) + assert.True(t, i < 0) +} + +func Test_ClusterCiVars_NoUpdate_IfNoChanges(t *testing.T) { + tenant := &synv1alpha1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "t-tenant", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.TenantSpec{ + CompilePipeline: &synv1alpha1.CompilePipelineSpec{ + Enabled: true, + }, + GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ + CIVariables: []synv1alpha1.EnvVar{ + { + Name: "COMMODORE_API_URL", + Value: "https://api-url", + }, + { + Name: "COMMODORE_API_TOKEN", + ValueFrom: &synv1alpha1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "t-tenant", + }, + Key: "token", + }, + }, + }, + { + Name: "CLUSTERS", + Value: "c-cluster1", + }, + { + Name: "ACCESS_TOKEN_c_cluster1", + ValueFrom: &synv1alpha1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "cluster1-token", + }, + Key: "token", + }, + }, + }, + }, + }, + }, + Status: synv1alpha1.TenantStatus{ + CompilePipeline: &synv1alpha1.CompilePipelineStatus{ + Clusters: []string{"c-cluster1"}, + }, + }, + } + cluster1 := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster1", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: v1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: true, + GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ + AccessToken: synv1alpha1.AccessToken{ + SecretRef: "cluster1-token", + }, + }, + }, + } + cluster2 := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster2", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: v1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: false, + GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ + AccessToken: synv1alpha1.AccessToken{ + SecretRef: "cluster2-token", + }, + }, + }, + } + + c := preparePipelineTestClient(t, tenant, cluster1, cluster2) + r := tenantCompilePipelineReconciler(c) + ctx := context.Background() + + orig_tenant := &synv1alpha1.Tenant{} + err := c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, orig_tenant) + assert.NoError(t, err) + + _, err = r.Reconcile(ctx, requestFor(tenant)) + assert.NoError(t, err) + + mod_tenant := &synv1alpha1.Tenant{} + err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, mod_tenant) + assert.NoError(t, err) + + assert.Equal(t, orig_tenant.ResourceVersion, mod_tenant.ResourceVersion) +} + +func tenantCompilePipelineReconciler(c client.Client) *controllers.TenantCompilePipelineReconciler { + r := controllers.TenantCompilePipelineReconciler{ + Client: c, + Scheme: c.Scheme(), + ApiUrl: "https://api-url", + } + return &r +} diff --git a/controllers/util.go b/controllers/util.go new file mode 100644 index 00000000..8045db89 --- /dev/null +++ b/controllers/util.go @@ -0,0 +1,79 @@ +package controllers + +import ( + "slices" + + synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1" + v1 "k8s.io/api/core/v1" +) + +func envVarIndex(name string, list *[]synv1alpha1.EnvVar) int { + for i, envvar := range *list { + if envvar.Name == name { + return i + } + } + return -1 +} + +func updateEnvVarValue(name string, value string, envVars []synv1alpha1.EnvVar) ([]synv1alpha1.EnvVar, bool) { + index := envVarIndex(name, &envVars) + changed := false + if index < 0 { + changed = true + envVars = append(envVars, synv1alpha1.EnvVar{ + GitlabOptions: synv1alpha1.EnvVarGitlabOptions{ + Raw: true, + }, + Name: name, + Value: value, + }) + } else if envVars[index].Value != value { + changed = true + envVars[index].Value = value + } + return envVars, changed +} +func updateEnvVarValueFrom(name string, secret string, key string, protected bool, envVars []synv1alpha1.EnvVar) ([]synv1alpha1.EnvVar, bool) { + index := envVarIndex(name, &envVars) + changed := false + if index < 0 { + changed = true + envVars = append(envVars, synv1alpha1.EnvVar{ + Name: name, + GitlabOptions: synv1alpha1.EnvVarGitlabOptions{ + Masked: true, + Raw: true, + Protected: protected, + }, + ValueFrom: &synv1alpha1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: secret, + }, + Key: key, + }, + }, + }) + } else if envVars[index].ValueFrom.SecretKeyRef.Name != secret || envVars[index].ValueFrom.SecretKeyRef.Key != key { + changed = true + envVars[index].ValueFrom = &synv1alpha1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: secret, + }, + Key: key, + }, + } + } + return envVars, changed +} + +func removeEnvVar(name string, envVars []synv1alpha1.EnvVar) ([]synv1alpha1.EnvVar, bool) { + index := envVarIndex(name, &envVars) + if index >= 0 { + return slices.Delete(envVars, index, index+1), true + + } + return envVars, false +} diff --git a/docs/modules/ROOT/pages/references/api-reference.adoc b/docs/modules/ROOT/pages/references/api-reference.adoc index 8efcebfb..5e3e87e0 100644 --- a/docs/modules/ROOT/pages/references/api-reference.adoc +++ b/docs/modules/ROOT/pages/references/api-reference.adoc @@ -197,6 +197,7 @@ Can point to a tag, branch or any other git reference. [cols="25a,75a", options="header"] |=== | Field | Description +| *`enabled`* __boolean__ | Enabled enables or disables the compile pipeline for this tenant | *`pipelineFiles`* __object (keys:string, values:string)__ | Pipelines contains a map of filenames and file contents, specifying files which are added to the GitRepoTemplate in order to set up the automatically configured compile pipeline |=== diff --git a/main.go b/main.go index 1ba7e41f..a99b3aae 100644 --- a/main.go +++ b/main.go @@ -59,10 +59,12 @@ func main() { var metricsAddr string var enableLeaderElection bool + var apiUrl string var probeAddr string var gitRepoMaxReconcileInterval time.Duration flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") + flag.StringVar(&apiUrl, "lieutenant-api-url", "localhost", "The URL at which the Lieutenant API is available externally") flag.DurationVar(&gitRepoMaxReconcileInterval, "git-repo-max-reconcile-interval", 3*time.Hour, "The maximum time between reconciliations of GitRepos.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ @@ -165,6 +167,14 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "Tenant") os.Exit(1) } + if err = (&controllers.TenantCompilePipelineReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ApiUrl: apiUrl, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "TenantCompilePipeline") + os.Exit(1) + } //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { From 1165e59d514ee2e9a4b5e49960a5b15ead81e079 Mon Sep 17 00:00:00 2001 From: Aline Abler Date: Thu, 25 Jul 2024 15:38:23 +0200 Subject: [PATCH 3/5] Implement code review suggestions --- api/v1alpha1/constants.go | 5 +- api/v1alpha1/tenant_types.go | 4 +- .../cluster_compile_pipeline_controller.go | 27 +++++- ...luster_compile_pipeline_controller_test.go | 90 +++++++++++++------ .../tenant_compile_pipeline_controller.go | 3 + controllers/util.go | 7 +- main.go | 5 +- 7 files changed, 104 insertions(+), 37 deletions(-) diff --git a/api/v1alpha1/constants.go b/api/v1alpha1/constants.go index f65efd99..1cf7b16a 100644 --- a/api/v1alpha1/constants.go +++ b/api/v1alpha1/constants.go @@ -1,8 +1,9 @@ package v1alpha1 const ( - LabelNameTenant = "syn.tools/tenant" - FinalizerName = "cluster.lieutenant.syn.tools" + LabelNameTenant = "syn.tools/tenant" + FinalizerName = "cluster.lieutenant.syn.tools" + PipelineFinalizerName = "cluster.lieutenant.syn.tools" // DeleteProtectionAnnotation defines the delete protection annotation name DeleteProtectionAnnotation = "syn.tools/protected-delete" diff --git a/api/v1alpha1/tenant_types.go b/api/v1alpha1/tenant_types.go index ed44c723..256a05d9 100644 --- a/api/v1alpha1/tenant_types.go +++ b/api/v1alpha1/tenant_types.go @@ -98,7 +98,7 @@ func (t *Tenant) GetGitTemplate() *GitRepoTemplate { // GetCompilePipelineStatus returns the compile pipeline status func (t *Tenant) GetCompilePipelineStatus() *CompilePipelineStatus { if t.Status.CompilePipeline == nil { - t.Status.CompilePipeline = &CompilePipelineStatus{} + return &CompilePipelineStatus{} } return t.Status.CompilePipeline } @@ -106,7 +106,7 @@ func (t *Tenant) GetCompilePipelineStatus() *CompilePipelineStatus { // GetCompilePipelineSpec returns the compile pipeline spec func (t *Tenant) GetCompilePipelineSpec() *CompilePipelineSpec { if t.Spec.CompilePipeline == nil { - t.Spec.CompilePipeline = &CompilePipelineSpec{} + return &CompilePipelineSpec{} } return t.Spec.CompilePipeline } diff --git a/controllers/cluster_compile_pipeline_controller.go b/controllers/cluster_compile_pipeline_controller.go index f1708560..d8d30c09 100644 --- a/controllers/cluster_compile_pipeline_controller.go +++ b/controllers/cluster_compile_pipeline_controller.go @@ -11,6 +11,7 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -25,6 +26,7 @@ type ClusterCompilePipelineReconciler struct { //+kubebuilder:rbac:groups=syn.tools,resources=clusters,verbs=get;list;watch; //+kubebuilder:rbac:groups=syn.tools,resources=tenants/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=syn.tools,resources=clusters/finalizers,verbs=update func (r *ClusterCompilePipelineReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { reqLogger := log.FromContext(ctx) @@ -40,6 +42,15 @@ func (r *ClusterCompilePipelineReconciler) Reconcile(ctx context.Context, reques return reconcile.Result{}, err } + if !controllerutil.ContainsFinalizer(instance, synv1alpha1.PipelineFinalizerName) { + if instance.GetDeletionTimestamp().IsZero() { + controllerutil.AddFinalizer(instance, synv1alpha1.PipelineFinalizerName) + return reconcile.Result{}, r.Client.Update(ctx, instance) + } else { + return reconcile.Result{}, nil + } + } + nsName := types.NamespacedName{Name: instance.GetTenantRef().Name, Namespace: instance.GetNamespace()} tenant := &synv1alpha1.Tenant{} if err := r.Client.Get(ctx, nsName, tenant); err != nil { @@ -55,7 +66,18 @@ func (r *ClusterCompilePipelineReconciler) Reconcile(ctx context.Context, reques if ensureClusterCiVariable(tenant, instance) { err = r.Client.Update(ctx, tenant) - return reconcile.Result{}, err + if err != nil { + return reconcile.Result{}, err + } + } + + // We can only get here if the cluster list and CI variables were successfully updated on the tenant. + // So in the case of deletion, we can clean up the finalizer here, because that update involves removing them. + if !instance.GetDeletionTimestamp().IsZero() { + if controllerutil.ContainsFinalizer(instance, synv1alpha1.PipelineFinalizerName) { + controllerutil.RemoveFinalizer(instance, synv1alpha1.PipelineFinalizerName) + return ctrl.Result{}, r.Client.Update(ctx, instance) + } } return reconcile.Result{}, nil @@ -68,16 +90,19 @@ func ensureTenantStatus(t *synv1alpha1.Tenant, c *synv1alpha1.Cluster) bool { if deleted && clusterInList { ind := slices.Index(pipelineStatus.Clusters, c.Name) pipelineStatus.Clusters = slices.Delete(pipelineStatus.Clusters, ind, ind+1) + t.Status.CompilePipeline = pipelineStatus return true } if c.GetEnableCompilePipeline() && !clusterInList { pipelineStatus.Clusters = append(pipelineStatus.Clusters, c.Name) slices.Sort(pipelineStatus.Clusters) + t.Status.CompilePipeline = pipelineStatus return true } if !c.GetEnableCompilePipeline() && clusterInList { ind := slices.Index(pipelineStatus.Clusters, c.Name) pipelineStatus.Clusters = slices.Delete(pipelineStatus.Clusters, ind, ind+1) + t.Status.CompilePipeline = pipelineStatus return true } return false diff --git a/controllers/cluster_compile_pipeline_controller_test.go b/controllers/cluster_compile_pipeline_controller_test.go index be1b462c..b5c329ad 100644 --- a/controllers/cluster_compile_pipeline_controller_test.go +++ b/controllers/cluster_compile_pipeline_controller_test.go @@ -24,8 +24,9 @@ func Test_AddClusterToPipelineStatus(t *testing.T) { } cluster := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-cluster", - Namespace: "lieutenant", + Name: "c-cluster", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ TenantRef: corev1.LocalObjectReference{ @@ -63,8 +64,9 @@ func Test_RemoveClusterFromPipelineStatus(t *testing.T) { } cluster := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-cluster", - Namespace: "lieutenant", + Name: "c-cluster", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ TenantRef: corev1.LocalObjectReference{ @@ -106,7 +108,7 @@ func Test_RemoveClusterFromPipelineStatus_WhenDeleting(t *testing.T) { Name: "c-cluster", Namespace: "lieutenant", DeletionTimestamp: &now, - Finalizers: []string{"foo"}, + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ TenantRef: corev1.LocalObjectReference{ @@ -128,6 +130,34 @@ func Test_RemoveClusterFromPipelineStatus_WhenDeleting(t *testing.T) { assert.NotNil(t, mod_tenant.Status.CompilePipeline) assert.NotContains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster") + assert.NotContains(t, mod_tenant.Finalizers, synv1alpha1.PipelineFinalizerName) +} + +func Test_FinalizerAdded(t *testing.T) { + cluster := &synv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-cluster", + Namespace: "lieutenant", + }, + Spec: synv1alpha1.ClusterSpec{ + TenantRef: corev1.LocalObjectReference{ + Name: "t-tenant", + }, + EnableCompilePipeline: false, + }, + } + c := preparePipelineTestClient(t, cluster) + r := clusterCompilePipelineReconciler(c) + ctx := context.Background() + + _, err := r.Reconcile(ctx, requestFor(cluster)) + assert.NoError(t, err) + + mod_cluster := &synv1alpha1.Cluster{} + err = c.Get(ctx, types.NamespacedName{Name: "c-cluster", Namespace: "lieutenant"}, mod_cluster) + assert.NoError(t, err) + + assert.Contains(t, mod_cluster.Finalizers, synv1alpha1.PipelineFinalizerName) } func Test_RemoveClusterFromPipelineStatus_EnableUnset(t *testing.T) { @@ -144,8 +174,9 @@ func Test_RemoveClusterFromPipelineStatus_EnableUnset(t *testing.T) { } cluster := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-cluster", - Namespace: "lieutenant", + Name: "c-cluster", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ TenantRef: corev1.LocalObjectReference{ @@ -182,8 +213,9 @@ func Test_NoChangeIfClusterInList(t *testing.T) { } cluster := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-cluster", - Namespace: "lieutenant", + Name: "c-cluster", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ TenantRef: corev1.LocalObjectReference{ @@ -221,8 +253,9 @@ func Test_LeaveOtherListEntriesBe_WhenRemoving(t *testing.T) { } cluster := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-cluster", - Namespace: "lieutenant", + Name: "c-cluster", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ TenantRef: corev1.LocalObjectReference{ @@ -261,8 +294,9 @@ func Test_LeaveOtherListEntriesBe_WhenAdding(t *testing.T) { } cluster := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-cluster", - Namespace: "lieutenant", + Name: "c-cluster", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ TenantRef: corev1.LocalObjectReference{ @@ -300,8 +334,9 @@ func Test_CiVariableNotUpdated_IfNotEnabled(t *testing.T) { } cluster := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-cluster", - Namespace: "lieutenant", + Name: "c-cluster", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ @@ -343,8 +378,9 @@ func Test_CiVariableNotUpdated_IfNoToken(t *testing.T) { } cluster := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-cluster", - Namespace: "lieutenant", + Name: "c-cluster", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ TenantRef: corev1.LocalObjectReference{ @@ -386,8 +422,9 @@ func Test_CiVariableUpdated_IfEnabled(t *testing.T) { } cluster := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-cluster", - Namespace: "lieutenant", + Name: "c-cluster", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ GitRepoTemplate: &synv1alpha1.GitRepoTemplate{ @@ -434,8 +471,9 @@ func Test_KeepListInAlphabeticalOrder(t *testing.T) { } clusterA := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-a", - Namespace: "lieutenant", + Name: "c-a", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ TenantRef: corev1.LocalObjectReference{ @@ -446,8 +484,9 @@ func Test_KeepListInAlphabeticalOrder(t *testing.T) { } clusterB := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-b", - Namespace: "lieutenant", + Name: "c-b", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ TenantRef: corev1.LocalObjectReference{ @@ -458,8 +497,9 @@ func Test_KeepListInAlphabeticalOrder(t *testing.T) { } clusterC := &synv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "c-c", - Namespace: "lieutenant", + Name: "c-c", + Namespace: "lieutenant", + Finalizers: []string{synv1alpha1.PipelineFinalizerName}, }, Spec: synv1alpha1.ClusterSpec{ TenantRef: corev1.LocalObjectReference{ diff --git a/controllers/tenant_compile_pipeline_controller.go b/controllers/tenant_compile_pipeline_controller.go index 0b3d3e62..6ec0ee2f 100644 --- a/controllers/tenant_compile_pipeline_controller.go +++ b/controllers/tenant_compile_pipeline_controller.go @@ -67,6 +67,9 @@ func (r *TenantCompilePipelineReconciler) Reconcile(ctx context.Context, request if err != nil && !errors.IsNotFound(err) { return reconcile.Result{}, fmt.Errorf("while reconciling CI variables for clusters: %w", err) } + if err != nil && errors.IsNotFound(err) { + reqLogger.Info("Could not find cluster from list in .Status.CompilePipeline.Clusters", "clusterName", clusterName) + } if err == nil { changed = ensureClusterCiVariable(tenant, cluster) || changed diff --git a/controllers/util.go b/controllers/util.go index 8045db89..de9eb912 100644 --- a/controllers/util.go +++ b/controllers/util.go @@ -8,12 +8,7 @@ import ( ) func envVarIndex(name string, list *[]synv1alpha1.EnvVar) int { - for i, envvar := range *list { - if envvar.Name == name { - return i - } - } - return -1 + return slices.IndexFunc(*list, func(e synv1alpha1.EnvVar) bool { return e.Name == name }) } func updateEnvVarValue(name string, value string, envVars []synv1alpha1.EnvVar) ([]synv1alpha1.EnvVar, bool) { diff --git a/main.go b/main.go index a99b3aae..bc0e22dd 100644 --- a/main.go +++ b/main.go @@ -64,7 +64,10 @@ func main() { var gitRepoMaxReconcileInterval time.Duration flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") - flag.StringVar(&apiUrl, "lieutenant-api-url", "localhost", "The URL at which the Lieutenant API is available externally") + flag.StringVar(&apiUrl, "lieutenant-api-url", "localhost", + "The URL at which the Lieutenant API is available externally. "+ + "This is provided to tenant git repositories where the Compile Pipeline is enabled.", + ) flag.DurationVar(&gitRepoMaxReconcileInterval, "git-repo-max-reconcile-interval", 3*time.Hour, "The maximum time between reconciliations of GitRepos.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ From 9b991607a7b98f4158ee772da8becda325c4a659 Mon Sep 17 00:00:00 2001 From: Aline Abler Date: Thu, 25 Jul 2024 16:05:48 +0200 Subject: [PATCH 4/5] Implement code review suggestion, round 2 --- api/v1alpha1/cluster_types.go | 2 +- api/v1alpha1/constants.go | 2 +- controllers/cluster_compile_pipeline_controller.go | 3 +-- controllers/util.go | 10 +++++----- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/api/v1alpha1/cluster_types.go b/api/v1alpha1/cluster_types.go index e7cebd0c..db895dfa 100644 --- a/api/v1alpha1/cluster_types.go +++ b/api/v1alpha1/cluster_types.go @@ -134,7 +134,7 @@ func init() { // GetGitTemplate returns the git repository template func (c *Cluster) GetGitTemplate() *GitRepoTemplate { if c.Spec.GitRepoTemplate == nil { - c.Spec.GitRepoTemplate = &GitRepoTemplate{} + return &GitRepoTemplate{} } return c.Spec.GitRepoTemplate } diff --git a/api/v1alpha1/constants.go b/api/v1alpha1/constants.go index 1cf7b16a..eead23e9 100644 --- a/api/v1alpha1/constants.go +++ b/api/v1alpha1/constants.go @@ -3,7 +3,7 @@ package v1alpha1 const ( LabelNameTenant = "syn.tools/tenant" FinalizerName = "cluster.lieutenant.syn.tools" - PipelineFinalizerName = "cluster.lieutenant.syn.tools" + PipelineFinalizerName = "cluster.lieutenant.syn.tools/pipelines" // DeleteProtectionAnnotation defines the delete protection annotation name DeleteProtectionAnnotation = "syn.tools/protected-delete" diff --git a/controllers/cluster_compile_pipeline_controller.go b/controllers/cluster_compile_pipeline_controller.go index d8d30c09..e4165b8d 100644 --- a/controllers/cluster_compile_pipeline_controller.go +++ b/controllers/cluster_compile_pipeline_controller.go @@ -74,8 +74,7 @@ func (r *ClusterCompilePipelineReconciler) Reconcile(ctx context.Context, reques // We can only get here if the cluster list and CI variables were successfully updated on the tenant. // So in the case of deletion, we can clean up the finalizer here, because that update involves removing them. if !instance.GetDeletionTimestamp().IsZero() { - if controllerutil.ContainsFinalizer(instance, synv1alpha1.PipelineFinalizerName) { - controllerutil.RemoveFinalizer(instance, synv1alpha1.PipelineFinalizerName) + if controllerutil.RemoveFinalizer(instance, synv1alpha1.PipelineFinalizerName) { return ctrl.Result{}, r.Client.Update(ctx, instance) } } diff --git a/controllers/util.go b/controllers/util.go index de9eb912..ffac4a35 100644 --- a/controllers/util.go +++ b/controllers/util.go @@ -7,12 +7,12 @@ import ( v1 "k8s.io/api/core/v1" ) -func envVarIndex(name string, list *[]synv1alpha1.EnvVar) int { - return slices.IndexFunc(*list, func(e synv1alpha1.EnvVar) bool { return e.Name == name }) +func envVarIndex(name string, list []synv1alpha1.EnvVar) int { + return slices.IndexFunc(list, func(e synv1alpha1.EnvVar) bool { return e.Name == name }) } func updateEnvVarValue(name string, value string, envVars []synv1alpha1.EnvVar) ([]synv1alpha1.EnvVar, bool) { - index := envVarIndex(name, &envVars) + index := envVarIndex(name, envVars) changed := false if index < 0 { changed = true @@ -30,7 +30,7 @@ func updateEnvVarValue(name string, value string, envVars []synv1alpha1.EnvVar) return envVars, changed } func updateEnvVarValueFrom(name string, secret string, key string, protected bool, envVars []synv1alpha1.EnvVar) ([]synv1alpha1.EnvVar, bool) { - index := envVarIndex(name, &envVars) + index := envVarIndex(name, envVars) changed := false if index < 0 { changed = true @@ -65,7 +65,7 @@ func updateEnvVarValueFrom(name string, secret string, key string, protected boo } func removeEnvVar(name string, envVars []synv1alpha1.EnvVar) ([]synv1alpha1.EnvVar, bool) { - index := envVarIndex(name, &envVars) + index := envVarIndex(name, envVars) if index >= 0 { return slices.Delete(envVars, index, index+1), true From 1daf931f80b6ed565bde92c65977f9d0b384965d Mon Sep 17 00:00:00 2001 From: Aline Abler Date: Fri, 26 Jul 2024 09:21:23 +0200 Subject: [PATCH 5/5] Update controllers/tenant_compile_pipeline_controller.go Co-authored-by: Sebastian Widmer --- controllers/tenant_compile_pipeline_controller.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/controllers/tenant_compile_pipeline_controller.go b/controllers/tenant_compile_pipeline_controller.go index 6ec0ee2f..bfcf6b50 100644 --- a/controllers/tenant_compile_pipeline_controller.go +++ b/controllers/tenant_compile_pipeline_controller.go @@ -64,16 +64,15 @@ func (r *TenantCompilePipelineReconciler) Reconcile(ctx context.Context, request nsName := types.NamespacedName{Name: clusterName, Namespace: tenant.GetNamespace()} err := r.Client.Get(ctx, nsName, cluster) - if err != nil && !errors.IsNotFound(err) { + if err != nil { + if errors.IsNotFound(err) { + reqLogger.Info("Could not find cluster from list in .Status.CompilePipeline.Clusters", "clusterName", clusterName) + continue + } return reconcile.Result{}, fmt.Errorf("while reconciling CI variables for clusters: %w", err) } - if err != nil && errors.IsNotFound(err) { - reqLogger.Info("Could not find cluster from list in .Status.CompilePipeline.Clusters", "clusterName", clusterName) - } - if err == nil { - changed = ensureClusterCiVariable(tenant, cluster) || changed - } + changed = ensureClusterCiVariable(tenant, cluster) || changed } if changed { err = r.Client.Update(ctx, tenant)