Page MenuHomePhabricator
Create Task
Maniphest T298976

Integrate cert-manager/issuer in ml-serve clusters
Closed, ResolvedPublic
Actions

Description

We are currently using TLS certificates issued by the Puppet CA and created via cergen. They are deployed to the deploy1002 node via puppet, that creates ad-hoc helmfile private configs that we can load in our helm charts.

The TLS certs that we created are:

  1. inference.discovery.wmnet, deployed on the istio gateway pods (in the istio-system namespace) via the knative helm chart (since it handles all L7 configs for istio).
  2. Webhooks certificates for various pods
  3. TLS certificate for the Istio Egress gateway

The SRE team is integrating cert-manager/issuer in their clusters: https://wikitech.wikimedia.org/wiki/Kubernetes/cert-manager

We could do the following:

  1. Use the same config to create inference.discovery.wmnet from the Discovery intermediate PKI (see documentation).
  2. Create a new Intermediate PKI to handle our certificates for webhooks and egress gateway. See https://wikitech.wikimedia.org/wiki/PKI/CA_Operations#Intermediate_Certificates

This would free us from the Puppet CA completely, and open the door to mTLS between pods.

Details

Related Changes in Gerrit:
Show related patches Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 ResolvedNoneT272917 Lift Wing proof of concept
 ResolvedelukeyT298976 Integrate cert-manager/issuer in ml-serve clusters

Event Timeline

elukey created this task.Jan 11 2022, 3:21 PM
elukey mentioned this in T289835: Create a LB service for inference.discovery.wmnet.Jan 11 2022, 3:23 PM
elukey added a comment.Jan 18 2022, 10:38 AM

I had a chat with Janis today, and IIUC for inference.discovery.wmnet we should:

  1. Create a new signing profile like https://gerrit.wikimedia.org/r/c/operations/puppet/+/745496 (and set the related cfssl-issuer-values.yaml file in deployment-charts)
  2. Set install_cert_manager: true in deployment-charts
  3. Follow https://wikitech.wikimedia.org/wiki/Kubernetes/cert-manager#Usage and add a CRD like the one stated in the docs. We can add one in the knative-serving chart, where we define the Istio gateways etc..
  4. Force Istio Gateway to use the new certificate/secret.

In this way we'll use the discovery PKI CA instead of the puppet one (the current cert for inference.d.wmnet we'll need to be revoked etc.. as clean up step).

A similar/different procedure may be needed for the other certs that we deployed (egress gw, webhooks, etc..) but we'll also likely need another dedicated intermediate CA for us.

gerritbot added a comment.Jan 18 2022, 11:00 AM

Change 754885 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/puppet@production] role::pki::multirootca: add dedicated profile for ml-serve k8s

https://gerrit.wikimedia.org/r/754885

gerritbot added a project: Patch-For-Review.Jan 18 2022, 11:00 AM
gerritbot added a comment.Jan 18 2022, 11:16 AM

Change 754890 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] helmfile.d: deploy cert-manager for ml-serve nodes

https://gerrit.wikimedia.org/r/754890

gerritbot added a comment.Jan 18 2022, 3:29 PM

Change 754885 merged by Elukey:

[operations/puppet@production] role::pki::multirootca: add dedicated profile for ml-serve k8s

https://gerrit.wikimedia.org/r/754885

gerritbot added a comment.Jan 18 2022, 3:35 PM

Change 754890 merged by Elukey:

[operations/deployment-charts@master] helmfile.d: deploy cert-manager for ml-serve nodes

https://gerrit.wikimedia.org/r/754890

Maintenance_bot removed a project: Patch-For-Review.Jan 18 2022, 4:10 PM
gerritbot added a comment.Jan 18 2022, 4:12 PM

Change 754981 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] helmfile.d: add 'cert-manager' namespace to ml-serve

https://gerrit.wikimedia.org/r/754981

gerritbot added a project: Patch-For-Review.Jan 18 2022, 4:12 PM
gerritbot added a comment.Jan 19 2022, 7:22 AM

Change 755259 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/puppet@production] role::pki::multirootca: add expiry for k8s_mlserve

https://gerrit.wikimedia.org/r/755259

gerritbot added a comment.Jan 19 2022, 7:26 AM

Change 755259 merged by Elukey:

[operations/puppet@production] role::pki::multirootca: add expiry for k8s_mlserve

https://gerrit.wikimedia.org/r/755259

gerritbot added a comment.Jan 19 2022, 7:49 AM

Change 754981 merged by Elukey:

[operations/deployment-charts@master] helmfile.d: add 'cert-manager' namespace to ml-serve

https://gerrit.wikimedia.org/r/754981

Maintenance_bot removed a project: Patch-For-Review.Jan 19 2022, 8:10 AM
elukey added a comment.Jan 19 2022, 8:12 AM
NAMESPACE                   NAME                                                              READY   STATUS    RESTARTS   AGE
cert-manager                cert-manager-5f788b6dbb-sxz28                                     1/1     Running   0          15m
cert-manager                cert-manager-cainjector-6b48b95c9-c9sbg                           1/1     Running   0          15m
cert-manager                cert-manager-cainjector-6b48b95c9-q875f                           1/1     Running   0          15m
cert-manager                cert-manager-webhook-7496cbfb78-jdjhn                             1/1     Running   0          15m
cert-manager                cert-manager-webhook-7496cbfb78-z4gmw                             1/1     Running   0          15m
cert-manager                cfssl-issuer-8567cb7675-kkj8x                                     1/1     Running   0          67s
cert-manager                cfssl-issuer-8567cb7675-phl57                                     1/1     Running   0          67s
gerritbot added a comment.Jan 19 2022, 10:38 AM

Change 755333 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] Expand helmfile_namespace_certs to support the ml-serve use case

https://gerrit.wikimedia.org/r/755333

gerritbot added a project: Patch-For-Review.Jan 19 2022, 10:38 AM
gerritbot added a comment.Jan 19 2022, 10:45 AM

Change 755334 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] Add cert-manager settings for the ml-serve clusters

https://gerrit.wikimedia.org/r/755334

gerritbot added a comment.Jan 19 2022, 3:50 PM

Change 755333 merged by Elukey:

[operations/deployment-charts@master] Expand helmfile_namespace_certs to support the ml-serve use case

https://gerrit.wikimedia.org/r/755333

gerritbot added a comment.Jan 19 2022, 3:50 PM

Change 755334 merged by Elukey:

[operations/deployment-charts@master] Add cert-manager settings for the ml-serve clusters

https://gerrit.wikimedia.org/r/755334

Maintenance_bot removed a project: Patch-For-Review.Jan 19 2022, 4:10 PM
gerritbot added a comment.Jan 19 2022, 4:10 PM

Change 755431 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] admin_ng: update tls secret settings for ml-serve

https://gerrit.wikimedia.org/r/755431

gerritbot added a project: Patch-For-Review.Jan 19 2022, 4:10 PM
gerritbot added a comment.Jan 19 2022, 4:30 PM

Change 755431 merged by Elukey:

[operations/deployment-charts@master] admin_ng: update knative's tls secret settings for ml-serve

https://gerrit.wikimedia.org/r/755431

gerritbot added a comment.Jan 19 2022, 4:39 PM

Change 755441 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] admin_ng: remove the secrets chart from knative-serving

https://gerrit.wikimedia.org/r/755441

elukey added a project: Machine-Learning-Team.Jan 19 2022, 6:16 PM
elukey claimed this task.Jan 19 2022, 6:30 PM
elukey moved this task from Unsorted to Active Tasks on the Machine-Learning-Team board.
elukey edited projects, added Machine-Learning-Team (Active Tasks); removed Machine-Learning-Team.
elukey moved this task from Parked to In Progress on the Machine-Learning-Team (Active Tasks) board.
gerritbot added a comment.Jan 20 2022, 8:40 AM

Change 755640 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] helmfile.d: remove secrets chart from knative-serving

https://gerrit.wikimedia.org/r/755640

gerritbot added a comment.Jan 20 2022, 8:44 AM

Change 755640 merged by Elukey:

[operations/deployment-charts@master] helmfile.d: remove secrets chart from knative-serving

https://gerrit.wikimedia.org/r/755640

gerritbot added a comment.Jan 20 2022, 8:56 AM

Change 755441 merged by Elukey:

[operations/deployment-charts@master] admin_ng: remove the secrets chart from knative-serving

https://gerrit.wikimedia.org/r/755441

elukey added a comment.Jan 20 2022, 9:17 AM

Cleaned up all config and certs related to inference.discovery.wmnet (on the puppet private repo and on k8s secrets etc..). The Puppet-based cert has also been revoked.

elukey added a comment.Jan 20 2022, 10:08 AM

The remaining puppet-based certificates are:

  • kserve-webhook-server-service.kserve.svc.cluster.local
  • istio-egressgateway.istio-system.svc.cluster.local

Plus of course if we'll enable mTLS in istio in the future, all the ones related to InferenceService etc..

gerritbot added a comment.Jan 20 2022, 10:19 AM

Change 755651 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/puppet@production] role::pki::root: add the ml_serve intermediate PKI

https://gerrit.wikimedia.org/r/755651

elukey added a comment.Jan 20 2022, 5:29 PM

After a chat with John in https://gerrit.wikimedia.org/r/755651 it seems that for our use case we can use the discovery intermediate (without creating a new one).

Next steps: move kserve-webhook-server-service.kserve.svc.cluster.local and istio-egressgateway.istio-system.svc.cluster.local to the discovery intermediate CA.

gerritbot added a comment.Jan 21 2022, 9:32 AM

Change 755651 abandoned by Elukey:

[operations/puppet@production] role::pki::root: add the ml_serve intermediate PKI

Reason:

https://gerrit.wikimedia.org/r/755651

gerritbot added a comment.Jan 21 2022, 11:16 AM

Change 755955 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] kserve: move to cert-manager

https://gerrit.wikimedia.org/r/755955

gerritbot added a comment.Jan 21 2022, 11:26 AM

Change 755955 merged by Elukey:

[operations/deployment-charts@master] kserve: move to cert-manager

https://gerrit.wikimedia.org/r/755955

gerritbot added a comment.Jan 21 2022, 12:39 PM

Change 755970 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] knative-serving: move egress gateway to cert-manager

https://gerrit.wikimedia.org/r/755970

gerritbot added a comment.Jan 21 2022, 2:38 PM

Change 755970 merged by Elukey:

[operations/deployment-charts@master] knative-serving: move egress gateway to cert-manager

https://gerrit.wikimedia.org/r/755970

gerritbot added a comment.Jan 21 2022, 2:45 PM

Change 755984 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] Move ml-services to the new CA cert bundle

https://gerrit.wikimedia.org/r/755984

gerritbot added a comment.Jan 21 2022, 2:49 PM

Change 755984 merged by Elukey:

[operations/deployment-charts@master] Move ml-services to the new CA cert bundle

https://gerrit.wikimedia.org/r/755984

elukey added a comment.Jan 21 2022, 6:21 PM

It sems that the kserve webhook doesn't work properly with the new settings, this is what I get when trying to deploy/modify an InferenceService:

Error: UPGRADE FAILED: an error occurred while rolling back the release. original upgrade error: cannot patch "enwiki-goodfaith" with kind InferenceService: Internal error occurred: failed calling webhook "inferenceservice.kserve-webhook-server.defaulter": Post https://kserve-webhook-server-service.kserve.svc:443/mutate-serving-kserve-io-v1beta1-inferenceservice?timeout=10s: x509: certificate signed by unknown authority && cannot patch "enwiki-damaging" with kind InferenceService: Internal error occurred: failed calling webhook "inferenceservice.kserve-webhook-server.defaulter": Post https://kserve-webhook-server-service.kserve.svc:443/mutate-serving-kserve-io-v1beta1-inferenceservice?timeout=10s: x509: certificate signed by unknown authority: no NetworkPolicy with the name "kserve-inference-main" found

From https://cert-manager.io/v1.2-docs/concepts/certificate/:

Additionally, if the Certificate Authority is known, the corresponding CA certificate will be stored in the secret with key ca.crt. For example, with the ACME issuer, the CA is not known and ca.crt will not exist in acme-crt-secret.

There is probably something missing our config, the caBundle field in the kserve webhook CRDs is not injected with the Discovery's CA crt value and the k8s api fails to validate TLS connections to the kserve webhook.

elukey added a comment.Jan 26 2022, 8:26 AM

This task is currently blocked by T299906

• ACraze mentioned this in T298989: Draftquality transformer.Jan 26 2022, 4:19 PM
gerritbot added a comment.Jan 26 2022, 7:03 PM

Change 757502 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] knative-serving: add more SANs to the Istio Egress gw's Certificate

https://gerrit.wikimedia.org/r/757502

gerritbot added a comment.Jan 26 2022, 7:14 PM

Change 757502 merged by Elukey:

[operations/deployment-charts@master] knative-serving: add more SANs to the Istio Egress gw's Certificate

https://gerrit.wikimedia.org/r/757502

elukey closed this task as Resolved.EditedJan 26 2022, 7:25 PM

Finally done!

The work in T299906 was enough to make all errors disappearing, the kserve webhook now works fine and we can deploy.

The egress tls certificate is also picked up and valid.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits