Default TLS for BentoDeployment model servers

rauerhans commented 1 year ago

Closes https://github.com/bentoml/yatai-deployment/issues/87

This feature allows the yatai-deployment controller to add TLS config to the ingress of a Bento deployment through added parameters to the network ConfigMap.

Previously this was only possible by adding the ingress TLS config to the BentoDeployment CRD, which limited the usability of the UI. (Cert-manager dedicated ingress annotations could be added through the network ConfigMap, but the ingress TLS config with host and secretName had to be set manually in the CRD.)

Changes

Add ingress-tls-mode field to network ConfigMap
Add ingress-static-tls-secret field to network ConfigMap
Implement controller logic to add TLS to ingress depending which on ingress-tls-mode field

Setup can be validated using this image rauerhans/yatai-deployment:1.1.12 in the yatai-deployment deployment and playing by adding the new fields to the network ConfigMap.

Functionality

Depending on the new ingress-tls-mode field in the network ConfigMap, the controller will add the TLS config (hosts is reused, secretName is inferred, based on setting) to the ingress of the Bento deployment.

If the mode is set to none, the controller will not add any TLS config to the ingress, as previously.
If the mode is set to static, the controller will reference to a wildcard certificate to the ingress of the Bento deployment.
If the mode is set to auto, reuse the name of the BentoDeployment CRD for the name of the TLS secret. This is useful in a scenario where the user wants to use cert-manager to generate a certificate for the ingress of the Bento deployment, so in addition you would have the appropriate annotations in the network ConfigMap, see below.

Mode: None

As previously, only way to add TLS is through the field in the deployment CRD.

Mode: Static

The controller will create a wildcard certificate for each deployment.

Network Configmap for mode "static"

apiVersion: v1
kind: ConfigMap
metadata:
name: network
namespace: yatai
data:
domain-suffix: yatai.dev.plural.sh
ingress-annotations: ""
ingress-class: nginx
ingress-path: /
ingress-path-type: ImplementationSpecific
ingress-static-tls-secret: yatai-deployment-static-tls
ingress-tls-mode: static

One wildcard certificate. Needs to be created manually.

apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
name: yatai-deployment-static-tls
namespace: yatai
spec:
dnsNames:
- '*.yatai.dev.plural.sh'
issuerRef:
group: cert-manager.io
kind: ClusterIssuer
name: letsencrypt-prod
secretName: yatai-deployment-static-tls
usages:
- digital signature
- key encipherment

results in one secret to be used by all deployments

apiVersion: v1
kind: Secret
metadata:
name: yatai-deployment-static-tls
namespace: yatai
labels:
controller.cert-manager.io/fao: 'true'
type: kubernetes.io/tls
data:
tls.crt: xxx
tls.key: xxx

ingress of deployment A created by yatai-deployment controller

kind: Ingress
metadata:
name: dep-a
namespace: yatai
labels:
yatai.ai/bento-deployment: dep-a
yatai.ai/bento-deployment-component-type: api-server
yatai.ai/bento-deployment-target-type: production
yatai.ai/bento-repository: iris_classifier_pydantic_test
yatai.ai/bento-version: zavubixtgkiqotwt
yatai.ai/creator: yatai-deployment
annotations:
cert-manager.io/cluster-issuer: letsencrypt-prod
kubernetes.io/tls-acme: 'true'
yatai.ai/bento-repository: iris_classifier_pydantic_test
yatai.ai/bento-version: zavubixtgkiqotwt
spec:
ingressClassName: nginx
tls:
- hosts:
    - dep-a-yatai.yatai.dev.plural.sh
  secretName: yatai-deployment-static-tls
rules:
- host: dep-a-yatai.yatai.dev.plural.sh
  http:
    paths:
      - path: /
        pathType: ImplementationSpecific
        backend:
          service:
            name: dep-a
            port:
              name: http

ingress deployment B

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: dep-b
namespace: yatai
labels:
yatai.ai/bento-deployment: dep-b
yatai.ai/bento-deployment-component-type: api-server
yatai.ai/bento-deployment-target-type: production
yatai.ai/bento-repository: iris_classifier_pydantic_test
yatai.ai/bento-version: f26pbxhtfczdgtwt
yatai.ai/creator: yatai-deployment
annotations:
cert-manager.io/cluster-issuer: letsencrypt-prod
kubernetes.io/tls-acme: 'true'
nginx.ingress.kubernetes.io/ssl-redirect: 'false'
yatai.ai/bento-repository: iris_classifier_pydantic_test
yatai.ai/bento-version: f26pbxhtfczdgtwt
spec:
ingressClassName: nginx
tls:
- hosts:
    - dep-b-yatai.yatai.dev.plural.sh
  secretName: yatai-deployment-static-tls
rules:
- host: dep-b-yatai.yatai.dev.plural.sh
  http:
    paths:
      - path: /
        pathType: ImplementationSpecific
        backend:
          service:
            name: iris-classifier-pydantic-test
            port:
              name: http

Mode: Auto

Use default annotations to add TLS to ingress through cert-manager. Results in a concrete TLS secret for each Bento deployment.

