From bd452ba359382cdd6441866d4b34e338d73c850f Mon Sep 17 00:00:00 2001 From: Hans Rauer Date: Thu, 22 Jun 2023 10:14:32 +0200 Subject: [PATCH] add default tls option logic to controller add ingress tls options to configmap refactor, allow auto vs static modes adapt helm chart to changes renamt to secret name add validations fix helm validation parametrize tls mode in quick installation script test installation with ingress validate ingress/tls behaviour add ingress test action default installtion tls mode should be none use my image make kind ingress compatible need an example where ingress is enabled polish test code rename example with ingress fix template condition commit before we delete it delete env vars for local testing make sure configmap is refreshed add gh action for all three modes for 1-26 we need the kind cluster config too fix lynt issues fix lint Signed-off-by: David van der Spek --- controllers/bentodeployment_controller.go | 70 ++++++++++++++++--- .../templates/configmap-network.yaml | 15 +++- helm/yatai-deployment/values.yaml | 5 ++ tests/e2e/e2e_test.go | 50 ++++++++++++- tests/e2e/installation_test_ingress.sh | 34 +++++++++ 5 files changed, 163 insertions(+), 11 deletions(-) diff --git a/controllers/bentodeployment_controller.go b/controllers/bentodeployment_controller.go index 4ab3c02..6bb8d1d 100644 --- a/controllers/bentodeployment_controller.go +++ b/controllers/bentodeployment_controller.go @@ -2728,11 +2728,21 @@ func (r *BentoDeploymentReconciler) generateDefaultHostname(ctx context.Context, return fmt.Sprintf("%s-%s.%s", bentoDeployment.Name, bentoDeployment.Namespace, domainSuffix), nil } +type TLSModeOpt string + +const ( + TLSModeNone TLSModeOpt = "none" + TLSModeAuto TLSModeOpt = "auto" + TLSModeStatic TLSModeOpt = "static" +) + type IngressConfig struct { - ClassName *string - Annotations map[string]string - Path string - PathType networkingv1.PathType + ClassName *string + Annotations map[string]string + Path string + PathType networkingv1.PathType + TLSMode TLSModeOpt + StaticTLSSecretName string } func GetIngressConfig(ctx context.Context, cliset *kubernetes.Clientset) (ingressConfig *IngressConfig, err error) { @@ -2772,11 +2782,31 @@ func GetIngressConfig(ctx context.Context, cliset *kubernetes.Clientset) (ingres pathType = networkingv1.PathType(pathType_) } + tlsMode := TLSModeNone + tlsModeStr := strings.TrimSpace(configMap.Data["ingress-tls-mode"]) + if tlsModeStr != "" && tlsModeStr != "none" { + if tlsModeStr == "auto" || tlsModeStr == "static" { + tlsMode = TLSModeOpt(tlsModeStr) + } else { + fmt.Println("Invalid TLS mode:", tlsModeStr) + err = errors.Wrapf(err, "Invalid TLS mode: %s", tlsModeStr) + return + } + } + + staticTLSSecretName := strings.TrimSpace(configMap.Data["ingress-static-tls-secret-name"]) + if tlsMode == TLSModeStatic && staticTLSSecretName == "" { + err = errors.Wrapf(err, "TLS mode is static but ingress-static-tls-secret isn't set") + return + } + ingressConfig = &IngressConfig{ - ClassName: className, - Annotations: annotations, - Path: path, - PathType: pathType, + ClassName: className, + Annotations: annotations, + Path: path, + PathType: pathType, + TLSMode: tlsMode, + StaticTLSSecretName: staticTLSSecretName, } return @@ -2849,6 +2879,8 @@ more_set_headers "X-Yatai-Bento: %s"; ingressAnnotations := ingressConfig.Annotations ingressPath := ingressConfig.Path ingressPathType := ingressConfig.PathType + ingressTLSMode := ingressConfig.TLSMode + ingressStaticTLSSecretName := ingressConfig.StaticTLSSecretName for k, v := range ingressAnnotations { annotations[k] = v @@ -2864,6 +2896,28 @@ more_set_headers "X-Yatai-Bento: %s"; var tls []networkingv1.IngressTLS + // set default tls from network configmap + switch ingressTLSMode { + case TLSModeNone: + case TLSModeAuto: + tls = make([]networkingv1.IngressTLS, 0, 1) + tls = append(tls, networkingv1.IngressTLS{ + Hosts: []string{internalHost}, + SecretName: kubeName, + }) + + case TLSModeStatic: + tls = make([]networkingv1.IngressTLS, 0, 1) + tls = append(tls, networkingv1.IngressTLS{ + Hosts: []string{internalHost}, + SecretName: ingressStaticTLSSecretName, + }) + default: + err = errors.Wrapf(err, "TLS mode is invalid: %s", ingressTLSMode) + return + } + + // override default tls if BentoDeployment defines its own tls section if opt.bentoDeployment.Spec.Ingress.TLS != nil && opt.bentoDeployment.Spec.Ingress.TLS.SecretName != "" { tls = make([]networkingv1.IngressTLS, 0, 1) tls = append(tls, networkingv1.IngressTLS{ diff --git a/helm/yatai-deployment/templates/configmap-network.yaml b/helm/yatai-deployment/templates/configmap-network.yaml index 7820848..bd48a59 100644 --- a/helm/yatai-deployment/templates/configmap-network.yaml +++ b/helm/yatai-deployment/templates/configmap-network.yaml @@ -21,4 +21,17 @@ data: {{- if .Values.layers.network.domainSuffix }} domain-suffix: {{ .Values.layers.network.domainSuffix }} {{- end }} - + {{- if .Values.layers.network.ingressTlsMode }} + {{- $validModes := (list "none" "auto" "static") }} + {{- $mode := .Values.layers.network.ingressTlsMode }} + {{- if not (has $mode $validModes) }} + {{- fail (printf "Invalid value for ingressTlsMode: %s. Expected one of: %s" $mode $validModes) }} + {{- end }} + {{- if and (eq $mode "static") (eq (trim .Values.layers.network.ingressStaticTlsSecretName) "") }} + {{- fail "ingressStaticTlsSecretName cannot be an empty string when ingressTlsMode is set to 'static'" }} + {{- end }} + ingress-tls-mode: {{ .Values.layers.network.ingressTlsMode | quote }} + {{- if .Values.layers.network.ingressStaticTlsSecretName }} + ingress-static-tls-secret-name: {{ .Values.layers.network.ingressStaticTlsSecretName | quote }} + {{- end }} + {{- end }} \ No newline at end of file diff --git a/helm/yatai-deployment/values.yaml b/helm/yatai-deployment/values.yaml index d94a20a..128efd2 100644 --- a/helm/yatai-deployment/values.yaml +++ b/helm/yatai-deployment/values.yaml @@ -85,6 +85,11 @@ layers: ingressPath: / ingressPathType: ImplementationSpecific + # can be one of "none", "auto", "static" + ingressTlsMode: "none" + # if ingressTlsMode is "static" ingressStaticTlsSecretName must not be empty + ingressStaticTlsSecretName: "" + # To configure DNS for Yatai BentoDeployment, take an External IP or CNAME from setting up networking, and configure it with your DNS provider as follows: # If the networking layer produced an External IP address, then configure a wildcard `A` record for the domain: # ``` diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 395dc18..9b6b0b7 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "os/exec" + "strings" "time" "github.com/pkg/errors" @@ -80,7 +81,7 @@ var _ = Describe("yatai-deployment", Ordered, func() { } if os.Getenv("E2E_CHECK_NAME") != "" { By("Cleaning up BentoDeployment resources") - cmd = exec.Command("kubectl", "delete", "-f", "tests/e2e/example.yaml") + cmd = exec.Command("kubectl", "delete", "-f", "tests/e2e/example_with_ingress.yaml") _, _ = utils.Run(cmd) } }) @@ -88,7 +89,7 @@ var _ = Describe("yatai-deployment", Ordered, func() { Context("BentoDeployment Operator", func() { It("Should run successfully", func() { By("Creating a BentoDeployment CR") - cmd := exec.Command("kubectl", "apply", "-f", "tests/e2e/example.yaml") + cmd := exec.Command("kubectl", "apply", "-f", "tests/e2e/example_with_ingress.yaml") out, err := utils.Run(cmd) Expect(err).To(BeNil(), "Failed to create BentoDeployment CR: %s", string(out)) @@ -130,6 +131,51 @@ var _ = Describe("yatai-deployment", Ordered, func() { daemonProcess, err = utils.RunAsDaemon(cmd) Expect(err).To(BeNil(), "Failed to port-forward the bento api-server service") + // Test ingress creation + By("Validating the creation of Ingress resource") + ingress, err := cliset.NetworkingV1().Ingresses("yatai").Get(ctx, "test", metav1.GetOptions{}) + Expect(err).To(BeNil(), "Failed to get ingress %s", "test") + + // Test ingress tls mode behavior + By("Getting ConfigMap and retrieving values") + configMap, err := cliset.CoreV1().ConfigMaps("yatai-deployment").Get(ctx, "network", metav1.GetOptions{}) + Expect(err).To(BeNil(), "Failed to get ConfigMap %s", "network") + + ingressTLSMode, ok := configMap.Data["ingress-tls-mode"] + Expect(ok).To(BeTrue(), "Failed to get value for key %s in ConfigMap %s", "ingress-tls-mode", "network") + ingressTLSMode = strings.TrimSpace(ingressTLSMode) + + if ingressTLSMode == "auto" { + By("Validating the creation of Ingress resource with correct configuration for mode 'auto'") + if len(ingress.Spec.TLS) > 0 { + tls := ingress.Spec.TLS[0] + Expect(tls.Hosts[0]).To(Equal(ingress.Spec.Rules[0].Host), "TLS host configuration is not correct") + Expect(tls.SecretName).To(Equal("test"), "TLS secretName configuration is not correct") + } else { + Fail("No TLS configuration found in the ingress") + } + } + if ingressTLSMode == "static" { + By("Validating the creation of Ingress resource with correct configuration for mode 'static'") + ingressStaticTLSSecretName, ok := configMap.Data["ingress-static-tls-secret-name"] + Expect(ok).To(BeTrue(), "Failed to get value for key %s in ConfigMap %s", "ingress-static-tls-secret-name", "network") + ingressStaticTLSSecretName = strings.TrimSpace(ingressStaticTLSSecretName) + + if len(ingress.Spec.TLS) > 0 { + tls := ingress.Spec.TLS[0] + Expect(tls.Hosts[0]).To(Equal(ingress.Spec.Rules[0].Host), "TLS host configuration is not correct") + Expect(tls.SecretName).To(Equal(ingressStaticTLSSecretName), "TLS secretName configuration is not correct") + } else { + Fail("No TLS configuration found in the ingress") + } + } + if ingressTLSMode == "none" { + By("Validating the creation of Ingress resource with correct configuration for mode 'none'") + // mode 'none' does not mean that there is no TLS configuration in the Ingress + // it could still be the case the the BentoDeployment CRD has TLS configuration and so there should be an Ingress with TLS configuration as well + // hence, there's nothing to validate here + } + By("Sleeping for 5 seconds") time.Sleep(5 * time.Second) diff --git a/tests/e2e/installation_test_ingress.sh b/tests/e2e/installation_test_ingress.sh index d97aa8c..74cc6c4 100644 --- a/tests/e2e/installation_test_ingress.sh +++ b/tests/e2e/installation_test_ingress.sh @@ -32,3 +32,37 @@ UPGRADE_CRDS=false \ bash ./scripts/quick-install-yatai-deployment.sh echo "yatai-deployment helm release values:" helm get values yatai-deployment -n yatai-deployment +#!/bin/bash + +set -xe + +# install ingress, needs to happen before yatai-deployment, otherwise quick-install-yatai-deployment.sh will complain +# INGRESS_CLASS should then be set automatically by the script if I understand correctly +kubectl apply -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/controller-v1.0.4/deploy/static/provider/kind/deploy.yaml +kubectl wait --namespace ingress-nginx --for=condition=ready pod --selector=app.kubernetes.io/component=controller --timeout=90s + +# install yatai +kubectl create ns yatai-system || true + +echo "🚀 Installing yatai-image-builder..." +YATAI_ENDPOINT='empty' bash <(curl -s "https://raw.githubusercontent.com/bentoml/yatai-image-builder/main/scripts/quick-install-yatai-image-builder.sh") +echo "yatai-image-builder helm release values:" +helm get values yatai-image-builder -n yatai-image-builder +echo "🚀 Installing yatai-deployment..." + +# before we install yatai-deployment, we need to make sure that the network configmap is recreated by the quick-install script +# otherwise the ingress tls mode will not be set correctly in the next test action of the GHA job +kubectl delete configmap network -n yatai-deployment || true + +INGRESS_TLS_MODE=${INGRESS_TLS_MODE} \ +INGRESS_STATIC_TLS_SECRET_NAME=${INGRESS_STATIC_TLS_SECRET_NAME:-""} \ +CHECK_YATAI_IMAGE_BUILDER=false \ +YATAI_ENDPOINT='empty' \ +USE_LOCAL_HELM_CHART=true \ +IGNORE_INGRESS=false \ +SKIP_METRICS_SERVER=true \ +DOMAIN_SUFFIX='test.com' \ +UPGRADE_CRDS=false \ +bash ./scripts/quick-install-yatai-deployment.sh +echo "yatai-deployment helm release values:" +helm get values yatai-deployment -n yatai-deployment