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

(helm/v1-alpha) Generates invalid chart in operator without webhooks #4582

Open
erikgb opened this issue Feb 27, 2025 · 5 comments · May be fixed by #4584
Open

(helm/v1-alpha) Generates invalid chart in operator without webhooks #4582

erikgb opened this issue Feb 27, 2025 · 5 comments · May be fixed by #4584
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@erikgb
Copy link
Contributor

erikgb commented Feb 27, 2025

What broke? What's expected?

We are considering providing a simple Helm chart to install https://github.com/statnett/image-scanner-operator - even if it is currently marked as an eschewed feature. 😉 So I am testing the new Helm plugin in kubebuilder, but it generates an invalid chart:

$ helm template dist/chart
Error: template: image-scanner-operator/templates/crd/stas.statnett.no_containerimagescans.yaml:18:16: executing "image-scanner-operator/templates/crd/stas.statnett.no_containerimagescans.yaml" at <.Values.webhook.enable>: nil pointer evaluating interface {}.enable

To me, it seems like the generator uses values not declared in values.yaml of the Helm chart - when the operator has no webhooks.

This is the output from kubebuilder:

$ kubebuilder edit --plugins=helm/v1-alpha
INFO Generating Helm Chart to distribute project  
INFO webhook manifests were not found at config/webhook/manifests.yaml 
INFO Successfully copied config/rbac/auth_proxy_client_clusterrole.yaml to dist/chart/templates/rbac/auth_proxy_client_clusterrole.yaml 
INFO Successfully copied config/rbac/auth_proxy_role.yaml to dist/chart/templates/rbac/auth_proxy_role.yaml 
INFO Successfully copied config/rbac/auth_proxy_role_binding.yaml to dist/chart/templates/rbac/auth_proxy_role_binding.yaml 
INFO Successfully copied config/rbac/auth_proxy_service.yaml to dist/chart/templates/rbac/auth_proxy_service.yaml 
INFO Successfully copied config/rbac/leader_election_role.yaml to dist/chart/templates/rbac/leader_election_role.yaml 
INFO Successfully copied config/rbac/leader_election_role_binding.yaml to dist/chart/templates/rbac/leader_election_role_binding.yaml 
INFO Successfully copied config/rbac/role.yaml to dist/chart/templates/rbac/role.yaml 
INFO Successfully copied config/rbac/role_binding.yaml to dist/chart/templates/rbac/role_binding.yaml 
INFO Successfully copied config/rbac/service_account.yaml to dist/chart/templates/rbac/service_account.yaml 
INFO Successfully copied config/rbac/stas_containerimagescan_editor_role.yaml to dist/chart/templates/rbac/stas_containerimagescan_editor_role.yaml 
INFO Successfully copied config/rbac/stas_containerimagescan_viewer_role.yaml to dist/chart/templates/rbac/stas_containerimagescan_viewer_role.yaml 
INFO Successfully copied config/crd/bases/stas.statnett.no_containerimagescans.yaml to dist/chart/templates/crd/stas.statnett.no_containerimagescans.yaml 

Reproducing this issue

I would assume the issue is reproducible by scaffolding a new operator without webhooks but with CRDs. Then try generating the Helm chart. And then template the chart.

KubeBuilder (CLI) Version

4.5.0

PROJECT version

3

Plugin versions

layout:
- go.kubebuilder.io/v4
multigroup: true
plugins:
  helm.kubebuilder.io/v1-alpha: {}

Other versions

No response

Extra Labels

No response

@erikgb erikgb added the kind/bug Categorizes issue or PR as related to a bug. label Feb 27, 2025
@camilamacedo86
Copy link
Member

Thank you for raising this one !!!
We need to give a look at it.

@camilamacedo86 camilamacedo86 added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Feb 28, 2025
@camilamacedo86 camilamacedo86 added this to the helm milestone Feb 28, 2025
@camilamacedo86
Copy link
Member

Hi @erikgb

I would assume the issue is reproducible by scaffolding a new operator without webhooks but with CRDs. Then try generating the Helm chart. And then template the chart.

This scenario is tested here:

https://github.com/kubernetes-sigs/kubebuilder/blob/master/docs/book/src/getting-started/testdata/project/dist/chart/values.yaml

We validate it with the CI. So, it seems that that is not the root cause.

@erikgb
Copy link
Contributor Author

erikgb commented Feb 28, 2025

Ok, then it's probably something with our project. Not very easy to keep up with the rescaffolding. Thanks for looking into this.

@erikgb erikgb closed this as completed Feb 28, 2025
@camilamacedo86
Copy link
Member

camilamacedo86 commented Feb 28, 2025

Hi @erikgb

I test out with your project and kubebuilder from master

$ kubebuilder version
Version: main.version{KubeBuilderVersion:"4.5.1", KubernetesVendor:"1.32.1", GitCommit:"0ace7a8753c52b35014e43edc2a0b0454b78e769", BuildDate:"2025-02-21T20:16:18Z", GoOs:"darwin", GoArch:"arm64"}

And I could find the issue. I validate the getting-started and I could helm template dist/chart from there all fine.
But lets re-open and see if we can find the root cause

@camilamacedo86
Copy link
Member

Bug Report: Helm Plugin Scaffold Issue with Webhook Conversion

Hi @erikgb,

I found a bug in the scaffold of the Helm Plugin that we need to address. Please see the PR linked to this issue. However, note that your project was scaffolded with older versions of Kubebuilder. Unfortunately, in the past, webhook conversion was added but not properly/fully implemented.

Now that we have been working on ensuring that the project has been fully converted and implemented webhooks conversion, this issue has surfaced.

Recommended Action

I would recommend manually applying the fixes to your project. Specifically, you should remove config/crd/patches.

Relevant Fixes

  • Issue fixed in release 4.3.1: (kustomize/v2, go/v4)
    • Corrected the generation of manifests under config/crd/patches to ensure the /convert service patch is only created for webhooks configured with --conversion.
    • #4280

Summary

**To resolve this scaffolded issue, you mainly need to apply the changes of #4280.

Let me know if you need further assistance!

Thanks. And please feel free to review the PR: #4584

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
2 participants