diff --git a/CHANGELOG.md b/CHANGELOG.md index 79a5a2f7..ea92348b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ - **BREAKING**: Change `KafkaSchema` deletion to perform a hard delete instead of soft delete only. The subject is no longer visible in the registry's listing after deletion, and re-applying a `KafkaSchema` with the same `subjectName` after deletion starts at version 1. +- Fix service resources: wait for connection Secret publication before reporting Ready. ## v0.38.0 - 2026-05-18 diff --git a/controllers/basic_controller.go b/controllers/basic_controller.go index 70a2ecaa..4a3f410e 100644 --- a/controllers/basic_controller.go +++ b/controllers/basic_controller.go @@ -13,6 +13,7 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -191,7 +192,7 @@ func (i *instanceReconcilerHelper) reconcile(ctx context.Context, o v1alpha1.Aiv return false, nil } - if IsReadyToUse(o) && !hasPendingMigration(o) { + if IsReadyToUse(o) && !hasPendingMigration(o) && !i.resourceNeedsConnectionSecretSync(ctx, o) { return false, nil } @@ -327,6 +328,34 @@ func hasPendingMigration(o v1alpha1.AivenManagedObject) bool { return cond != nil && cond.Reason == v1alpha1.MigrationReasonInProgress } +func (i *instanceReconcilerHelper) resourceNeedsConnectionSecretSync(ctx context.Context, o v1alpha1.AivenManagedObject) bool { + if IsMarkedAsPoweredOff(o) { + return false + } + + withSecret, ok := connectionSecretOwner(o) + if !ok { + return false + } + + if hasConnectionSecretPublishError(o) { + return true + } + + secret := &corev1.Secret{} + if err := i.k8s.Get(ctx, types.NamespacedName{Name: connectionSecretName(withSecret), Namespace: withSecret.GetNamespace()}, secret); err != nil { + if !apierrors.IsNotFound(err) { + i.log.Info("unable to verify connection secret ownership", "error", err) + } + return true + } + + // Don't treat a same-named Secret as fresh connection details unless it was published for this exact object. + // Otherwise a Secret left behind by a resource deleted and recreated quickly could make the new resource look Ready too early. + ref := metav1.GetControllerOf(secret) + return ref == nil || o.GetUID() == "" || ref.UID != o.GetUID() +} + func (i *instanceReconcilerHelper) checkPreconditions(ctx context.Context, o client.Object, refs []client.Object) (bool, error) { i.rec.Event(o, corev1.EventTypeNormal, eventWaitingForPreconditions, "waiting for preconditions of the instance") @@ -555,11 +584,18 @@ func (i *instanceReconcilerHelper) updateInstanceStateAndSecretUntilRunning(ctx goalSecret, err := i.h.get(ctx, i.avnGen, o) if goalSecret == nil || err != nil { + if err != nil && goalSecret != nil { + markConnectionSecretPublishFailed(o, err) + } + if err == nil && hasIsRunningAnnotation(o) { + clearConnectionSecretPublishError(o) + } return err } if o.NoSecret() { i.rec.Event(o, corev1.EventTypeNormal, eventConnInfoSecretCreationDisabled, "connInfoSecretTargetDisabled is true, secret will not be created") + clearConnectionSecretPublishError(o) return nil } @@ -590,8 +626,47 @@ func (i *instanceReconcilerHelper) updateInstanceStateAndSecretUntilRunning(ctx return controllerutil.SetControllerReference(o, secret, i.k8s.Scheme()) }) + if err != nil { + markConnectionSecretPublishFailed(o, err) + return err + } + + clearConnectionSecretPublishError(o) + return nil +} + +func connectionSecretOwner(o v1alpha1.AivenManagedObject) (objWithSecret, bool) { + if o.NoSecret() { + return nil, false + } + + withSecret, ok := any(o).(objWithSecret) + return withSecret, ok +} + +func hasConnectionSecretPublishError(o v1alpha1.AivenManagedObject) bool { + cond := meta.FindStatusCondition(*o.Conditions(), ConditionTypeError) + return cond != nil && cond.Reason == string(errConditionConnInfoSecret) +} + +func markConnectionSecretPublishFailed(o v1alpha1.AivenManagedObject, err error) { + if _, ok := connectionSecretOwner(o); !ok { + return + } + + delete(o.GetAnnotations(), instanceIsRunningAnnotation) + meta.SetStatusCondition(o.Conditions(), getRunningCondition( + metav1.ConditionUnknown, + string(errConditionConnInfoSecret), + "Connection details are not published", + )) + meta.SetStatusCondition(o.Conditions(), getErrorCondition(errConditionConnInfoSecret, err)) +} - return err +func clearConnectionSecretPublishError(o v1alpha1.AivenManagedObject) { + if hasConnectionSecretPublishError(o) { + meta.RemoveStatusCondition(o.Conditions(), ConditionTypeError) + } } func setupLogger(log logr.Logger, o client.Object) logr.Logger { diff --git a/controllers/basic_controller_test.go b/controllers/basic_controller_test.go new file mode 100644 index 00000000..329f1a9e --- /dev/null +++ b/controllers/basic_controller_test.go @@ -0,0 +1,233 @@ +package controllers + +import ( + "context" + "testing" + + avngen "github.com/aiven/go-client-codegen" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/aiven/aiven-operator/api/v1alpha1" +) + +func TestInstanceReconcilerHelper_updateInstanceStateAndSecretUntilRunning(t *testing.T) { + connectionSecret := func(pg *v1alpha1.PostgreSQL) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: pg.Name, + Namespace: pg.Namespace, + }, + StringData: map[string]string{ + "PGPASSWORD": "pw", + }, + } + } + + t.Run("Marks resource not ready when connection Secret publish fails", func(t *testing.T) { + pg := newReadyPostgreSQL(t) + controller := true + + scheme := newBasicControllerTestScheme(t) + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: pg.Name, + Namespace: pg.Namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "aiven.io/v1alpha1", + Kind: "PostgreSQL", + Name: "other", + UID: types.UID("other-uid"), + Controller: &controller, + }, + }, + }, + }). + Build() + + h := NewMockHandlers(t) + h.EXPECT(). + get(mock.Anything, mock.Anything, pg). + Run(func(_ context.Context, _ avngen.Client, _ client.Object) { + metav1.SetMetaDataAnnotation(&pg.ObjectMeta, instanceIsRunningAnnotation, "true") + meta.SetStatusCondition(&pg.Status.Conditions, getRunningCondition(metav1.ConditionTrue, "CheckRunning", "Instance is running on Aiven side")) + }). + Return(connectionSecret(pg), nil). + Once() + + helper := &instanceReconcilerHelper{ + k8s: k8sClient, + h: h, + rec: record.NewFakeRecorder(10), + } + + err := helper.updateInstanceStateAndSecretUntilRunning(t.Context(), pg) + require.Error(t, err) + + require.False(t, hasIsRunningAnnotation(pg)) + + running := meta.FindStatusCondition(pg.Status.Conditions, conditionTypeRunning) + require.NotNil(t, running) + require.Equal(t, metav1.ConditionUnknown, running.Status) + require.Equal(t, string(errConditionConnInfoSecret), running.Reason) + + errCond := meta.FindStatusCondition(pg.Status.Conditions, ConditionTypeError) + require.NotNil(t, errCond) + require.Equal(t, metav1.ConditionUnknown, errCond.Status) + require.Equal(t, string(errConditionConnInfoSecret), errCond.Reason) + }) + + t.Run("Clears connection Secret publish error after successful publish", func(t *testing.T) { + pg := newReadyPostgreSQL(t) + meta.SetStatusCondition(&pg.Status.Conditions, getErrorCondition(errConditionConnInfoSecret, assert.AnError)) + + scheme := newBasicControllerTestScheme(t) + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + h := NewMockHandlers(t) + h.EXPECT(). + get(mock.Anything, mock.Anything, pg). + Run(func(_ context.Context, _ avngen.Client, _ client.Object) { + metav1.SetMetaDataAnnotation(&pg.ObjectMeta, instanceIsRunningAnnotation, "true") + meta.SetStatusCondition(&pg.Status.Conditions, getRunningCondition(metav1.ConditionTrue, "CheckRunning", "Instance is running on Aiven side")) + }). + Return(connectionSecret(pg), nil). + Once() + + helper := &instanceReconcilerHelper{ + k8s: k8sClient, + h: h, + rec: record.NewFakeRecorder(10), + } + + require.NoError(t, helper.updateInstanceStateAndSecretUntilRunning(t.Context(), pg)) + require.True(t, hasIsRunningAnnotation(pg)) + require.Nil(t, meta.FindStatusCondition(pg.Status.Conditions, ConditionTypeError)) + + got := &corev1.Secret{} + require.NoError(t, k8sClient.Get(t.Context(), types.NamespacedName{Name: pg.Name, Namespace: pg.Namespace}, got)) + require.Equal(t, []byte("pw"), got.Data["PGPASSWORD"]) + }) +} + +func TestInstanceReconcilerHelper_resourceNeedsConnectionSecretSync(t *testing.T) { + ownedSecret := func(pg *v1alpha1.PostgreSQL) *corev1.Secret { + controller := true + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: pg.Name, + Namespace: pg.Namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "aiven.io/v1alpha1", + Kind: "PostgreSQL", + Name: pg.Name, + UID: pg.UID, + Controller: &controller, + }, + }, + }, + Data: map[string][]byte{ + "PGPASSWORD": []byte("pw"), + }, + } + } + + tests := map[string]struct { + mutate func(*v1alpha1.PostgreSQL) + objects []runtime.Object + wantNeedsSync bool + }{ + "does not need sync for powered-off resource": { + mutate: func(pg *v1alpha1.PostgreSQL) { + metav1.SetMetaDataAnnotation(&pg.ObjectMeta, instanceIsRunningAnnotation, "false") + }, + wantNeedsSync: false, + }, + "does not need sync when connection Secret creation is disabled": { + mutate: func(pg *v1alpha1.PostgreSQL) { + pg.Spec.ConnInfoSecretTargetDisabled = new(bool) + *pg.Spec.ConnInfoSecretTargetDisabled = true + }, + wantNeedsSync: false, + }, + "needs sync when connection Secret is missing": { + wantNeedsSync: true, + }, + "needs sync when connection Secret is not controlled by the resource": { + objects: []runtime.Object{ + &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "pg-no-ref", Namespace: "default"}}, + }, + wantNeedsSync: true, + }, + "needs sync when a stale connection Secret error is present": { + mutate: func(pg *v1alpha1.PostgreSQL) { + meta.SetStatusCondition(&pg.Status.Conditions, getErrorCondition(errConditionConnInfoSecret, assert.AnError)) + }, + objects: []runtime.Object{ + ownedSecret(newReadyPostgreSQL(t)), + }, + wantNeedsSync: true, + }, + "does not need sync when connection Secret is controlled by the resource": { + objects: []runtime.Object{ + ownedSecret(newReadyPostgreSQL(t)), + }, + wantNeedsSync: false, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + pg := newReadyPostgreSQL(t) + if tt.mutate != nil { + tt.mutate(pg) + } + + scheme := newBasicControllerTestScheme(t) + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(tt.objects...). + Build() + + helper := &instanceReconcilerHelper{k8s: k8sClient} + require.Equal(t, tt.wantNeedsSync, helper.resourceNeedsConnectionSecretSync(t.Context(), pg)) + }) + } +} + +func newBasicControllerTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, v1alpha1.AddToScheme(scheme)) + return scheme +} + +func newReadyPostgreSQL(t *testing.T) *v1alpha1.PostgreSQL { + t.Helper() + + pg := newObjectFromYAML[v1alpha1.PostgreSQL](t, yamlPostgres) + pg.UID = types.UID("pg-uid") + pg.Generation = 1 + metav1.SetMetaDataAnnotation(&pg.ObjectMeta, processedGenerationAnnotation, "1") + metav1.SetMetaDataAnnotation(&pg.ObjectMeta, instanceIsRunningAnnotation, "true") + meta.SetStatusCondition(&pg.Status.Conditions, getRunningCondition(metav1.ConditionTrue, "CheckRunning", "Instance is running on Aiven side")) + return pg +} diff --git a/controllers/common.go b/controllers/common.go index ff1457de..9483efb9 100644 --- a/controllers/common.go +++ b/controllers/common.go @@ -220,6 +220,16 @@ func GetIsRunningAnnotation(o client.Object) string { return o.GetAnnotations()[instanceIsRunningAnnotation] } +// IsMarkedAsPoweredOff returns true when the running annotation explicitly marks a service as powered off. +func IsMarkedAsPoweredOff(o client.Object) bool { + return GetIsRunningAnnotation(o) == "false" +} + +// IsMarkedAsPoweredOn returns true when the running annotation explicitly marks a resource as running. +func IsMarkedAsPoweredOn(o client.Object) bool { + return GetIsRunningAnnotation(o) == "true" +} + // IsReadyToUse returns true when the client.Object's controller has processed the latest manifest changes // and the resource is in a running state in Aiven. For services, this includes both running and powered-off states. // This indicates the resource is ready for use and has reached its desired state. @@ -272,16 +282,12 @@ type objWithSecret interface { func newSecret(o objWithSecret, stringData map[string]string, addPrefix bool) *corev1.Secret { target := o.GetConnInfoSecretTarget() meta := metav1.ObjectMeta{ - Name: o.GetName(), + Name: connectionSecretName(o), Namespace: o.GetNamespace(), Annotations: target.Annotations, Labels: target.Labels, } - if target.Name != "" { - meta.Name = target.Name - } - // fixme: set this as default behaviour // when legacy secrets removed if addPrefix { @@ -298,6 +304,13 @@ func newSecret(o objWithSecret, stringData map[string]string, addPrefix bool) *c } } +func connectionSecretName(o objWithSecret) string { + if target := o.GetConnInfoSecretTarget(); target.Name != "" { + return target.Name + } + return o.GetName() +} + // getSecretPrefix returns user's prefix or kind name func getSecretPrefix(o objWithSecret) string { target := o.GetConnInfoSecretTarget() diff --git a/controllers/common_test.go b/controllers/common_test.go index 9dd6bd46..81288b69 100644 --- a/controllers/common_test.go +++ b/controllers/common_test.go @@ -1,10 +1,14 @@ package controllers import ( + "fmt" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/aiven/aiven-operator/api/v1alpha1" kafkaconnectuserconfig "github.com/aiven/aiven-operator/api/v1alpha1/userconfig/integration/kafka_connect" ) @@ -15,3 +19,78 @@ func TestCreateEmptyUserConfiguration(t *testing.T) { assert.Empty(t, m) assert.NoError(t, err) } + +func TestConnectionSecretName(t *testing.T) { + tests := map[string]struct { + targetName string + want string + }{ + "uses resource name by default": { + want: "pg", + }, + "uses connInfoSecretTarget name when set": { + targetName: "custom-secret", + want: "custom-secret", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + obj := &mockObjWithSecret{ + name: "pg", + target: v1alpha1.ConnInfoSecretTarget{ + Name: tt.targetName, + }, + } + + assert.Equal(t, tt.want, connectionSecretName(obj)) + }) + } +} + +func TestPowerStateAnnotations(t *testing.T) { + tests := []struct { + annotation string + wantOn bool + wantOff bool + }{ + {annotation: "false", wantOff: true}, + {annotation: "true", wantOn: true}, + {wantOn: false, wantOff: false}, + } + + for i, tt := range tests { + t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { + obj := &v1alpha1.PostgreSQL{} + if tt.annotation != "" { + obj.SetAnnotations(map[string]string{ + instanceIsRunningAnnotation: tt.annotation, + }) + } + + require.Equal(t, tt.wantOn, IsMarkedAsPoweredOn(obj)) + require.Equal(t, tt.wantOff, IsMarkedAsPoweredOff(obj)) + }) + } +} + +type mockObjWithSecret struct { + name string + target v1alpha1.ConnInfoSecretTarget +} + +func (t *mockObjWithSecret) GetName() string { + return t.name +} + +func (t *mockObjWithSecret) GetNamespace() string { + return "default" +} + +func (t *mockObjWithSecret) GetObjectKind() schema.ObjectKind { + return schema.EmptyObjectKind +} + +func (t *mockObjWithSecret) GetConnInfoSecretTarget() v1alpha1.ConnInfoSecretTarget { + return t.target +} diff --git a/controllers/generic_service_handler.go b/controllers/generic_service_handler.go index 82a9eeed..c6fb296a 100644 --- a/controllers/generic_service_handler.go +++ b/controllers/generic_service_handler.go @@ -248,8 +248,6 @@ func (h *genericServiceHandler) get(ctx context.Context, avnGen avngen.Client, o return nil, nil } - meta.SetStatusCondition(&status.Conditions, getRunningCondition(metav1.ConditionTrue, "CheckRunning", msg)) - if mp, ok := o.(migrationSecretProvider); ok && mp.getMigrationSecretSource() != nil { // Skip polling Aiven once the migration has completed; the status won't // change, and ServiceGetMigrationStatus would 404 on every reconcile. @@ -269,12 +267,16 @@ func (h *genericServiceHandler) get(ctx context.Context, avnGen avngen.Client, o // If service is powered off, we don't need to return a secret if !isPowered { + meta.SetStatusCondition(&status.Conditions, getRunningCondition(metav1.ConditionTrue, "CheckRunning", msg)) metav1.SetMetaDataAnnotation(o.getObjectMeta(), instanceIsRunningAnnotation, "false") return nil, nil } - // Service is powered - metav1.SetMetaDataAnnotation(o.getObjectMeta(), instanceIsRunningAnnotation, "true") + if managed, ok := obj.(v1alpha1.AivenManagedObject); ok && managed.NoSecret() { + meta.SetStatusCondition(&status.Conditions, getRunningCondition(metav1.ConditionTrue, "CheckRunning", msg)) + metav1.SetMetaDataAnnotation(o.getObjectMeta(), instanceIsRunningAnnotation, "true") + return nil, nil + } // Some services get secrets after they are running only, // like ip addresses (hosts) @@ -287,6 +289,8 @@ func (h *genericServiceHandler) get(ctx context.Context, avnGen avngen.Client, o case serviceTypeKafka, serviceTypePostgreSQL, serviceTypeMySQL: // CA_CERT can be used with these service types only default: + meta.SetStatusCondition(&status.Conditions, getRunningCondition(metav1.ConditionTrue, "CheckRunning", msg)) + metav1.SetMetaDataAnnotation(o.getObjectMeta(), instanceIsRunningAnnotation, "true") return secret, nil } @@ -302,6 +306,8 @@ func (h *genericServiceHandler) get(ctx context.Context, avnGen avngen.Client, o // todo: backward compatibility, remove in future releases secret.StringData["CA_CERT"] = cert } + meta.SetStatusCondition(&status.Conditions, getRunningCondition(metav1.ConditionTrue, "CheckRunning", msg)) + metav1.SetMetaDataAnnotation(o.getObjectMeta(), instanceIsRunningAnnotation, "true") return secret, nil } diff --git a/controllers/generic_service_handler_test.go b/controllers/generic_service_handler_test.go index 0755aa73..dc1d0c3e 100644 --- a/controllers/generic_service_handler_test.go +++ b/controllers/generic_service_handler_test.go @@ -1,6 +1,7 @@ package controllers import ( + "errors" "net/http" "testing" @@ -339,6 +340,76 @@ func TestGet_SecretCleanupRunsWhenPoweredOff(t *testing.T) { "migration Secret should have been deleted even though service is powered off, got err: %v", err) } +func TestGet_DoesntMarkReadyBeforeConnectionSecretDetails(t *testing.T) { + t.Parallel() + + pg := newObjectFromYAML[v1alpha1.PostgreSQL](t, yamlPostgres) + avn := avngen.NewMockClient(t) + avn.EXPECT(). + ServiceGet(mock.Anything, pg.Spec.Project, pg.Name, mock.Anything). + Return(&service.ServiceGetOut{ + State: service.ServiceStateTypeRunning, + ServiceUriParams: map[string]string{ + "host": "pg.example.com", + "port": "5432", + "dbname": "defaultdb", + "user": "avnadmin", + "password": "secret", + "sslmode": "require", + }, + ServiceUri: "postgres://avnadmin:secret@pg.example.com:5432/defaultdb?sslmode=require", + }, nil).Once() + avn.EXPECT(). + ProjectKmsGetCA(mock.Anything, pg.Spec.Project). + Return("", errors.New("kms ca is not ready")).Once() + + h := &genericServiceHandler{ + fabric: newPostgreSQLAdapterFactory(nil), + log: logr.Discard(), + } + + secret, err := h.get(t.Context(), avn, pg) + require.Error(t, err) + require.Nil(t, secret) + require.False(t, hasIsRunningAnnotation(pg)) + require.False(t, meta.IsStatusConditionTrue(pg.Status.Conditions, conditionTypeRunning)) +} + +func TestGet_MarksNoSecretServiceReadyWhenRunning(t *testing.T) { + t.Parallel() + + kafkaConnect := &v1alpha1.KafkaConnect{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kafka-connect", + Namespace: "default", + }, + Spec: v1alpha1.KafkaConnectSpec{ + BaseServiceFields: v1alpha1.BaseServiceFields{ + ProjectDependant: v1alpha1.ProjectDependant{ + ProjectField: v1alpha1.ProjectField{Project: "project"}, + }, + }, + }, + } + + avn := avngen.NewMockClient(t) + avn.EXPECT(). + ServiceGet(mock.Anything, kafkaConnect.Spec.Project, kafkaConnect.Name, mock.Anything). + Return(&service.ServiceGetOut{State: service.ServiceStateTypeRunning}, nil).Once() + + h := &genericServiceHandler{ + fabric: newKafkaConnectAdapter, + log: logr.Discard(), + } + + secret, err := h.get(t.Context(), avn, kafkaConnect) + require.NoError(t, err) + require.Nil(t, secret) + require.True(t, hasIsRunningAnnotation(kafkaConnect)) + require.True(t, IsMarkedAsPoweredOn(kafkaConnect)) + require.True(t, meta.IsStatusConditionTrue(kafkaConnect.Status.Conditions, conditionTypeRunning)) +} + func TestHasPendingMigration(t *testing.T) { t.Parallel() diff --git a/tests/generic_service_handler_test.go b/tests/generic_service_handler_test.go index f45f67d0..d47ea90e 100644 --- a/tests/generic_service_handler_test.go +++ b/tests/generic_service_handler_test.go @@ -86,7 +86,7 @@ func TestCreateUpdateService(t *testing.T) { // Waits kube objects pg := new(v1alpha1.PostgreSQL) require.NoError(t, s.GetRunning(pg, pgName)) - assert.Equal(t, "true", controllers.GetIsRunningAnnotation(pg)) + assert.True(t, controllers.IsMarkedAsPoweredOn(pg)) // THEN // Validates tags @@ -130,7 +130,7 @@ func TestCreateUpdateService(t *testing.T) { avnPgPoweredOff, err := avnGen.ServiceGet(ctx, cfg.Project, pgName) require.NoError(t, err) assert.Equal(t, service.ServiceStateTypePoweroff, avnPgPoweredOff.State) - assert.Equal(t, "false", controllers.GetIsRunningAnnotation(pgPoweredOff)) + assert.True(t, controllers.IsMarkedAsPoweredOff(pgPoweredOff)) // Validates the service is powered on ymlPowerOn := getUpdateServiceYaml(cfg.Project, pgName, true) @@ -142,7 +142,7 @@ func TestCreateUpdateService(t *testing.T) { avnPgPoweredOn, err := avnGen.ServiceGet(ctx, cfg.Project, pgName) require.NoError(t, err) assert.Equal(t, service.ServiceStateTypeRunning, avnPgPoweredOn.State) - assert.Equal(t, "true", controllers.GetIsRunningAnnotation(pgPoweredOn)) + assert.True(t, controllers.IsMarkedAsPoweredOn(pgPoweredOn)) } // TestTerminationProtectionDeletion verifies that the controller can delete a service diff --git a/tests/kafkaconnector_test.go b/tests/kafkaconnector_test.go index f0a2ece2..e2508da0 100644 --- a/tests/kafkaconnector_test.go +++ b/tests/kafkaconnector_test.go @@ -22,14 +22,17 @@ func TestKafkaConnector(t *testing.T) { defer cancel() kafkaName := randName("kafka-service") + kafkaSecretName := randName("kafka-secret") osName := randName("opensearch-service") + osSecretName := randName("os-secret") topicName := randName("kafka-topic") connectorName := randName("kafka-connector") yml, err := loadExampleYaml("kafkaconnector.yaml", map[string]string{ // Kafka - "doc[0].metadata.name": kafkaName, - "doc[0].spec.project": cfg.Project, - "doc[0].spec.cloudName": cfg.PrimaryCloudName, + "doc[0].metadata.name": kafkaName, + "doc[0].spec.project": cfg.Project, + "doc[0].spec.cloudName": cfg.PrimaryCloudName, + "doc[0].spec.connInfoSecretTarget.name": kafkaSecretName, // Kafka Topic "doc[1].metadata.name": topicName, @@ -37,15 +40,17 @@ func TestKafkaConnector(t *testing.T) { "doc[1].spec.serviceName": kafkaName, // OpenSearch - "doc[2].metadata.name": osName, - "doc[2].spec.project": cfg.Project, - "doc[2].spec.cloudName": cfg.PrimaryCloudName, + "doc[2].metadata.name": osName, + "doc[2].spec.project": cfg.Project, + "doc[2].spec.cloudName": cfg.PrimaryCloudName, + "doc[2].spec.connInfoSecretTarget.name": osSecretName, // Kafka Connector - "doc[3].metadata.name": connectorName, - "doc[3].spec.project": cfg.Project, - "doc[3].spec.serviceName": kafkaName, - "doc[3].spec.userConfig.topics": topicName, + "doc[3].metadata.name": connectorName, + "doc[3].spec.project": cfg.Project, + "doc[3].spec.serviceName": kafkaName, + "doc[3].spec.userConfig.topics": topicName, + "doc[3].spec.userConfig.'connection.url'": `{{ fromSecret "` + osSecretName + `" "OPENSEARCH_URI" }}`, }) require.NoError(t, err) s := NewSession(ctx, k8sClient) diff --git a/tests/kafkanativeacl_test.go b/tests/kafkanativeacl_test.go index 320ef17e..aa797645 100644 --- a/tests/kafkanativeacl_test.go +++ b/tests/kafkanativeacl_test.go @@ -21,13 +21,15 @@ func TestKafkaNativeACL(t *testing.T) { defer cancel() kafkaName := randName("kafka-native-acl") + kafkaSecretName := randName("kafka-secret") aclName := randName("kafka-acl") yml, err := loadExampleYaml("kafkanativeacl.yaml", map[string]string{ - "doc[0].metadata.name": kafkaName, - "doc[0].spec.project": cfg.Project, - "doc[1].metadata.name": aclName, - "doc[1].spec.project": cfg.Project, - "doc[1].spec.serviceName": kafkaName, + "doc[0].metadata.name": kafkaName, + "doc[0].spec.project": cfg.Project, + "doc[0].spec.connInfoSecretTarget.name": kafkaSecretName, + "doc[1].metadata.name": aclName, + "doc[1].spec.project": cfg.Project, + "doc[1].spec.serviceName": kafkaName, }) require.NoError(t, err) s := NewSession(ctx, k8sClient) diff --git a/tests/postgresql_test.go b/tests/postgresql_test.go index 19b45235..fde98ea9 100644 --- a/tests/postgresql_test.go +++ b/tests/postgresql_test.go @@ -9,9 +9,13 @@ import ( "github.com/aiven/go-client-codegen/handler/service" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "github.com/aiven/aiven-operator/api/v1alpha1" + "github.com/aiven/aiven-operator/controllers" ) func getPgReadReplicaYaml(project, masterName, replicaName, cloudName string) string { @@ -183,6 +187,75 @@ spec: `, project, pgName, cloudName) } +func TestPgReadyRequiresConnectionSecretCredentials(t *testing.T) { + t.Parallel() + defer recoverPanic(t) + + ctx, cancel := testCtx() + defer cancel() + + pgName := randName("secret-ready") + secretName := randName("pg-secret") + yml, err := loadExampleYaml("postgresql.yaml", map[string]string{ + "metadata.name": pgName, + "spec.project": cfg.Project, + "spec.cloudName": cfg.PrimaryCloudName, + "spec.connInfoSecretTarget.name": secretName, + "spec.connInfoSecretTarget.prefix": "REMOVE", + "spec.connInfoSecretTarget.annotations": "REMOVE", + "spec.connInfoSecretTarget.labels": "REMOVE", + "spec.maintenanceWindowDow": "REMOVE", + "spec.maintenanceWindowTime": "REMOVE", + }) + require.NoError(t, err) + s := NewSession(ctx, k8sClient) + + immutableSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: defaultNamespace, + }, + Immutable: anyPointer(true), + Data: map[string][]byte{ + "placeholder": []byte("immutable"), + }, + } + require.NoError(t, k8sClient.Create(ctx, immutableSecret)) + + defer func() { + if err := k8sClient.Delete(ctx, immutableSecret); err != nil && !isNotFound(err) { + t.Errorf("failed to delete immutable Secret %q: %s", secretName, err) + } + }() + defer s.Destroy(t) + + require.NoError(t, s.Apply(yml)) + + pg := new(v1alpha1.PostgreSQL) + require.NoError(t, retryForever(ctx, "verify PostgreSQL waits for connection Secret credentials", func() (bool, error) { + err := k8sClient.Get(ctx, types.NamespacedName{Name: pgName, Namespace: defaultNamespace}, pg) + if err != nil { + return isNotFound(err), err + } + + if controllers.IsReadyToUse(pg) { + return false, fmt.Errorf("PostgreSQL became ready before connection Secret credentials were published") + } + + errCond := meta.FindStatusCondition(pg.Status.Conditions, controllers.ConditionTypeError) + if pg.Status.State == serviceRunningState && errCond != nil && errCond.Reason == "ConnInfoSecret" { + return false, nil + } + + return true, nil + })) + + secret, err := s.GetSecret(secretName) + require.NoError(t, err) + require.Empty(t, secret.Data["PGPASSWORD"]) + require.Empty(t, secret.Data["POSTGRESQL_PASSWORD"]) +} + func TestPgCustomPrefix(t *testing.T) { t.Parallel() defer recoverPanic(t) diff --git a/tests/suite_lib_test.go b/tests/suite_lib_test.go index 1b0f0b65..e694fc05 100644 --- a/tests/suite_lib_test.go +++ b/tests/suite_lib_test.go @@ -103,6 +103,17 @@ data: }, expected: []string{"new-nested-value"}, }, + { + name: "should_update_key_with_dot", + yamlContent: ` +data: + config: + connection.url: old-url`, + replacements: map[string]string{ + "data.config.'connection.url'": "new-url", + }, + expected: []string{"new-url"}, + }, }, "removal_operations": { {