diff --git a/api/v1alpha1/backstage_types.go b/api/v1alpha1/backstage_types.go index 6327b4d7..6d61ea47 100644 --- a/api/v1alpha1/backstage_types.go +++ b/api/v1alpha1/backstage_types.go @@ -48,8 +48,8 @@ type Database struct { //+kubebuilder:default=true EnableLocalDb *bool `json:"enableLocalDb,omitempty"` - // Name of the secret for database authentication. Required for external database access. - // Optional for a local database (EnableLocalDb=true) and if absent a secret will be auto generated. + // Name of the secret for database authentication. Optional. + // For a local database deployment (EnableLocalDb=true), a secret will be auto generated if it does not exist. // The secret shall include information used for the database access. // An example for PostgreSQL DB access: // "POSTGRES_PASSWORD": "rl4s3Fh4ng3M4" @@ -100,7 +100,7 @@ type Application struct { // Image Pull Secrets to use in all containers (including Init Containers) // +optional - ImagePullSecrets []string `json:"imagePullSecrets,omitempty"` + ImagePullSecrets *[]string `json:"imagePullSecrets,omitempty"` // Route configuration. Used for OpenShift only. Route *Route `json:"route,omitempty"` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 030da0a1..63e6f197 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -76,8 +76,12 @@ func (in *Application) DeepCopyInto(out *Application) { } if in.ImagePullSecrets != nil { in, out := &in.ImagePullSecrets, &out.ImagePullSecrets - *out = make([]string, len(*in)) - copy(*out, *in) + *out = new([]string) + if **in != nil { + in, out := *in, *out + *out = make([]string, len(*in)) + copy(*out, *in) + } } if in.Route != nil { in, out := &in.Route, &out.Route diff --git a/bundle/manifests/backstage-operator.clusterserviceversion.yaml b/bundle/manifests/backstage-operator.clusterserviceversion.yaml index 3b58a364..e81f51a3 100644 --- a/bundle/manifests/backstage-operator.clusterserviceversion.yaml +++ b/bundle/manifests/backstage-operator.clusterserviceversion.yaml @@ -21,7 +21,7 @@ metadata: } ] capabilities: Basic Install - createdAt: "2024-01-08T02:20:13Z" + createdAt: "2024-01-08T19:47:18Z" operators.operatorframework.io/builder: operator-sdk-v1.33.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 name: backstage-operator.v0.0.1 diff --git a/bundle/manifests/janus-idp.io_backstages.yaml b/bundle/manifests/janus-idp.io_backstages.yaml index ddb368d3..6bf98fef 100644 --- a/bundle/manifests/janus-idp.io_backstages.yaml +++ b/bundle/manifests/janus-idp.io_backstages.yaml @@ -267,14 +267,13 @@ spec: properties: authSecretName: description: 'Name of the secret for database authentication. - Required for external database access. Optional for a local - database (EnableLocalDb=true) and if absent a secret will be - auto generated. The secret shall include information used for - the database access. An example for PostgreSQL DB access: "POSTGRES_PASSWORD": - "rl4s3Fh4ng3M4" "POSTGRES_PORT": "5432" "POSTGRES_USER": "postgres" - "POSTGRESQL_ADMIN_PASSWORD": "rl4s3Fh4ng3M4" "POSTGRES_HOST": - "backstage-psql-bs1" # For local database, set to "backstage-psql-".' + Optional. For a local database deployment (EnableLocalDb=true), + a secret will be auto generated if it does not exist. The secret + shall include information used for the database access. An example + for PostgreSQL DB access: "POSTGRES_PASSWORD": "rl4s3Fh4ng3M4" + "POSTGRES_PORT": "5432" "POSTGRES_USER": "postgres" "POSTGRESQL_ADMIN_PASSWORD": + "rl4s3Fh4ng3M4" "POSTGRES_HOST": "backstage-psql-bs1" # For + local database, set to "backstage-psql-".' type: string enableLocalDb: default: true diff --git a/config/crd/bases/janus-idp.io_backstages.yaml b/config/crd/bases/janus-idp.io_backstages.yaml index 3e95bde8..fd52e185 100644 --- a/config/crd/bases/janus-idp.io_backstages.yaml +++ b/config/crd/bases/janus-idp.io_backstages.yaml @@ -268,14 +268,13 @@ spec: properties: authSecretName: description: 'Name of the secret for database authentication. - Required for external database access. Optional for a local - database (EnableLocalDb=true) and if absent a secret will be - auto generated. The secret shall include information used for - the database access. An example for PostgreSQL DB access: "POSTGRES_PASSWORD": - "rl4s3Fh4ng3M4" "POSTGRES_PORT": "5432" "POSTGRES_USER": "postgres" - "POSTGRESQL_ADMIN_PASSWORD": "rl4s3Fh4ng3M4" "POSTGRES_HOST": - "backstage-psql-bs1" # For local database, set to "backstage-psql-".' + Optional. For a local database deployment (EnableLocalDb=true), + a secret will be auto generated if it does not exist. The secret + shall include information used for the database access. An example + for PostgreSQL DB access: "POSTGRES_PASSWORD": "rl4s3Fh4ng3M4" + "POSTGRES_PORT": "5432" "POSTGRES_USER": "postgres" "POSTGRESQL_ADMIN_PASSWORD": + "rl4s3Fh4ng3M4" "POSTGRES_HOST": "backstage-psql-bs1" # For + local database, set to "backstage-psql-".' type: string enableLocalDb: default: true diff --git a/controllers/backstage_controller.go b/controllers/backstage_controller.go index 07376c17..11be59ac 100644 --- a/controllers/backstage_controller.go +++ b/controllers/backstage_controller.go @@ -131,7 +131,7 @@ func (r *BackstageReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, fmt.Errorf("failed to sync Database Service: %w", err) } setStatusCondition(&backstage, bs.LocalDbSynced, v1.ConditionTrue, bs.SyncOK, "") - } else if isLocalDbDeployed(backstage) { // EnableLocalDb is off but local db has been deployed. Clean up the deployed local db resources + } else { // Clean up the deployed local db resources if any if err := r.cleanupLocalDbResources(ctx, backstage); err != nil { setStatusCondition(&backstage, bs.LocalDbSynced, v1.ConditionFalse, bs.SyncFailed, fmt.Sprintf("failed to delete Database Services:%s", err.Error())) return ctrl.Result{}, fmt.Errorf("failed to delete Database Service: %w", err) @@ -263,28 +263,7 @@ func setStatusCondition(backstage *bs.Backstage, condType string, status v1.Cond }) } -func isSynced(backstage bs.Backstage) bool { - if cond := meta.FindStatusCondition(backstage.Status.Conditions, bs.RuntimeConditionSynced); cond != nil { - return cond.Status == v1.ConditionTrue - } - return false -} - -func isRouteDeployed(backstage bs.Backstage) bool { - if cond := meta.FindStatusCondition(backstage.Status.Conditions, bs.RouteSynced); cond != nil { - return cond.Status == v1.ConditionTrue && cond.Reason == bs.SyncOK - } - return false -} - -func isLocalDbDeployed(backstage bs.Backstage) bool { - if cond := meta.FindStatusCondition(backstage.Status.Conditions, bs.LocalDbSynced); cond != nil { - return cond.Status == v1.ConditionTrue && cond.Reason == bs.SyncOK - } - return false -} - -// cleanupResource deletes the resource from the cluster if exists +// cleanupResource deletes the resource that was previously deployed by the operator from the cluster func (r *BackstageReconciler) cleanupResource(ctx context.Context, obj client.Object, backstage bs.Backstage) (bool, error) { err := r.Get(ctx, types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()}, obj) if err != nil { diff --git a/controllers/backstage_controller_test.go b/controllers/backstage_controller_test.go index 46f38cd3..020c2528 100644 --- a/controllers/backstage_controller_test.go +++ b/controllers/backstage_controller_test.go @@ -25,6 +25,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/pointer" @@ -1345,7 +1346,7 @@ plugins: [] BeforeEach(func() { backstage = buildBackstageCR(bsv1alpha1.BackstageSpec{ Application: &bsv1alpha1.Application{ - ImagePullSecrets: []string{ips1, ips2}, + ImagePullSecrets: &[]string{ips1, ips2}, }, }) err := k8sClient.Create(ctx, backstage) @@ -1507,7 +1508,7 @@ plugins: [] verifyBackstageInstance(ctx) }) }) - It("should fail to reconcile a custom resource for default Backstage without existing secret", func() { + It("should reconcile a custom resource for default Backstage without existing secret", func() { backstage := buildBackstageCR(bsv1alpha1.BackstageSpec{ Database: bsv1alpha1.Database{ EnableLocalDb: pointer.Bool(false), @@ -1526,8 +1527,7 @@ plugins: [] _, err = backstageReconciler.Reconcile(ctx, reconcile.Request{ NamespacedName: types.NamespacedName{Name: backstageName, Namespace: ns}, }) - Expect(err).Should(HaveOccurred()) - Expect(err.Error()).Should(ContainSubstring("authSecretName is required if enableLocalDb is false")) + Expect(err).Should(Not(HaveOccurred())) }) }) }) @@ -1540,3 +1540,17 @@ func findElementsByPredicate[T any](l []T, predicate func(t T) bool) (result []T } return result } + +func isLocalDbDeployed(backstage bsv1alpha1.Backstage) bool { + if cond := meta.FindStatusCondition(backstage.Status.Conditions, bsv1alpha1.LocalDbSynced); cond != nil { + return cond.Status == metav1.ConditionTrue && cond.Reason == bsv1alpha1.SyncOK + } + return false +} + +func isSynced(backstage bsv1alpha1.Backstage) bool { + if cond := meta.FindStatusCondition(backstage.Status.Conditions, bsv1alpha1.RuntimeConditionSynced); cond != nil { + return cond.Status == metav1.ConditionTrue + } + return false +} diff --git a/controllers/backstage_deployment.go b/controllers/backstage_deployment.go index dbc8ea23..40ff6995 100644 --- a/controllers/backstage_deployment.go +++ b/controllers/backstage_deployment.go @@ -22,7 +22,6 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" bs "janus-idp.io/backstage-operator/api/v1alpha1" @@ -161,8 +160,11 @@ func (r *BackstageReconciler) reconcileBackstageDeployment(ctx context.Context, return nil } -func (r *BackstageReconciler) deploymentObjectMutFun(ctx context.Context, deployment *appsv1.Deployment, backstage bs.Backstage, ns string) controllerutil.MutateFn { +func (r *BackstageReconciler) deploymentObjectMutFun(ctx context.Context, targetDeployment *appsv1.Deployment, backstage bs.Backstage, ns string) controllerutil.MutateFn { return func() error { + deployment := &appsv1.Deployment{} + targetDeployment.ObjectMeta.DeepCopyInto(&deployment.ObjectMeta) + err := r.readConfigMapOrDefault(ctx, backstage.Spec.RawRuntimeConfig.BackstageConfigName, "deployment.yaml", ns, deployment) if err != nil { return fmt.Errorf("failed to read config: %s", err) @@ -202,6 +204,9 @@ func (r *BackstageReconciler) deploymentObjectMutFun(ctx context.Context, deploy return fmt.Errorf("failed to set owner reference: %s", err) } } + + deployment.ObjectMeta.DeepCopyInto(&targetDeployment.ObjectMeta) + deployment.Spec.DeepCopyInto(&targetDeployment.Spec) return nil } } @@ -255,9 +260,6 @@ func (r *BackstageReconciler) addEnvVars(backstage bs.Backstage, ns string, depl } func (r *BackstageReconciler) validateAndUpdatePsqlSecretRef(backstage bs.Backstage, deployment *appsv1.Deployment) error { - if len(backstage.Spec.Database.AuthSecretName) == 0 && !pointer.BoolDeref(backstage.Spec.Database.EnableLocalDb, true) { - return fmt.Errorf("authSecretName is required if enableLocalDb is false") - } for i, c := range deployment.Spec.Template.Spec.Containers { if c.Name != _defaultBackstageMainContainerName { continue @@ -301,13 +303,14 @@ func (r *BackstageReconciler) applyApplicationParamsFromCR(backstage bs.Backstag container.Image = *backstage.Spec.Application.Image }) } - - if len(backstage.Spec.Application.ImagePullSecrets) > 0 { // use image pull secrets from the CR spec + if backstage.Spec.Application.ImagePullSecrets != nil { // use image pull secrets from the CR spec deployment.Spec.Template.Spec.ImagePullSecrets = nil - for _, imagePullSecret := range backstage.Spec.Application.ImagePullSecrets { - deployment.Spec.Template.Spec.ImagePullSecrets = append(deployment.Spec.Template.Spec.ImagePullSecrets, v1.LocalObjectReference{ - Name: imagePullSecret, - }) + if len(*backstage.Spec.Application.ImagePullSecrets) > 0 { + for _, imagePullSecret := range *backstage.Spec.Application.ImagePullSecrets { + deployment.Spec.Template.Spec.ImagePullSecrets = append(deployment.Spec.Template.Spec.ImagePullSecrets, v1.LocalObjectReference{ + Name: imagePullSecret, + }) + } } } } diff --git a/controllers/backstage_route.go b/controllers/backstage_route.go index dfde75fe..5fbdaf9d 100644 --- a/controllers/backstage_route.go +++ b/controllers/backstage_route.go @@ -38,14 +38,11 @@ func (r *BackstageReconciler) reconcileBackstageRoute(ctx context.Context, backs } if !shouldCreateRoute(*backstage) { - if isRouteDeployed(*backstage) { - deleted, err := r.cleanupResource(ctx, route, *backstage) - if err == nil && deleted { - setStatusCondition(backstage, bs.RouteSynced, metav1.ConditionTrue, bs.Deleted, "") - } - return err + deleted, err := r.cleanupResource(ctx, route, *backstage) + if err == nil && deleted { + setStatusCondition(backstage, bs.RouteSynced, metav1.ConditionTrue, bs.Deleted, "") } - return nil + return err } if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, route, r.routeObjectMutFun(ctx, route, *backstage, ns)); err != nil { @@ -59,8 +56,11 @@ func (r *BackstageReconciler) reconcileBackstageRoute(ctx context.Context, backs return nil } -func (r *BackstageReconciler) routeObjectMutFun(ctx context.Context, route *openshift.Route, backstage bs.Backstage, ns string) controllerutil.MutateFn { +func (r *BackstageReconciler) routeObjectMutFun(ctx context.Context, targetRoute *openshift.Route, backstage bs.Backstage, ns string) controllerutil.MutateFn { return func() error { + route := &openshift.Route{} + targetRoute.ObjectMeta.DeepCopyInto(&route.ObjectMeta) + err := r.readConfigMapOrDefault(ctx, backstage.Spec.RawRuntimeConfig.BackstageConfigName, "route.yaml", ns, route) if err != nil { return err @@ -80,6 +80,9 @@ func (r *BackstageReconciler) routeObjectMutFun(ctx context.Context, route *open return fmt.Errorf("failed to set owner reference: %s", err) } } + + route.ObjectMeta.DeepCopyInto(&targetRoute.ObjectMeta) + route.Spec.DeepCopyInto(&targetRoute.Spec) return nil } } diff --git a/controllers/backstage_service.go b/controllers/backstage_service.go index 305577a9..e3ecbecd 100644 --- a/controllers/backstage_service.go +++ b/controllers/backstage_service.go @@ -44,9 +44,11 @@ func (r *BackstageReconciler) reconcileBackstageService(ctx context.Context, bac // selector for deploy.spec.template.spec.meta.label // targetPort: http for deploy.spec.template.spec.containers.ports.name=http -func (r *BackstageReconciler) serviceObjectMutFun(ctx context.Context, service *corev1.Service, backstage bs.Backstage, ns string) controllerutil.MutateFn { +func (r *BackstageReconciler) serviceObjectMutFun(ctx context.Context, targetService *corev1.Service, backstage bs.Backstage, ns string) controllerutil.MutateFn { return func() error { - tmp := service.DeepCopy() + service := &corev1.Service{} + targetService.ObjectMeta.DeepCopyInto(&service.ObjectMeta) + err := r.readConfigMapOrDefault(ctx, backstage.Spec.RawRuntimeConfig.BackstageConfigName, "service.yaml", ns, service) if err != nil { return err @@ -64,18 +66,21 @@ func (r *BackstageReconciler) serviceObjectMutFun(ctx context.Context, service * } } - if len(tmp.Spec.ClusterIP) > 0 && service.Spec.ClusterIP != "" && service.Spec.ClusterIP != "None" && service.Spec.ClusterIP != tmp.Spec.ClusterIP { - return fmt.Errorf("backstage service IP can not be updated: %s, %s, %s", tmp.Name, tmp.Spec.ClusterIP, service.Spec.ClusterIP) + if len(targetService.Spec.ClusterIP) > 0 && service.Spec.ClusterIP != "" && service.Spec.ClusterIP != "None" && service.Spec.ClusterIP != targetService.Spec.ClusterIP { + return fmt.Errorf("backstage service IP can not be updated: %s, current: %s, new: %s", targetService.Name, targetService.Spec.ClusterIP, service.Spec.ClusterIP) } - service.Spec.ClusterIP = tmp.Spec.ClusterIP - for _, ip1 := range tmp.Spec.ClusterIPs { + service.Spec.ClusterIP = targetService.Spec.ClusterIP + for _, ip1 := range targetService.Spec.ClusterIPs { for _, ip2 := range service.Spec.ClusterIPs { if len(ip1) > 0 && ip2 != "" && ip2 != "None" && ip1 != ip2 { - return fmt.Errorf("backstage service IPs can not be updated: %s, %v, %v", tmp.Name, tmp.Spec.ClusterIPs, service.Spec.ClusterIPs) + return fmt.Errorf("backstage service IPs can not be updated: %s, current: %v, new: %v", targetService.Name, targetService.Spec.ClusterIPs, service.Spec.ClusterIPs) } } } - service.Spec.ClusterIPs = tmp.Spec.ClusterIPs + service.Spec.ClusterIPs = targetService.Spec.ClusterIPs + + service.ObjectMeta.DeepCopyInto(&targetService.ObjectMeta) + service.Spec.DeepCopyInto(&targetService.Spec) return nil } } diff --git a/controllers/local_db_services.go b/controllers/local_db_services.go index df8a0f36..ae850b31 100644 --- a/controllers/local_db_services.go +++ b/controllers/local_db_services.go @@ -88,14 +88,15 @@ func (r *BackstageReconciler) reconcilePsqlService(ctx context.Context, backstag return nil } -func (r *BackstageReconciler) psqlServiceObjectMutFun(ctx context.Context, service *corev1.Service, backstage bs.Backstage, label, ns, key string) controllerutil.MutateFn { +func (r *BackstageReconciler) psqlServiceObjectMutFun(ctx context.Context, targetService *corev1.Service, backstage bs.Backstage, label, ns, key string) controllerutil.MutateFn { return func() error { - tmp := service.DeepCopy() + service := &corev1.Service{} + targetService.ObjectMeta.DeepCopyInto(&service.ObjectMeta) err := r.readConfigMapOrDefault(ctx, backstage.Spec.RawRuntimeConfig.LocalDbConfigName, key, ns, service) if err != nil { return err } - service.SetName(tmp.Name) + service.SetName(targetService.Name) setBackstageLocalDbLabel(&service.ObjectMeta.Labels, label) setBackstageLocalDbLabel(&service.Spec.Selector, label) @@ -104,19 +105,21 @@ func (r *BackstageReconciler) psqlServiceObjectMutFun(ctx context.Context, servi return fmt.Errorf(ownerRefFmt, err) } } - if len(tmp.Spec.ClusterIP) > 0 && service.Spec.ClusterIP != "" && service.Spec.ClusterIP != "None" && service.Spec.ClusterIP != tmp.Spec.ClusterIP { - return fmt.Errorf("db service IP can not be updated: %s, %s, %s", tmp.Name, tmp.Spec.ClusterIP, service.Spec.ClusterIP) + if len(targetService.Spec.ClusterIP) > 0 && service.Spec.ClusterIP != "" && service.Spec.ClusterIP != "None" && service.Spec.ClusterIP != targetService.Spec.ClusterIP { + return fmt.Errorf("db service IP can not be updated: %s, %s, %s", targetService.Name, targetService.Spec.ClusterIP, service.Spec.ClusterIP) } - service.Spec.ClusterIP = tmp.Spec.ClusterIP - for _, ip1 := range tmp.Spec.ClusterIPs { + service.Spec.ClusterIP = targetService.Spec.ClusterIP + for _, ip1 := range targetService.Spec.ClusterIPs { for _, ip2 := range service.Spec.ClusterIPs { if len(ip1) > 0 && ip2 != "" && ip2 != "None" && ip1 != ip2 { - return fmt.Errorf("db service IPs can not be updated: %s, %v, %v", tmp.Name, tmp.Spec.ClusterIPs, service.Spec.ClusterIPs) + return fmt.Errorf("db service IPs can not be updated: %s, %v, %v", targetService.Name, targetService.Spec.ClusterIPs, service.Spec.ClusterIPs) } } } - service.Spec.ClusterIPs = tmp.Spec.ClusterIPs + service.Spec.ClusterIPs = targetService.Spec.ClusterIPs + service.ObjectMeta.DeepCopyInto(&targetService.ObjectMeta) + service.Spec.DeepCopyInto(&targetService.Spec) return nil } } diff --git a/controllers/local_db_statefulset.go b/controllers/local_db_statefulset.go index 275f11ea..01447ea2 100644 --- a/controllers/local_db_statefulset.go +++ b/controllers/local_db_statefulset.go @@ -153,8 +153,10 @@ func (r *BackstageReconciler) reconcileLocalDbStatefulSet(ctx context.Context, b return nil } -func (r *BackstageReconciler) localDBStatefulSetMutFun(ctx context.Context, statefulSet *appsv1.StatefulSet, backstage bs.Backstage, ns string) controllerutil.MutateFn { +func (r *BackstageReconciler) localDBStatefulSetMutFun(ctx context.Context, targetStatefulSet *appsv1.StatefulSet, backstage bs.Backstage, ns string) controllerutil.MutateFn { return func() error { + statefulSet := &appsv1.StatefulSet{} + targetStatefulSet.ObjectMeta.DeepCopyInto(&statefulSet.ObjectMeta) err := r.readConfigMapOrDefault(ctx, backstage.Spec.RawRuntimeConfig.LocalDbConfigName, "db-statefulset.yaml", ns, statefulSet) if err != nil { return err @@ -194,6 +196,9 @@ func (r *BackstageReconciler) localDBStatefulSetMutFun(ctx context.Context, stat return fmt.Errorf(ownerRefFmt, err) } } + + statefulSet.ObjectMeta.DeepCopyInto(&targetStatefulSet.ObjectMeta) + statefulSet.Spec.DeepCopyInto(&targetStatefulSet.Spec) return nil } }