From 52053cbfe3eedeb4d5e467e3eafb2b0c2e5ed8ea Mon Sep 17 00:00:00 2001 From: jgilaber Date: Wed, 11 Dec 2024 12:48:42 +0100 Subject: [PATCH] Follow-up changes for WatcherAPI from PR 13 Following the review on PR 13, there were a few changes suggested. This PR does 2 of them: 1. Make the Secret field on the Spec required, and remove the default from WatcherAPI. This means that the field is no longer shared between the Watcher and WatcherAPI kinds. 2. Add more functional tests to test scenarios where the secret or database are not present. To test the change 1, this commit also changes the watcherapi kuttl test to explicitely create a Secret, mimicking what we'll later do via the Watcher controller. --- .../watcher.openstack.org_watcherapis.yaml | 2 +- api/v1beta1/common_types.go | 9 +-- api/v1beta1/watcherapi_types.go | 3 + .../watcher.openstack.org_watcherapis.yaml | 2 +- .../samples/watcher_v1beta1_watcherapi.yaml | 1 + controllers/watcherapi_controller.go | 6 +- .../functional/watcherapi_controller_test.go | 65 ++++++++++++++++++- .../watcher-api/01-cleanup-watcherapi.yaml | 3 + .../default/watcher-api/03-assert.yaml | 2 +- .../watcher-api/03-deploy-watcher-api.yaml | 11 +++- 10 files changed, 92 insertions(+), 12 deletions(-) diff --git a/api/bases/watcher.openstack.org_watcherapis.yaml b/api/bases/watcher.openstack.org_watcherapis.yaml index 992fdba..a7e0f63 100644 --- a/api/bases/watcher.openstack.org_watcherapis.yaml +++ b/api/bases/watcher.openstack.org_watcherapis.yaml @@ -62,7 +62,6 @@ spec: type: string type: object secret: - default: osp-secret description: Secret containing all passwords / keys needed type: string serviceUser: @@ -72,6 +71,7 @@ spec: type: string required: - databaseInstance + - secret type: object status: description: WatcherAPIStatus defines the observed state of WatcherAPI diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go index ce06af8..611cbc6 100644 --- a/api/v1beta1/common_types.go +++ b/api/v1beta1/common_types.go @@ -18,10 +18,6 @@ package v1beta1 // WatcherCommon defines a spec based reusable for all the CRDs type WatcherCommon struct { - // +kubebuilder:validation:Optional - // +kubebuilder:default=osp-secret - // Secret containing all passwords / keys needed - Secret string `json:"secret"` // +kubebuilder:validation:Optional // +kubebuilder:default=watcher @@ -55,6 +51,11 @@ type WatcherTemplate struct { // RabbitMQ instance name // Needed to request a transportURL that is created and used in Barbican RabbitMqClusterName string `json:"rabbitMqClusterName"` + + // +kubebuilder:validation:Optional + // +kubebuilder:default=osp-secret + // Secret containing all passwords / keys needed + Secret string `json:"secret"` } // PasswordSelector to identify the DB and AdminUser password from the Secret diff --git a/api/v1beta1/watcherapi_types.go b/api/v1beta1/watcherapi_types.go index 03a49d3..90cacf2 100644 --- a/api/v1beta1/watcherapi_types.go +++ b/api/v1beta1/watcherapi_types.go @@ -27,6 +27,9 @@ type WatcherAPISpec struct { // Important: Run "make" to regenerate code after modifying this file WatcherCommon `json:",inline"` + // +kubebuilder:validation:Required + // Secret containing all passwords / keys needed + Secret string `json:"secret"` } // WatcherAPIStatus defines the observed state of WatcherAPI diff --git a/config/crd/bases/watcher.openstack.org_watcherapis.yaml b/config/crd/bases/watcher.openstack.org_watcherapis.yaml index 992fdba..a7e0f63 100644 --- a/config/crd/bases/watcher.openstack.org_watcherapis.yaml +++ b/config/crd/bases/watcher.openstack.org_watcherapis.yaml @@ -62,7 +62,6 @@ spec: type: string type: object secret: - default: osp-secret description: Secret containing all passwords / keys needed type: string serviceUser: @@ -72,6 +71,7 @@ spec: type: string required: - databaseInstance + - secret type: object status: description: WatcherAPIStatus defines the observed state of WatcherAPI diff --git a/config/samples/watcher_v1beta1_watcherapi.yaml b/config/samples/watcher_v1beta1_watcherapi.yaml index 734ba1b..b593cf1 100644 --- a/config/samples/watcher_v1beta1_watcherapi.yaml +++ b/config/samples/watcher_v1beta1_watcherapi.yaml @@ -7,3 +7,4 @@ metadata: name: watcherapi-sample spec: databaseInstance: "openstack" + secret: "osp-secret" diff --git a/controllers/watcherapi_controller.go b/controllers/watcherapi_controller.go index c7b4bfc..19fee10 100644 --- a/controllers/watcherapi_controller.go +++ b/controllers/watcherapi_controller.go @@ -152,11 +152,11 @@ func (r *WatcherAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) db, err := mariadbv1.GetDatabaseByNameAndAccount(ctx, helper, watcher.DatabaseCRName, instance.Spec.DatabaseAccount, instance.Namespace) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( - condition.ServiceConfigReadyCondition, + condition.InputReadyCondition, condition.ErrorReason, condition.SeverityWarning, - condition.ServiceConfigReadyErrorMessage, - err.Error())) + condition.InputReadyErrorMessage, + fmt.Sprintf("couldn't get database %s and account %s", watcher.DatabaseCRName, instance.Spec.DatabaseAccount))) return ctrl.Result{}, err } diff --git a/tests/functional/watcherapi_controller_test.go b/tests/functional/watcherapi_controller_test.go index daaade8..e1ef701 100644 --- a/tests/functional/watcherapi_controller_test.go +++ b/tests/functional/watcherapi_controller_test.go @@ -11,13 +11,21 @@ import ( . "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers" mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" watcherv1beta1 "github.com/openstack-k8s-operators/watcher-operator/api/v1beta1" + "github.com/openstack-k8s-operators/watcher-operator/pkg/watcher" corev1 "k8s.io/api/core/v1" ) +var ( + MinimalWatcherAPISpec = map[string]interface{}{ + "secret": "osp-secret", + "databaseInstance": "openstack", + } +) + var _ = Describe("WatcherAPI controller with minimal spec values", func() { When("A Watcher instance is created from minimal spec", func() { BeforeEach(func() { - DeferCleanup(th.DeleteInstance, CreateWatcherAPI(watcherTest.Instance, MinimalWatcherSpec)) + DeferCleanup(th.DeleteInstance, CreateWatcherAPI(watcherTest.Instance, MinimalWatcherAPISpec)) }) It("should have the Spec fields defaulted", func() { @@ -173,4 +181,59 @@ var _ = Describe("WatcherAPI controller", func() { ) }) }) + When("A WatcherAPI instance without secret is created", func() { + BeforeEach(func() { + DeferCleanup(th.DeleteInstance, CreateWatcherAPI(watcherTest.Instance, GetDefaultWatcherAPISpec())) + }) + It("is missing the secret", func() { + th.ExpectConditionWithDetails( + watcherTest.Instance, + ConditionGetterFunc(WatcherAPIConditionGetter), + condition.InputReadyCondition, + corev1.ConditionFalse, + condition.RequestedReason, + condition.InputReadyWaitingMessage, + ) + }) + }) + When("A WatcherAPI instance without a database but with a secret is created", func() { + BeforeEach(func() { + secret := th.CreateSecret( + watcherTest.InternalTopLevelSecretName, + map[string][]byte{ + "WatcherPassword": []byte("service-password"), + }, + ) + DeferCleanup(k8sClient.Delete, ctx, secret) + DeferCleanup(th.DeleteInstance, CreateWatcherAPI(watcherTest.Instance, GetDefaultWatcherAPISpec())) + }) + It("should have input not ready", func() { + WatcherAPI := GetWatcherAPI(watcherTest.Instance) + customErrorString := fmt.Sprintf( + "couldn't get database %s and account %s", + watcher.DatabaseCRName, + WatcherAPI.Spec.DatabaseAccount, + ) + errorString := fmt.Sprintf( + condition.InputReadyErrorMessage, + customErrorString, + ) + th.ExpectConditionWithDetails( + watcherTest.Instance, + ConditionGetterFunc(WatcherAPIConditionGetter), + condition.InputReadyCondition, + corev1.ConditionFalse, + condition.ErrorReason, + errorString, + ) + }) + It("should have config service unknown", func() { + th.ExpectCondition( + watcherTest.Instance, + ConditionGetterFunc(WatcherAPIConditionGetter), + condition.ServiceConfigReadyCondition, + corev1.ConditionUnknown, + ) + }) + }) }) diff --git a/tests/kuttl/test-suites/default/watcher-api/01-cleanup-watcherapi.yaml b/tests/kuttl/test-suites/default/watcher-api/01-cleanup-watcherapi.yaml index b9c7650..f78c95e 100644 --- a/tests/kuttl/test-suites/default/watcher-api/01-cleanup-watcherapi.yaml +++ b/tests/kuttl/test-suites/default/watcher-api/01-cleanup-watcherapi.yaml @@ -4,3 +4,6 @@ delete: - apiVersion: watcher.openstack.org/v1beta1 kind: WatcherAPI name: watcherapi-kuttl +- apiVersion: v1 + kind: Secret + name: watcherapi-secret diff --git a/tests/kuttl/test-suites/default/watcher-api/03-assert.yaml b/tests/kuttl/test-suites/default/watcher-api/03-assert.yaml index f3c4cab..84ab971 100644 --- a/tests/kuttl/test-suites/default/watcher-api/03-assert.yaml +++ b/tests/kuttl/test-suites/default/watcher-api/03-assert.yaml @@ -9,7 +9,7 @@ spec: databaseInstance: openstack passwordSelectors: service: WatcherPassword - secret: osp-secret + secret: watcherapi-secret status: conditions: - message: Setup complete diff --git a/tests/kuttl/test-suites/default/watcher-api/03-deploy-watcher-api.yaml b/tests/kuttl/test-suites/default/watcher-api/03-deploy-watcher-api.yaml index b4f6a41..38e6684 100644 --- a/tests/kuttl/test-suites/default/watcher-api/03-deploy-watcher-api.yaml +++ b/tests/kuttl/test-suites/default/watcher-api/03-deploy-watcher-api.yaml @@ -1,6 +1,15 @@ +apiVersion: v1 +kind: Secret +metadata: + name: watcherapi-secret +type: Opaque +stringData: + WatcherPassword: password +--- apiVersion: watcher.openstack.org/v1beta1 kind: WatcherAPI metadata: name: watcherapi-kuttl spec: - databaseInstance: "openstack" + databaseInstance: openstack + secret: watcherapi-secret