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.

Sign up for GitHub

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

Jump to bottom

"cert-manager.io/alt-names" annotation under Ingress resources #5897

Closed
ruiengana opened this issue Mar 28, 2023 · 12 comments · May be fixed by #6190
Closed

"cert-manager.io/alt-names" annotation under Ingress resources #5897

ruiengana opened this issue Mar 28, 2023 · 12 comments · May be fixed by #6190
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. triage/support Indicates an issue that is a support question.

Comments

@ruiengana
Copy link

ruiengana commented Mar 28, 2023 •
edited

Describe the bug:
When annotating Ingress resources with cert-manager annotations, "cert-manager.io/alt-names" doesn't seem to be picked up.

Expected behaviour:
Expected cert-manager to manage "cert-manager.io/alt-names" annotations from Ingress resources

Steps to reproduce the bug:
Create a Ingress resource like below, where you use annotation "cert-manager.io/alt-names"

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    cert-manager.io/alt-names: service.example.com,service.region.cloud.example.com
    cert-manager.io/cluster-issuer: letsencrypt-prod
    kubernetes.io/service.class: nginx
    nginx.service.kubernetes.io/server-alias: service.example.com
    nginx.service.kubernetes.io/service-upstream: "true"
  name: service-ingress
  namespace: default
spec:
  rules:
  - host: service.region.cloud.example.com
    http:
      paths:
      - backend:
          service:
            name: service-backend
            port:
              number: 80
        path: /
        pathType: ImplementationSpecific
  tls:
  - hosts:
    - service.region.cloud.example.com
    secretName: service.region.cloud.example.com

After cert-manager do its magic, you will see the Certificate and associated Secret will not have the expected "cert-manager.io/alt-names". You will see something like below.

apiVersion: v1
kind: Secret
metadata:
  annotations:
    cert-manager.io/alt-names: service.region.cloud.example.com
    cert-manager.io/certificate-name: service.region.cloud.example.com
    cert-manager.io/common-name: service.region.cloud.example.com
    cert-manager.io/ip-sans: ""
    cert-manager.io/issuer-group: cert-manager.io
    cert-manager.io/issuer-kind: ClusterIssuer
    cert-manager.io/issuer-name: letsencrypt-prod
    cert-manager.io/uri-sans: ""
  labels:
    controller.cert-manager.io/fao: "true"
  name: service.region.cloud.example.com
  namespace: default
type: kubernetes.io/tls

Environment details::

  • Kubernetes version: 1.25
  • Cloud-provider/provisioner: AWS
  • cert-manager version: 1.11
  • Install method: Helm

/kind bug
@jetstack-bot jetstack-bot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 28, 2023
@irbekrm
Copy link
Contributor

irbekrm commented Mar 31, 2023

cert-manager.io/alt-names

As far as I am aware, we do not have such annotation for Ingress resources
These are the annotations that are supported https://cert-manager.io/docs/usage/ingress/#supported-annotations

@irbekrm irbekrm added triage/support Indicates an issue that is a support question. and removed kind/bug Categorizes issue or PR as related to a bug. labels Mar 31, 2023
@ruiengana
Copy link
Author

cert-manager.io/alt-names

As far as I am aware, we do not have such annotation for Ingress resources These are the annotations that are supported https://cert-manager.io/docs/usage/ingress/#supported-annotations

But this annotation exists for Certificate resources, right? Why would it not be applicable for Ingress resources too...

@ruiengana
Copy link
Author

@irbekrm why would this annotation be available for Certificate resources and not for Ingress resources?

@irbekrm
Copy link
Contributor

irbekrm commented Apr 12, 2023

@irbekrm why would this annotation be available for Certificate resources and not for Ingress resources?

Certificate is not configured via annotations.
You can however configure SANs via spec.uris field.

The annotations on the Ingress will always be a very limited subset of Certificate functionality - it's really just a shortcut for generating a Certificate from the Ingress's TLS block and generally annotations is not a great configuration mechanism as there are limited validation capabilities etc. If you need a more complex setup, you can use Certificate directly.
Specifically for SANs we've not had anyone come up with a valid use case why this kind of annotation would be needed for Ingresses.

@MesterJoda
Copy link

+1 for the feature request. I'd like to say that this is really important as a feature and here is why. All modern internet browsers first try https and then it's up to a user to try http only after a warning about insecure connection. For a very simple www to non-www redirect, one can use annotation to fix this nginx.ingress.kubernetes.io/from-to-www-redirect: "true". For this to work you need two certificates, one for non-www and the other for www, since the SSL connection comes first, and only after the connection has been made, browser will follow any redirects present in annotations.
Not to mention that a lot of modern websites buy numerous similar domains. All the redirections are done a web server level. That's because in the DNS zones, you cannot use CNAME on a "root" or "top-level" DNS name. You have to use A record. For a "www." you can use CNAME to point to domain name you actually want it to go. Because of that A record of the "root" domain name, all requests go to web server, hence the need to have alt-names in ingress definition. Currently, there should be multiple ingress hosts, and it could be made simple, with just one, if this feature existed.
I respect the fact that the annotations on the Ingress will always be a very limited subset of Certificate functionality, this feature actually fits perfectly.
Thank you

@ruiengana
Copy link
Author

ruiengana commented Jun 6, 2023 •
edited

We simply want to ability to describe the certificate SAN attribute via cert-manager.io annotations on Ingress resources, this is extremely helpful in scenarios where you need to rebuild the cluster in another region and/or cloud provider where you need certificates with the region / cloud provider information and keep vanity SANs for disaster recovery options.

We understand that we can manage the Certificate resource directly and all that, but point is it completely unnecessary if a simple annotation is made available at the Ingress level.

Looking to the code in

cert-manager/pkg/controller/certificate-shim/helper.go

Line 68 in f8940ab

if commonName, found := ingLikeAnnotations[cmapi.CommonNameAnnotationKey]; found {
already support cert-manager.io/common-name annotation over Ingress resources. Wouldn't this be a simple extension to the existing feature?

Can you please consider it?

@ruiengana
Copy link
Author

@irbekrm would you consider a PR? The code base already support a similar annotation so change looks minimal.

@vanveele vanveele mentioned this issue Jun 27, 2023
Open
@jetstack-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle stale

@jetstack-bot jetstack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 13, 2023
@jetstack-bot
Copy link
Contributor

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle rotten
/remove-lifecycle stale

@jetstack-bot jetstack-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 13, 2023
@ruiengana
Copy link
Author

ruiengana commented Oct 13, 2023 via email

@jetstack-bot
Copy link
Contributor

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

@jetstack-bot
Copy link
Contributor

@jetstack-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. triage/support Indicates an issue that is a support question.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Adds ingress annotation support for alt-names vanveele/cert-manager
4 participants
@ruiengana @MesterJoda @irbekrm @jetstack-bot