From 326f8ac90ecec417000caf0f76ac52a69a5b288c Mon Sep 17 00:00:00 2001 From: Veronika Fisarova Date: Mon, 23 Feb 2026 14:52:04 +0100 Subject: [PATCH] AppCred fixes Fix propagation of ApplicationCredentialSecret into correct Octavia Auth spec field. Fix reconcile on AC config changes (such as roles, expiry...). Fix deleting AC CRs when app creds are disabled (globally and for the service). Enhance kuttl test scenario to check the AC CR deletion. Signed-off-by: Veronika Fisarova Co-authored-by: Milana Levy --- internal/openstack/applicationcredential.go | 33 ++++++++++ internal/openstack/barbican.go | 8 ++- internal/openstack/cinder.go | 8 ++- internal/openstack/designate.go | 8 ++- internal/openstack/glance.go | 9 ++- internal/openstack/heat.go | 8 ++- internal/openstack/ironic.go | 11 +++- internal/openstack/manila.go | 8 ++- internal/openstack/neutron.go | 8 ++- internal/openstack/nova.go | 8 ++- internal/openstack/octavia.go | 12 ++-- internal/openstack/placement.go | 8 ++- internal/openstack/swift.go | 8 ++- internal/openstack/telemetry.go | 34 +++++++--- internal/openstack/watcher.go | 8 ++- .../03-assert-appcred-propagation.yaml | 66 +++++++++++++++++++ .../03-update-appcred-config.yaml | 11 ++++ .../04-assert-appcred-cleanup.yaml | 51 ++++++++++++++ .../04-disable-appcred-config.yaml | 5 ++ .../{03-cleanup.yaml => 05-cleanup.yaml} | 0 ...rs-cleanup.yaml => 05-errors-cleanup.yaml} | 0 21 files changed, 273 insertions(+), 39 deletions(-) create mode 100644 test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-assert-appcred-propagation.yaml create mode 100644 test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-update-appcred-config.yaml create mode 100644 test/kuttl/tests/ctlplane-basic-deployment-with-appcred/04-assert-appcred-cleanup.yaml create mode 100644 test/kuttl/tests/ctlplane-basic-deployment-with-appcred/04-disable-appcred-config.yaml rename test/kuttl/tests/ctlplane-basic-deployment-with-appcred/{03-cleanup.yaml => 05-cleanup.yaml} (100%) rename test/kuttl/tests/ctlplane-basic-deployment-with-appcred/{03-errors-cleanup.yaml => 05-errors-cleanup.yaml} (100%) diff --git a/internal/openstack/applicationcredential.go b/internal/openstack/applicationcredential.go index 02d402ba97..3c699df561 100644 --- a/internal/openstack/applicationcredential.go +++ b/internal/openstack/applicationcredential.go @@ -59,6 +59,34 @@ func isACEnabled(globalAC corev1beta1.ApplicationCredentialSection, serviceAC *c return serviceAC != nil && serviceAC.Enabled } +// CleanupApplicationCredentialForService deletes the AC CR for a service if it exists. +// Used when a service or its AC is disabled - deletes the AC CR if it exists regardless +// of the AC enabled flag. +func CleanupApplicationCredentialForService( + ctx context.Context, + helper *helper.Helper, + instance *corev1beta1.OpenStackControlPlane, + serviceName string, +) error { + acName := keystonev1.GetACCRName(serviceName) + acCR := &keystonev1.KeystoneApplicationCredential{ + ObjectMeta: metav1.ObjectMeta{ + Name: acName, + Namespace: instance.Namespace, + }, + } + Log := GetLogger(ctx) + err := helper.GetClient().Delete(ctx, acCR) + if k8s_errors.IsNotFound(err) { + return nil + } + if err != nil { + return err + } + Log.Info("Service disabled, deleted existing KeystoneApplicationCredential CR", "service", serviceName, "acName", acName) + return nil +} + // EnsureApplicationCredentialForService handles AC creation for a single service. // If service is not ready, AC creation is deferred // If AC already exists and is ready, it's used immediately @@ -125,6 +153,11 @@ func EnsureApplicationCredentialForService( // Check if AC CR exists and is ready if acExists { + // We want to run reconcileApplicationCredential to update the AC CR if it exists and is ready and AC config fields changed + err = reconcileApplicationCredential(ctx, helper, instance, acName, serviceUser, secretName, passwordSelector, merged) + if err != nil { + return "", ctrl.Result{}, err + } if acCR.IsReady() { Log.Info("Application Credential is ready", "service", serviceName, "acName", acName, "secretName", acCR.Status.SecretName) return acCR.Status.SecretName, ctrl.Result{}, nil diff --git a/internal/openstack/barbican.go b/internal/openstack/barbican.go index 17aaa88b05..1c566ed738 100644 --- a/internal/openstack/barbican.go +++ b/internal/openstack/barbican.go @@ -37,6 +37,10 @@ func ReconcileBarbican(ctx context.Context, instance *corev1beta1.OpenStackContr instance.Status.ContainerImages.BarbicanAPIImage = nil instance.Status.ContainerImages.BarbicanWorkerImage = nil instance.Status.ContainerImages.BarbicanKeystoneListenerImage = nil + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, barbican.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -73,8 +77,8 @@ func ReconcileBarbican(ctx context.Context, instance *corev1beta1.OpenStackContr barbicanSecret = instance.Spec.Secret } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Barbican.ApplicationCredential) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Barbican.ApplicationCredential != nil || instance.Spec.Barbican.Template.Auth.ApplicationCredentialSecret != "" { acSecretName, acResult, err := EnsureApplicationCredentialForService( diff --git a/internal/openstack/cinder.go b/internal/openstack/cinder.go index e68c58ce44..1d64d425a4 100644 --- a/internal/openstack/cinder.go +++ b/internal/openstack/cinder.go @@ -59,6 +59,10 @@ func ReconcileCinder(ctx context.Context, instance *corev1beta1.OpenStackControl instance.Status.ContainerImages.CinderSchedulerImage = nil instance.Status.ContainerImages.CinderBackupImage = nil instance.Status.ContainerImages.CinderVolumeImages = make(map[string]*string) + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, cinder.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -96,8 +100,8 @@ func ReconcileCinder(ctx context.Context, instance *corev1beta1.OpenStackControl cinderSecret = instance.Spec.Secret } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Cinder.ApplicationCredential) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Cinder.ApplicationCredential != nil || instance.Spec.Cinder.Template.Auth.ApplicationCredentialSecret != "" { acSecretName, acResult, err := EnsureApplicationCredentialForService( diff --git a/internal/openstack/designate.go b/internal/openstack/designate.go index eb10935db6..5ef092b0e0 100644 --- a/internal/openstack/designate.go +++ b/internal/openstack/designate.go @@ -41,6 +41,10 @@ func ReconcileDesignate(ctx context.Context, instance *corev1beta1.OpenStackCont instance.Status.ContainerImages.DesignateBackendbind9Image = nil instance.Status.ContainerImages.DesignateUnboundImage = nil instance.Status.ContainerImages.NetUtilsImage = nil + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, designate.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -85,8 +89,8 @@ func ReconcileDesignate(ctx context.Context, instance *corev1beta1.OpenStackCont designateSecret = instance.Spec.Secret } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Designate.ApplicationCredential) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Designate.ApplicationCredential != nil || instance.Spec.Designate.Template.DesignateAPI.Auth.ApplicationCredentialSecret != "" { acSecretName, acResult, err := EnsureApplicationCredentialForService( diff --git a/internal/openstack/glance.go b/internal/openstack/glance.go index 594f9a7226..2ab1eaf63d 100644 --- a/internal/openstack/glance.go +++ b/internal/openstack/glance.go @@ -64,6 +64,10 @@ func ReconcileGlance(ctx context.Context, instance *corev1beta1.OpenStackControl instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneGlanceReadyCondition) instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneExposeGlanceReadyCondition) instance.Status.ContainerImages.GlanceAPIImage = nil + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, glance.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -128,9 +132,8 @@ func ReconcileGlance(ctx context.Context, instance *corev1beta1.OpenStackControl } } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Glance.ApplicationCredential) || hasACConfigured { - + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Glance.ApplicationCredential != nil || hasACConfigured { acSecretName, acResult, err := EnsureApplicationCredentialForService( ctx, helper, diff --git a/internal/openstack/heat.go b/internal/openstack/heat.go index 19c0cb7a1f..a9bad1d9b0 100644 --- a/internal/openstack/heat.go +++ b/internal/openstack/heat.go @@ -40,6 +40,10 @@ func ReconcileHeat(ctx context.Context, instance *corev1beta1.OpenStackControlPl instance.Status.ContainerImages.HeatAPIImage = nil instance.Status.ContainerImages.HeatCfnapiImage = nil instance.Status.ContainerImages.HeatEngineImage = nil + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, heat.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -120,8 +124,8 @@ func ReconcileHeat(ctx context.Context, instance *corev1beta1.OpenStackControlPl heatSecret = instance.Spec.Secret } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Heat.ApplicationCredential) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Heat.ApplicationCredential != nil || instance.Spec.Heat.Template.Auth.ApplicationCredentialSecret != "" { heatACSecretName, acResult, err := EnsureApplicationCredentialForService( diff --git a/internal/openstack/ironic.go b/internal/openstack/ironic.go index 2f76e9e8b2..37b3ff4222 100644 --- a/internal/openstack/ironic.go +++ b/internal/openstack/ironic.go @@ -40,6 +40,13 @@ func ReconcileIronic(ctx context.Context, instance *corev1beta1.OpenStackControl instance.Status.ContainerImages.IronicNeutronAgentImage = nil instance.Status.ContainerImages.IronicPxeImage = nil instance.Status.ContainerImages.IronicPythonAgentImage = nil + // Clean up AC CRs when service is disabled (ironic has two: ironic and ironic-inspector) + if err := CleanupApplicationCredentialForService(ctx, helper, instance, ironic.Name); err != nil { + return ctrl.Result{}, err + } + if err := CleanupApplicationCredentialForService(ctx, helper, instance, "ironic-inspector"); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -124,8 +131,8 @@ func ReconcileIronic(ctx context.Context, instance *corev1beta1.OpenStackControl ironicSecret = instance.Spec.Secret } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Ironic.ApplicationCredential) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Ironic.ApplicationCredential != nil || instance.Spec.Ironic.Template.Auth.ApplicationCredentialSecret != "" || instance.Spec.Ironic.Template.IronicInspector.Auth.ApplicationCredentialSecret != "" { diff --git a/internal/openstack/manila.go b/internal/openstack/manila.go index 8c176b7477..19a6f8244a 100644 --- a/internal/openstack/manila.go +++ b/internal/openstack/manila.go @@ -38,6 +38,10 @@ func ReconcileManila(ctx context.Context, instance *corev1beta1.OpenStackControl instance.Status.ContainerImages.ManilaAPIImage = nil instance.Status.ContainerImages.ManilaSchedulerImage = nil instance.Status.ContainerImages.ManilaShareImages = make(map[string]*string) + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, manila.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -75,8 +79,8 @@ func ReconcileManila(ctx context.Context, instance *corev1beta1.OpenStackControl manilaSecret = instance.Spec.Secret } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Manila.ApplicationCredential) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Manila.ApplicationCredential != nil || instance.Spec.Manila.Template.Auth.ApplicationCredentialSecret != "" { acSecretName, acResult, err := EnsureApplicationCredentialForService( diff --git a/internal/openstack/neutron.go b/internal/openstack/neutron.go index f36cca3a4b..45479df0ee 100644 --- a/internal/openstack/neutron.go +++ b/internal/openstack/neutron.go @@ -39,6 +39,10 @@ func ReconcileNeutron(ctx context.Context, instance *corev1beta1.OpenStackContro instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneNeutronReadyCondition) instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneExposeNeutronReadyCondition) instance.Status.ContainerImages.NeutronAPIImage = nil + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, neutronAPI.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -119,8 +123,8 @@ func ReconcileNeutron(ctx context.Context, instance *corev1beta1.OpenStackContro neutronSecret = instance.Spec.Secret } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Neutron.ApplicationCredential) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Neutron.ApplicationCredential != nil || instance.Spec.Neutron.Template.Auth.ApplicationCredentialSecret != "" { acSecretName, acResult, err := EnsureApplicationCredentialForService( diff --git a/internal/openstack/nova.go b/internal/openstack/nova.go index aa27677a8e..83d9c94cb9 100644 --- a/internal/openstack/nova.go +++ b/internal/openstack/nova.go @@ -61,6 +61,10 @@ func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPl instance.Status.ContainerImages.NovaConductorImage = nil instance.Status.ContainerImages.NovaNovncImage = nil instance.Status.ContainerImages.NovaSchedulerImage = nil + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, nova.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -191,8 +195,8 @@ func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPl novaSecret = instance.Spec.Secret } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Nova.ApplicationCredential) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Nova.ApplicationCredential != nil || instance.Spec.Nova.Template.Auth.ApplicationCredentialSecret != "" { acSecretName, acResult, err := EnsureApplicationCredentialForService( diff --git a/internal/openstack/octavia.go b/internal/openstack/octavia.go index 5765b01528..825cdd9dab 100644 --- a/internal/openstack/octavia.go +++ b/internal/openstack/octavia.go @@ -59,6 +59,10 @@ func ReconcileOctavia(ctx context.Context, instance *corev1beta1.OpenStackContro instance.Status.ContainerImages.OctaviaHousekeepingImage = nil instance.Status.ContainerImages.OctaviaApacheImage = nil instance.Status.ContainerImages.OctaviaRsyslogImage = nil + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, octavia.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -167,9 +171,9 @@ func ReconcileOctavia(ctx context.Context, instance *corev1beta1.OpenStackContro octaviaSecret = instance.Spec.Secret } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Octavia.ApplicationCredential) || - instance.Spec.Octavia.Template.OctaviaAPI.Auth.ApplicationCredentialSecret != "" { + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Octavia.ApplicationCredential != nil || + instance.Spec.Octavia.Template.Auth.ApplicationCredentialSecret != "" { acSecretName, acResult, err := EnsureApplicationCredentialForService( ctx, @@ -194,7 +198,7 @@ func ReconcileOctavia(ctx context.Context, instance *corev1beta1.OpenStackContro // Set ApplicationCredentialSecret based on what the helper returned: // - If AC disabled: returns "" // - If AC enabled and ready: returns the AC secret name - instance.Spec.Octavia.Template.OctaviaAPI.Auth.ApplicationCredentialSecret = acSecretName + instance.Spec.Octavia.Template.Auth.ApplicationCredentialSecret = acSecretName } svcs, err := service.GetServicesListWithLabel( diff --git a/internal/openstack/placement.go b/internal/openstack/placement.go index 3658b1263a..60c17ad57d 100644 --- a/internal/openstack/placement.go +++ b/internal/openstack/placement.go @@ -34,6 +34,10 @@ func ReconcilePlacementAPI(ctx context.Context, instance *corev1beta1.OpenStackC } instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlanePlacementAPIReadyCondition) instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneExposePlacementAPIReadyCondition) + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, placementAPI.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -79,8 +83,8 @@ func ReconcilePlacementAPI(ctx context.Context, instance *corev1beta1.OpenStackC placementSecret = instance.Spec.Secret } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Placement.ApplicationCredential) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Placement.ApplicationCredential != nil || instance.Spec.Placement.Template.Auth.ApplicationCredentialSecret != "" { acSecretName, acResult, err := EnsureApplicationCredentialForService( diff --git a/internal/openstack/swift.go b/internal/openstack/swift.go index 54c7186147..e7dc468a7f 100644 --- a/internal/openstack/swift.go +++ b/internal/openstack/swift.go @@ -40,6 +40,10 @@ func ReconcileSwift(ctx context.Context, instance *corev1beta1.OpenStackControlP instance.Status.ContainerImages.SwiftContainerImage = nil instance.Status.ContainerImages.SwiftObjectImage = nil instance.Status.ContainerImages.SwiftProxyImage = nil + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, swift.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -109,8 +113,8 @@ func ReconcileSwift(ctx context.Context, instance *corev1beta1.OpenStackControlP swiftSecret = instance.Spec.Secret } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Swift.ApplicationCredential) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Swift.ApplicationCredential != nil || instance.Spec.Swift.Template.SwiftProxy.Auth.ApplicationCredentialSecret != "" { acSecretName, acResult, err := EnsureApplicationCredentialForService( diff --git a/internal/openstack/telemetry.go b/internal/openstack/telemetry.go index c2009bc8c3..0d302cb4cd 100644 --- a/internal/openstack/telemetry.go +++ b/internal/openstack/telemetry.go @@ -54,6 +54,12 @@ func ReconcileTelemetry(ctx context.Context, instance *corev1beta1.OpenStackCont instance.Status.ContainerImages.AodhListenerImage = nil instance.Status.ContainerImages.CloudKittyAPIImage = nil instance.Status.ContainerImages.CloudKittyProcImage = nil + // Clean up AC CRs when service is disabled (telemetry has three: aodh, ceilometer, cloudkitty) + for _, svcName := range []string{"aodh", "ceilometer", "cloudkitty"} { + if err := CleanupApplicationCredentialForService(ctx, helper, instance, svcName); err != nil { + return ctrl.Result{}, err + } + } return ctrl.Result{}, nil } @@ -127,8 +133,8 @@ func ReconcileTelemetry(ctx context.Context, instance *corev1beta1.OpenStackCont // AC for Aodh (if service enabled) if instance.Spec.Telemetry.Template.Autoscaling.Enabled != nil && *instance.Spec.Telemetry.Template.Autoscaling.Enabled { - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Telemetry.ApplicationCredentialAodh) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Telemetry.ApplicationCredentialAodh != nil || instance.Spec.Telemetry.Template.Autoscaling.Aodh.Auth.ApplicationCredentialSecret != "" { // Apply same fallback logic as in CreateOrPatch to avoid passing empty values to AC @@ -163,14 +169,17 @@ func ReconcileTelemetry(ctx context.Context, instance *corev1beta1.OpenStackCont instance.Spec.Telemetry.Template.Autoscaling.Aodh.Auth.ApplicationCredentialSecret = aodhACSecretName } } else { - // Aodh service disabled, clear the field + // Aodh sub-service disabled - clean up AC CR and clear secret field + if err := CleanupApplicationCredentialForService(ctx, helper, instance, "aodh"); err != nil { + return ctrl.Result{}, err + } instance.Spec.Telemetry.Template.Autoscaling.Aodh.Auth.ApplicationCredentialSecret = "" } // AC for Ceilometer (if service enabled) if instance.Spec.Telemetry.Template.Ceilometer.Enabled != nil && *instance.Spec.Telemetry.Template.Ceilometer.Enabled { - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Telemetry.ApplicationCredentialCeilometer) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Telemetry.ApplicationCredentialCeilometer != nil || instance.Spec.Telemetry.Template.Ceilometer.Auth.ApplicationCredentialSecret != "" { // Apply same fallback logic as in CreateOrPatch to avoid passing empty values to AC @@ -205,14 +214,16 @@ func ReconcileTelemetry(ctx context.Context, instance *corev1beta1.OpenStackCont instance.Spec.Telemetry.Template.Ceilometer.Auth.ApplicationCredentialSecret = ceilometerACSecretName } } else { - // Ceilometer service disabled, clear the field + // Ceilometer sub-service disabled - clean up AC CR and clear secret field + if err := CleanupApplicationCredentialForService(ctx, helper, instance, "ceilometer"); err != nil { + return ctrl.Result{}, err + } instance.Spec.Telemetry.Template.Ceilometer.Auth.ApplicationCredentialSecret = "" } - // AC for CloudKitty (if service enabled) if instance.Spec.Telemetry.Template.CloudKitty.Enabled != nil && *instance.Spec.Telemetry.Template.CloudKitty.Enabled { - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Telemetry.ApplicationCredentialCloudKitty) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Telemetry.ApplicationCredentialCloudKitty != nil || instance.Spec.Telemetry.Template.CloudKitty.Auth.ApplicationCredentialSecret != "" { // Apply same fallback logic as in CreateOrPatch to avoid passing empty values to AC @@ -247,7 +258,10 @@ func ReconcileTelemetry(ctx context.Context, instance *corev1beta1.OpenStackCont instance.Spec.Telemetry.Template.CloudKitty.Auth.ApplicationCredentialSecret = cloudkittyACSecretName } } else { - // CloudKitty service disabled, clear the field + // CloudKitty sub-service disabled - clean up AC CR and clear secret field + if err := CleanupApplicationCredentialForService(ctx, helper, instance, "cloudkitty"); err != nil { + return ctrl.Result{}, err + } instance.Spec.Telemetry.Template.CloudKitty.Auth.ApplicationCredentialSecret = "" } diff --git a/internal/openstack/watcher.go b/internal/openstack/watcher.go index daf7162321..bfed839c50 100644 --- a/internal/openstack/watcher.go +++ b/internal/openstack/watcher.go @@ -36,6 +36,10 @@ func ReconcileWatcher(ctx context.Context, instance *corev1beta1.OpenStackContro instance.Status.ContainerImages.WatcherAPIImage = nil instance.Status.ContainerImages.WatcherApplierImage = nil instance.Status.ContainerImages.WatcherDecisionEngineImage = nil + // Clean up AC CRs when service is disabled + if err := CleanupApplicationCredentialForService(ctx, helper, instance, watcher.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -88,8 +92,8 @@ func ReconcileWatcher(ctx context.Context, instance *corev1beta1.OpenStackContro return "" } - // Only call if AC enabled or currently configured - if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Watcher.ApplicationCredential) || + // Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle. + if instance.Spec.Watcher.ApplicationCredential != nil || instance.Spec.Watcher.Template.Auth.ApplicationCredentialSecret != "" { acSecretName, acResult, err := EnsureApplicationCredentialForService( diff --git a/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-assert-appcred-propagation.yaml b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-assert-appcred-propagation.yaml new file mode 100644 index 0000000000..903109b618 --- /dev/null +++ b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-assert-appcred-propagation.yaml @@ -0,0 +1,66 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +commands: + - script: |- + set -euo pipefail + NS="${NAMESPACE}" + + wait_ready() { + echo "Waiting for appcred/ac-$1 to be Ready..." + oc wait appcred/ac-$1 -n "$NS" --for=condition=Ready --timeout=180s + } + + check_field() { + local name=$1 field=$2 expected=$3 + local actual + actual=$(oc get appcred ac-$name -n "$NS" -o jsonpath="{.spec.$field}" 2>/dev/null || echo "") + if [ "$actual" != "$expected" ]; then + echo "ERROR: ac-$name.$field: expected '$expected', got '$actual'" + exit 1 + fi + echo "✓ ac-$name.$field = $expected" + } + + check_roles() { + local name=$1 + shift + local expected_roles=("$@") + local roles + roles=$(oc get appcred ac-$name -n "$NS" -o jsonpath='{.spec.roles[*]}') + + for role in "${expected_roles[@]}"; do + if [[ ! " $roles " =~ " $role " ]]; then + echo "ERROR: ac-$name: Role '$role' not found. Got: $roles" + exit 1 + fi + done + + local role_count + role_count=$(echo "$roles" | wc -w) + if [ "$role_count" -ne "${#expected_roles[@]}" ]; then + echo "ERROR: ac-$name: Expected ${#expected_roles[@]} roles, got $role_count: $roles" + exit 1 + fi + + echo "✓ ac-$name.roles = [${expected_roles[*]}]" + } + + echo "=========================================" + echo "Testing ApplicationCredential Spec Propagation" + echo "=========================================" + echo + + # ---- ac-barbican: expirationDays 730 -> 365, gracePeriodDays 364 -> 180 ---- + echo "=== Verifying expirationDays propagation on ac-barbican ===" + wait_ready barbican + check_field barbican expirationDays 365 + check_field barbican gracePeriodDays 180 + echo + + # ---- ac-glance: roles [admin,service] -> [service] ---- + echo "=== Verifying roles propagation on ac-glance ===" + wait_ready glance + check_roles glance "service" + echo + + echo "All spec changes propagated to existing ApplicationCredential CRs successfully" diff --git a/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-update-appcred-config.yaml b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-update-appcred-config.yaml new file mode 100644 index 0000000000..4011046cb9 --- /dev/null +++ b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-update-appcred-config.yaml @@ -0,0 +1,11 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: | + # Change barbican's expirationDays from the default (730) to 365 + oc patch openstackcontrolplane openstack -n $NAMESPACE --type merge -p \ + '{"spec":{"barbican":{"applicationCredential":{"expirationDays":365,"gracePeriodDays":180}}}}' + + # Change glance's roles from [admin,service] to [service] only + oc patch openstackcontrolplane openstack -n $NAMESPACE --type merge -p \ + '{"spec":{"glance":{"applicationCredential":{"roles":["service"]}}}}' diff --git a/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/04-assert-appcred-cleanup.yaml b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/04-assert-appcred-cleanup.yaml new file mode 100644 index 0000000000..0027dc0faa --- /dev/null +++ b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/04-assert-appcred-cleanup.yaml @@ -0,0 +1,51 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +commands: + - script: |- + set -euo pipefail + NS="${NAMESPACE}" + + SERVICES=(barbican cinder glance swift neutron placement nova ceilometer) + + wait_deleted() { + local kind=$1 name=$2 timeout=${3:-180} + echo "Waiting for $kind/$name to be deleted..." + local end=$((SECONDS + timeout)) + while [ $SECONDS -lt $end ]; do + if ! oc get "$kind" "$name" -n "$NS" &>/dev/null; then + echo "✓ $kind/$name deleted" + return 0 + fi + sleep 5 + done + echo "ERROR: $kind/$name still exists after ${timeout}s" + exit 1 + } + + echo "=========================================" + echo "Testing Application Credential Cleanup" + echo "=========================================" + echo + + echo "=== Verifying global ApplicationCredential is disabled ===" + global_enabled=$(oc get openstackcontrolplane openstack -n "$NS" -o jsonpath='{.spec.applicationCredential.enabled}') + if [ "$global_enabled" != "false" ]; then + echo "ERROR: OpenStackControlPlane.spec.applicationCredential.enabled expected 'false', got '$global_enabled'" + exit 1 + fi + echo "✓ OpenStackControlPlane.spec.applicationCredential.enabled = false" + echo + + echo "=== Verifying AC CRs are deleted ===" + for svc in "${SERVICES[@]}"; do + wait_deleted appcred "ac-$svc" 180 + done + echo + + echo "=== Verifying AC Secrets are cleaned up ===" + for svc in "${SERVICES[@]}"; do + wait_deleted secret "ac-$svc-secret" 120 + done + echo + + echo "All ApplicationCredential CRs and Secrets cleaned up successfully" diff --git a/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/04-disable-appcred-config.yaml b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/04-disable-appcred-config.yaml new file mode 100644 index 0000000000..f468cb921f --- /dev/null +++ b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/04-disable-appcred-config.yaml @@ -0,0 +1,5 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: | + oc patch openstackcontrolplane openstack -n $NAMESPACE --type merge -p '{"spec":{"applicationCredential":{"enabled":false}}}' diff --git a/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-cleanup.yaml b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/05-cleanup.yaml similarity index 100% rename from test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-cleanup.yaml rename to test/kuttl/tests/ctlplane-basic-deployment-with-appcred/05-cleanup.yaml diff --git a/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-errors-cleanup.yaml b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/05-errors-cleanup.yaml similarity index 100% rename from test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-errors-cleanup.yaml rename to test/kuttl/tests/ctlplane-basic-deployment-with-appcred/05-errors-cleanup.yaml