Skip to content

Commit 581e9c0

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1822 from Deydra71/appcred-fixes
AppCred fixes and enhancing the AppCred kuttl tests
2 parents da1efd1 + d7de042 commit 581e9c0

21 files changed

+273
-39
lines changed

‎internal/openstack/applicationcredential.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,34 @@ func isACEnabled(globalAC corev1beta1.ApplicationCredentialSection, serviceAC *c
5959
return serviceAC != nil && serviceAC.Enabled
6060
}
6161

62+
// CleanupApplicationCredentialForService deletes the AC CR for a service if it exists.
63+
// Used when a service or its AC is disabled - deletes the AC CR if it exists regardless
64+
// of the AC enabled flag.
65+
func CleanupApplicationCredentialForService(
66+
ctx context.Context,
67+
helper *helper.Helper,
68+
instance *corev1beta1.OpenStackControlPlane,
69+
serviceName string,
70+
) error {
71+
acName := keystonev1.GetACCRName(serviceName)
72+
acCR := &keystonev1.KeystoneApplicationCredential{
73+
ObjectMeta: metav1.ObjectMeta{
74+
Name: acName,
75+
Namespace: instance.Namespace,
76+
},
77+
}
78+
Log := GetLogger(ctx)
79+
err := helper.GetClient().Delete(ctx, acCR)
80+
if k8s_errors.IsNotFound(err) {
81+
return nil
82+
}
83+
if err != nil {
84+
return err
85+
}
86+
Log.Info("Service disabled, deleted existing KeystoneApplicationCredential CR", "service", serviceName, "acName", acName)
87+
return nil
88+
}
89+
6290
// EnsureApplicationCredentialForService handles AC creation for a single service.
6391
// If service is not ready, AC creation is deferred
6492
// If AC already exists and is ready, it's used immediately
@@ -125,6 +153,11 @@ func EnsureApplicationCredentialForService(
125153

126154
// Check if AC CR exists and is ready
127155
if acExists {
156+
// We want to run reconcileApplicationCredential to update the AC CR if it exists and is ready and AC config fields changed
157+
err = reconcileApplicationCredential(ctx, helper, instance, acName, serviceUser, secretName, passwordSelector, merged)
158+
if err != nil {
159+
return "", ctrl.Result{}, err
160+
}
128161
if acCR.IsReady() {
129162
Log.Info("Application Credential is ready", "service", serviceName, "acName", acName, "secretName", acCR.Status.SecretName)
130163
return acCR.Status.SecretName, ctrl.Result{}, nil

‎internal/openstack/barbican.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ func ReconcileBarbican(ctx context.Context, instance *corev1beta1.OpenStackContr
3737
instance.Status.ContainerImages.BarbicanAPIImage = nil
3838
instance.Status.ContainerImages.BarbicanWorkerImage = nil
3939
instance.Status.ContainerImages.BarbicanKeystoneListenerImage = nil
40+
// Clean up AC CRs when service is disabled
41+
if err := CleanupApplicationCredentialForService(ctx, helper, instance, barbican.Name); err != nil {
42+
return ctrl.Result{}, err
43+
}
4044
return ctrl.Result{}, nil
4145
}
4246

@@ -73,8 +77,8 @@ func ReconcileBarbican(ctx context.Context, instance *corev1beta1.OpenStackContr
7377
barbicanSecret = instance.Spec.Secret
7478
}
7579

76-
// Only call if AC enabled or currently configured
77-
if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Barbican.ApplicationCredential) ||
80+
// Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle.
81+
if instance.Spec.Barbican.ApplicationCredential != nil ||
7882
instance.Spec.Barbican.Template.Auth.ApplicationCredentialSecret != "" {
7983

8084
acSecretName, acResult, err := EnsureApplicationCredentialForService(

‎internal/openstack/cinder.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ func ReconcileCinder(ctx context.Context, instance *corev1beta1.OpenStackControl
5959
instance.Status.ContainerImages.CinderSchedulerImage = nil
6060
instance.Status.ContainerImages.CinderBackupImage = nil
6161
instance.Status.ContainerImages.CinderVolumeImages = make(map[string]*string)
62+
// Clean up AC CRs when service is disabled
63+
if err := CleanupApplicationCredentialForService(ctx, helper, instance, cinder.Name); err != nil {
64+
return ctrl.Result{}, err
65+
}
6266
return ctrl.Result{}, nil
6367
}
6468

@@ -96,8 +100,8 @@ func ReconcileCinder(ctx context.Context, instance *corev1beta1.OpenStackControl
96100
cinderSecret = instance.Spec.Secret
97101
}
98102

99-
// Only call if AC enabled or currently configured
100-
if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Cinder.ApplicationCredential) ||
103+
// Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle.
104+
if instance.Spec.Cinder.ApplicationCredential != nil ||
101105
instance.Spec.Cinder.Template.Auth.ApplicationCredentialSecret != "" {
102106

103107
acSecretName, acResult, err := EnsureApplicationCredentialForService(

‎internal/openstack/designate.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ func ReconcileDesignate(ctx context.Context, instance *corev1beta1.OpenStackCont
4141
instance.Status.ContainerImages.DesignateBackendbind9Image = nil
4242
instance.Status.ContainerImages.DesignateUnboundImage = nil
4343
instance.Status.ContainerImages.NetUtilsImage = nil
44+
// Clean up AC CRs when service is disabled
45+
if err := CleanupApplicationCredentialForService(ctx, helper, instance, designate.Name); err != nil {
46+
return ctrl.Result{}, err
47+
}
4448
return ctrl.Result{}, nil
4549
}
4650

@@ -85,8 +89,8 @@ func ReconcileDesignate(ctx context.Context, instance *corev1beta1.OpenStackCont
8589
designateSecret = instance.Spec.Secret
8690
}
8791

88-
// Only call if AC enabled or currently configured
89-
if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Designate.ApplicationCredential) ||
92+
// Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle.
93+
if instance.Spec.Designate.ApplicationCredential != nil ||
9094
instance.Spec.Designate.Template.DesignateAPI.Auth.ApplicationCredentialSecret != "" {
9195

9296
acSecretName, acResult, err := EnsureApplicationCredentialForService(

‎internal/openstack/glance.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ func ReconcileGlance(ctx context.Context, instance *corev1beta1.OpenStackControl
6464
instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneGlanceReadyCondition)
6565
instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneExposeGlanceReadyCondition)
6666
instance.Status.ContainerImages.GlanceAPIImage = nil
67+
// Clean up AC CRs when service is disabled
68+
if err := CleanupApplicationCredentialForService(ctx, helper, instance, glance.Name); err != nil {
69+
return ctrl.Result{}, err
70+
}
6771
return ctrl.Result{}, nil
6872
}
6973

@@ -128,9 +132,8 @@ func ReconcileGlance(ctx context.Context, instance *corev1beta1.OpenStackControl
128132
}
129133
}
130134

131-
// Only call if AC enabled or currently configured
132-
if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Glance.ApplicationCredential) || hasACConfigured {
133-
135+
// Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle.
136+
if instance.Spec.Glance.ApplicationCredential != nil || hasACConfigured {
134137
acSecretName, acResult, err := EnsureApplicationCredentialForService(
135138
ctx,
136139
helper,

‎internal/openstack/heat.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ func ReconcileHeat(ctx context.Context, instance *corev1beta1.OpenStackControlPl
4040
instance.Status.ContainerImages.HeatAPIImage = nil
4141
instance.Status.ContainerImages.HeatCfnapiImage = nil
4242
instance.Status.ContainerImages.HeatEngineImage = nil
43+
// Clean up AC CRs when service is disabled
44+
if err := CleanupApplicationCredentialForService(ctx, helper, instance, heat.Name); err != nil {
45+
return ctrl.Result{}, err
46+
}
4347
return ctrl.Result{}, nil
4448
}
4549

@@ -120,8 +124,8 @@ func ReconcileHeat(ctx context.Context, instance *corev1beta1.OpenStackControlPl
120124
heatSecret = instance.Spec.Secret
121125
}
122126

123-
// Only call if AC enabled or currently configured
124-
if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Heat.ApplicationCredential) ||
127+
// Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle.
128+
if instance.Spec.Heat.ApplicationCredential != nil ||
125129
instance.Spec.Heat.Template.Auth.ApplicationCredentialSecret != "" {
126130

127131
heatACSecretName, acResult, err := EnsureApplicationCredentialForService(

‎internal/openstack/ironic.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ func ReconcileIronic(ctx context.Context, instance *corev1beta1.OpenStackControl
4040
instance.Status.ContainerImages.IronicNeutronAgentImage = nil
4141
instance.Status.ContainerImages.IronicPxeImage = nil
4242
instance.Status.ContainerImages.IronicPythonAgentImage = nil
43+
// Clean up AC CRs when service is disabled (ironic has two: ironic and ironic-inspector)
44+
if err := CleanupApplicationCredentialForService(ctx, helper, instance, ironic.Name); err != nil {
45+
return ctrl.Result{}, err
46+
}
47+
if err := CleanupApplicationCredentialForService(ctx, helper, instance, "ironic-inspector"); err != nil {
48+
return ctrl.Result{}, err
49+
}
4350
return ctrl.Result{}, nil
4451
}
4552

@@ -124,8 +131,8 @@ func ReconcileIronic(ctx context.Context, instance *corev1beta1.OpenStackControl
124131
ironicSecret = instance.Spec.Secret
125132
}
126133

127-
// Only call if AC enabled or currently configured
128-
if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Ironic.ApplicationCredential) ||
134+
// Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle.
135+
if instance.Spec.Ironic.ApplicationCredential != nil ||
129136
instance.Spec.Ironic.Template.Auth.ApplicationCredentialSecret != "" ||
130137
instance.Spec.Ironic.Template.IronicInspector.Auth.ApplicationCredentialSecret != "" {
131138

‎internal/openstack/manila.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ func ReconcileManila(ctx context.Context, instance *corev1beta1.OpenStackControl
3838
instance.Status.ContainerImages.ManilaAPIImage = nil
3939
instance.Status.ContainerImages.ManilaSchedulerImage = nil
4040
instance.Status.ContainerImages.ManilaShareImages = make(map[string]*string)
41+
// Clean up AC CRs when service is disabled
42+
if err := CleanupApplicationCredentialForService(ctx, helper, instance, manila.Name); err != nil {
43+
return ctrl.Result{}, err
44+
}
4145
return ctrl.Result{}, nil
4246
}
4347

@@ -75,8 +79,8 @@ func ReconcileManila(ctx context.Context, instance *corev1beta1.OpenStackControl
7579
manilaSecret = instance.Spec.Secret
7680
}
7781

78-
// Only call if AC enabled or currently configured
79-
if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Manila.ApplicationCredential) ||
82+
// Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle.
83+
if instance.Spec.Manila.ApplicationCredential != nil ||
8084
instance.Spec.Manila.Template.Auth.ApplicationCredentialSecret != "" {
8185

8286
acSecretName, acResult, err := EnsureApplicationCredentialForService(

‎internal/openstack/neutron.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ func ReconcileNeutron(ctx context.Context, instance *corev1beta1.OpenStackContro
3939
instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneNeutronReadyCondition)
4040
instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneExposeNeutronReadyCondition)
4141
instance.Status.ContainerImages.NeutronAPIImage = nil
42+
// Clean up AC CRs when service is disabled
43+
if err := CleanupApplicationCredentialForService(ctx, helper, instance, neutronAPI.Name); err != nil {
44+
return ctrl.Result{}, err
45+
}
4246
return ctrl.Result{}, nil
4347
}
4448

@@ -119,8 +123,8 @@ func ReconcileNeutron(ctx context.Context, instance *corev1beta1.OpenStackContro
119123
neutronSecret = instance.Spec.Secret
120124
}
121125

122-
// Only call if AC enabled or currently configured
123-
if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Neutron.ApplicationCredential) ||
126+
// Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle.
127+
if instance.Spec.Neutron.ApplicationCredential != nil ||
124128
instance.Spec.Neutron.Template.Auth.ApplicationCredentialSecret != "" {
125129

126130
acSecretName, acResult, err := EnsureApplicationCredentialForService(

‎internal/openstack/nova.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPl
6161
instance.Status.ContainerImages.NovaConductorImage = nil
6262
instance.Status.ContainerImages.NovaNovncImage = nil
6363
instance.Status.ContainerImages.NovaSchedulerImage = nil
64+
// Clean up AC CRs when service is disabled
65+
if err := CleanupApplicationCredentialForService(ctx, helper, instance, nova.Name); err != nil {
66+
return ctrl.Result{}, err
67+
}
6468
return ctrl.Result{}, nil
6569
}
6670

@@ -191,8 +195,8 @@ func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPl
191195
novaSecret = instance.Spec.Secret
192196
}
193197

194-
// Only call if AC enabled or currently configured
195-
if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Nova.ApplicationCredential) ||
198+
// Always reconcile AC - EnsureApplicationCredentialForService checks cluster state and handles the full AC lifecycle.
199+
if instance.Spec.Nova.ApplicationCredential != nil ||
196200
instance.Spec.Nova.Template.Auth.ApplicationCredentialSecret != "" {
197201

198202
acSecretName, acResult, err := EnsureApplicationCredentialForService(

0 commit comments

Comments
 (0)