Network Configmap for mode "auto"

apiVersion: v1
kind: ConfigMap
metadata:
name: network
namespace: yatai
data:
domain-suffix: yatai.dev.plural.sh
ingress-annotations: >-
{"cert-manager.io/cluster-issuer":"letsencrypt-prod","kubernetes.io/tls-acme":"true"}
ingress-class: nginx
ingress-path: /
ingress-path-type: ImplementationSpecific
ingress-static-tls-secret: ""
ingress-tls-mode: auto

To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1204609820998601

yetone commented 1 year ago

Thank you! You have done a great job!

rauerhans commented 1 year ago

cool, lmk if you want to change anything, this is an important feature for us

yetone commented 1 year ago

How about adding a relevant e2e test?

rauerhans commented 1 year ago

okay I will see what I can do when I find the time, where do I put this, somewhere here I suppose? https://github.com/bentoml/yatai-deployment/tree/main/tests/e2e

rauerhans commented 1 year ago

Hey just finished with the tests, I tested everything locally. Because of your triage job I have no way of testing it your GH action setup, but I can vouch that "it works on my machine" 😄 with the image I built locally. Hope you find it to your liking.

.github/workflows/e2e.yaml: This file includes changes to the workflow for running end-to-end tests. It adds new steps to test the BentoML deployment with different ingress TLS modes: 'none', 'auto', and 'static'. The appropriate environment variables are set for each test.
controllers/bentodeployment_controller.go: Updated ingress configuration handling for different TLS modes.
helm/yatai-deployment/templates/configmap-network.yaml: Added ingress TLS mode and static TLS secret name configuration options.
helm/yatai-deployment/values.yaml: Added ingress TLS mode and static TLS secret name configuration options. The layers.network section includes ingressTlsMode and ingressStaticTlsSecretName values to configure the ingress TLS mode and static TLS secret name.
scripts/quick-install-yatai-deployment.sh: Updated the script to include the INGRESS_TLS_MODE and INGRESS_STATIC_TLS_SECRET_NAME environment variables. These variables are used to set the ingress TLS mode and static TLS secret name during the installation of the BentoML deployment.
tests/e2e/e2e_test.go: Added tests for ingress creation and TLS mode configuration. It validates the creation of the Ingress resource and checks the correctness of the TLS configuration based on the configured TLS mode.
tests/e2e/example_with_ingress.yaml: New example configuration file for BentoML deployment with ingress enabled.
tests/e2e/installation_test.sh: Pass INGRESS_TLS_MODE and INGRESS_STATIC_TLS_SECRET_NAME
tests/e2e/installation_test_ingress.sh: Now also installs the Ingress controller and then installs the everything with the appropriate ingress TLS mode configuration.
tests/gh-actions/kind-cluster-1-23.yaml: Updated KinD cluster configuration with extra port mappings and node label to make it compatible with nginx ingress.
tests/gh-actions/kind-cluster-1-26.yaml: same as above

TIA for your time reviewing this ❤️

yetone commented 1 year ago

You are amazing, this PR is too professional!

yetone commented 1 year ago

/run-e2e

Update: You can check the progress here

rauerhans commented 1 year ago

hmmm, it's still running on the old test setup though, you need to set it up to use the new github actions

yetone commented 1 year ago

Can you work on a separate PR for .github/workflows? The current mechanism is that only .github/workflows under the main branch can be used to execute github actions

davidspek commented 1 year ago

@yetone I've just created https://github.com/bentoml/yatai-deployment/pull/120 to update the github workflow. I tried to do it in a way that hopefully doesn't break tests before the PR is merged but I'd still aim to merge this PR quickly after https://github.com/bentoml/yatai-deployment/pull/120.

yetone commented 1 year ago

CI related PR has been merged.

davidspek commented 1 year ago

@yetone This PR has been rebased on main so it should be possible to run the e2e test now.

FogDong commented 1 year ago

/run-e2e Update: You can check the progress here

davidspek commented 1 year ago

@FogDong @yetone It seems that I needed to do a chmod +x so that the script is executable. Can you rerun the e2e tests?

yetone commented 1 year ago

/run-e2e Update: You can check the progress here

davidspek commented 1 year ago

@yetone It looks like the e2e is erroring on waiting for the yatai-deployment-default-domain to be complete saying it can't be found, but the pods output from the previous command shows the job has already run and was successful.

davidspek commented 1 year ago

Could it be that the current e2e test is a bit flaky?

yetone commented 1 year ago

@DavidSpek You may need to change the configuration of the kind cluster so that it installs loadbalancer.

https://kind.sigs.k8s.io/docs/user/loadbalancer/

yetone commented 1 year ago

/run-e2e Update: You can check the progress here

yetone commented 1 year ago

/run-e2e Update: You can check the progress here

davidspek commented 1 year ago

Sorry @yetone I was away on vacation the past few weeks. Had you already tried updating the kind config to add the load balancer before rerunning the E2E tests? Or is there something we should do on our end?

a-pichard commented 8 months ago

Similar to https://github.com/bentoml/yatai-deployment/pull/123 I believe we should close this one

bentoml / yatai-deployment