From 13e6a20d850e20a97b4f82b07f4f9c6ee5276b54 Mon Sep 17 00:00:00 2001 From: Thuan Vo Date: Tue, 21 May 2024 14:42:38 -0700 Subject: [PATCH] fix(status): controller should set approriate status in CR (#825) Signed-off-by: Thuan Vo --- api/v1beta1/cryostat_types.go | 8 +-- api/v1beta2/cryostat_types.go | 11 ++++ ...yostat-operator.clusterserviceversion.yaml | 14 +++- .../operator.cryostat.io_cryostats.yaml | 25 ++++++-- .../bases/operator.cryostat.io_cryostats.yaml | 25 ++++++-- ...yostat-operator.clusterserviceversion.yaml | 12 ++++ internal/controllers/reconciler_test.go | 38 ++++++++--- internal/controllers/secrets.go | 64 +++++++++++-------- internal/test/resources.go | 15 ++++- 9 files changed, 158 insertions(+), 54 deletions(-) diff --git a/api/v1beta1/cryostat_types.go b/api/v1beta1/cryostat_types.go index 3b7b04000..b9f51a95e 100644 --- a/api/v1beta1/cryostat_types.go +++ b/api/v1beta1/cryostat_types.go @@ -141,8 +141,6 @@ type CryostatStatus struct { // +optional // +operator-sdk:csv:customresourcedefinitions:type=status,order=2,xDescriptors={"urn:alm:descriptor:io.kubernetes:Secret"} GrafanaSecret string `json:"grafanaSecret,omitempty"` - // Name of the Secret containing the cryostat storage connection key - StorageSecret string `json:"storageSecret,omitempty"` // Address of the deployed Cryostat web application. // +operator-sdk:csv:customresourcedefinitions:type=status,order=1,xDescriptors={"urn:alm:descriptor:org.w3:link"} ApplicationURL string `json:"applicationUrl"` @@ -315,12 +313,12 @@ type ServiceConfigList struct { // Specification for the service responsible for the Cryostat Grafana dashboard. // +optional GrafanaConfig *GrafanaServiceConfig `json:"grafanaConfig,omitempty"` - // Specification for the service responsible for the cryostat-reports sidecars. + // Specification for the service responsible for the Cryostat reports sidecars. // +optional ReportsConfig *ReportsServiceConfig `json:"reportsConfig,omitempty"` - // Specification for the service responsible for the cryostat storage container. + // Specification for the service responsible for the Cryostat storage container. // +optional - StorageConfig *StorageServiceConfig `json:"storageConfig,omitEmpty"` + StorageConfig *StorageServiceConfig `json:"storageConfig,omitempty"` } // NetworkConfiguration provides customization for how to expose a Cryostat diff --git a/api/v1beta2/cryostat_types.go b/api/v1beta2/cryostat_types.go index 81a189a21..c93a30c24 100644 --- a/api/v1beta2/cryostat_types.go +++ b/api/v1beta2/cryostat_types.go @@ -159,6 +159,14 @@ type CryostatStatus struct { // Address of the deployed Cryostat web application. // +operator-sdk:csv:customresourcedefinitions:type=status,order=1,xDescriptors={"urn:alm:descriptor:org.w3:link"} ApplicationURL string `json:"applicationUrl"` + // Name of the Secret containing the Cryostat storage connection key. + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=status,order=2,xDescriptors={"urn:alm:descriptor:io.kubernetes:Secret"} + StorageSecret string `json:"storageSecret,omitempty"` + // Name of the Secret containing the Cryostat database connection and encryption keys. + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=status,order=2,xDescriptors={"urn:alm:descriptor:io.kubernetes:Secret"} + DatabaseSecret string `json:"databaseSecret,omitempty"` } // CryostatConditionType refers to a Condition type that may be used in status.conditions @@ -414,6 +422,9 @@ type TargetConnectionCacheOptions struct { // to deploy the Cryostat application. // +operator-sdk:csv:customresourcedefinitions:resources={{Deployment,v1},{Ingress,v1},{PersistentVolumeClaim,v1},{Secret,v1},{Service,v1},{Route,v1},{ConsoleLink,v1}} // +kubebuilder:printcolumn:name="Application URL",type=string,JSONPath=`.status.applicationUrl` +// +kubebuilder:printcolumn:name="Target Namespaces",type=string,JSONPath=`.status.targetNamespaces` +// +kubebuilder:printcolumn:name="Storage Secret",type=string,JSONPath=`.status.storageSecret` +// +kubebuilder:printcolumn:name="Database Secret",type=string,JSONPath=`.status.databaseSecret` type Cryostat struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index 0d794ced6..abeaef42e 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -53,7 +53,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:3.0.0-dev - createdAt: "2024-05-11T06:47:54Z" + createdAt: "2024-05-20T05:12:58Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { @@ -889,6 +889,18 @@ spec: path: applicationUrl x-descriptors: - urn:alm:descriptor:org.w3:link + - description: Name of the Secret containing the Cryostat database connection + and encryption keys. + displayName: Database Secret + path: databaseSecret + x-descriptors: + - urn:alm:descriptor:io.kubernetes:Secret + - description: Name of the Secret containing the Cryostat storage connection + key. + displayName: Storage Secret + path: storageSecret + x-descriptors: + - urn:alm:descriptor:io.kubernetes:Secret - description: List of namespaces that Cryostat has been configured and authorized to access and profile. displayName: Target Namespaces diff --git a/bundle/manifests/operator.cryostat.io_cryostats.yaml b/bundle/manifests/operator.cryostat.io_cryostats.yaml index 5b4a334a8..b095d06d1 100644 --- a/bundle/manifests/operator.cryostat.io_cryostats.yaml +++ b/bundle/manifests/operator.cryostat.io_cryostats.yaml @@ -4646,7 +4646,7 @@ spec: type: object reportsConfig: description: Specification for the service responsible for the - cryostat-reports sidecars. + Cryostat reports sidecars. properties: annotations: additionalProperties: @@ -4672,7 +4672,7 @@ spec: type: object storageConfig: description: Specification for the service responsible for the - cryostat storage container. + Cryostat storage container. properties: annotations: additionalProperties: @@ -5085,10 +5085,6 @@ spec: grafanaSecret: description: Name of the Secret containing the generated Grafana credentials. type: string - storageSecret: - description: Name of the Secret containing the cryostat storage connection - key - type: string required: - applicationUrl type: object @@ -5101,6 +5097,15 @@ spec: - jsonPath: .status.applicationUrl name: Application URL type: string + - jsonPath: .status.targetNamespaces + name: Target Namespaces + type: string + - jsonPath: .status.storageSecret + name: Storage Secret + type: string + - jsonPath: .status.databaseSecret + name: Database Secret + type: string name: v1beta2 schema: openAPIV3Schema: @@ -9832,6 +9837,14 @@ spec: - type type: object type: array + databaseSecret: + description: Name of the Secret containing the Cryostat database connection + and encryption keys. + type: string + storageSecret: + description: Name of the Secret containing the Cryostat storage connection + key. + type: string targetNamespaces: description: List of namespaces that Cryostat has been configured and authorized to access and profile. diff --git a/config/crd/bases/operator.cryostat.io_cryostats.yaml b/config/crd/bases/operator.cryostat.io_cryostats.yaml index 7d6fb2d6b..3102d2e92 100644 --- a/config/crd/bases/operator.cryostat.io_cryostats.yaml +++ b/config/crd/bases/operator.cryostat.io_cryostats.yaml @@ -4636,7 +4636,7 @@ spec: type: object reportsConfig: description: Specification for the service responsible for the - cryostat-reports sidecars. + Cryostat reports sidecars. properties: annotations: additionalProperties: @@ -4662,7 +4662,7 @@ spec: type: object storageConfig: description: Specification for the service responsible for the - cryostat storage container. + Cryostat storage container. properties: annotations: additionalProperties: @@ -5075,10 +5075,6 @@ spec: grafanaSecret: description: Name of the Secret containing the generated Grafana credentials. type: string - storageSecret: - description: Name of the Secret containing the cryostat storage connection - key - type: string required: - applicationUrl type: object @@ -5091,6 +5087,15 @@ spec: - jsonPath: .status.applicationUrl name: Application URL type: string + - jsonPath: .status.targetNamespaces + name: Target Namespaces + type: string + - jsonPath: .status.storageSecret + name: Storage Secret + type: string + - jsonPath: .status.databaseSecret + name: Database Secret + type: string name: v1beta2 schema: openAPIV3Schema: @@ -9822,6 +9827,14 @@ spec: - type type: object type: array + databaseSecret: + description: Name of the Secret containing the Cryostat database connection + and encryption keys. + type: string + storageSecret: + description: Name of the Secret containing the Cryostat storage connection + key. + type: string targetNamespaces: description: List of namespaces that Cryostat has been configured and authorized to access and profile. diff --git a/config/manifests/bases/cryostat-operator.clusterserviceversion.yaml b/config/manifests/bases/cryostat-operator.clusterserviceversion.yaml index 49582ba1e..831922035 100644 --- a/config/manifests/bases/cryostat-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/cryostat-operator.clusterserviceversion.yaml @@ -428,6 +428,18 @@ spec: path: applicationUrl x-descriptors: - urn:alm:descriptor:org.w3:link + - description: Name of the Secret containing the Cryostat database connection + and encryption keys. + displayName: Database Secret + path: databaseSecret + x-descriptors: + - urn:alm:descriptor:io.kubernetes:Secret + - description: Name of the Secret containing the Cryostat storage connection + key. + displayName: Storage Secret + path: storageSecret + x-descriptors: + - urn:alm:descriptor:io.kubernetes:Secret - description: List of namespaces that Cryostat has been configured and authorized to access and profile. displayName: Target Namespaces diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index bd1bbd99b..6e09e6524 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -66,7 +66,7 @@ type cryostatTestInput struct { func (c *controllerTest) commonBeforeEach() *cryostatTestInput { t := &cryostatTestInput{ TestReconcilerConfig: test.TestReconcilerConfig{ - GeneratedPasswords: []string{"auth_cookie_secret", "credentials_database", "encryption_key", "object_storage", "jmx", "keystore"}, + GeneratedPasswords: []string{"auth_cookie_secret", "connection_key", "encryption_key", "object_storage", "jmx", "keystore"}, }, TestResources: &test.TestResources{ Name: "cryostat", @@ -145,7 +145,7 @@ func resourceChecks() []resourceCheck { {func(t *cryostatTestInput) { t.expectPVC(t.NewDefaultPVC()) }, "persistent volume claim"}, - {(*cryostatTestInput).expectCredentialsDatabaseSecret, "credentials database secret"}, + {(*cryostatTestInput).expectDatabaseSecret, "database secret"}, {(*cryostatTestInput).expectStorageSecret, "object storage secret"}, {(*cryostatTestInput).expectJMXSecret, "JMX secret"}, {(*cryostatTestInput).expectCoreService, "core service"}, @@ -164,6 +164,12 @@ func expectSuccessful(t **cryostatTestInput) { It("should set ApplicationURL in CR Status", func() { (*t).expectStatusApplicationURL() }) + It("should set Database Secret in CR Status", func() { + (*t).expectStatusDatabaseSecret() + }) + It("should set Storage Secret in CR Status", func() { + (*t).expectStatusStorageSecret() + }) It("should set TLSSetupComplete condition", func() { (*t).checkConditionPresent(operatorv1beta2.ConditionTypeTLSSetupComplete, metav1.ConditionTrue, "AllCertificatesReady") @@ -496,7 +502,7 @@ func (c *controllerTest) commonTests() { Expect(secret.StringData["CRYOSTAT_RJMX_PASS"]).To(Equal(oldSecret.StringData["CRYOSTAT_RJMX_PASS"])) }) }) - Context("with an existing Credentials Database Secret", func() { + Context("with an existing Database Secret", func() { var cr *model.CryostatInstance var oldSecret *corev1.Secret BeforeEach(func() { @@ -1592,9 +1598,11 @@ func (c *controllerTest) commonTests() { }) }) }) - Context("with secret provided for database password", func() { + Context("with secret provided for database", func() { + var customSecret *corev1.Secret BeforeEach(func() { - t.objs = append(t.objs, t.NewCryostatWithDatabaseSecretProvided().Object) + customSecret = t.NewCustomDatabaseSecret() + t.objs = append(t.objs, t.NewCryostatWithDatabaseSecretProvided().Object, customSecret) }) JustBeforeEach(func() { t.reconcileCryostatFully() @@ -1602,16 +1610,20 @@ func (c *controllerTest) commonTests() { It("should configure deployment appropriately", func() { t.expectMainDeployment() }) + It("should set Database Secret in CR Status", func() { + instance := t.getCryostatInstance() + Expect(instance.Status.DatabaseSecret).To(Equal(customSecret.Name)) + }) It("should not generate default secret", func() { secret := &corev1.Secret{} err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-db", Namespace: t.Namespace}, secret) Expect(kerrors.IsNotFound(err)).To(BeTrue()) }) - Context("with an existing Credentials Database Secret", func() { + Context("with an existing Database Secret", func() { BeforeEach(func() { t.objs = append(t.objs, t.NewDatabaseSecret()) }) - It("should not delete the existing Credentials Database Secret", func() { + It("should not delete the existing Database Secret", func() { secret := &corev1.Secret{} err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-db", Namespace: t.Namespace}, secret) Expect(err).ToNot(HaveOccurred()) @@ -2356,7 +2368,7 @@ func (t *cryostatTestInput) expectEmptyDir(expectedEmptyDir *corev1.EmptyDirVolu Expect(emptyDir.SizeLimit).To(Equal(expectedEmptyDir.SizeLimit)) } -func (t *cryostatTestInput) expectCredentialsDatabaseSecret() { +func (t *cryostatTestInput) expectDatabaseSecret() { secret := &corev1.Secret{} err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-db", Namespace: t.Namespace}, secret) Expect(err).ToNot(HaveOccurred()) @@ -2398,6 +2410,16 @@ func (t *cryostatTestInput) expectStatusApplicationURL() { Expect(instance.Status.ApplicationURL).To(Equal(fmt.Sprintf("https://%s.example.com", t.Name))) } +func (t *cryostatTestInput) expectStatusDatabaseSecret() { + instance := t.getCryostatInstance() + Expect(instance.Status.DatabaseSecret).To(Equal(fmt.Sprintf("%s-db", t.Name))) +} + +func (t *cryostatTestInput) expectStatusStorageSecret() { + instance := t.getCryostatInstance() + Expect(instance.Status.StorageSecret).To(Equal(fmt.Sprintf("%s-storage-secret-key", t.Name))) +} + func (t *cryostatTestInput) expectDeploymentHasCertSecrets() { deployment := &appsv1.Deployment{} err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name, Namespace: t.Namespace}, deployment) diff --git a/internal/controllers/secrets.go b/internal/controllers/secrets.go index 119a02d01..cb1961220 100644 --- a/internal/controllers/secrets.go +++ b/internal/controllers/secrets.go @@ -93,34 +93,42 @@ func (r *Reconciler) reconcileJMXSecret(ctx context.Context, cr *model.CryostatI } // databaseSecretNameSuffix is the suffix to be appended to the name of a -// Cryostat CR to name its credentials database secret +// Cryostat CR to name its database secret const databaseSecretNameSuffix = "-db" func (r *Reconciler) reconcileDatabaseConnectionSecret(ctx context.Context, cr *model.CryostatInstance) error { - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: cr.Name + databaseSecretNameSuffix, - Namespace: cr.InstallNamespace, - }, - } - + var secretName string secretProvided := cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecretName != nil if secretProvided { - return nil // Do not delete default secret to allow reverting to use default if needed - } - - return r.createOrUpdateSecret(ctx, secret, cr.Object, func() error { - if secret.StringData == nil { - secret.StringData = map[string]string{} + // Do not delete default secret to allow reverting to use default if needed + secretName = *cr.Spec.DatabaseOptions.SecretName + } else { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: cr.Name + databaseSecretNameSuffix, + Namespace: cr.InstallNamespace, + }, } - - // Password is generated, so don't regenerate it when updating - if secret.CreationTimestamp.IsZero() { - secret.StringData[constants.DatabaseSecretConnectionKey] = r.GenPasswd(32) - secret.StringData[constants.DatabaseSecretEncryptionKey] = r.GenPasswd(32) + err := r.createOrUpdateSecret(ctx, secret, cr.Object, func() error { + if secret.StringData == nil { + secret.StringData = map[string]string{} + } + + // Password is generated, so don't regenerate it when updating + if secret.CreationTimestamp.IsZero() { + secret.StringData[constants.DatabaseSecretConnectionKey] = r.GenPasswd(32) + secret.StringData[constants.DatabaseSecretEncryptionKey] = r.GenPasswd(32) + } + return nil + }) + + if err != nil { + return err } - return nil - }) + secretName = secret.Name + } + cr.Status.DatabaseSecret = secretName + return r.Client.Status().Update(ctx, cr.Object) } // storageSecretNameSuffix is the suffix to be appended to the name of a @@ -138,12 +146,7 @@ func (r *Reconciler) reconcileStorageSecret(ctx context.Context, cr *model.Cryos }, } - // secretProvided := cr.Spec.JmxCredentialsDatabaseOptions != nil && cr.Spec.JmxCredentialsDatabaseOptions.DatabaseSecretName != nil - // if secretProvided { - // return nil // Do not delete default secret to allow reverting to use default if needed - // } - - return r.createOrUpdateSecret(ctx, secret, cr.Object, func() error { + err := r.createOrUpdateSecret(ctx, secret, cr.Object, func() error { if secret.StringData == nil { secret.StringData = map[string]string{} } @@ -154,6 +157,13 @@ func (r *Reconciler) reconcileStorageSecret(ctx context.Context, cr *model.Cryos } return nil }) + + if err != nil { + return err + } + + cr.Status.StorageSecret = secret.Name + return r.Client.Status().Update(ctx, cr.Object) } func (r *Reconciler) createOrUpdateSecret(ctx context.Context, secret *corev1.Secret, owner metav1.Object, diff --git a/internal/test/resources.go b/internal/test/resources.go index 497d61de2..a736953ef 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -855,12 +855,25 @@ func (r *TestResources) NewDatabaseSecret() *corev1.Secret { Namespace: r.Namespace, }, StringData: map[string]string{ - "CONNECTION_KEY": "credentials_database", + "CONNECTION_KEY": "connection_key", "ENCRYPTION_KEY": "encryption_key", }, } } +func (r *TestResources) NewCustomDatabaseSecret() *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: providedDatabaseSecretName, + Namespace: r.Namespace, + }, + StringData: map[string]string{ + "CONNECTION_KEY": "custom-connection_database", + "ENCRYPTION_KEY": "custom-encryption_key", + }, + } +} + func (r *TestResources) NewStorageSecret() *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{