sysdiglabs / charts

The official source for Sysdig’s Helm charts
https://charts.sysdig.com
40 stars 125 forks source link

Support using cert-manager for admission controller certs #1761

Open yurrriq opened 2 months ago

yurrriq commented 2 months ago

It would be great if we could use cert-manager to provision the certs for the admission controller. KEDA supports this, for example: https://github.com/kedacore/charts/tree/main/keda/templates/cert-manager

I'll plan to prepare a PR for review.

yurrriq commented 2 months ago

The following should be a sufficient MWE for what I've hacked together to get around the absence of this feature...

helmfile.yaml:

repositories:
  - name: sysdig
    url: https://charts.sysdig.com

releases:
  - chart: sysdig/sysdig-deploy
    version: 1.55.3
    name: sysdig
    namespace: sysdig
    needs:
      - sysdig/sysdig-admissioncontroller-webhook-cert
    values:
      - admissionController:
          enabled: true
        # webhook:
        #   ssl:
        #     ca:
        #       existingCaSecret: sysdig-admissioncontroller-webhook-tls
    # NOTE: START HACKS
    jsonPatches:
      - target:
          group: admissionregistration.k8s.io
          kind: ValidatingWebhookConfiguration
          name: sysdig-admissioncontroller-webhook
          version: v1
        patch:
          - op: add
            path: /metadata/annotations
            value:
              cert-manager.io/inject-ca-from: sysdig/sysdig-admissioncontroller-webhook
          - op: remove
            path: /webhooks/0/clientConfig/caBundle
    strategicMergePatches:
      - $patch: delete
        apiVersion: v1
        kind: Secret
        metadata:
          name: sysdig-admissioncontroller-webhook-tls
          namespace: sysdig
    # NOTE: END HACKS
  - chart: sysdig-admissioncontroller-webhook-cert
    name: sysdig-admissioncontroller-webhook-cert
    namespace: sysdig

sysdig-admissioncontroller-webhook-cert/cert.yaml:

apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
  name: sysdig-admissioncontroller-webhook
  namespace: sysdig
spec:
  commonName: sysdig-admissioncontroller-webhook
  dnsNames:
  - sysdig-admissioncontroller-webhook.sysdig.svc
  issuerRef:
    group: cert-manager.io
    kind: ClusterIssuer
    name: selfsigning-issuer
  secretName: sysdig-admissioncontroller-webhook-tls

... where the following ClusterIssuer/selfsigning-issuer already exists.

apiVersion: cert-manager.io/v1
kind: ClusterIssuer
  name: selfsigning-issuer
spec:
  selfSigned: {}

... which results in the following ValidatingWebhookConfiguration/sysdig-admissioncontroller-webhook.

apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  annotations:
    cert-manager.io/inject-ca-from: sysdig/sysdig-admissioncontroller-webhook
  name: sysdig-admissioncontroller-webhook
  namespace: sysdig
webhooks:
- admissionReviewVersions:
  - v1
  - v1beta1
  clientConfig:
    service:
      name: sysdig-admissioncontroller-webhook
      namespace: sysdig
      path: /k8s-audit
      port: 443
  failurePolicy: Ignore
  matchPolicy: Equivalent
  name: audit.secure.sysdig.com
  rules:
  - apiGroups:
    - ""
    - apps
    - autoscaling
    - batch
    - networking.k8s.io
    - rbac.authorization.k8s.io
    - extensions
    apiVersions:
    - '*'
    operations:
    - '*'
    resources:
    - '*/*'
    scope: '*'
  sideEffects: None
  timeoutSeconds: 5
airadier commented 2 months ago

Thanks @yurrriq for this great contribution. We will take a look and wait for the PR to understand how we can officially include support for cert-manager for certificate provision of the AC webhook.

mavimo commented 2 months ago

@yurrriq thx!

We will look if we can make it as part of the official chart so you don't need to maintain your changes on top of it 😄

yurrriq commented 2 months ago

Sounds great, thank you! I'm following this issue so can test and verify once ready.

jalaziz commented 2 months ago

You can see https://github.com/aws/eks-charts/blob/master/stable/aws-load-balancer-controller/values.yaml#L116 for how the AWS Load Balancer Controller handles this exact issue.

yurrriq commented 2 months ago

Oops, sorry, I totally missed the "we'll wait for the PR". For some reason I though y'all were gonna implement this internally... I'm back to work tomorrow, so depending on priority conflicts I can probably whip something together in the next couple days.

yurrriq commented 2 months ago

I've created #1791 with the minimal set of features to support my use case for now.

github-actions[bot] commented 2 days ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days