Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,5 @@ replace k8s.io/component-base => k8s.io/component-base v0.31.13 //allow-merging
replace github.com/rabbitmq/cluster-operator/v2 => github.com/openstack-k8s-operators/rabbitmq-cluster-operator/v2 v2.6.1-0.20250929174222-a0d328fa4dec //allow-merging

replace k8s.io/kube-openapi => k8s.io/kube-openapi v0.0.0-20250627150254-e9823e99808e //allow-merging

replace github.com/openstack-k8s-operators/keystone-operator/api => github.com/Deydra71/keystone-operator/api v0.0.0-20251103091514-244e15fe5d63
55 changes: 55 additions & 0 deletions controllers/glanceapi_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,42 @@ func (r *GlanceAPIReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man
return nil
}

// Application Credential secret watching function
acSecretFn := func(_ context.Context, o client.Object) []reconcile.Request {
name := o.GetName()
ns := o.GetNamespace()
result := []reconcile.Request{}

// Only handle Secret objects
if _, isSecret := o.(*corev1.Secret); !isSecret {
return nil
}

// Check if this is a glance AC secret by name pattern (ac-glance-secret)
expectedSecretName := keystonev1.GetACSecretName("glance")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure we should hardcode glance here, especially because if this depends on the CR name, we might fail due to the fact we can append a suffix from the openstack-operator. If the openstack-operator ends up creating AppCred for glance using the CR name, you might end up having ac-glance-12345678-secret, and this might create side effects to this code (it wouldn't generate a reconcile loop).

Instead of matching a secret using the name, I think it might be solid to select if via LabelSelector, so you can remove the dependency on the name and any potential suffix on the CR.
I did something similar in [1], where you only reconcile if a LabelSelector matches.

However, I'm good with the current approach if we manage to resolve the problem with the hardcoded name, I just think we can make it more solid and less error prone.
Let's think a bit more about it and get some feedback from @stuggi too in case a better pattern already exists!

[1] #795

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @fmount for reviews! I still need to go through them.
But regarding this part - CR Names are not suffixed by openstack-operator [1]. The only place where a random suffix is added is in the Keystone Application Credential name (not the CR) [2].

Regarding matching by the label - currently we don't have any service-specific labels. All AppCred CRs share the same label structure, so using LabelSelector would not distinguish between services. But I'm open to create service specific labes, to bypass hardcoding, which can be always tricky... Let's see about Martin's additional comment.

[1] https://github.com/openstack-k8s-operators/openstack-operator/pull/1430/files#diff-c7b5c3051971f29e36eeddfd6ae38150becaabdf19a75569fb7c9e964c42e78eR113
[2] https://github.com/openstack-k8s-operators/keystone-operator/pull/567/files#diff-83ece155782727a148956f6e4b2b7203661c2ff5b9f128979326158f988bb777R233

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok interesting, I didn't see the openstack-operator patch and now I see that the service list is pretty much static:

svcs := []svcAC{
		{"glance", instance.Spec.Glance.Enabled, instance.Spec.Glance.ApplicationCredential},
		{"nova", instance.Spec.Nova.Enabled, instance.Spec.Nova.ApplicationCredential},
		{"swift", instance.Spec.Swift.Enabled, instance.Spec.Swift.ApplicationCredential},
		{"ceilometer", instance.Spec.Telemetry.Enabled, instance.Spec.Telemetry.ApplicationCredential},
		{"barbican", instance.Spec.Barbican.Enabled, instance.Spec.Barbican.ApplicationCredential},
		{"cinder", instance.Spec.Cinder.Enabled, instance.Spec.Cinder.ApplicationCredential},
		{"placement", instance.Spec.Placement.Enabled, instance.Spec.Placement.ApplicationCredential},
		{"neutron", instance.Spec.Neutron.Enabled, instance.Spec.Neutron.ApplicationCredential},
	}

LabelSelector would involve modifying [1] to pass at least something like service: <service, but if we do not have any aliasing problem then we can go with this version, where you can simply replace "glance" with glance.ServiceName.
Still let's wait for @stuggi's feedback as well, but this clarifies part of my concerns about using the name instead of something different.

[1] https://github.com/openstack-k8s-operators/keystone-operator/pull/567/files#diff-83ece155782727a148956f6e4b2b7203661c2ff5b9f128979326158f988bb777R235

if name == expectedSecretName {
// get all GlanceAPI CRs in this namespace
glanceAPIs := &glancev1.GlanceAPIList{}
listOpts := []client.ListOption{
client.InNamespace(ns),
}
if err := r.List(context.Background(), glanceAPIs, listOpts...); err != nil {
return nil
}

// Enqueue reconcile for all glance API instances
for _, cr := range glanceAPIs.Items {
objKey := client.ObjectKey{
Namespace: ns,
Name: cr.Name,
}
result = append(result, reconcile.Request{NamespacedName: objKey})
}
}

return result
}

return ctrl.NewControllerManagedBy(mgr).
For(&glancev1.GlanceAPI{}).
Owns(&keystonev1.KeystoneEndpoint{}).
Expand All @@ -393,6 +429,8 @@ func (r *GlanceAPIReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man
handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
).
Watches(&corev1.Secret{},
handler.EnqueueRequestsFromMapFunc(acSecretFn)).
Watches(&memcachedv1.Memcached{},
handler.EnqueueRequestsFromMapFunc(memcachedFn)).
Watches(&topologyv1.Topology{},
Expand Down Expand Up @@ -709,6 +747,11 @@ func (r *GlanceAPIReconciler) reconcileNormal(
instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)
// run check OpenStack secret - end

// Verify Application Credentials if available
if res, err := keystonev1.VerifyApplicationCredentialsForService(ctx, r.Client, instance.Namespace, instance.APIName(), &configVars, glance.NormalDuration); err != nil || res.RequeueAfter > 0 {
return res, err
}

//
// Check for required memcached used for caching
//
Expand Down Expand Up @@ -1164,6 +1207,7 @@ func (r *GlanceAPIReconciler) generateServiceConfig(
memcached *memcachedv1.Memcached,
wsgi bool,
) error {
Log := r.GetLogger(ctx)
labels := labels.GetLabels(instance, labels.GetGroupLabel(glance.ServiceName), GetServiceLabels(instance))

db, err := mariadbv1.GetDatabaseByNameAndAccount(ctx, h, glance.DatabaseName, instance.Spec.DatabaseAccount, instance.Namespace)
Expand Down Expand Up @@ -1261,6 +1305,17 @@ func (r *GlanceAPIReconciler) generateServiceConfig(
"Wsgi": wsgi,
}

templateParameters["UseApplicationCredentials"] = false
// Try to get Application Credential for this service (via keystone api helper)
if acData, err := keystonev1.GetApplicationCredentialFromSecret(ctx, r.Client, instance.Namespace, glance.ServiceName); err != nil {
Log.Error(err, "Failed to get ApplicationCredential for service", "service", glance.ServiceName)
} else if acData != nil {
templateParameters["UseApplicationCredentials"] = true
templateParameters["ACID"] = acData.ID
templateParameters["ACSecret"] = acData.Secret
Log.Info("Using ApplicationCredentials auth", "service", glance.ServiceName)
}

// (OSPRH-18291)Only set EndpointID parameter when the Endpoint has been
// created and the associated ID is set in the keystoneapi CR. Because we
// have the Keystone CR, we get the Region parameter mirrored in its
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,5 @@ replace k8s.io/component-base => k8s.io/component-base v0.31.13 //allow-merging
replace github.com/rabbitmq/cluster-operator/v2 => github.com/openstack-k8s-operators/rabbitmq-cluster-operator/v2 v2.6.1-0.20250929174222-a0d328fa4dec //allow-merging

replace k8s.io/kube-openapi => k8s.io/kube-openapi v0.0.0-20250627150254-e9823e99808e //allow-merging

replace github.com/openstack-k8s-operators/keystone-operator/api => github.com/Deydra71/keystone-operator/api v0.0.0-20251103091514-244e15fe5d63
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
github.com/Deydra71/keystone-operator/api v0.0.0-20251103091514-244e15fe5d63 h1:ug2YPMQJ/+0ifOjFyaPx1YtX0zsVnL02pB2ngacYviw=
github.com/Deydra71/keystone-operator/api v0.0.0-20251103091514-244e15fe5d63/go.mod h1:FMFoO4MjEQ85JpdLtDHxYSZxvJ9KzHua+HdKhpl0KRI=
github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1Xbatp0=
github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM=
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
Expand Down Expand Up @@ -104,8 +106,6 @@ github.com/openstack-k8s-operators/horizon-operator/api v0.6.1-0.20251027072356-
github.com/openstack-k8s-operators/horizon-operator/api v0.6.1-0.20251027072356-bf38df61afe6/go.mod h1:2TxZDR8H2IevsXCVpsoTElr8alzL7lV31q53qZjR5LE=
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251027233324-053afb24d94c h1:N/7dfFsOZwQQYPtg0sSCvsurLQC1RDFArYZ6j9b2x3Y=
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251027233324-053afb24d94c/go.mod h1:qq8BCRxTEmLRriUsQ4HeDUzqltWg32MQPDTMhgbBGK4=
github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20251027074845-ed8154b20ad1 h1:QohvX44nxoV2GwvvOURGXYyDuCn4SCrnwubTKJtzehY=
github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20251027074845-ed8154b20ad1/go.mod h1:FMFoO4MjEQ85JpdLtDHxYSZxvJ9KzHua+HdKhpl0KRI=
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251027074416-ab5c045dbe00 h1:Xih6tYYqiDVllo4fDGHqTPL+M2biO5YLOUmbiTqrW/I=
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251027074416-ab5c045dbe00/go.mod h1:PMoNILOdQ1Ij7DyrKgljN6RAiq8pFM2AGsUb6mcxe98=
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.6.1-0.20251027074416-ab5c045dbe00 h1:YwkGrTpeeAq9bk09u9Hp96BEZb8X3XgnMfoyxypelVM=
Expand Down
30 changes: 24 additions & 6 deletions templates/common/config/00-config.conf
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,18 @@ default_backend=default_backend
[keystone_authtoken]
www_authenticate_uri={{ .KeystonePublicURL }}
auth_url={{ .KeystoneInternalURL }}
auth_type=password
username={{ .ServiceUser }}
{{ if .UseApplicationCredentials -}}
auth_type = v3applicationcredential
application_credential_id = {{ .ACID }}
application_credential_secret = {{ .ACSecret }}
{{ else -}}
auth_type = password
username = {{ .ServiceUser }}
password = {{ .ServicePassword }}
project_domain_name = Default
user_domain_name = Default
project_name = service
{{- end }}
{{ if (index . "MemcachedServers") }}
memcached_servers = {{ .MemcachedServers }}
memcache_pool_dead_retry = 10
Expand All @@ -55,12 +64,16 @@ memcache_tls_keyfile = {{ .MemcachedAuthKey }}
memcache_tls_cafile = {{ .MemcachedAuthCa }}
memcache_tls_enabled = true
{{end}}
project_domain_name=Default
user_domain_name=Default
project_name=service

[service_user]
{{ if .UseApplicationCredentials -}}
auth_type = v3applicationcredential
application_credential_id = {{ .ACID }}
application_credential_secret = {{ .ACSecret }}
{{ else -}}
auth_type = password
password = {{ .ServicePassword }}
{{- end }}

[oslo_messaging_notifications]
{{ if (index . "TransportURL") -}}
Expand Down Expand Up @@ -94,11 +107,16 @@ filesystem_store_datadir = /var/lib/glance/os_glance_tasks_store/

[oslo_limit]
auth_url={{ .KeystoneInternalURL }}
auth_type = password
auth_type = {{ if .UseApplicationCredentials }}v3applicationcredential{{ else }}password{{ end }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

{{ if .UseApplicationCredentials -}}
application_credential_id = {{ .ACID }}
application_credential_secret = {{ .ACSecret }}
{{ else -}}
username={{ .ServiceUser }}
password = {{ .ServicePassword }}
system_scope = all
user_domain_id = default
{{- end }}
{{ if (index . "EndpointID") -}}
endpoint_id = {{ .EndpointID }}
{{ end -}}
Expand Down
114 changes: 114 additions & 0 deletions test/functional/glanceapi_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ import (
. "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports
. "github.com/onsi/gomega" //revive:disable:dot-imports
glancev1 "github.com/openstack-k8s-operators/glance-operator/api/v1beta1"
"github.com/openstack-k8s-operators/glance-operator/pkg/glance"
memcachedv1 "github.com/openstack-k8s-operators/infra-operator/apis/memcached/v1beta1"
topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1"
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"

//revive:disable-next-line:dot-imports
Expand Down Expand Up @@ -1286,4 +1288,116 @@ var _ = Describe("Glanceapi controller", func() {
}, timeout, interval).Should(Succeed())
})
})

When("An ApplicationCredential is created for Glance", func() {
var (
acName string
acSecretName string
servicePasswordSecret string
passwordSelector string
)
BeforeEach(func() {
servicePasswordSecret = "ac-test-osp-secret" //nolint:gosec // G101
passwordSelector = "GlancePassword"

DeferCleanup(k8sClient.Delete, ctx, CreateGlanceSecret(glanceTest.Instance.Namespace, servicePasswordSecret))
DeferCleanup(k8sClient.Delete, ctx, CreateGlanceMessageBusSecret(glanceTest.Instance.Namespace, glanceTest.RabbitmqSecretName))
DeferCleanup(th.DeleteInstance, CreateDefaultGlance(glanceTest.Instance))
DeferCleanup(
mariadb.DeleteDBService,
mariadb.CreateDBService(
glanceTest.Instance.Namespace,
glanceTest.GlanceDatabaseName.Name,
corev1.ServiceSpec{
Ports: []corev1.ServicePort{{Port: 3306}}}))
mariadb.CreateMariaDBDatabase(glanceTest.GlanceDatabaseName.Namespace, glanceTest.GlanceDatabaseName.Name, mariadbv1.MariaDBDatabaseSpec{})
DeferCleanup(k8sClient.Delete, ctx, mariadb.GetMariaDBDatabase(glanceTest.GlanceDatabaseName))

DeferCleanup(keystone.DeleteKeystoneAPI, keystone.CreateKeystoneAPI(glanceTest.Instance.Namespace))
DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(glanceTest.Instance.Namespace, MemcachedInstance, memcachedv1.MemcachedSpec{}))
infra.SimulateMemcachedReady(glanceTest.GlanceMemcached)

// Create AC secret with test credentials
acName = fmt.Sprintf("ac-%s", glance.ServiceName)
acSecretName = acName + "-secret"
acSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: glanceTest.Instance.Namespace,
Name: acSecretName,
},
Data: map[string][]byte{
"AC_ID": []byte("test-ac-id"),
"AC_SECRET": []byte("test-ac-secret"),
},
}
DeferCleanup(k8sClient.Delete, ctx, acSecret)
Expect(k8sClient.Create(ctx, acSecret)).To(Succeed())

// Create AC CR
ac := &keystonev1.KeystoneApplicationCredential{
ObjectMeta: metav1.ObjectMeta{
Namespace: glanceTest.Instance.Namespace,
Name: acName,
},
Spec: keystonev1.KeystoneApplicationCredentialSpec{
UserName: glance.ServiceName,
Secret: servicePasswordSecret,
PasswordSelector: passwordSelector,
Roles: []string{"admin", "member"},
AccessRules: []keystonev1.ACRule{{Service: "identity", Method: "POST", Path: "/auth/tokens"}},
ExpirationDays: 30,
GracePeriodDays: 5,
},
}
DeferCleanup(k8sClient.Delete, ctx, ac)
Expect(k8sClient.Create(ctx, ac)).To(Succeed())

// Simulate AC controller updating the status
fetched := &keystonev1.KeystoneApplicationCredential{}
key := types.NamespacedName{Namespace: ac.Namespace, Name: ac.Name}
Expect(k8sClient.Get(ctx, key, fetched)).To(Succeed())

fetched.Status.SecretName = acSecretName
now := metav1.Now()
readyCond := condition.Condition{
Type: condition.ReadyCondition,
Status: corev1.ConditionTrue,
Reason: condition.ReadyReason,
Message: condition.ReadyMessage,
LastTransitionTime: now,
}
fetched.Status.Conditions = condition.Conditions{readyCond}
Expect(k8sClient.Status().Update(ctx, fetched)).To(Succeed())

// Create GlanceAPI using the service password secret
spec := CreateGlanceAPISpec(GlanceAPITypeInternal)
spec["secret"] = servicePasswordSecret
DeferCleanup(th.DeleteInstance, CreateGlanceAPI(glanceTest.GlanceInternal, spec))

mariadb.SimulateMariaDBAccountCompleted(glanceTest.GlanceDatabaseAccount)
mariadb.SimulateMariaDBDatabaseCompleted(glanceTest.GlanceDatabaseName)
th.SimulateStatefulSetReplicaReady(glanceTest.GlanceInternalStatefulSet)

keystone.SimulateKeystoneEndpointReady(glanceTest.GlanceInternal)
})

It("should render ApplicationCredential auth in 00-config.conf", func() {
keystone.SimulateKeystoneEndpointReady(glanceTest.GlanceInternal)

// Wait for the config to be generated and updated with AC auth
Eventually(func(g Gomega) {
cfgSecret := th.GetSecret(glanceTest.GlanceInternalConfigMapData)
g.Expect(cfgSecret).NotTo(BeNil())

conf := string(cfgSecret.Data["00-config.conf"])

g.Expect(conf).To(ContainSubstring(
"application_credential_id = test-ac-id"),
)
g.Expect(conf).To(ContainSubstring(
"application_credential_secret = test-ac-secret"),
)
}, timeout, interval).Should(Succeed())
})
})
})
Loading