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

cert-manager's Gateway API support is no longer experimental in cert-manager 1.15 #33840

Closed
wants to merge 2 commits into from

Conversation

maelvls
Copy link

@maelvls maelvls commented Jul 16, 2024

Hey,

I'm happy to report that cert-manager has graduated the Gateway API support to beta! You no longer have to pass the feature flag ExperimentalGatewayAPISupport anymore. Most importantly, the feature flag ExperimentalGatewayAPISupport was removed, so folks who used it in cert-manager 1.14 and below will need to remove it and add the new flag --enable-gateway-api.

Source: https://cert-manager.io/docs/releases/upgrading/upgrading-1.14-1.15#gatewayapi-promotion-to-beta

Ref: cert-manager/website#851 (comment)

NONE

@maelvls maelvls requested review from a team as code owners July 16, 2024 09:12
@maintainer-s-little-helper
Copy link

Commit b32d538 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jul 16, 2024
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jul 16, 2024
Signed-off-by: Maël Valais <mael@vls.dev>
@maelvls maelvls force-pushed the gateway-api-cert-manager branch from b32d538 to 9675e62 Compare July 16, 2024 09:19
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jul 16, 2024
@sayboras sayboras added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Jul 16, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jul 16, 2024
I also changed "installCRDs" to "crds.enabled" since installCRDs has
been deprecated in cert-manager 1.15.

Signed-off-by: Maël Valais <mael@vls.dev>
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

LGTM and thanks a lot

Copy link
Member

@mhofstetter mhofstetter left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! 👍 Looks like we still need to improve the command a little bit - see my comment inline.

@@ -32,11 +32,11 @@ Create TLS Certificate and Private Key
.. code-block:: shell-session

$ helm repo add jetstack https://charts.jetstack.io
$ helm install cert-manager jetstack/cert-manager --version v1.10.0 \
$ helm install cert-manager jetstack/cert-manager --version v1.15.1 \
Copy link
Member

@mhofstetter mhofstetter Jul 16, 2024

Choose a reason for hiding this comment

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

it seems as this command is failing with the following error whereas the previous command worked without an issue.

❯ helm install cert-manager jetstack/cert-manager --version v1.15.1 \
                --namespace cert-manager \
                --create-namespace \
                --set crds.enabled=true \
                --set config.enableGatewayAPI=true
Error: INSTALLATION FAILED: execution error at (cert-manager/templates/controller-config.yaml:2:38): .Values.config.apiVersion must be set !

It looks like we need wait for cert-manager/cert-manager#7126 being released or add the missing helm values too.

--set config.apiVersion="controller.config.cert-manager.io/v1alpha1" \
--set config.kind="ControllerConfiguration" \

edit: a quick test with the missing helm values starts the Helm installation - but it doesn't finish, as the startupapicheck pod doesn't succeed. 🤔 not sure whether i miss something in my setup though (local kind cluster with Cilium installed)

I0716 15:18:10.341850       1 api.go:108] "Not ready" logger="cert-manager.startupapicheck.checkAPI" err="Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": failed to call webhook: Post \"https://cert-manager-webhook.cert-manager.svc:443/validate?timeout=30s\": tls: failed to verify certificate: x509: certificate signed by unknown authority"

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for testing out the changes @mhofstetter, I appreciate the attention to detail there.

Copy link

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Aug 16, 2024
Copy link

This pull request has not seen any activity since it was marked stale.
Closing.

@github-actions github-actions bot closed this Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. kind/community-contribution This was a contribution made by a community member. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants