cert-manager / aws-privateca-issuer

Addon for cert-manager that issues certificates using AWS ACM PCA.
Apache License 2.0
192 stars 81 forks source link

[Bug]: cert-manager.io/cluster-issuer annotation does not work for AWSPCAClusterIssuer #252

Closed brsolomon-deloitte closed 1 year ago

brsolomon-deloitte commented 1 year ago

Describe the expected outcome

The cert-manager.io/cluster-issuer annotation for Ingress should be allowed to point to a AWSPCAClusterIssuer.

Describe the actual outcome

To refer to a AWSPCAClusterIssuer, cert-manager only recognizes cert-manager.io/issuer and not cert-manager.io/cluster-issuer.

This is counter-intuitive given that cluster-issuer is supposed to point to a non-namespaced resource.

https://cert-manager.io/docs/usage/ingress/#supported-annotations

Steps to reproduce

  1. Deploy cert-manager and aws-privateca-issuer using Helm
  2. Create a AWSPCAClusterIssuer
  3. Create a test app with an Ingress and annotate it with cert-manager.io/cluster-issuer

The result is that nothing happens at all - the annotation is effectively ignored.

Relevant log output

No response

Version

1.2.4

Have you tried the following?

Category

Other

Severity

Severity 3

brsolomon-deloitte commented 1 year ago

If this is somehow the intended behavior - then it doesn't make much sense.

cert-manager.io/issuer should point to an AWSPCAIssuer

cert-manager.io/cluster-issuer should point to an AWSPCAClusterIssuer

This does not seem to be formally documented nor tested.

KyleBS commented 1 year ago

Thank you for raising this issue with the AWS Private CA Issuer plugin. We will review your submission and respond back to you here as soon as possible.

Hamidhasan commented 1 year ago

Hello @brsolomon-deloitte, thanks for raising this issue, I have been looking into this and am trying to reproduce it on our end.

Can you post the Ingress resource you’ve tested this with (a sample definition file is fine). May be an obvious ask but, after reading the cert-manager source code, you cannot specify a cluster-issuer at the same time as specifying a kind or a group. https://github.com/cert-manager/cert-manager/blob/706ad574b9bb7669ae94d293d4136cc37cb7d09f/pkg/controller/certificate-shim/sync.go#L705-L715

Hamidhasan commented 1 year ago

I was able to test this offline, and I can confirm that even with just the cluster-issuer annotation (i.e. not specifying the kind or group), we're unable to issue the certificate. The regular issuer, when using the kind and group annotations, is able to issue it, but the cluster issuer is not. Here's the error messages I get (it seems like cert-manager attempts to use a few different issuer types but they don't include ours, and thus the cert issuance fails):

  Normal  WaitingForApproval  53s   cert-manager-certificaterequests-issuer-ca          Not signing CertificateRequest until it is Approved
  Normal  WaitingForApproval  53s   cert-manager-certificaterequests-issuer-vault       Not signing CertificateRequest until it is Approved
  Normal  WaitingForApproval  53s   cert-manager-certificaterequests-issuer-venafi      Not signing CertificateRequest until it is Approved
  Normal  WaitingForApproval  53s   cert-manager-certificaterequests-issuer-selfsigned  Not signing CertificateRequest until it is Approved
  Normal  WaitingForApproval  53s   cert-manager-certificaterequests-issuer-acme        Not signing CertificateRequest until it is Approved
  Normal  IssuerNotFound      53s   cert-manager-certificaterequests-issuer-selfsigned  Referenced "ClusterIssuer" not found: clusterissuer.cert-manager.io "pca-cluster-issuer-rsa" not found
  Normal  IssuerNotFound      53s   cert-manager-certificaterequests-issuer-acme        Referenced "ClusterIssuer" not found: clusterissuer.cert-manager.io "pca-cluster-issuer-rsa" not found
  Normal  IssuerNotFound      53s   cert-manager-certificaterequests-issuer-ca          Referenced "ClusterIssuer" not found: clusterissuer.cert-manager.io "pca-cluster-issuer-rsa" not found
  Normal  IssuerNotFound      53s   cert-manager-certificaterequests-issuer-vault       Referenced "ClusterIssuer" not found: clusterissuer.cert-manager.io "pca-cluster-issuer-rsa" not found
  Normal  IssuerNotFound      53s   cert-manager-certificaterequests-issuer-venafi      Referenced "ClusterIssuer" not found: clusterissuer.cert-manager.io "pca-cluster-issuer-rsa" not found
  Normal  cert-manager.io     53s   cert-manager-certificaterequests-approver           Certificate request has been approved by cert-manager.io

For reference, here is the yaml file I applied in order to get those messages.

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    # add an annotation indicating the issuer to use.
    cert-manager.io/cluster-issuer: pca-cluster-issuer-rsa
  name: my-ingress-cluster-issuer
  namespace: aws-privateca-issuer
spec:
  rules:
  - host: example.com
    http:
      paths:
      - pathType: Prefix
        path: /
        backend:
          service:
            name: myservice
            port:
              number: 80
  tls: # < placing a host in the TLS config will determine what ends up in the cert's subjectAltNames
  - hosts:
    - example.com
    secretName: myingress-issuer-cert # < cert-manager will store the created certificate in this secret.

There is a biweekly sync with cert-manager happening tomorrow, we'll raise this issue with them and get guidance on how to resolve it with them. There may need to be some support on cert-manager's side to make this work correctly.

brsolomon-deloitte commented 1 year ago

@Hamidhasan

Can you post the Ingress resource you’ve tested this with. ... For reference, here is the yaml file I applied in order to get those messages.

Seems like you've been able to reproduce exactly what we are seeing, e.g. that a kind: Ingress definition will fail to issue a cert when using cert-manager.io/cluster-issuer, but will succeed with cert-manager.io/issuer, even though the issuer in question is a non-namespaced cluster-wide AWSPCAClusterIssuer.

Hamidhasan commented 1 year ago

Leaving some more notes in here, plan to present to cert-manager weekly meeting:

This is related to the securing ingresses feature of cert-manager.

For our aws-privateca-issuer, only cert-manager.io/issuer annotation will work, and only if it is combined with the cert-manager.io/issuer-kind and cert-manager.io/issuer-group annotations. (I believe the group is optional, just the Kind works). The other 3 combinations fail:

  1. Just issuer, fails with the error messages above
  2. Just cluster-issuer, fails with the error messages above
  3. cluster-issuer with issuer-kind and issuer-group, fails due to the check in cert-manager ingress-shim code.

Basically, we are not able to get the ingress to automatically obtain a cert in the case of a cluster-issuer.

Hamidhasan commented 1 year ago

OK, while convoluted (and I think you alluded to this in your original comment), you can get around this limitation by specifying the cluster-issuer in the issuer annotation. This works, but note the commented line.

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    cert-manager.io/issuer: pca-cluster-issuer-rsa # bit weird to specify issuer instead of cluster-issuer here
    cert-manager.io/issuer-kind: AWSPCAClusterIssuer
    cert-manager.io/issuer-group: awspca.cert-manager.io
  name: my-ingress-issuer-test
  namespace: aws-privateca-issuer
spec:
  rules:
  - host: example.com
    http:
      paths:
      - pathType: Prefix
        path: /
        backend:
          service:
            name: myservice
            port:
              number: 80
  tls:
  - hosts:
    - example.com
    secretName: myingress-issuer-cert
Hamidhasan commented 1 year ago

One question to you @brsolomon-deloitte is: while this is not ideal, does the potential workaround of specifying the cluster issuer in the issuer annotation actually work for you?

Put another way: if this was fixed, does it unblock/enable any more functionality for your use cases, or can you use the workaround to do everything you need?

brsolomon-deloitte commented 1 year ago

while this is not ideal, does the potential workaround of specifying the cluster issuer in the issuer annotation actually work for you?

Yes, we realize that using issuer to point to a AWSPCAClusterIssuer does work, when combined with issuer-kind. However, from our experience, it seems buggy and confusing from a usability perspective, and doesn't seem to match with the semantics/definitions of issuer versus cluster-issuer, since AWSPCAClusterIssuer arguably best fits with cluster-issuer. At the very least, it seems like this behavior should be documented, either by aws-privateca-issuer or cert-manager.

Either way, thanks for laying out the details and giving attention to this issue.

irbekrm commented 1 year ago

PR to cert-manager docs to clarify the behaviour https://github.com/cert-manager/website/pull/1203

cert-manager.io/cluster-issuer annotation is just a shortcut to for cert-manager.io ClusterIssuer and cert-manager.io/issuer is not intended for namespace scoped issuers only- it defaults to cert-manager.io Issuer, however with cert-manager.io/issuer-kind, cert-manager.io/issuer-group specified will refer to an issuer (cluster or namespace scoped) of that kind and group.

Do let us know if the updated docs make sense

brsolomon-deloitte commented 1 year ago

Do let us know if the updated docs make sense

Looks alright to me.

divyansh-gupta commented 1 year ago

Thanks for creating this issue @brsolomon-deloitte and thank you @irbekrm for the help in fixing the documentation! I am closing this issue now.