gardener / cert-management Goto Github PK

What would you like to be added:
Currently Issuer resources are only watched on the default cluster in the configured "issuer" namespace.
It should be possible to specify Issuer resources on the source cluster additionally, i.e. on the same cluster, certificates are watched and handled.

Why is this needed:
In the Gardener context, you can add issuer for the shoot-cert-service via the shoot manifest, i.e. as configuration of the shoot-cert-service extension (see Request X.509 Certificates, chapter "Issuers" ).
But this needs write permissions to the Gardener project, which is not always feasible from a shoot cluster.
If the Issuer can be created directly on the shoot cluster, RBAC permissions are only needed on the shoot cluster itself. The issuer can then either be watched directly or be replicated to the control plane and handled like any other issuer there.
This would simplify use cases like using kyma-runtime with Istio.

Note: This is related to Add source controller for replicating DNSProviders

A very good new feature of kubectl is the kubectl wait.... options to wait until a resource has a dedicate condition.
e.g. kubectl wait pod --for condition=ready -n mynamespace --all . You wait until all pods in the named namespace are ready.

Adding some conditions to the status.condition fields to the certificate resource enables kubectl to use the same pattern for certificates. So we can easy wait until a cert is issued without looping and check if the secreted is available. Makes CI/CD and testing more stable.

example pod state.condition fields. You can wait on each of the status.condition attributes to get "True"

status:
  conditions:
  - lastProbeTime: null
    lastTransitionTime: "2021-08-18T14:21:04Z"
    status: "True"
    type: Initialized
  - lastProbeTime: null
    lastTransitionTime: "2021-08-18T14:21:15Z"
    status: "True"
    type: Ready
  - lastProbeTime: null
    lastTransitionTime: "2021-08-18T14:21:15Z"
    status: "True"
    type: ContainersReady
  - lastProbeTime: null
    lastTransitionTime: "2021-08-18T14:21:04Z"
    status: "True"
    type: PodScheduled

What would you like to be added:
The cert-management controller should be able to create DNSRecords (ref) instead of DNSEntry resources, to fulfill the acme challenge.

Why is this needed:
In the scope of gardener/gardener#9635, gardener-operator will be able to create Certificates for the Garden cluster and use registered provider extensions for creating required TXT challenge entries.

What would you like to be added:
Source resources like ingresses and services should support an optional annotation cert.gardener.cloud/commonname

Why is this needed:
It is always required to specify a CN with 64 characters at max, when requesting a certificate from Let’s encrypt. To use longer names, additional alternative names can be used.
For services the annotation dns.gardener.cloud/dnsnames is reused to specify the certificate domain names. The first name is used for the CN. But this annotation is also used to produce follower DNS entries. This can cause problems if there are multiple services using the same short domain name, as then multiple follower DNS entries with the same dnsname are generated.
To avoid this problem, it should be possible to specify the CN independently with an own annotation.

What happened:
The generated secret is of type: Opaque and that cannot be changed because the field is immutable.
In my contour ingress I see error messages:

level=error msg="unresolved secret reference" context=IngressProcessor error="Secret not found" name=github-actions-runner-self-service namespace=github-actions secret=github-actions/ingress-tls

In another cluster the same setup is working. Only difference I was able to find is the secret type: kubernetes.io/tls field.
According to https://projectcontour.io/docs/main/config/tls-termination/ the type should be kubernetes.io/tls

What you expected to happen:

My ingress secret should be generated with the correct type kubernetes.io/tls.

How to reproduce it (as minimally and precisely as possible):
N/A

Anything else we need to know:

• Related: #8
• you are awesome! 🙌

Environment:

• develop/eval

What would you like to be added:
Make cert-management aware of Istio gateway resource to simplify certificate management via annotations.

apiVersion: networking.istio.io/v1beta1
kind: Gateway
metadata:
  name: my-gateway
  annotations:
    cert.gardener.cloud/purpose: managed
spec:
...
  servers:
  - port:
...
      protocol: HTTPS
    hosts:
    - app1.example.com
    - app2.example.com
    tls:
      mode: SIMPLE # enables HTTPS on this port
      credentialName: tls-credential # must be the same as secret

see https://istio.io/docs/reference/config/networking/gateway/ and https://istio.io/docs/tasks/traffic-management/ingress/secure-ingress-sds/

Why is this needed:
Istio has graduated its API to beta and also become very easy to install with the istioctl utility:

istioctl manifest apply --set profile=default \
--set values.gateways.istio-ingressgateway.sds.enabled=true 

For now, many users will probably use the backward compatible path with Kubernetes Ingress support:

# to enable ingress gateway with Kubernetes Ingress support, SDS.
istioctl manifest apply \
  --set values.gateways.istio-ingressgateway.sds.enabled=true \
  --set values.global.k8sIngress.enabled=true \
  --set values.global.k8sIngress.enableHttps=true \
  --set values.global.k8sIngress.gatewayName=ingressgateway

But many users will start using Istio's advanced request routing capabilities and then this feature will make it comfortable.

What happened:
We are using gardener/cert-management as a dependency in our operator project to use the types API's provided by cert-management. A security vulnerability was reported during scans of our repository which point to CVE-2022-1996 which is a dependency of cert-management.

What you expected to happen:
We expect that the security vulnerability is addressed by updating the dependency emicklei/go-restful to 3.8.0 (or higher).

How to reproduce it (as minimally and precisely as possible):
By running security scans (for example using WhiteSource/MEND) on the repository code.

Anything else we need to know:
See details of vulnerability and versions at CVE-2022-1996

Environment:
All environments.

Normally the CertManager writes the events of the certificate issuing status into the resource.
You can inspect this pretty well in the k8s Ingress resource.

This is missing in the cert.gardener.cloud/v1alpha1/Certificate resource. After issuing the certificate the
event part is empty

Status:
  Common Name:  controlling.klacks.shoot.canary.k8s-hana.ondemand.com
  Dns Names:
    *.zone.controlling.klacks.shoot.canary.k8s-hana.ondemand.com
  Expiration Date:  2021-06-01T14:52:54Z
  Issuer Ref:
    Name:               garden
    Namespace:          shoot--klacks--controlling
  Observed Generation:  1
  State:                Ready
Events:                 <none>

The event entries helps a lot to debug the complete issuing process. Please add this to the certificate as well.

What would you like to be added:
The Gardener cert manager has the ability to name the tls secret for a managed cert with the annotation:
cert.gardener.cloud/secretname: mycoolsecret-tls
This capability however is not mentioned in the documentation. More precisely it is mentioned but in a different section. My suggestion would be to add this annotation to the first example-yaml in section Requesting a Certificate for Ingress / Process / Create the Ingress Resource (optional) as well as in the example echoheaders

Why is this needed:
The reason I would like to see this added is that probably many users will need it. Many users likely will start with deploying an nginx displaying some static content. This straight-forward example deploying a helm chart with nginx will not work without this. Unfortunately the nginx-helm-chart is very inflexible with respect to this setting and has a hard coded syntax for the tls-secret:

  tls:
      secretName: {{ printf "%s-tls" .Values.ingress.hostname }}

Gardener is so nicely able to make this easy. But here you will likely fail if the secret naming feature is not mentioned.

One more remark:
I also suggest to restructure this documentation page to become more beginner-friendly. The simple case with "let Gardener manage DNS and cert and I do not need to do anything except adding a couple of annotations" is hard to extract from this wealth of information. Why not start with this simple case and add the more sophisticated stuff below? It took me hours to figure out how exactly the annotations have to be even that I new it is possible. (just my personal opinion)

What happened:
When an error occurs during the cert creation (e.g. hostname > 64 chars) there an event of type normal fired.

What you expected to happen:
I would expect that in an error case the event type should be warning.

How to reproduce it (as minimally and precisely as possible):
Create an ingress with a hostname longer than 64 chars and list the events of the corresponding ingress.

What would you like to be added:
We can enhance the stability by removing resource limit scaling for the cert-controller-manager pod. In practice, that is typically achieved by:

• If the pod is critical to Gardener operation, remove the existing resource limits and set the ControlledValues in the respective VPA to RequestsOnly (in Spec > Resource Policy > Container Policies > Controlled Values).
• If the pod is not critical to Gardener operation, set an appropriate large fixed value for memory limit and set the ControlledValues in the respective VPA to RequestsOnly.

Why is this needed:
The component was seen stuck in an OOMkill loop in the field. Fundamentally that's due to a bug in upstream VPA, but the pragmatic approach is to remove reliance on limit scaling.

What would you like to be added:
It'd be helpful to have metrics about certificates which are about to expire (configurable). Cert controller tries to renew certificates automatically but this might fail for various and non-recoverable reasons.

Why is this needed:
Respective metrics will allow to setup alerts to warn operators about potential issues.

What happened:
After a certificate had been obtained successfully from Let's Encrypt, the data section in the referenced Secret was deleted. The Certificate object turned into a persisting error state, without trying to obtain the certificate again.

What you expected to happen:
Cert-Management controller to obtain the certificate and save it in the Secret again.

How to reproduce it (as minimally and precisely as possible):

1. Create Certificate
2. Wait for issuance, i.e. until Certificate is Ready
3. Delete .data. from referenced Secret.
4. Wait until Secret has .status.state: Error
5. Error state is irrecoverable

/kind bug

What happened:

E0913 12:56:23.275332       1 runtime.go:78] Observed a panic: "integer divide by zero" (runtime error: integer divide by zero)

See the full stacktrace in cert-controller-manager-5cb8c4b94f-rl2zm.log

What you expected to happen:

No such panic.

How to reproduce it (as minimally and precisely as possible):

Not clear for now.

Environment:
cert-manager version: v0.8.4

If you have an ingress like

spec:
  tls:
    - hosts:
      - base.mydomain.com
      - '*.base.mydomain.com'

two DNS challenges for the same domain base.mydomain.com are triggered. Currently each DNS challenge creates a separate DNSEntry, but in this case there is a clash resulting in the error
"error presenting token: creating DNSEntry xxx failed with dnsentries.dns.gardener.cloud "base.mydomain.com" is invalid: metadata.resourceVersion: Invalid value: 0x0: must be specified for an update.
The correct solution is probably to present one DNSEntry with multiple tokens.

What would you like to be added:
It'd be helpful to have proper validation for the Kinds Certificate and Issuer.
See https://github.com/gardener/cert-management/blob/master/examples/10-crds.yaml

Why is this needed:
It prevents wrong specs from being created in the first place.

Right now the generated secret contains two keys - tls.key and tls.cert.

To use this secret with Istio and MUTUAL TLS we must patch the secret and add the ca.crt attribute with the allowed root certificates for the mutual TLS. Unfortunately the ca.crt part get lost if the cert-manager updates the certificate for any reason.

Istio supports three kind of secret formats which helps a lot to decouble the CA certificate for mutual TLS and the HTTPS certificate. Maybe you can add a flag to the gardener certificate to indicate which format the cert-manager should generate.

Documentation from Istio
(at the bottom on this page https://istio.io/latest/docs/tasks/traffic-management/ingress/secure-ingress/)

Istio supports reading a few different Secret formats, to support integration with various tools such as cert-manager:

• A TLS Secret with keys tls.key and tls.crt, as described above. For mutual TLS, a ca.crt key can be used.
• A generic Secret with keys key and cert. For mutual TLS, a cacert key can be used.
• A generic Secret with keys key and cert. For mutual TLS, a separate generic Secret named <secret>-cacert, with a cacert key. For example, httpbin-credential has key and cert, and httpbin-credential-cacert has cacert.

The third part is the interesting part to decouple cert-manager updates and mutual TLS.

What would you like to be added:

I would need a feature like https://cert-manager.io/docs/concepts/ca-injector/ so that resources like MutatingWebhookConfiguration get the caBundle injected by a certificate or tls secret. Is something available or planned with gardener cert-management?

Why is this needed:

a very popular gitops-promotion-project kargo uses this feature in its webhooks: https://github.com/akuity/kargo/blob/93940f957ec5eda563f8b163a3fc3d78474e3120/charts/kargo/templates/webhooks/webhooks.yaml#L10C5-L10C35

The following error message is encountered when trying to use Google CloudDNS and the cert-management controller for a SAN certificate.

Error: obtaining certificate failed: error: one or more domains had a problem: [abc5.evsapfgl.com] time limit exceeded: last error: %!w()

CommonName: fgdvgceu0014.evsapfgl.com
DNSNames:

• abc5.evsapfgl.com
• abc6.evsapfgl.com

This error does not occur when using a single CommonName and removing DNSNames from the configuration. This is not an option as we have multiple SANs for our customers that need to be applied. Wildcard is not acceptable based on customer need in our use case.

If possible, can a configurable Recheck Interval and frequency be added to allow for both shorter and longer DNS propagation times?

What would you like to be added:

Gardener Cert-Manager Issuer should be able to create self-signed certificates.

Why is this needed:

We use the project Kargo and there in https://github.com/akuity/kargo/blob/main/charts/kargo/templates/webhooks-server/cert.yaml
a certificate for dnsName "kargo-webhooks-server.{{ .Release.Namespace }}.svc" is created, to enable a TLS connection between the Kubernetes-API server and this admissioncontroller webhook.

The Project normally creates its own issuer to create a selfSigned certificate: https://github.com/akuity/kargo/blob/main/charts/kargo/templates/common/cert-issuer.yaml

Look at .spec.selfSigned

So we do not want to install cert-manager but use the gardener cert-manager to create a certificate. However, I didn't find a way to create self-signed certificates the way the cert-manager does.

Maybe someone can explain how to achieve this with the current features of gardener cert-management?

What would you like to be added:
Some of the APIs have a more stable version and it is good practice to use it. For example, (Custer)Role(Binding)s are using rbac.authorization.k8s.io/v1beta1 while there is rbac.authorization.k8s.io/v1.

Ref: https://github.com/gardener/cert-management/blob/master/charts/cert-management/templates/clusterrole.yaml#L2

Why is this needed:
To not use deprecated versions, v1beta1 will be disabled starting from k8s v1.20 kubernetes/kubernetes#84758

What would you like to be added:
Enhance the cert-manager metrics to get more information of the certificate, such as the certificate expiration date.

Why is this needed:
There are several metrics provided: https://github.com/gardener/cert-management#metrics.
If want to monitor the certificate expiration information, only "cert_management_overdue_renewal_certificates" is related, it shows "Number of certificate objects with certificate's renewal overdue". But the metrics is only returned the number of overdue certificate instead of the expiration date, we can monitor the expiration date to know when the certificates will be expired.

If an ingress manifest does not specify a TLS host, the cert-controller-manager crashes with panic: runtime error: index out of range [0] with length 0.

Here an example of a wrong/correct ingress manifest:

wrong (without spec.tls.hosts)

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: echoheaders
  namespace: default
  annotations:
    cert.gardener.cloud/purpose: managed
spec:
  tls:
    - secretName: cert-echoheaders
  rules:
    - host: echoheaders.demo.mydomain.com
      http:
        paths:
          - backend:
              serviceName: echoheaders
              servicePort: 80
            path: /

correct (with spec.tls.hosts)

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: echoheaders
  namespace: default
  annotations:
    cert.gardener.cloud/purpose: managed
spec:
  tls:
    - hosts:
        - echoheaders.demo.mydomain.com
      secretName: cert-echoheaders
  rules:
    - host: echoheaders.demo.mydomain.com
      http:
        paths:
          - backend:
              serviceName: echoheaders
              servicePort: 80
            path: /

In the documentation the resource networking.k8s.io/v1beta1is used, which is dropped and not valid anymore.
Please adapt the documentation to a valid resource version. e.g. networking.k8s.io/v1

Docu PAge: https://github.com/gardener/cert-management#requesting-a-certificate-for-ingress

What happened:
As an side-effect of the PR #137 which includes an update of the github.com/go-acme/lego/v4from version v4.8.0 to v4.14.0, the logic has been changed in this library to automatically use FollowCNAME.

What you expected to happen:
FollowCNAME should be disabled by default.

How to reproduce it (as minimally and precisely as possible):

Anything else we need to know:

Environment:

Hi,

I try to use gardener certificate service to request the let's encrypt certificate on AliCloud.

Enable gardener certificate in shoot cluster :

  dns:
    domain: hana.inumber-haas-hc-dev.dev.the.use.subzone
    providers:
      - primary: true
        secretName: hc-dev
        type: alicloud-dns
  extensions:
    - type: shoot-dns-service
    - type: shoot-cert-service
      providerConfig:
        apiVersion: service.cert.extensions.gardener.cloud/v1alpha1
        issuers:
          - email: my.email@address
            name: christina-issuer
            server: 'https://acme-v02.api.letsencrypt.org/directory'
        kind: CertConfig

Request a certificate via Certificate and the file yaml as:

apiVersion: cert.gardener.cloud/v1alpha1
kind: Certificate
metadata:
  name: cert-custom
  namespace: default
spec:
  commonName: the.use.subzone
  dnsNames:
  - '*.ingress.hana.inumber-haas-hc-dev.dev.the.use.subzone'
  secretname: cert-custom
  issuerRef:
    name: christina-issuer

Dns provider like this:

kubectl get dnspr
NAME                    TYPE           STATUS   AGE
alicloud-dns-provider   alicloud-dns   Ready    52m

Status:
  Domains:
    Included:
      dev.the.use.subzone
  Message:              provider operational
  Observed Generation:  1
  State:                Ready
  Zones:
    Excluded:
      e8412dfdb88d4a6e9cce562f6c2ceac1

But the Certificate status :

kubectl get cert
NAME           COMMON NAME                                                     ISSUER             STATUS    AGE
cert-custom   the.use.subzone                                                  christina-issuer   Pending   49m

pending with log

Status:
Common Name:  the.use.subzone
Dns Names:
  *.ingress.hana.inumber-haas-hc-dev.dev.the.use.subzone
Issuer Ref:
  Name:                  christina-issuer
  Namespace:             shoot--hc-dev--inumber-haas
Last Pending Timestamp:  2020-07-07T07:36:12Z
Message:                 certificate requested, preparing/waiting for successful DNS01 challenge
Observed Generation:     1
State:                   Pending
Events:
Type     Reason  Age                From                     Message
----     ------  ----               ----                     -------
Warning  sync    32m (x7 over 32m)  cert-controller-manager  waiting for end of pending rate limiting in 23 seconds

I don't know why the status is pending, Could you please help us to debug the issue ? Thanks a lot!

What happened:
For shoot side issuers and invalid certificates, the status of a certificate assigns a different issuer than given in the spec.
More details must be evaluated by reproducing the issue.

What you expected to happen:

How to reproduce it (as minimally and precisely as possible):

Anything else we need to know:

Environment:

We use gardener cert extension service to work together with nginx ingress to automatically create certificate and renew certificates.
And also we heard that currently within gardener's certificate manager, we can only use DNS based challenges, but this conflicts with our akamai DNS challenges.

For akamai, in order to validate certificate domain, we need to create cname DNS records using gardener dns extension service, but in recent issue, we found that ingress failed to create certificate due to below error:

  Warning  cert-annotation  11m                cert-controller-manager   eu10-stage1.dws.workzone.ondemand.com: errornous certificate: obtaining certificate failed: error: one or more domains had a problem: [assets0-eu10-stage1.dws.workzone.ondemand.com] propagation: time limit exceeded [assets1-eu10-stage1.dws.workzone.ondemand.com] propagation: time limit exceeded [assets2-eu10-stage1.dws.workzone.ondemand.com] propagation: time limit exceeded [assets3-eu10-stage1.dws.workzone.ondemand.com] propagation: time limit exceeded [cdn-eu10-stage1.dws.workzone.ondemand.com] propagation: time limit exceeded . Details: DNS TXT record '_acme-challenge.cdn-eu10-stage1.dws.workzone.ondemand.com' is not visible on public (or precheck) name servers. Failed check: DNS entry getting ready

  Warning  cert-annotation  5m26s              cert-controller-manager   eu10-stage1.dws.workzone.ondemand.com: errornous certificate: obtaining certificate failed: error: one or more domains had a problem: [assets0-eu10-stage1.dws.workzone.ondemand.com] propagation: time limit exceeded [assets1-eu10-stage1.dws.workzone.ondemand.com] propagation: time limit exceeded [assets2-eu10-stage1.dws.workzone.ondemand.com] propagation: time limit exceeded [assets3-eu10-stage1.dws.workzone.ondemand.com] propagation: time limit exceeded [cdn-eu10-stage1.dws.workzone.ondemand.com] propagation: time limit exceeded . Details: DNS TXT record '_acme-challenge.assets2-eu10-stage1.dws.workzone.ondemand.com' is not visible on public (or precheck) name servers. Failed check: DNS entry getting ready

  Normal   cert-annotation  86s (x3 over 15m)  cert-controller-manager   eu10-stage1.dws.workzone.ondemand.com: certificate pending: certificate requested, preparing/waiting for successful DNS01 challenge

This issue is caused by the two DNS challenges conflicts with each other, as gardener's cert manager is trying to set and read the same entry as our cdn validator.

Since in akamai, we use secure by default certificate for our hosts, and it only supports cname DNS validation, there is no other options to validate certificate domain, is it possible that Gardener's certificate manager can also support HTTP based challenges?

thanks

Jane

What would you like to be added:
We've been observing an immense amount of memory usage if the source cluster contains many secrets.

Example:

Secret count (not referenced by certificates): ~ 3200
Volume per Secret: ~ 1 MB
Memory needed for Cert-Management component: ~ 6.7 GB

Please check if the memory footprint can be reduced in such scenarios.

Suggestions:

• Use informer with tweaked list options, e.g. only consider special labelled secerts
• Use Metadata only informer (ref kubernetes-sigs/controller-runtime#1174)
• Bypass informer cache if possible

If the same domain is requested by multiple certificate requests at the same time, multiple DNS challenges for the same domain are started concurrently, but only one of them can successfully create a DNSEntry. All other requests will fail and are retried later. As multiple challenge tokens have been created, the token in the DNSEntry may not reflect the latest and therefore the challenge may fail. As a result all requests fail with error and may not recover on the next try, as the same conflicts will happen again.

What happened:
Cert-Management controller fails to update the status of a certificate to Ready.

time="2020-06-05T07:38:06Z" level=warning msg="updating status failed with: Certificate.cert.gardener.cloud \"test\" is invalid: status.lastPendingTimestamp: Invalid value: \"null\": status.lastPendingTimestamp in body must be of type string: \"null\""

What you expected to happen:
Certificate status is set to Ready.

How to reproduce it (as minimally and precisely as possible):

1. Create a certificate and wait until it was issues successfully.
2. Delete certificate resource, don't touch the referenced secret.
3. Create certificate from (1.).
4. Observe that status is not updated and above message appears in log.

Environment:
v0.2.15

The certificate secret is not copied to the namespace of the second ingress if an identical certificate is used by two ingresses in two different namespaces.

cert-controller-manager writes certificate secrets with type Opaque.
If the secret is already existing and has type kubernetes.io/tls, updating the secret fails,
as the type field is immutable.
This can happen, e.g. if an ingress was managed with cert-broker/cert-manager and is migrated to cert-management

What happened:
If two instances of cert-management are working on the same source cluster but responsibility for different cert-class annotations, they delete the secrets belonging to certificates they are not responsible for.

What you expected to happen:
A source controller should ignore secrets of certificates, if it is not responsible, i.e. if it is foreign.