Skip to content

fix: conversion setting on bundle crds#4673

Merged
Skarlso merged 1 commit intomainfrom
gc/fix-helm-charts
Apr 16, 2025
Merged

fix: conversion setting on bundle crds#4673
Skarlso merged 1 commit intomainfrom
gc/fix-helm-charts

Conversation

@gusfcarvalho
Copy link
Copy Markdown
Member

@gusfcarvalho gusfcarvalho commented Apr 15, 2025

when users specify kubectl apply -f, they need to have spec.conversion: {} explicitly. While, with a helm upgrade, they need to have no conversion set for helm to upgrade the manifest.

With helm, the error is:

 Required value && cannot patch "webhooks.generators.external-secrets.io" with kind CustomResourceDefinition: CustomResourceDefinition.apiextensions.k8s.io "webhooks.generators.external-secrets.io" is invalid: spec.conversion.strategy: Required value

While with kubectl apply (regardless if server side or client side), the error is:

for: "deploy/crds/bundle.yaml": error when patching "deploy/crds/bundle.yaml": CustomResourceDefinition.apiextensions.k8s.io "webhooks.generators.external-secrets.io" is invalid: [spec.conversion.webhookClientConfig: Forbidden: should not be set when strategy is not set to Webhook, spec.conversion.conversionReviewVersions: Forbidden: should not be set when strategy is not set to Webhook]

I think this covers most of the ways people would apply this patch, so the proposal here is to fix the bundle so that kubectl apply works, and opt out (we already did) helm charts from it.

I also did a lot of fixes to Makefile now that I am using nerdctl instead of docker

@gusfcarvalho
fix: conversion setting on bundle crds
4326fc9 
Signed-off-by: Gustavo Carvalho <gustavo@externalsecrets.com>
@gusfcarvalho gusfcarvalho requested a review from a team as a code owner April 15, 2025 21:41
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 15, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@gusfcarvalho gusfcarvalho mentioned this pull request Apr 15, 2025
Closed
@Skarlso Skarlso merged commit d6dcfff into main Apr 16, 2025
23 of 24 checks passed
@Skarlso Skarlso deleted the gc/fix-helm-charts branch April 16, 2025 04:30
@Syed-Shahidh-Ilhan
Copy link
Copy Markdown

Syed-Shahidh-Ilhan commented Apr 16, 2025 •
edited
Loading

Heyy @gusfcarvalho 👋 is this incompatible with versions of k8s < 1.30? ( Not sure if its a k8s problem anyway )

We run
kubectl create -f 'https://raw.githubusercontent.com/external-secrets/external-secrets/main/deploy/crds/bundle.yaml
in our CI pipeline and have been facing this error

Error from server (Invalid): error when creating "https://raw.githubusercontent.com/external-secrets/external-secrets/main/deploy/crds/bundle.yaml": CustomResourceDefinition.apiextensions.k8s.io "gcraccesstokens.generators.external-secrets.io" is invalid: spec.conversion.strategy: Required value

@devOwlish
Copy link
Copy Markdown

devOwlish commented Apr 16, 2025

Also incompatible with at least Kind k8s 1.30 - 1.32.

I use the ESO bundle in CI pipelines to verify that the chart is installable, and the change has broken all of them. I've verified with Kind versions 1.29 - 1.32.

Error from server (Invalid): error when creating "https://raw.githubusercontent.com/external-secrets/external-secrets/main/deploy/crds/bundle.yaml": CustomResourceDefinition.apiextensions.k8s.io "webhooks.generators.external-secrets.io" is invalid: spec.conversion.strategy: Required value

@Skarlso
Copy link
Copy Markdown
Contributor

Skarlso commented Apr 16, 2025

This most likely isn't a k8s version problem per say. I'll take a look at what's up.

@Syed-Shahidh-Ilhan
Copy link
Copy Markdown

Syed-Shahidh-Ilhan commented Apr 16, 2025

Thank you @Skarlso !

@Skarlso Skarlso mentioned this pull request Apr 16, 2025
Merged
5 tasks
@Skarlso
Copy link
Copy Markdown
Contributor

Skarlso commented Apr 16, 2025

Removed the hook completely. I'll test this out see if it works. #4675

@Skarlso
Copy link
Copy Markdown
Contributor

Skarlso commented Apr 16, 2025

Please try the pull request. I tried and it seems to be okay since conversion is an optional field.

➜ kubectl apply -f deploy/crds/bundle.yaml
customresourcedefinition.apiextensions.k8s.io/clusterexternalsecrets.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/clusterpushsecrets.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/clustersecretstores.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/externalsecrets.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/pushsecrets.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/secretstores.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/acraccesstokens.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/clustergenerators.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/ecrauthorizationtokens.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/fakes.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/gcraccesstokens.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/generatorstates.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/githubaccesstokens.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/grafanas.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/passwords.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/quayaccesstokens.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/stssessiontokens.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/uuids.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/vaultdynamicsecrets.generators.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/webhooks.generators.external-secrets.io created

@harshit-workk
Copy link
Copy Markdown

harshit-workk commented Apr 16, 2025 •
edited
Loading

Removed the hook completely. I'll test this out see if it works.

@Skarlso are the changes merged? Its still the same issue right now!! Can you please confirm this once? TY for understanding!

@Skarlso
Copy link
Copy Markdown
Contributor

Skarlso commented Apr 16, 2025

No, it's only on a branch for now. It's not merged yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Skarlso Skarlso Skarlso approved these changes

@knelasevero knelasevero Awaiting requested review from knelasevero knelasevero was automatically assigned from external-secrets/maintainers

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@gusfcarvalho @Syed-Shahidh-Ilhan @devOwlish @Skarlso @harshit-workk