diff --git a/api/bases/placement.openstack.org_placementapis.yaml b/api/bases/placement.openstack.org_placementapis.yaml index 73ef0098b..8608b73b3 100644 --- a/api/bases/placement.openstack.org_placementapis.yaml +++ b/api/bases/placement.openstack.org_placementapis.yaml @@ -412,6 +412,13 @@ spec: status: description: PlacementAPIStatus defines the observed state of PlacementAPI properties: + applicationCredentialSecret: + description: |- + ApplicationCredentialSecret - the AC secret placement is currently + consuming and protecting with the openstack.org/placementapi-ac-consumer + finalizer. Tracked so the controller can remove its finalizer from the + old secret when the openstack-operator rotates the reference. + type: string conditions: description: Conditions items: diff --git a/api/placement/v1beta1/api_types.go b/api/placement/v1beta1/api_types.go index 3354e3b50..bdaf922a0 100644 --- a/api/placement/v1beta1/api_types.go +++ b/api/placement/v1beta1/api_types.go @@ -182,6 +182,12 @@ type PlacementAPIStatus struct { // LastAppliedTopology - the last applied Topology LastAppliedTopology *topologyv1.TopoRef `json:"lastAppliedTopology,omitempty"` + + // ApplicationCredentialSecret - the AC secret placement is currently + // consuming and protecting with the openstack.org/placementapi-ac-consumer + // finalizer. Tracked so the controller can remove its finalizer from the + // old secret when the openstack-operator rotates the reference. + ApplicationCredentialSecret string `json:"applicationCredentialSecret,omitempty"` } // PlacementAPI is the Schema for the placementapis API diff --git a/config/crd/bases/placement.openstack.org_placementapis.yaml b/config/crd/bases/placement.openstack.org_placementapis.yaml index 73ef0098b..8608b73b3 100644 --- a/config/crd/bases/placement.openstack.org_placementapis.yaml +++ b/config/crd/bases/placement.openstack.org_placementapis.yaml @@ -412,6 +412,13 @@ spec: status: description: PlacementAPIStatus defines the observed state of PlacementAPI properties: + applicationCredentialSecret: + description: |- + ApplicationCredentialSecret - the AC secret placement is currently + consuming and protecting with the openstack.org/placementapi-ac-consumer + finalizer. Tracked so the controller can remove its finalizer from the + old secret when the openstack-operator rotates the reference. + type: string conditions: description: Conditions items: diff --git a/internal/controller/placement/api_controller.go b/internal/controller/placement/api_controller.go index dc44b95fe..67c15533f 100644 --- a/internal/controller/placement/api_controller.go +++ b/internal/controller/placement/api_controller.go @@ -455,6 +455,21 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, nil } + if instance.Spec.Auth.ApplicationCredentialSecret != "" { + if err := keystonev1.ManageACSecretFinalizer(ctx, h, instance.Namespace, + instance.Spec.Auth.ApplicationCredentialSecret, + "", + placement.ACConsumerFinalizer); err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.ServiceConfigReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.ServiceConfigReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err + } + } + instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) serviceAnnotations, result, err := r.ensureNetworkAttachments(ctx, h, instance) @@ -502,6 +517,26 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, err } + // Manage the old AC secret's finalizer and status tracking. + // On rotation (old != new), only remove the old secret's finalizer after + // all sub-services are ready with the new credentials. This prevents + // premature revocation during rapid rotations. + isRotation := instance.Status.ApplicationCredentialSecret != "" && + instance.Status.ApplicationCredentialSecret != instance.Spec.Auth.ApplicationCredentialSecret + + if isRotation { + allServicesReady := instance.Status.Conditions.AllSubConditionIsTrue() + if allServicesReady { + if err := keystonev1.RemoveACSecretConsumerFinalizer(ctx, h, instance.Namespace, + instance.Status.ApplicationCredentialSecret, placement.ACConsumerFinalizer); err != nil { + return ctrl.Result{}, err + } + instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret + } + } else { + instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret + } + return ctrl.Result{}, nil } @@ -1094,6 +1129,17 @@ func (r *PlacementAPIReconciler) reconcileDelete(ctx context.Context, instance * } } + // Remove consumer finalizer from AC secrets placement was consuming. + for _, secretName := range []string{ + instance.Status.ApplicationCredentialSecret, + instance.Spec.Auth.ApplicationCredentialSecret, + } { + if err := keystonev1.RemoveACSecretConsumerFinalizer(ctx, helper, instance.Namespace, + secretName, placement.ACConsumerFinalizer); err != nil { + return ctrl.Result{}, err + } + } + // We did all the cleanup on the objects we created so we can remove the // finalizer from ourselves to allow the deletion controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) @@ -1332,6 +1378,25 @@ func (r *PlacementAPIReconciler) ensureDeployment( } // create Deployment - end + // Manage the old AC secret's finalizer and status tracking. + // On rotation (old != new), only remove the old secret's finalizer after + // all sub-services are ready with the new credentials. This prevents + // premature revocation during rapid rotations. + isRotation := instance.Status.ApplicationCredentialSecret != "" && instance.Status.ApplicationCredentialSecret != instance.Spec.Auth.ApplicationCredentialSecret + + if isRotation { + allServicesReady := instance.Status.Conditions.AllSubConditionIsTrue() + if allServicesReady { + if err := keystonev1.RemoveACSecretConsumerFinalizer(ctx, h, instance.Namespace, + instance.Status.ApplicationCredentialSecret, placement.ACConsumerFinalizer); err != nil { + return ctrl.Result{}, err + } + instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret + } + } else { + instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret + } + Log.Info("Reconciled Service successfully") return ctrl.Result{}, nil } diff --git a/internal/placement/const.go b/internal/placement/const.go index f058387ef..b678734d1 100644 --- a/internal/placement/const.go +++ b/internal/placement/const.go @@ -36,4 +36,7 @@ const ( // PlacementUserID is the linux user ID used by Kolla for the placement // user in the service containers PlacementUserID int64 = 42482 + + // ACConsumerFinalizer is added to AC secrets that placement is actively consuming + ACConsumerFinalizer = "openstack.org/placementapi-ac-consumer" ) diff --git a/test/functional/placement/api_controller_test.go b/test/functional/placement/api_controller_test.go index fd2da4302..1e0ec412d 100644 --- a/test/functional/placement/api_controller_test.go +++ b/test/functional/placement/api_controller_test.go @@ -32,6 +32,7 @@ import ( mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" "github.com/openstack-k8s-operators/nova-operator/internal/placement" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) @@ -1526,4 +1527,285 @@ var _ = Describe("PlacementAPI reconfiguration", func() { }) }) + When("ApplicationCredential consumer finalizer is managed", func() { + var acSecretName string + + BeforeEach(func() { + acSecretName = "ac-placement-a1b2c-secret" //nolint:gosec // G101 + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: names.Namespace, + Name: acSecretName, + }, + Data: map[string][]byte{ + keystonev1.ACIDSecretKey: []byte("a1b2ctest-ac-id"), + keystonev1.ACSecretSecretKey: []byte("test-ac-secret"), + }, + } + DeferCleanup(k8sClient.Delete, ctx, secret) + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + + spec := GetDefaultPlacementAPISpec() + spec["auth"] = map[string]any{ + "applicationCredentialSecret": acSecretName, + } + + DeferCleanup(th.DeleteInstance, CreatePlacementAPI(names.PlacementAPIName, spec)) + DeferCleanup( + k8sClient.Delete, ctx, CreatePlacementAPISecret(names.Namespace, SecretName)) + keystoneAPIName := keystone.CreateKeystoneAPI(names.Namespace) + DeferCleanup(keystone.DeleteKeystoneAPI, keystoneAPIName) + DeferCleanup( + mariadb.DeleteDBService, + mariadb.CreateDBService( + names.Namespace, + GetDefaultPlacementAPISpec()["databaseInstance"].(string), + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}, + }, + ), + ) + mariadb.SimulateMariaDBDatabaseCompleted(names.MariaDBDatabaseName) + mariadb.SimulateMariaDBAccountCompleted(names.MariaDBAccount) + }) + + It("should add the consumer finalizer to the AC secret", func() { + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: names.Namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(placement.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + }) + + It("should track the consumed AC secret in status", func() { + th.SimulateJobSuccess(names.DBSyncJobName) + th.SimulateDeploymentReplicaReady(names.DeploymentName) + keystone.SimulateKeystoneServiceReady(names.KeystoneServiceName) + keystone.SimulateKeystoneEndpointReady(names.KeystoneEndpointName) + Eventually(func(g Gomega) { + p := GetPlacementAPI(names.PlacementAPIName) + g.Expect(p.Status.ApplicationCredentialSecret).To(Equal(acSecretName)) + }, timeout, interval).Should(Succeed()) + }) + + It("should move the finalizer from the old to the new secret on rotation", func() { + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: names.Namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(placement.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + th.SimulateJobSuccess(names.DBSyncJobName) + th.SimulateDeploymentReplicaReady(names.DeploymentName) + keystone.SimulateKeystoneServiceReady(names.KeystoneServiceName) + keystone.SimulateKeystoneEndpointReady(names.KeystoneEndpointName) + Eventually(func(g Gomega) { + p := GetPlacementAPI(names.PlacementAPIName) + g.Expect(p.Status.Conditions.IsTrue(condition.ReadyCondition)).To(BeTrue()) + }, timeout, interval).Should(Succeed()) + + newACSecretName := "ac-placement-x9y8z-secret" //nolint:gosec // G101 + newSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: names.Namespace, + Name: newACSecretName, + }, + Data: map[string][]byte{ + keystonev1.ACIDSecretKey: []byte("x9y8zrotated-ac-id"), + keystonev1.ACSecretSecretKey: []byte("rotated-ac-secret"), + }, + } + DeferCleanup(k8sClient.Delete, ctx, newSecret) + Expect(k8sClient.Create(ctx, newSecret)).To(Succeed()) + + Eventually(func(g Gomega) { + p := GetPlacementAPI(names.PlacementAPIName) + p.Spec.Auth.ApplicationCredentialSecret = newACSecretName + g.Expect(k8sClient.Update(ctx, p)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: names.Namespace, + Name: newACSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(placement.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + th.SimulateDeploymentReplicaReady(names.DeploymentName) + keystone.SimulateKeystoneEndpointReady(names.KeystoneEndpointName) + secret := th.GetSecret(types.NamespacedName{ + Namespace: names.Namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).NotTo( + ContainElement(placement.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + p := GetPlacementAPI(names.PlacementAPIName) + g.Expect(p.Status.ApplicationCredentialSecret).To(Equal(newACSecretName)) + }, timeout, interval).Should(Succeed()) + }) + + It("should remove the consumer finalizer from AC secret on CR deletion", func() { + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: names.Namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(placement.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + th.DeleteInstance(GetPlacementAPI(names.PlacementAPIName)) + + secret := th.GetSecret(types.NamespacedName{ + Namespace: names.Namespace, + Name: acSecretName, + }) + Expect(secret.Finalizers).NotTo( + ContainElement(placement.ACConsumerFinalizer)) + }) + + It("should remove the consumer finalizer when AC auth is cleared from spec", func() { + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: names.Namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(placement.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + th.SimulateJobSuccess(names.DBSyncJobName) + th.SimulateDeploymentReplicaReady(names.DeploymentName) + keystone.SimulateKeystoneServiceReady(names.KeystoneServiceName) + keystone.SimulateKeystoneEndpointReady(names.KeystoneEndpointName) + Eventually(func(g Gomega) { + p := GetPlacementAPI(names.PlacementAPIName) + g.Expect(p.Status.Conditions.IsTrue(condition.ReadyCondition)).To(BeTrue()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + p := GetPlacementAPI(names.PlacementAPIName) + p.Spec.Auth.ApplicationCredentialSecret = "" + g.Expect(k8sClient.Update(ctx, p)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + th.SimulateDeploymentReplicaReady(names.DeploymentName) + keystone.SimulateKeystoneEndpointReady(names.KeystoneEndpointName) + secret := th.GetSecret(types.NamespacedName{ + Namespace: names.Namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).NotTo( + ContainElement(placement.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + p := GetPlacementAPI(names.PlacementAPIName) + g.Expect(p.Status.ApplicationCredentialSecret).To(BeEmpty()) + }, timeout, interval).Should(Succeed()) + }) + }) + + When("ApplicationCredential secret is not found", func() { + BeforeEach(func() { + DeferCleanup( + k8sClient.Delete, ctx, CreatePlacementAPISecret(names.Namespace, SecretName)) + keystoneAPIName := keystone.CreateKeystoneAPI(names.Namespace) + DeferCleanup(keystone.DeleteKeystoneAPI, keystoneAPIName) + + spec := GetDefaultPlacementAPISpec() + spec["auth"] = map[string]any{ + "applicationCredentialSecret": "nonexistent-ac-secret", + } + + DeferCleanup(th.DeleteInstance, CreatePlacementAPI(names.PlacementAPIName, spec)) + + DeferCleanup( + mariadb.DeleteDBService, + mariadb.CreateDBService( + names.Namespace, + GetDefaultPlacementAPISpec()["databaseInstance"].(string), + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}, + }, + ), + ) + mariadb.SimulateMariaDBDatabaseCompleted(names.MariaDBDatabaseName) + mariadb.SimulateMariaDBAccountCompleted(names.MariaDBAccount) + }) + + It("should set ServiceConfigReady to False", func() { + th.ExpectCondition( + names.PlacementAPIName, + ConditionGetterFunc(PlacementConditionGetter), + condition.ServiceConfigReadyCondition, + corev1.ConditionFalse, + ) + }) + }) + + When("ApplicationCredential secret is missing required keys", func() { + BeforeEach(func() { + DeferCleanup( + k8sClient.Delete, ctx, CreatePlacementAPISecret(names.Namespace, SecretName)) + keystoneAPIName := keystone.CreateKeystoneAPI(names.Namespace) + DeferCleanup(keystone.DeleteKeystoneAPI, keystoneAPIName) + + badACSecretName := "ac-placement-bad-secret" //nolint:gosec // G101 + badSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: names.Namespace, + Name: badACSecretName, + }, + Data: map[string][]byte{ + "WRONG_KEY": []byte("some-value"), + }, + } + DeferCleanup(k8sClient.Delete, ctx, badSecret) + Expect(k8sClient.Create(ctx, badSecret)).To(Succeed()) + + spec := GetDefaultPlacementAPISpec() + spec["auth"] = map[string]any{ + "applicationCredentialSecret": badACSecretName, + } + + DeferCleanup(th.DeleteInstance, CreatePlacementAPI(names.PlacementAPIName, spec)) + + DeferCleanup( + mariadb.DeleteDBService, + mariadb.CreateDBService( + names.Namespace, + GetDefaultPlacementAPISpec()["databaseInstance"].(string), + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}, + }, + ), + ) + mariadb.SimulateMariaDBDatabaseCompleted(names.MariaDBDatabaseName) + mariadb.SimulateMariaDBAccountCompleted(names.MariaDBAccount) + }) + + It("should set ServiceConfigReady to False", func() { + th.ExpectCondition( + names.PlacementAPIName, + ConditionGetterFunc(PlacementConditionGetter), + condition.ServiceConfigReadyCondition, + corev1.ConditionFalse, + ) + }) + }) + })