Skip to content

⚠️ Upgrade kubebuilder scaffold from v4.2.0 to v4.5.2#6928

Merged
acornett21 merged 5 commits intooperator-framework:masterfrom
camilamacedo86:upgrade-kubebuilder
May 27, 2025
Merged

⚠️ Upgrade kubebuilder scaffold from v4.2.0 to v4.5.2#6928
acornett21 merged 5 commits intooperator-framework:masterfrom
camilamacedo86:upgrade-kubebuilder

Conversation

@camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Apr 25, 2025

Update the scaffolds as part of: #6927

"--version", mh.ctx.Version,
"--kind", mh.ctx.Kind,
"--defaulting",
"--defaulting")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@acornett21, it was duplicated. If we want it in the sample, default + validating, then we need to add it here.
But would not be call twice defaulting
I keep only defaulting here. We can add the validating as well in a follow up if we need to

err := kbutil.ReplaceInFile(webhookPath,
"// TODO(user): fill in your defaulting logic.",
"if memcached.Spec.Size == 0 {\n\t\tmemcached.Spec.Size = 3\n\t}")
pkg.CheckError("injecting defaulting logic", err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@acornett21 just added the code to implement the defaulting logic, but in the right place, ancleaned up all the other staff.

return errors.New("Cluster size must be an odd number")
}
return nil
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@acornett21 if we want the validating webhook here then we need to add the option to scaffold this type and then just replace TODO:(user) implement your logic for the logic that we want instead.

# delimiter: '.'
# index: 1
# create: true
# - source: # Uncomment the following block to enable certificates for metrics
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@acornett21 the changes here mainly happened due bug fixes and enhancements.

New feature

Bug Fixes

  • Fixed CA injection for conversion webhooks. Previously, the CA injection patch was not accurate; The injection should occur only for CRDs, which are conversion types and not for all CRDs when a webhook with --conversion option is scaffolded. The issue goes back to release 3.5.0 (where to replace vars for replacements was done and the kustomize/v2-alpha plugin was introduced). It was not previously found, likely because conversion webhook features were incomplete, which is addressed in this release. Now, users can use the tool to generate the conversion webhooks properly

  • Corrected the generation of manifests under config/crd/patches to ensure the /convert service patch is only created for webhooks configured with --conversion.

@camilamacedo86 camilamacedo86 mentioned this pull request Apr 25, 2025
10 tasks
@$(KIND) get clusters | grep -q 'kind' || { \
echo "No Kind cluster is running. Please start a Kind cluster before running the e2e tests."; \
exit 1; \
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@acornett21 just for you be aware of.
It should not be added to the scaffold.
We changed already on master, so it is fixed. I am cleaning up.

@camilamacedo86 camilamacedo86 changed the title ⚠️ Upgrade kubebuilder deps add support for k8s 1.32 ⚠️ Upgrade kubebuilder scaffold from v4.2.0 to v4.5.2 May 14, 2025
@acornett21
Copy link
Contributor

acornett21 commented May 20, 2025

@camilamacedo86 Testing this locally I believe this is the error for the package manifest tests

oc logs memcached-operator-controller-manager-56f6565475-rx2fv
2025-05-20T21:48:49Z    INFO    setup   Initializing webhook certificate watcher using provided certificates    {"webhook-cert-path": "/tmp/k8s-webhook-server/serving-certs", "webhook-cert-name": "tls.crt", "webhook-cert-key": "tls.key"}
2025-05-20T21:48:49Z    INFO    controller-runtime.certwatcher  Updated current TLS certificate
2025-05-20T21:48:49Z    INFO    setup   Initializing metrics certificate watcher using provided certificates    {"metrics-cert-path": "/tmp/k8s-metrics-server/metrics-certs", "metrics-cert-name": "tls.crt", "metrics-cert-key": "tls.key"}
2025-05-20T21:48:49Z    ERROR   setup   to initialize metrics certificate watcher       {"error": "open /tmp/k8s-metrics-server/metrics-certs/tls.crt: no such file or directory", "error": "open /tmp/k8s-metrics-server/metrics-certs/tls.crt: no such file or directory"}
main.main
        /workspace/cmd/main.go:173
runtime.main
        /usr/local/go/src/runtime/proc.go:272

I can't tell what would be going on here, since the only diff between the static test data, and the test for the speicifc test is below

24c24,27
<     createdAt: "2022-11-08T17:26:37Z"
---
>     createdAt: "2025-05-21T15:11:36Z"
>     operators.operatorframework.io/builder: operator-sdk-v1.39.2+git
>     operators.operatorframework.io/project_layout: go.kubebuilder.io/v4
>   creationTimestamp: null
28a32,33
>   cleanup:
>     enabled: false
143a149
>               creationTimestamp: null
160c166
<                 image: quay.io/example/memcached-operator:v0.0.1
---
>                 image: quay.io/integration/memcached-operator:0.0.1
281a288,289
> status:
>   cleanup: {}

Copy link
Contributor

Choose a reason for hiding this comment

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

Are these new files coming from kubebuilder? Or from your local?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is new from Kubebuilder.
Now, projects built within have the devcontainer file so that people can test out and do things within, for example, GitHub Codespaces. ( new feature does not impact old projects )

@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented May 23, 2025

Hi @acornett21 👋

Following up on this comment — I believe I found the issue with the package manifest tests. Here's the fix.

What happened?

Previously, we relied on kube-rbac-proxy, which generated TLS certificates on the fly. While convenient, that approach is no longer recommended for production. We’ve since transitioned to using real certificates issued by cert-manager (see kubernetes-sigs/kubebuilder#3907 for more context).

You can see a working example of this setup in the Memcached sample, where metrics certificates are configured correctly:
https://github.com/operator-framework/operator-sdk/pull/6928/files#diff-e0eef45157ac7767286524a85be1093cefd53432bde1ec4b02d01f800d3ac658R44-R50

However, packagemanifest is unable to work within.

So the fix here is just comment the configuration and allow the test still working to test the deprecated command with what it supports.

Thanks!

camilamacedo86 and others added 5 commits May 23, 2025 13:39
Signed-off-by: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com>
Signed-off-by: Adam D. Cornett <adc@redhat.com>
Signed-off-by: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com>
Comment kustomize code to pass certs for the metric server for tests with packagemanifest

Signed-off-by: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com>
Signed-off-by: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com>
Signed-off-by: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com>
@camilamacedo86
Copy link
Contributor Author

Hi @acornett21

Please feel free to take a look at this one.
I think we are good now regarding the scaffold changes.
As a follow-up we need to change the k8s version for SDK itself: https://github.com/operator-framework/operator-sdk/pull/6941/files

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.

3 participants