-
Notifications
You must be signed in to change notification settings - Fork 284
✨ Add OpenStackClusterIdentity for centralized credential management #2682
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
✨ Add OpenStackClusterIdentity for centralized credential management #2682
Conversation
✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hi @bnallapeta. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
19a467e to
68dbd07
Compare
|
/ok-to-test |
68dbd07 to
ffa81c1
Compare
|
/test pull-cluster-api-provider-openstack-e2e-test |
lentzi90
left a comment
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.
I am almost through reviewing this. A few left that I will try to get done tomorrow. Here are the comments so far
Signed-off-by: Bharath Nallapeta <[email protected]>
Signed-off-by: Bharath Nallapeta <[email protected]>
Signed-off-by: Bharath Nallapeta <[email protected]>
74f2ac4 to
52a62f9
Compare
lentzi90
left a comment
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.
I am quite happy with this now. Still a few nits below.
Could you also squash the commits? Then I think this is good to go 🙂
52a62f9 to
880481c
Compare
Signed-off-by: Bharath Nallapeta <[email protected]>
880481c to
174e2e8
Compare
@lentzi90 I have logically grouped the commits since its a relatively big PR. Will be easier to manage in the future too. Hope this structure works. Please let me know. |
lentzi90
left a comment
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.
Thanks! Yes, it is ok to keep the 4 commits I think, the fifth "duplicate" e2e commit was unnecessary but these are good.
/approve
@smoshiur1237 could you also please review this? You should have org membership soon and this is a good way to get more familiar with the code.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lentzi90 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test pull-cluster-api-provider-openstack-e2e-full-test |
|
I have started to check commit by commit. I may need more time to do final check.. I will try to finish it by tomorrow.. @lentzi90 you have already reviewed all the parts and initially it is in good shape. Thanks for this addition. |
@smoshiur1237 Thanks for reviewing this. Just checking if you have any updates. I was hoping that we can close this out as soon as possible. |
|
I am happy with the changes and it looks good to me. We can take it in. |
|
/hold cancel |
What this PR does / why we need it:
Introduces OpenStackClusterIdentity CRD to centralize OpenStack credentials across namespaces with access controls.
Key Features:
namespaceSelectorUsage:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #2386
/hold