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.

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

adding support for webhook-admission validation #146

Closed
wants to merge 0 commits into from

Conversation

sepulworld
Copy link
Contributor

@sepulworld sepulworld commented Jan 12, 2024

@sepulworld
Copy link
Contributor Author

I considered adding

{{- if (Capabilities.APIVersions.Has "cert-manager.io/v1") }}

Up to you though... it is sort of nice but if people are using ArgoCD (using helm template) these Capabilities checks don't play nice in charts. I say leave it out and just document cert-manager is required if you want to use it.

@rlex
Copy link
Owner

rlex commented Jan 14, 2024

looks like helm-docs needs to be run again. Or maybe it's difference in versions?

Additionally, what will happen if cert isn't present but value is set to true? pyrra will skip webhook creation?

@rlex rlex self-assigned this Jan 14, 2024
@rlex rlex closed this Jan 14, 2024
@rlex rlex reopened this Jan 14, 2024
@rlex
Copy link
Owner

rlex commented Jan 14, 2024

Seems like CI passes now, all that's needed is to run helm-docs because variables in values.yaml and readme doesn't match

@sepulworld
Copy link
Contributor Author

looks like helm-docs needs to be run again. Or maybe it's difference in versions?

Additionally, what will happen if cert isn't present but value is set to true? pyrra will skip webhook creation?

It would skip the creation if we used

{{- if (Capabilities.APIVersions.Has "cert-manager.io/v1") }}

I say we leverage this API capability check built into Helm. I will add a new commit

@rlex
Copy link
Owner

rlex commented Jan 17, 2024

Might be worth bumping chart version as well, so it will release automatically after merge

@sepulworld sepulworld force-pushed the webhook-admission-support branch from 1a95c81 to 73f768c Compare January 18, 2024 08:36
@sepulworld sepulworld closed this Jan 18, 2024
@sepulworld sepulworld force-pushed the webhook-admission-support branch from 73f768c to 8de2164 Compare January 18, 2024 08:40
@sepulworld sepulworld deleted the webhook-admission-support branch January 18, 2024 08:44
@sepulworld sepulworld restored the webhook-admission-support branch January 18, 2024 08:44
@sepulworld
Copy link
Contributor Author

sorry my rebase somehow auto closed this MR. odd! I opened up a new one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants