-
Notifications
You must be signed in to change notification settings - Fork 22
Re-scaffold with kubebuilder 4.5.0 #55
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: Erik Godding Boye <[email protected]>
| var enableHTTP2 bool | ||
| var tlsOpts []func(*tls.Config) | ||
| flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+ | ||
| "Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.") |
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.
This doesn't make sense. Is it 127.0.0.1:8443 / 0.0.0.0:8443, or is it really :8443? And if it's the latter, why is it a string?
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.
Well, that's just how it is configurable in controller-runtime - ref. https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/metrics/server#Options. 😉
Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Erik Godding Boye <[email protected]>
wallrj
left a comment
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.
Thanks @erikgb
And thank @jsoref for your reviews.
I skimmed through the changes and compared some of the files to the kubebuilder files:
A lot has changed!
I last updated the skaffolding in #38
The release.yaml file is quite important because that's how we make the sample-external-issuer available in the cert-manager E2E tests.
We can revive that in a followup PR if you like.
/approve
/lgtm
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.
This file is important.
We periodically do a release and update the cert-manager E2E tests to use that release version.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wallrj The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR re-scaffolds the project using the latest release of kubebuilder (4.5.0). I have intentionally tried to deviate as little as possible from the suggested/recommended structure, regardless of whether I like it. 😉 Hopefully this re-scaffold process will be simpler to do the next time. At the same time I have attempted not to leave any "loose ends" and not make unnecessary/unrelated changes.
Main motivation for this PR was to remove use of the long-time deprecated kube-rbac-proxy and avoid using deprecated kustomize features.
Relates to #52 (initial attempt to do this).