Explicitly setting CertManager APIVersion to V1

[hawkesn]

Issue

aws/eks-charts#942
and
Fixes: #3188

Description

This PR explicitly sets v1 for the certmanager API version:
apiVersion: cert-manager.io/v1

Checklist

• Added tests that cover your change (if possible)
• Added/modified documentation as required (such as the README.md, or the docs directory)
• Manually tested
• Made sure the title of the PR is a good description that can go into the release notes

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

• Backfilled missing tests for code in same general area 🎉
• Refactored something and made the world a better place 🌟

Testing

Ran helm template --set clusterName=test .

---
# Source: aws-load-balancer-controller/templates/webhook.yaml
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
  name: aws-load-balancer-webhook
  labels:
    helm.sh/chart: aws-load-balancer-controller-1.5.3
    app.kubernetes.io/name: aws-load-balancer-controller
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "v2.5.1"
    app.kubernetes.io/managed-by: Helm
webhooks:
- clientConfig:
    caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURRRENDQWlpZ0F3SUJBZ0lSQU9qaDIxMFlBakRsUjdsK2hqNUluN2d3RFFZSktvWklodmNOQVFFTEJRQXcKS..........

This continues to function as normal.

Ran helm template --set clusterName=test --set enableCertManager=true .

---
# Source: aws-load-balancer-controller/templates/webhook.yaml
apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
  name: aws-load-balancer-serving-cert
  namespace: kube-system
  labels:
    helm.sh/chart: aws-load-balancer-controller-1.5.3
    app.kubernetes.io/name: aws-load-balancer-controller
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "v2.5.1"
    app.kubernetes.io/managed-by: Helm
spec:
  dnsNames:
  - aws-load-balancer-webhook-service.kube-system.svc
  - aws-load-balancer-webhook-service.kube-system.svc.cluster.local
  issuerRef:
    kind: Issuer
    name: aws-load-balancer-selfsigned-issuer
  secretName: aws-load-balancer-tls
---
# Source: aws-load-balancer-controller/templates/webhook.yaml
apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
  name: aws-load-balancer-selfsigned-issuer
  namespace: kube-system
  labels:
    helm.sh/chart: aws-load-balancer-controller-1.5.3
    app.kubernetes.io/name: aws-load-balancer-controller
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "v2.5.1"
    app.kubernetes.io/managed-by: Helm
spec:
  selfSigned: {}

apiVersion of cert-manager is set to v1.

[k8s-ci-robot]

Welcome @hawkesn!

It looks like this is your first PR to kubernetes-sigs/aws-load-balancer-controller 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/aws-load-balancer-controller has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @hawkesn. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[kishorj]

Lets specify the v1 version explicitly and we specify the min cert-manager version in our documents.

[johngmyers]

Why would one want to explicitly specify this?

[codecov-commenter]

Codecov Report

Patch and project coverage have no change.

Comparison is base (ba7df31) 54.70% compared to head (964e4b9) 54.70%.

❗ Current head 964e4b9 differs from pull request most recent head 568c28c. Consider uploading reports for the commit 568c28c to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3189   +/-   ##
=======================================
  Coverage   54.70%   54.70%           
=======================================
  Files         148      148           
  Lines        8590     8590           
=======================================
  Hits         4699     4699           
  Misses       3559     3559           
  Partials      332      332           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[kishorj]

Why would one want to explicitly specify this?

Since cert-manager.io/v1 is the stable version, we should stick with it rather than providing an option to choose between v1alpha2 and v1 unless there is a compelling reason. This will also simplify future maintenance.

[johngmyers]

My question was to the author as to why an admin would want to explicitly specify apiVersion. The existing .Capabilities.APIVersions.Has mechanism is reasonable for a transitition time.

The v1 API was introduced in cert-manager 1.0.0, released September 2020. It's probably time to drop support for older cert-manager.

[hawkesn]

My question was to the author as to why an admin would want to explicitly specify apiVersion. The existing .Capabilities.APIVersions.Has mechanism is reasonable for a transitition time.

The v1 API was introduced in cert-manager 1.0.0, released September 2020. It's probably time to drop support for older cert-manager.

I agree with you that we should drop support for the older cert-manager, I let a user specify it for backwards compatibility reasons (just in case), but you're both right @kishorj and @johngmyers, it's 2023 now and we should use v1. I see the comment from @kishorj and I'll be changing it to use v1 with updated documentation

[kishorj]

/ok-to-test

[hawkesn]

@kishorj thanks for approving to test! Looks like the tests have passed 🎉

[kishorj]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hawkesn, kishorj

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:

• OWNERS [kishorj]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sudermanjr]

should this have been released with chart version 1.5.3, or is it still pending a release?

▶ helm template eks/aws-load-balancer-controller --set enableCertManager=true --set clusterName=test --version=1.5.3 | grep cert-manager
apiVersion: cert-manager.io/v1alpha2
apiVersion: cert-manager.io/v1alpha2
    cert-manager.io/inject-ca-from: argocd/aws-load-balancer-serving-cert
    cert-manager.io/inject-ca-from: argocd/aws-load-balancer-serving-cert

[kishorj]

@sudermanjr, we will need a new chart release. Chart version 1.5.3 doesn't contain the changes from this PR.