-
Notifications
You must be signed in to change notification settings - Fork 47
spire-controller-manager 0.4.0 support #60
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
Conversation
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
5151f29
to
ef589a9
Compare
Depends on this already merged feature getting a release: spiffe/spire-controller-manager#230 |
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: kfox1111 <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: kfox1111 <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
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.
Couple of suggestions on renaming some stuff and adding more context for understanding the feature. I think people new to the feature (like me) will be unable to follow allong.
charts/spire/charts/spire-server/templates/controller-manager-cluster-ids.yaml
Outdated
Show resolved
Hide resolved
…cluster-ids.yaml Co-authored-by: Marco Franssen <[email protected]> Signed-off-by: kfox1111 <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: kfox1111 <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
@marcofranssen What would be the best path forward here? Would it help to have a call? |
charts/spire/charts/spire-server/templates/controller-manager-webhook.yaml
Show resolved
Hide resolved
@kfox1111 Overall looks good. Thanks for adding the other missing controller manager features. Just some minor questions inline. |
Co-authored-by: Faisal Memon <[email protected]> Signed-off-by: kfox1111 <[email protected]>
Signed-off-by: kfox1111 <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
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.
LGTM 🚀 Lets get some community feedback on this feature and its UX from a Helm perspective.
* 6c0a4fb Auto tornjak url (#89) * f2758a8 Autogen jwtIssuer (#88) * 93c20c6 Merge pull request #83 from spiffe/doc-ingress-patch * c662b0c Merge branch 'main' into doc-ingress-patch * de8cbc8 Bump github.com/onsi/gomega from 1.29.0 to 1.30.0 in /tests (#90) * 1de838e Bump helm.sh/helm/v3 from 3.13.1 to 3.13.2 in /tests (#91) * daf9d55 Merge branch 'main' into doc-ingress-patch * aaeca00 Fix tests after renaming files * 0320c3f Cleanup documentation * 37cd9f2 Update to SPIRE 1.8.4 (#84) * bd7879a Fix the typo in the production ingress README * 805d869 spire-controller-manager 0.4.0 support (#60) * edf2381 Bump sigstore/cosign-installer from 3.1.2 to 3.2.0 (#85) * 0ed6d92 Bump test chart dependencies * f7b6140 Fix docs * a7e8785 Add another missing one * 3778597 Add missing ClusterSPIFFEID fields * 9b31f3d Bump helm/chart-testing-action from 2.6.0 to 2.6.1 * d6583be Add missing no cleanup flags to example tests (#79) * 81cc2dc Ingress type openshift (#52) * 2dce90f Bump helm/chart-releaser-action from 1.5.0 to 1.6.0 (#75) * f08ebd3 Patch Tornjak ingress (#74) * 6a6aa1c Cosign download fix (#72) * 3b01684 Support Openshift deployment (#13) * ed23d8b Simple ingress support (#48) * b354413 Fix incorrect nesting of priorityClassName (#67) * 09ebfc9 Bump test chart dependencies (#68) * d50d78d Fix ingress annotation variable for tornjak server (#66) * b813807 Update docs to simplify, clarify, and prioritize ease of install/upgrade (#62) * 5ad35bd Merge pull request #59 from sabre1041/spire-agent-nodename * 02d445c Update charts/spire/README.md * 9bd7f43 Added environment variable to spire-agent to inject node name * fb64f8d Bump github.com/onsi/gomega from 1.28.1 to 1.29.0 in /tests (#61) * 30d0f59 Update helm-charts repository URL (#58) * 7924323 Specify ingress controller type (#43) * 0b17434 Bump test chart dependencies (#54) * a419903 Bump github.com/onsi/gomega from 1.28.0 to 1.28.1 in /tests (#53) * 5025025 Fix doc missing end tag (#46) * cf226a7 Bump actions/checkout from 4.1.0 to 4.1.1 (#42) Signed-off-by: Faisal Memon <[email protected]>
* 6c0a4fb Auto tornjak url (#89) * f2758a8 Autogen jwtIssuer (#88) * 93c20c6 Merge pull request #83 from spiffe/doc-ingress-patch * c662b0c Merge branch 'main' into doc-ingress-patch * de8cbc8 Bump github.com/onsi/gomega from 1.29.0 to 1.30.0 in /tests (#90) * 1de838e Bump helm.sh/helm/v3 from 3.13.1 to 3.13.2 in /tests (#91) * daf9d55 Merge branch 'main' into doc-ingress-patch * aaeca00 Fix tests after renaming files * 0320c3f Cleanup documentation * 37cd9f2 Update to SPIRE 1.8.4 (#84) * bd7879a Fix the typo in the production ingress README * 805d869 spire-controller-manager 0.4.0 support (#60) * edf2381 Bump sigstore/cosign-installer from 3.1.2 to 3.2.0 (#85) * 0ed6d92 Bump test chart dependencies * f7b6140 Fix docs * a7e8785 Add another missing one * 3778597 Add missing ClusterSPIFFEID fields * 9b31f3d Bump helm/chart-testing-action from 2.6.0 to 2.6.1 * d6583be Add missing no cleanup flags to example tests (#79) * 81cc2dc Ingress type openshift (#52) * 2dce90f Bump helm/chart-releaser-action from 1.5.0 to 1.6.0 (#75) * f08ebd3 Patch Tornjak ingress (#74) * 6a6aa1c Cosign download fix (#72) * 3b01684 Support Openshift deployment (#13) * ed23d8b Simple ingress support (#48) * b354413 Fix incorrect nesting of priorityClassName (#67) * 09ebfc9 Bump test chart dependencies (#68) * d50d78d Fix ingress annotation variable for tornjak server (#66) * b813807 Update docs to simplify, clarify, and prioritize ease of install/upgrade (#62) * 5ad35bd Merge pull request #59 from sabre1041/spire-agent-nodename * 02d445c Update charts/spire/README.md * 9bd7f43 Added environment variable to spire-agent to inject node name * fb64f8d Bump github.com/onsi/gomega from 1.28.1 to 1.29.0 in /tests (#61) * 30d0f59 Update helm-charts repository URL (#58) * 7924323 Specify ingress controller type (#43) * 0b17434 Bump test chart dependencies (#54) * a419903 Bump github.com/onsi/gomega from 1.28.0 to 1.28.1 in /tests (#53) * 5025025 Fix doc missing end tag (#46) * cf226a7 Bump actions/checkout from 4.1.0 to 4.1.1 (#42) Signed-off-by: Faisal Memon <[email protected]>
{{- $webhookname := printf "%s-%s-webhook" .Release.Namespace (include "spire-controller-manager.fullname" .) }} | ||
{{- $oldwebhookname := printf "%s-webhook" (include "spire-controller-manager.fullname" .) }} | ||
{{- if not (lookup "admissionregistration.k8s.io/v1" "ValidatingWebhookConfiguration" "" $webhookname) }} | ||
{{- $webhookname = $oldwebhookname }} | ||
{{- end }} |
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.
These short term object renames need to be major releases. It's impossible to upgrade from 0.14.0 to 0.20.0 due to this rename logic being removed somewhere b/t here and 0.20.0. Under semver, minor releases should just work, but that's not the case.
Updates image and CRD's to 0.4.0. Adds autoPopulateDNSNames and className support which were added in 0.4.0.