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.

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

Support using cert-manager for admission controller certs #1761

Open
yurrriq opened this issue May 31, 2024 · 7 comments · May be fixed by #1791
Open

Support using cert-manager for admission controller certs #1761

yurrriq opened this issue May 31, 2024 · 7 comments · May be fixed by #1791

Comments

@yurrriq
Copy link
Contributor

yurrriq commented May 31, 2024

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
Copy link
Contributor Author

yurrriq commented Jun 3, 2024

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
Copy link
Collaborator

airadier commented Jun 4, 2024

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
Copy link
Contributor

mavimo commented Jun 4, 2024

@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
Copy link
Contributor Author

yurrriq commented Jun 4, 2024

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

@jalaziz
Copy link

jalaziz commented Jun 6, 2024

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
Copy link
Contributor Author

yurrriq commented Jun 20, 2024

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
Copy link
Contributor Author

yurrriq commented Jun 26, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants