diff --git a/internal/openstack/applicationcredential.go b/internal/openstack/applicationcredential.go index 02d402ba9..87129be80 100644 --- a/internal/openstack/applicationcredential.go +++ b/internal/openstack/applicationcredential.go @@ -125,6 +125,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 17aaa88b0..ab5d20b3b 100644 --- a/internal/openstack/barbican.go +++ b/internal/openstack/barbican.go @@ -73,8 +73,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 e68c58ce4..f514a47ad 100644 --- a/internal/openstack/cinder.go +++ b/internal/openstack/cinder.go @@ -96,8 +96,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 eb10935db..955bed177 100644 --- a/internal/openstack/designate.go +++ b/internal/openstack/designate.go @@ -85,8 +85,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 594f9a722..39f11861b 100644 --- a/internal/openstack/glance.go +++ b/internal/openstack/glance.go @@ -128,9 +128,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 { - + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 19c0cb7a1..46e1a10b1 100644 --- a/internal/openstack/heat.go +++ b/internal/openstack/heat.go @@ -120,8 +120,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 2f76e9e8b..c9bdc89fd 100644 --- a/internal/openstack/ironic.go +++ b/internal/openstack/ironic.go @@ -124,8 +124,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + if instance.Spec.Ironic.ApplicationCredential != nil || instance.Spec.Ironic.Template.Auth.ApplicationCredentialSecret != "" || instance.Spec.Ironic.Template.IronicInspector.Auth.ApplicationCredentialSecret != "" { @@ -145,7 +145,6 @@ func ReconcileIronic(ctx context.Context, instance *corev1beta1.OpenStackControl return ctrl.Result{}, err } - // If AC is not ready, return immediately without updating the service CR if (acResult != ctrl.Result{}) { return acResult, nil } @@ -162,7 +161,7 @@ func ReconcileIronic(ctx context.Context, instance *corev1beta1.OpenStackControl instance, "ironic-inspector", ironicReady, - ironicSecret, // Inspector shares the same secret as ironic + ironicSecret, instance.Spec.Ironic.Template.IronicInspector.PasswordSelectors.Service, instance.Spec.Ironic.Template.IronicInspector.ServiceUser, instance.Spec.Ironic.ApplicationCredential, @@ -171,7 +170,6 @@ func ReconcileIronic(ctx context.Context, instance *corev1beta1.OpenStackControl return ctrl.Result{}, err } - // If AC is not ready, return immediately without updating the service CR if (inspectorACResult != ctrl.Result{}) { return inspectorACResult, nil } diff --git a/internal/openstack/manila.go b/internal/openstack/manila.go index 8c176b747..d461dfe03 100644 --- a/internal/openstack/manila.go +++ b/internal/openstack/manila.go @@ -75,8 +75,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 f36cca3a4..cb835e857 100644 --- a/internal/openstack/neutron.go +++ b/internal/openstack/neutron.go @@ -119,8 +119,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 aa27677a8..7cffeafe1 100644 --- a/internal/openstack/nova.go +++ b/internal/openstack/nova.go @@ -191,8 +191,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 5765b0152..c702893ef 100644 --- a/internal/openstack/octavia.go +++ b/internal/openstack/octavia.go @@ -167,9 +167,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 != "" { + // Reconcile AC if configured (enabled or disabled) or secret previously set + if instance.Spec.Octavia.ApplicationCredential != nil || + instance.Spec.Octavia.Template.Auth.ApplicationCredentialSecret != "" { acSecretName, acResult, err := EnsureApplicationCredentialForService( ctx, @@ -194,7 +194,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 3658b1263..0c704a35c 100644 --- a/internal/openstack/placement.go +++ b/internal/openstack/placement.go @@ -79,8 +79,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 54c718614..33797bf0a 100644 --- a/internal/openstack/swift.go +++ b/internal/openstack/swift.go @@ -109,8 +109,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 c2009bc8c..fd1c6c61b 100644 --- a/internal/openstack/telemetry.go +++ b/internal/openstack/telemetry.go @@ -127,8 +127,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 @@ -162,15 +162,12 @@ func ReconcileTelemetry(ctx context.Context, instance *corev1beta1.OpenStackCont // - If AC enabled and ready: returns the AC secret name instance.Spec.Telemetry.Template.Autoscaling.Aodh.Auth.ApplicationCredentialSecret = aodhACSecretName } - } else { - // Aodh service disabled, clear the field - 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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 @@ -204,15 +201,11 @@ func ReconcileTelemetry(ctx context.Context, instance *corev1beta1.OpenStackCont // - If AC enabled and ready: returns the AC secret name instance.Spec.Telemetry.Template.Ceilometer.Auth.ApplicationCredentialSecret = ceilometerACSecretName } - } else { - // Ceilometer service disabled, clear the field - 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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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 @@ -246,9 +239,6 @@ func ReconcileTelemetry(ctx context.Context, instance *corev1beta1.OpenStackCont // - If AC enabled and ready: returns the AC secret name instance.Spec.Telemetry.Template.CloudKitty.Auth.ApplicationCredentialSecret = cloudkittyACSecretName } - } else { - // CloudKitty service disabled, clear the field - instance.Spec.Telemetry.Template.CloudKitty.Auth.ApplicationCredentialSecret = "" } // preserve any previously set TLS certs, set CA cert diff --git a/internal/openstack/watcher.go b/internal/openstack/watcher.go index daf716232..d8295ee68 100644 --- a/internal/openstack/watcher.go +++ b/internal/openstack/watcher.go @@ -88,8 +88,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) || + // Reconcile AC if configured (enabled or disabled) or secret previously set + 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-cleanup.yaml b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-assert-appcred-cleanup.yaml new file mode 100644 index 000000000..0027dc0fa --- /dev/null +++ b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-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/03-disable-appcred-config.yaml b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-disable-appcred-config.yaml new file mode 100644 index 000000000..f468cb921 --- /dev/null +++ b/test/kuttl/tests/ctlplane-basic-deployment-with-appcred/03-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/04-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/04-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/04-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/04-errors-cleanup.yaml