From 724837d7b48ac392b195d30727f1509ae9c78f0d Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Tue, 17 Oct 2023 16:39:38 -0400 Subject: [PATCH] feat: parameterizes gateway namespace for kubeflow Update components/workbenches/workbenches.go Co-authored-by: Bartosz Majsak consolidate regex and string replace funcs into one w helper move to package, add testing move out gateway const rename to ossmcommon clean up unit tests rename pkg, move io funcs, rename gw func --- components/dashboard/dashboard.go | 4 +- components/workbenches/workbenches.go | 4 ++ controllers/dscinitialization/monitoring.go | 6 +- pkg/common/common.go | 16 +++-- pkg/feature/raw_resources.go | 8 ++- pkg/feature/servicemesh/manifests_config.go | 21 +++++++ .../servicemesh/manifests_config_unit_test.go | 59 +++++++++++++++++++ .../servicemesh_suite_unit_test.go | 13 ++++ 8 files changed, 117 insertions(+), 14 deletions(-) create mode 100644 pkg/feature/servicemesh/manifests_config.go create mode 100644 pkg/feature/servicemesh/manifests_config_unit_test.go create mode 100644 pkg/feature/servicemesh/servicemesh_suite_unit_test.go diff --git a/components/dashboard/dashboard.go b/components/dashboard/dashboard.go index 25d7816595c..b862ed5e8ed 100644 --- a/components/dashboard/dashboard.go +++ b/components/dashboard/dashboard.go @@ -212,7 +212,7 @@ func (d *Dashboard) applyRhodsSpecificConfigs(cli client.Client, owner metav1.Ob deploy.ManagedRhods: "dedicated-admins", }[platform] - if err := common.ReplaceStringsInFile(dashboardConfig, map[string]string{"": adminGroups}); err != nil { + if err := common.ReplaceInFile(dashboardConfig, map[string]string{"": adminGroups}); err != nil { return err } @@ -261,7 +261,7 @@ func (d *Dashboard) deployConsoleLink(cli client.Client, owner metav1.Object, na domainIndex := strings.Index(consoleRoute.Spec.Host, ".") consoleLinkDomain := consoleRoute.Spec.Host[domainIndex+1:] - err := common.ReplaceStringsInFile(pathConsoleLink, map[string]string{ + err := common.ReplaceInFile(pathConsoleLink, map[string]string{ "": "https://rhods-dashboard-" + namespace + "." + consoleLinkDomain, "": sectionTitle, }) diff --git a/components/workbenches/workbenches.go b/components/workbenches/workbenches.go index 01648f328aa..045f801a3bb 100644 --- a/components/workbenches/workbenches.go +++ b/components/workbenches/workbenches.go @@ -2,6 +2,7 @@ package workbenches import ( + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/servicemesh" "path/filepath" "strings" @@ -130,6 +131,9 @@ func (w *Workbenches) ReconcileComponent(cli client.Client, owner metav1.Object, } if shouldConfigureServiceMesh { actualNbCtrlPath = notebookControllerServiceMeshPath + if err := servicemesh.OverwriteIstioGatewayVar(dscispec.ApplicationsNamespace, kfnotebookControllerServiceMeshPath); err != nil { + return err + } } if err := deploy.DeployManifestsFromPath(cli, owner, actualNbCtrlPath, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil { diff --git a/controllers/dscinitialization/monitoring.go b/controllers/dscinitialization/monitoring.go index 5355f58d896..39545aa3e28 100644 --- a/controllers/dscinitialization/monitoring.go +++ b/controllers/dscinitialization/monitoring.go @@ -89,7 +89,7 @@ func configurePrometheus(ctx context.Context, dsciInit *dsci.DSCInitialization, r.Log.Info("Success: read prometheus data from prometheus.yaml from CM") // Update prometheus manifests - err = common.ReplaceStringsInFile(filepath.Join(prometheusManifestsPath, "prometheus.yaml"), + err = common.ReplaceInFile(filepath.Join(prometheusManifestsPath, "prometheus.yaml"), map[string]string{ "": alertmanagerRoute.Spec.Host, "": alertmanagerData, @@ -100,7 +100,7 @@ func configurePrometheus(ctx context.Context, dsciInit *dsci.DSCInitialization, return err } - err = common.ReplaceStringsInFile(filepath.Join(prometheusManifestsPath, "prometheus-viewer-rolebinding.yaml"), + err = common.ReplaceInFile(filepath.Join(prometheusManifestsPath, "prometheus-viewer-rolebinding.yaml"), map[string]string{ "": dsciInit.Spec.Monitoring.Namespace, }) @@ -163,7 +163,7 @@ func configureAlertManager(ctx context.Context, dsciInit *dsci.DSCInitialization // Replace variables in alertmanager configmap // TODO: Following variables can later be exposed by the API - err = common.ReplaceStringsInFile(filepath.Join(deploy.DefaultManifestPath, "monitoring", "alertmanager", "alertmanager-configs.yaml"), + err = common.ReplaceInFile(filepath.Join(deploy.DefaultManifestPath, "monitoring", "alertmanager", "alertmanager-configs.yaml"), map[string]string{ "": b64.StdEncoding.EncodeToString(deadmansnitchSecret.Data["SNITCH_URL"]), "": b64.StdEncoding.EncodeToString(pagerDutySecret.Data["PAGERDUTY_KEY"]), diff --git a/pkg/common/common.go b/pkg/common/common.go index 80cfa9b717e..53d41b72957 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -20,21 +20,25 @@ package common import ( "fmt" "os" - "strings" + "regexp" ) -// ReplaceStringsInFile replaces variable with value in manifests during runtime. -func ReplaceStringsInFile(fileName string, replacements map[string]string) error { +// ReplaceInFile replaces content in the given file either by plain strings or regex patterns based on the content. +func ReplaceInFile(fileName string, replacements map[string]string) error { // Read the contents of the file fileContent, err := os.ReadFile(fileName) if err != nil { return fmt.Errorf("failed to read file: %w", err) } - // Replace all occurrences of the strings in the map + // Replace content using string or regex newContent := string(fileContent) - for string1, string2 := range replacements { - newContent = strings.ReplaceAll(newContent, string1, string2) + for pattern, replacement := range replacements { + regexPattern, err := regexp.Compile(pattern) + if err != nil { + return fmt.Errorf("failed to compile pattern: %w", err) + } + newContent = regexPattern.ReplaceAllString(newContent, replacement) } // Write the modified content back to the file diff --git a/pkg/feature/raw_resources.go b/pkg/feature/raw_resources.go index 4ea955c222e..09718943a56 100644 --- a/pkg/feature/raw_resources.go +++ b/pkg/feature/raw_resources.go @@ -16,16 +16,18 @@ package feature import ( "context" "fmt" + "os" + "regexp" + "strings" + "github.com/ghodss/yaml" "github.com/pkg/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" k8stypes "k8s.io/apimachinery/pkg/types" - "os" - "regexp" - "strings" ) const ( diff --git a/pkg/feature/servicemesh/manifests_config.go b/pkg/feature/servicemesh/manifests_config.go new file mode 100644 index 00000000000..6bb53c3fb81 --- /dev/null +++ b/pkg/feature/servicemesh/manifests_config.go @@ -0,0 +1,21 @@ +package servicemesh + +import ( + "fmt" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/common" + "path/filepath" +) + +const ( + gatewayPattern = `ISTIO_GATEWAY=(.*)` +) + +// OverwriteIstioGatewayVar replaces the ISTIO_GATEWAY with given namespace and "odh-gateway" in the specified ossm.env file. +// This is used in conjunction with kustomize overlays for Kubeflow notebook controllers. By overwritting referenced we can set +// proper values for environment variables populated through Kustomize. +func OverwriteIstioGatewayVar(namespace, path string) error { + envFile := filepath.Join(path, "ossm.env") + replacement := fmt.Sprintf("ISTIO_GATEWAY=%s", namespace+"/odh-gateway") + + return common.ReplaceInFile(envFile, map[string]string{gatewayPattern: replacement}) +} diff --git a/pkg/feature/servicemesh/manifests_config_unit_test.go b/pkg/feature/servicemesh/manifests_config_unit_test.go new file mode 100644 index 00000000000..1b41c79826f --- /dev/null +++ b/pkg/feature/servicemesh/manifests_config_unit_test.go @@ -0,0 +1,59 @@ +package servicemesh_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/servicemesh" + "os" + "path/filepath" +) + +var _ = Describe("Overwriting gateway name in env file", func() { + + It("should replace gateway name in the file", func() { + namespace := "test-namespace" + path := createTempEnvFile() + Expect(servicemesh.OverwriteIstioGatewayVar(namespace, path)).To(Succeed()) + + updatedContents, err := os.ReadFile(filepath.Join(path, "ossm.env")) + Expect(err).NotTo(HaveOccurred()) + + Expect(string(updatedContents)).To(ContainSubstring("ISTIO_GATEWAY=test-namespace/odh-gateway")) + }) + + It("should fail if the file does not exist", func() { + Expect(servicemesh.OverwriteIstioGatewayVar("test-namespace", "wrong_directory")).To(Not(Succeed())) + }) + + It("should not modify other text in the file", func() { + namespace := "test-namespace" + path := createTempEnvFile() + + Expect(servicemesh.OverwriteIstioGatewayVar(namespace, path)).To(Succeed()) + + updatedContents, err := os.ReadFile(filepath.Join(path, "ossm.env")) + Expect(err).NotTo(HaveOccurred()) + + Expect(string(updatedContents)).To(ContainSubstring("AnotherSetting=value")) + }) +}) + +const testContent = ` +ISTIO_GATEWAY=default-namespace/odh-gateway +AnotherSetting=value` + +func createTempEnvFile() string { + var err error + + tempDir := GinkgoT().TempDir() + + tempFilePath := filepath.Join(tempDir, "ossm.env") + tempFile, err := os.Create(tempFilePath) + Expect(err).NotTo(HaveOccurred()) + + _, err = tempFile.WriteString(testContent) + Expect(err).NotTo(HaveOccurred()) + defer tempFile.Close() + + return tempDir +} diff --git a/pkg/feature/servicemesh/servicemesh_suite_unit_test.go b/pkg/feature/servicemesh/servicemesh_suite_unit_test.go new file mode 100644 index 00000000000..2386b14327f --- /dev/null +++ b/pkg/feature/servicemesh/servicemesh_suite_unit_test.go @@ -0,0 +1,13 @@ +package servicemesh_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestServiceMeshSetup(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Service Mesh setup unit tests") +}