Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add configuration for keystone webhook #91

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Add configuration for keystone webhook #91

merged 1 commit into from
Nov 28, 2023

Conversation

heytrav
Copy link
Contributor

@heytrav heytrav commented Sep 14, 2023 •
edited
Loading

@heytrav heytrav requested a review from mkjpryor as a code owner September 14, 2023 01:09
@heytrav heytrav mentioned this pull request Sep 14, 2023
Merged
@heytrav heytrav force-pushed the keystone-patch-api-server branch from 7963f87 to 06008a0 Compare October 1, 2023 20:59
@mkjpryor mkjpryor mentioned this pull request Oct 2, 2023
Merged
@heytrav heytrav force-pushed the keystone-patch-api-server branch from 06008a0 to dfc6ad6 Compare October 4, 2023 23:31
@github-actions
Copy link

github-actions bot commented Oct 4, 2023

@mkjpryor

Approval is required for workflow run #6412667704 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from dfc6ad6 to 32c90d7 Compare October 12, 2023 19:13
@github-actions
Copy link

@mkjpryor

Approval is required for workflow run #6499934847 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from 32c90d7 to 115b12a Compare October 13, 2023 23:04
@github-actions
Copy link

@mkjpryor

Approval is required for workflow run #6513862027 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from 115b12a to 078a381 Compare October 13, 2023 23:09
@github-actions
Copy link

@mkjpryor

Approval is required for workflow run #6513904012 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from 078a381 to b875df8 Compare October 18, 2023 17:54
@github-actions
Copy link

@mkjpryor

Approval is required for workflow run #6564700955 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from b875df8 to 733e5d6 Compare October 18, 2023 19:42
@github-actions
Copy link

@mkjpryor

Approval is required for workflow run #6565780568 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav
Copy link
Contributor Author

heytrav commented Oct 18, 2023

I've refactored this to remove a lot of the code that I'd added. The only additional code now is the Secret/HelmRelease definition for the k8s-keystone-auth subchart.

Otherwise I've added documentation to explain what settings need to be overridden in values.yaml to add the webhook.

@mkjpryor
Copy link
Collaborator

Similar to the other patch, I'm happy to include a layer to make the required changes to the kubeadmConfigSpec in the short- to medium-term. If we are going to support the Keystone auth webhook I would prefer that it is just a flag that needs flipping.

Let me know if you are happy to make the changes now or whether you would prefer to merge this for now.

@github-actions
Copy link

@mkjpryor

Approval is required for workflow run #6632355933 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav
Copy link
Contributor Author

heytrav commented Oct 24, 2023 •
edited
Loading

Refactored a bit

  • broke up generic webhook setup to allow for other webhooks
  • integrated k8s-keystone-auth into layered template processing

@mkjpryor I haven't tested this yet but keen to know if you think of the approach makes sense

@heytrav heytrav force-pushed the keystone-patch-api-server branch from 46e15f7 to b8df5a6 Compare October 24, 2023 20:55
@github-actions
Copy link

@mkjpryor

Approval is required for workflow run #6632446870 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from b8df5a6 to 9ec8164 Compare October 24, 2023 22:07
@github-actions
Copy link

@mkjpryor

Approval is required for workflow run #6633079599 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from 9ec8164 to 3173136 Compare October 25, 2023 01:49
@github-actions
Copy link

@mkjpryor

Approval is required for workflow run #6634720470 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from 3173136 to 6ae0ad3 Compare October 25, 2023 06:45
@github-actions
Copy link

@mkjpryor

Approval is required for workflow run #6636743395 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from 6ae0ad3 to d1fb9a2 Compare October 25, 2023 20:06
@github-actions
Copy link

@mkjpryor

Approval is required for workflow run #6645571699 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from d8c4c4f to 359a49d Compare November 2, 2023 06:42
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 2, 2023

@mkjpryor

Approval is required for workflow run #6729199866 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from 359a49d to 17213a6 Compare November 2, 2023 06:48
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 2, 2023

@mkjpryor

Approval is required for workflow run #6729238112 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from 17213a6 to ac9e6ac Compare November 12, 2023 18:20
@github-actions GitHub Actions
Copy link

@mkjpryor

Approval is required for workflow run #6842375543 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from ac9e6ac to 82bdb1f Compare November 15, 2023 18:12
@github-actions GitHub Actions
Copy link

@mkjpryor

Approval is required for workflow run #6881247771 for this PR.

Please review the code that will be executed by this workflow run and give either a 👍 or 👎 on this comment to approve or deny execution.

@heytrav heytrav force-pushed the keystone-patch-api-server branch from 82bdb1f to 074202a Compare November 19, 2023 17:59
@heytrav heytrav force-pushed the keystone-patch-api-server branch from 074202a to 8de4333 Compare November 23, 2023 01:52
@heytrav heytrav force-pushed the keystone-patch-api-server branch from 8de4333 to c56928c Compare November 23, 2023 01:53
@heytrav heytrav force-pushed the keystone-patch-api-server branch from c56928c to a6a51ac Compare November 23, 2023 17:28
@heytrav
Copy link
Contributor Author

heytrav commented Nov 23, 2023 •
edited
Loading

Hi @mkjpryor, would it be possible to get some feedback on this please? Is there anything that you would like me to change in the implementation?

@heytrav heytrav closed this Nov 25, 2023
@heytrav heytrav force-pushed the keystone-patch-api-server branch from a6a51ac to 38d01f5 Compare November 25, 2023 00:36
Authentication/Authorization webhook integration
2bfe594 
* Set up webhook for k8s-keystone-auth and other plugins in the future
@heytrav heytrav reopened this Nov 25, 2023
mkjpryor
mkjpryor approved these changes Nov 28, 2023
View reviewed changes
Copy link
Collaborator

@mkjpryor mkjpryor left a comment

Choose a reason for hiding this comment

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

My only comment would be that I can't help thinking there must be a way to populate the auth URL and project ID from the appcred that is used to create the cluster. But I'm not going to hold up merging for it.

@heytrav
Copy link
Contributor Author

heytrav commented Nov 28, 2023

My only comment would be that I can't help thinking there must be a way to populate the auth URL and project ID from the appcred that is used to create the cluster. But I'm not going to hold up merging for it.

awesome! thank you.

@mkjpryor mkjpryor merged commit 70a2159 into azimuth-cloud:main Nov 28, 2023
10 checks passed
@heytrav heytrav deleted the keystone-patch-api-server branch December 4, 2023 18:35
@sd109 sd109 mentioned this pull request Mar 25, 2024
Open
@waipeng waipeng mentioned this pull request Apr 8, 2024
Open
openstack-mirroring referenced this pull request in openstack/magnum-capi-helm Apr 17, 2024
@waipeng
Disable keystone-auth by default
8f802d0 
A recent commit in capi-helm-charts[1] added support for keystone-auth.
However, the feature is not working yet and now kubeadm fails to init
the cluster.

Disable keystone-auth by default for now, until the feature is fixed in
the charts.

[1] https://github.com/stackhpc/capi-helm-charts/pull/91
[2] https://github.com/stackhpc/capi-helm-charts/issues/301

Change-Id: Idb603f4e5b57e004453af2460c3d84225cacf6fa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkjpryor mkjpryor mkjpryor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@heytrav @mkjpryor