[WIP] Add Application Credentials authentication support for Nova services #1006
Conversation
This commit implements Application Credentials (AC) authentication as an alternative to service user authentication for Nova services, providing enhanced security and simplified credential management. Changes include: * **API Types**: Add ApplicationCredential fields to NovaAPISpec, NovaSchedulerSpec, NovaMetadataSpec, and NovaCellSpec to support AC configuration propagation * **Controller Logic**: Implement AC detection with graceful fallback to service user authentication when AC is unavailable. Add AC hash tracking in Nova status for change detection and reconciliation * **Utility Package**: Add pkg/util/appcred with helper functions for AC secret loading, validation, hashing, and logging * **RBAC**: Add permissions for ApplicationCredential resources to enable AC CR and Secret watching * **Event Handling**: Add watches for ac-nova ApplicationCredential CR and ac-nova-secret Secret with proper event mapping to trigger Nova reconciliation on AC changes * **Dependencies**: Update go.mod to include keystone-operator API with ApplicationCredential CRD support * **Testing**: Add comprehensive functional and KUTTL test suites covering AC usage, fallback scenarios, and credential updates The implementation maintains backward compatibility by requiring only the metadata secret when using Application Credentials, while falling back to requiring the service password when AC is not available. ApplicationCredential authentication is automatically detected by the presence of the 'ac-nova-secret' Secret containing AC_ID and AC_SECRET keys, eliminating the need for manual configuration changes. Implements: https://issues.redhat.com/browse/OSPRH-16626 Signed-off-by: Mauricio Harley <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mauricioharley The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@mauricioharley: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/ce9340d9de40461f9e3d71fc04df1881 ❌ openstack-meta-content-provider FAILURE in 14m 41s |
|
nite: Implements: https://issues.redhat.com/browse/OSPRH-16626 shoudl be Closes: OSPRH-16626 if you want the bot to properly do the linkage its import to not use the full url you need to use jsut the short slug. the both will turn that into a link but it wont match proeprly on the full url |
|
by the way im gladd this propoal is findally movign forward. i proposed doing this before we gaed https://issues.redhat.com/browse/OSPRH-335 but there was not enough capstiy to do that at the teime |
| // +listType=set | ||
| IPs []string `json:"ips,omitempty"` | ||
| } | ||
|
|
There was a problem hiding this comment.
these CRD change do not look related and shoudl not be done as part of this PR if we need to make the CRD change to use this new type that shoudl be its won PR that is doen before or aftere the applciation credential pr
| allowedKeys, | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
again non of this is correct to includ in this PR plwase serperate all cred validation chagne to a sepreate pr, not just a sperate commit and this shoudl have its own jira tracker.
also we need to ensure that this change is semanticly equilvent and that exsitign CR will still be compatibale that means to make this chagne we shoudl be able to do it without any kuttl or tempest/edpm job changes
althought eh envtests may need to be modifed.
| NodeSelector *map[string]string `json:"nodeSelector,omitempty"` | ||
| // +listType=map | ||
| // +listMapKey=key | ||
| NodeSelector []KeyValuePair `json:"nodeSelector,omitempty"` |
There was a problem hiding this comment.
im going to stop commeitng on the ohter crd change but assume the same applies.
| // +kubebuilder:validation:Enum="true";"false" | ||
| // +kubebuilder:default="false" | ||
| // UseApplicationCredential - indicates if ApplicationCredential authentication is used | ||
| UseApplicationCredential string `json:"useApplicationCredential"` |
There was a problem hiding this comment.
this is inapprotiate.
by design we are not allowed to have boolean feature flag in the CRDs
usage of application credentials shoudl either by unconditional or determined by the presence of the application Credtial secreat in the CR.
|
|
||
| // +kubebuilder:validation:Optional | ||
| // ApplicationCredentialID - the ID of the ApplicationCredential | ||
| ApplicationCredentialID string `json:"applicationCredentialID"` |
There was a problem hiding this comment.
this i guess will funciton similar to the passworkd selector however this also feels like a desgin flaw.
really we shoudl nto have multiple applciation credtial in the ApplicationCredentialSecret
so this information should be encoded in that object
| @@ -0,0 +1,52 @@ | |||
| package appcred | |||
There was a problem hiding this comment.
i dont think this si really corerct. we shoudl take the time to put these helper fucntion into lib-common so that they are nto cargo culted around all the service operators..
There was a problem hiding this comment.
Hello, @SeanMooney. Thank you for all the comments. This was just the very first release of the feature. I know there's room for more improvement.
Anyway, I'd like to ask you: regarding the package appcred part and the corresponding appcred.go, should we move the code to lib-common and remove it from here?
There was a problem hiding this comment.
i think that would be good to move to lib-common if its reusabel yes isntead of doing it here
Thank you. Yes, the actual support is being implemented for both, Keystone and OpenStack operators, on specific PRs. |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
3 participants
This PR implements Application Credentials authentication support for Nova services as an alternative to service user authentication, providing enhanced security and simplified credential management.
What has changed:
How to Test:
oc apply -f examples/ac-secret.yamloc apply -f examples/ac-cr.yamloc kuttl test test/kuttl/test-suites/default/application-credentials-tests/Backward Compatibility:
Fully backward compatible - existing deployments continue to use service user authentication
Closes: OSPRH-16626