From 66c53dd3f69a14d40a390a066b54aee5a261b8ce Mon Sep 17 00:00:00 2001 From: i325261 Date: Wed, 6 Mar 2024 14:06:36 +0100 Subject: [PATCH 1/4] Added controller.dnsTarget to chart --- chart/Chart.yaml | 4 ++-- chart/README.md | 1 + chart/templates/controller-deployment.yaml | 4 ++++ chart/values.yaml | 2 ++ 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/chart/Chart.yaml b/chart/Chart.yaml index cf84d09..80cdba9 100644 --- a/chart/Chart.yaml +++ b/chart/Chart.yaml @@ -1,5 +1,5 @@ apiVersion: v2 description: Helm chart to deploy CAP Operator https://sap.github.io/cap-operator/ name: cap-operator -version: 0.2.0 -appVersion: 0.2.0 +version: 0.2.1 +appVersion: 0.2.1 diff --git a/chart/README.md b/chart/README.md index c0b9628..eb114f4 100644 --- a/chart/README.md +++ b/chart/README.md @@ -33,6 +33,7 @@ Helm chart to deploy CAP Operator https://sap.github.io/cap-operator/ | controller.resources.limits.cpu | float | `0.2` | CPU limit | | controller.resources.requests.memory | string | `"50Mi"` | Memory request | | controller.resources.requests.cpu | float | `0.02` | CPU request | +| controller.dnsTarget | string | `""` | The dns target mentioned on the public ingress gateway service used in the cluster | | subscriptionServer.replicas | int | `1` | Replicas | | subscriptionServer.image.repository | string | `"ghcr.io/sap/cap-operator/server"` | Image repository | | subscriptionServer.image.tag | string | `""` | Image tag | diff --git a/chart/templates/controller-deployment.yaml b/chart/templates/controller-deployment.yaml index 12847b3..d3b00f2 100644 --- a/chart/templates/controller-deployment.yaml +++ b/chart/templates/controller-deployment.yaml @@ -64,4 +64,8 @@ spec: value: {{ .Capabilities.APIVersions.Has "dns.gardener.cloud/v1alpha1" | ternary "gardener" "kubernetes" }} - name: MTX_JOB_IMAGE value: "ghcr.io/sap/cap-operator/mtx-job" + {{- if .Values.controller.dnsTarget }} + - name: DNS_TARGET + value: {{ .Values.controller.dnsTarget }} + {{- end }} serviceAccountName: {{.Release.Name}}-controller diff --git a/chart/values.yaml b/chart/values.yaml index f529e22..b80834d 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -55,6 +55,8 @@ controller: memory: 50Mi # -- CPU request cpu: 0.02 + # -- The dns target mentioned on the public ingress gateway service used in the cluster + dnsTarget: "" subscriptionServer: # -- Replicas From 1015fa5aa6582c07003b478cb26ec524c6ed463c Mon Sep 17 00:00:00 2001 From: i325261 Date: Wed, 6 Mar 2024 15:52:58 +0100 Subject: [PATCH 2/4] dnsTarget defaulting using cluster domain --- internal/transformer/transformer.go | 42 +++++++++--- internal/transformer/transformer_test.go | 82 +++++++++++++++--------- 2 files changed, 85 insertions(+), 39 deletions(-) diff --git a/internal/transformer/transformer.go b/internal/transformer/transformer.go index e268f79..3cedf5f 100644 --- a/internal/transformer/transformer.go +++ b/internal/transformer/transformer.go @@ -14,6 +14,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" apitypes "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" componentoperatorruntimetypes "github.com/sap/component-operator-runtime/pkg/types" @@ -30,6 +31,8 @@ type transformer struct { client client.Client } +var setupLog = ctrl.Log.WithName("transformer") + func NewParameterTransformer(client client.Client) *transformer { return &transformer{client: client} } @@ -54,30 +57,52 @@ func replaceAsteriskDNSTarget(dnsTarget string) string { } func (t *transformer) fillDNSTarget(parameters map[string]any) error { - // get DNSTarget subscriptionServer := parameters["subscriptionServer"].(map[string]interface{}) - if parameters["dnsTarget"] != nil { // already filled in CRO - subscriptionServer["dnsTarget"] = replaceAsteriskDNSTarget(parameters["dnsTarget"].(string)) + // DNSTarget given - use it + if parameters["dnsTarget"] != nil { + replacedDnsTarget := replaceAsteriskDNSTarget(parameters["dnsTarget"].(string)) + + // set the dnsTarget in the subscriptionServer + subscriptionServer["dnsTarget"] = replacedDnsTarget + // set the dnsTarget in the controller + parameters["controller"] = map[string]interface{}{ + "dnsTarget": replacedDnsTarget, + } + delete(parameters, "dnsTarget") return nil } // DNSTarget not given - read it from the load balancer service in istio namespace if parameters["ingressGatewayLabels"] == nil { - return fmt.Errorf("cannot get dnsTarget; provide either dnsTarget or ingressGatewayLabels in the CRO") + return fmt.Errorf("unable to retrieve dnsTarget; please specify either dnsTarget or ingressGatewayLabels in the CAP Operator CRO") } - dnsTarget, err := t.getDNSTarget(parameters["ingressGatewayLabels"].([]interface{})) + dnsTarget, err := t.getDNSTargetUsingIngressGatewayLabels(parameters["ingressGatewayLabels"].([]interface{})) if err != nil { - return err + setupLog.Info("dnsTarget not found using ingressGatewayLabels", "error", err) + + // default the dnsTarget to the x. + dnsTarget, err = t.getDomain("x") + if err != nil { + return err + } + + setupLog.Info("defaulting dnsTarget to " + dnsTarget) } - subscriptionServer["dnsTarget"] = replaceAsteriskDNSTarget(dnsTarget) + replacedDnsTarget := replaceAsteriskDNSTarget(dnsTarget) + // set the dnsTarget in the subscriptionServer + subscriptionServer["dnsTarget"] = replacedDnsTarget + // set the dnsTarget in the controller + parameters["controller"] = map[string]interface{}{ + "dnsTarget": replacedDnsTarget, + } delete(parameters, "ingressGatewayLabels") return nil } -func (t *transformer) getDNSTarget(ingressGatewayLabels []interface{}) (dnsTarget string, err error) { +func (t *transformer) getDNSTargetUsingIngressGatewayLabels(ingressGatewayLabels []interface{}) (dnsTarget string, err error) { ctx := context.TODO() @@ -125,7 +150,6 @@ func (t *transformer) getDNSTarget(ingressGatewayLabels []interface{}) (dnsTarge return "", fmt.Errorf("ingress gateway service not annotated with dns target name") } - // Return ingress Gateway info (Namespace and DNS target) return dnsTarget, nil } diff --git a/internal/transformer/transformer_test.go b/internal/transformer/transformer_test.go index 35994c0..8dbf870 100644 --- a/internal/transformer/transformer_test.go +++ b/internal/transformer/transformer_test.go @@ -16,41 +16,69 @@ import ( func TestTransformer(t *testing.T) { tests := []struct { - name string - dnsTargetFilled bool - ingressGatewayLabelsFilled bool - longDomain bool - expectError bool + name string + dnsTargetFilled bool + ingressGatewayLabelsFilled bool + longDomain bool + expectError bool + withoutIngressGatewaySvcAnnotation bool }{ { - name: "Test with dnsTarget and without ingress gateway labels", + name: "With dnsTarget and without ingress gateway labels", dnsTargetFilled: true, ingressGatewayLabelsFilled: false, expectError: false, }, { - name: "Test without dnsTarget and with ingress gateway labels", + name: "Without dnsTarget and with ingress gateway labels", dnsTargetFilled: false, ingressGatewayLabelsFilled: true, expectError: false, }, { - name: "Test without dnsTarget and ingress gateway labels", + name: "Without dnsTarget and ingress gateway labels", dnsTargetFilled: false, ingressGatewayLabelsFilled: false, expectError: true, }, { - name: "Test with more than 64 character domain", + name: "With more than 64 character domain", ingressGatewayLabelsFilled: true, longDomain: true, expectError: true, }, + { + name: "Without annotation in ingress gateway labels", + ingressGatewayLabelsFilled: true, + longDomain: false, + withoutIngressGatewaySvcAnnotation: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { clientBuilder := fake.NewClientBuilder() + + istioSvc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "istioingress-gateway", + Namespace: "istio-system", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Selector: map[string]string{ + "istio": "ingress", + "app": "istio-ingress", + }, + }, + } + + if !tt.withoutIngressGatewaySvcAnnotation { + istioSvc.ObjectMeta.Annotations = map[string]string{ + "dns.gardener.cloud/dnsnames": "public-ingress.some.cluster.sap", + } + } + clientBuilder.WithObjects( &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{ @@ -75,22 +103,7 @@ func TestTransformer(t *testing.T) { }, }, }, - &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "istioingress-gateway", - Namespace: "istio-system", - Annotations: map[string]string{ - "dns.gardener.cloud/dnsnames": "public-ingress.some.cluster.sap", - }, - }, - Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeLoadBalancer, - Selector: map[string]string{ - "istio": "ingress", - "app": "istio-ingress", - }, - }, - }) + istioSvc) kubeClient := clientBuilder.Build() @@ -98,8 +111,6 @@ func TestTransformer(t *testing.T) { parameter := make(map[string]interface{}) - parameter["spec"] = map[string]interface{}{} - parameter["subscriptionServer"] = map[string]interface{}{} subscriptionServer := parameter["subscriptionServer"].(map[string]interface{}) @@ -138,12 +149,23 @@ func TestTransformer(t *testing.T) { t.Log(err) return } + + var expectedDnsTarget string + if tt.withoutIngressGatewaySvcAnnotation { + expectedDnsTarget = "x.some.cluster.sap" + } else { + expectedDnsTarget = "public-ingress.some.cluster.sap" + } + transformedParametersMap := transformedParameters.ToUnstructured() - if transformedParametersMap["subscriptionServer"].(map[string]interface{})["dnsTarget"].(string) != "public-ingress.some.cluster.sap" { - t.Error("unexpected value returned") + if transformedParametersMap["subscriptionServer"].(map[string]interface{})["dnsTarget"].(string) != expectedDnsTarget { + t.Error("unexpected value returned for subscriptionServer.dnsTarget") } if transformedParametersMap["subscriptionServer"].(map[string]interface{})["domain"].(string) != "cop.some.cluster.sap" { - t.Error("unexpected value returned") + t.Error("unexpected value returned for subscriptionServer.domain") + } + if transformedParametersMap["controller"].(map[string]interface{})["dnsTarget"].(string) != expectedDnsTarget { + t.Error("unexpected value returned for controller.dnsTarget") } }) } From 37233c84ee219c4578916d169f372378496de0a4 Mon Sep 17 00:00:00 2001 From: i325261 Date: Wed, 6 Mar 2024 16:14:15 +0100 Subject: [PATCH 3/4] Helm chart minor version bumped --- chart/Chart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chart/Chart.yaml b/chart/Chart.yaml index 80cdba9..292dc62 100644 --- a/chart/Chart.yaml +++ b/chart/Chart.yaml @@ -1,5 +1,5 @@ apiVersion: v2 description: Helm chart to deploy CAP Operator https://sap.github.io/cap-operator/ name: cap-operator -version: 0.2.1 -appVersion: 0.2.1 +version: 0.3.0 +appVersion: 0.3.0 From cf502ac48f2b53163b79f88634e2ca009f15665d Mon Sep 17 00:00:00 2001 From: Pavan Date: Thu, 7 Mar 2024 10:55:58 +0100 Subject: [PATCH 4/4] [Misc] chart: readme updated --- chart/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chart/README.md b/chart/README.md index eb114f4..ac9c5e3 100644 --- a/chart/README.md +++ b/chart/README.md @@ -1,6 +1,6 @@ # cap-operator -![Version: 0.2.0](https://img.shields.io/badge/Version-0.2.0-informational?style=flat-square) ![AppVersion: 0.2.0](https://img.shields.io/badge/AppVersion-0.2.0-informational?style=flat-square) +![Version: 0.3.0](https://img.shields.io/badge/Version-0.3.0-informational?style=flat-square) ![AppVersion: 0.3.0](https://img.shields.io/badge/AppVersion-0.3.0-informational?style=flat-square) Helm chart to deploy CAP Operator https://sap.github.io/cap-operator/