-
Notifications
You must be signed in to change notification settings - Fork 26
Application Credential Support #317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -472,6 +472,18 @@ func (r *WatcherAPIReconciler) generateServiceConfigs( | |
| if string(secret.Data[NotificationURLSelector]) != "" { | ||
| templateParameters["NotificationURL"] = string(secret.Data[NotificationURLSelector]) | ||
| } | ||
|
|
||
| // Check for Application Credentials | ||
| templateParameters["UseApplicationCredentials"] = false | ||
| if acData, err := keystonev1.GetApplicationCredentialFromSecret(ctx, r.Client, instance.Namespace, watcher.ServiceName); err != nil { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok so keystone is going to provide a helper function it its api module instead of each fo the service operators using ensureSecret this is not quite correct for watcher nova or placement because we create separate secrets for sub cr each so the top level watcher controller shoudl receive the application credential and look up the info form the keystone created secret and then populate that into the per CR secrete so we should look this up once here then extend createSubLevelSecret to propagate the application credential info into the service secrets. https://github.com/openstack-k8s-operators/watcher-operator/blob/main/internal/controller/watcher_controller.go#L355-L360 even if we were to ignore that pattenr which i dont think we should we would add a new fucntion to the base reconsolie instead to aovid repeating this per service contoler. |
||
| Log.Error(err, "Failed to get ApplicationCredential for service", "service", watcher.ServiceName) | ||
| } else if acData != nil { | ||
| templateParameters["UseApplicationCredentials"] = true | ||
| templateParameters["ACID"] = acData.ID | ||
| templateParameters["ACSecret"] = acData.Secret | ||
| Log.Info("Using ApplicationCredentials auth", "service", watcher.ServiceName) | ||
| } | ||
|
|
||
| // MTLS | ||
| if memcachedInstance.GetMemcachedMTLSSecret() != "" { | ||
| templateParameters["MemcachedAuthCert"] = fmt.Sprint(memcachedv1.CertMountPath()) | ||
|
|
@@ -929,6 +941,36 @@ func (r *WatcherAPIReconciler) SetupWithManager(mgr ctrl.Manager) error { | |
| return err | ||
| } | ||
|
|
||
| // Application Credential secret watching function | ||
| acSecretFn := func(_ context.Context, o client.Object) []reconcile.Request { | ||
| result := []reconcile.Request{} | ||
|
|
||
| // Check if this is a watcher AC secret by name pattern (ac-watcher-secret) | ||
| expectedSecretName := keystonev1.GetACSecretName(watcher.ServiceName) | ||
| if o.GetName() == expectedSecretName { | ||
| // get all WatcherAPI CRs in this namespace | ||
| watcherAPIs := &watcherv1beta1.WatcherAPIList{} | ||
| listOpts := []client.ListOption{ | ||
| client.InNamespace(o.GetNamespace()), | ||
| } | ||
| if err := r.Client.List(context.Background(), watcherAPIs, listOpts...); err != nil { | ||
| return nil | ||
| } | ||
|
|
||
| // Enqueue reconcile for all WatcherAPI instances | ||
| for _, cr := range watcherAPIs.Items { | ||
| result = append(result, reconcile.Request{ | ||
| NamespacedName: types.NamespacedName{ | ||
| Namespace: o.GetNamespace(), | ||
| Name: cr.Name, | ||
| }, | ||
| }) | ||
| } | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this looks incorrect to me. we watcher secrets or other CRDs genericly the looping over the relevant objects and creation of reconsile request is don by so you shoudl not be generating the reconsile reqeust hree you would follw the same patthern as the passwordSecretFiled and return a slice of strings []string with that said as i indicated above the applcition credtial shoudl be manged at the toplevel contoler and the API/applier/descison engine should jus twhatc there input secret. basiclly the top level contoler will watch the appcliatce credtial secret/CRD and update the subCR secrete, the subcr contoler like the water api contole only watch there secret not teh applcation credital one. |
||
|
|
||
| return result | ||
| } | ||
|
|
||
| return ctrl.NewControllerManagedBy(mgr). | ||
| For(&watcherv1beta1.WatcherAPI{}). | ||
| Owns(&corev1.Secret{}). | ||
|
|
@@ -940,6 +982,11 @@ func (r *WatcherAPIReconciler) SetupWithManager(mgr ctrl.Manager) error { | |
| handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc), | ||
| builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), | ||
| ). | ||
| Watches( | ||
| &corev1.Secret{}, | ||
| handler.EnqueueRequestsFromMapFunc(acSecretFn), | ||
| builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), | ||
| ). | ||
| Watches( | ||
| &memcachedv1.Memcached{}, | ||
| handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we normally do not use booleans for this
we isntead follow the pattern of chekcign if the parmares are empty in the template.
https://github.com/openstack-k8s-operators/watcher-operator/blob/main/templates/00-default.conf#L8-L10
can you refactor this to use that pattern
in this case you shoudl generate the approate section if
"ACID" is populated
{{ if (index . "ACID") }}
...
{{ end}}