1.17 - Relax gloogateway.Context validation (#10153)

Co-authored-by: Sam Heilbron <[email protected]>
Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>

changelog/v1.17.13/production-helm-profiles-in-tests-part2.yaml

@@ -0,0 +1,6 @@
+changelog:
+  - type: NON_USER_FACING
+    issueLink: https://github.com/solo-io/docs/issues/489
+    resolvesIssue: false
+    description: >-
+      Validate the gloogateway.Context object in a way that it can be invoked by an Enterprise installation

pkg/utils/helmutils/unmarshal.go

@@ -0,0 +1,46 @@
+package helmutils
+
+import (
+	"os"
+
+	k8syamlutil "sigs.k8s.io/yaml"
+)
+
+// UnmarshalValuesFile reads a file and returns a map containing the unmarshalled values
+func UnmarshalValuesFile(filePath string) (map[string]interface{}, error) {
+	mapFromFile := map[string]interface{}{}
+
+	bytes, err := os.ReadFile(filePath)
+	if err != nil {
+		return nil, err
+	}
+
+	// NOTE: This is not the default golang yaml.Unmarshal, because that implementation
+	// does not unmarshal into a map[string]interface{}; it unmarshals the file into a map[interface{}]interface{}
+	// https://github.com/go-yaml/yaml/issues/139
+	if err := k8syamlutil.Unmarshal(bytes, &mapFromFile); err != nil {
+		return nil, err
+	}
+
+	return mapFromFile, nil
+}
+
+// MergeMaps comes from Helm internals: https://github.com/helm/helm/blob/release-3.0/pkg/cli/values/options.go#L88
+func MergeMaps(a, b map[string]interface{}) map[string]interface{} {
+	out := make(map[string]interface{}, len(a))
+	for k, v := range a {
+		out[k] = v
+	}
+	for k, v := range b {
+		if v, ok := v.(map[string]interface{}); ok {
+			if bv, ok := out[k]; ok {
+				if bv, ok := bv.(map[string]interface{}); ok {
+					out[k] = MergeMaps(bv, v)
+					continue
+				}
+			}
+		}
+		out[k] = v
+	}
+	return out
+}

test/kubernetes/e2e/test.go

@@ -53,6 +53,11 @@ func CreateTestInstallation(
 	runtimeContext := testruntime.NewContext()
 	clusterContext := cluster.MustKindContext(runtimeContext.ClusterName)
 
+	if err := gloogateway.ValidateGlooGatewayContext(glooGatewayContext); err != nil {
+		// We error loudly if the context is misconfigured
+		panic(err)
+	}
+
 	return CreateTestInstallationForCluster(t, runtimeContext, clusterContext, glooGatewayContext)
 }
 
@@ -68,11 +73,6 @@ func CreateTestInstallationForCluster(
 	clusterContext *cluster.Context,
 	glooGatewayContext *gloogateway.Context,
 ) *TestInstallation {
-	if err := glooGatewayContext.Validate(); err != nil {
-		// We error loudly if the context is misconfigured
-		panic(err)
-	}
-
 	installation := &TestInstallation{
 		// RuntimeContext contains the set of properties that are defined at runtime by whoever is invoking tests
 		RuntimeContext: runtimeContext,

test/kubernetes/testutils/gloogateway/context.go

@@ -47,22 +47,12 @@ type Context struct {
 	ChartUri string
 }
 
-// Validate returns an error if the defined Context is invalid for a test
-func (c *Context) Validate() error {
-	// We are intentionally restrictive, and expect a ProfileValuesManifestFile to be defined.
-	// This is because we want all existing and future tests to rely on this concept
-	if err := c.validateValuesManifest("ProfileValuesManifestFile", c.ProfileValuesManifestFile); err != nil {
-		return err
-	}
-
-	if err := c.validateValuesManifest("ValuesManifestFile", c.ValuesManifestFile); err != nil {
-		return err
-	}
-
-	return nil
+// ValidateGlooGatewayContext returns an error if the provided Context is invalid
+func ValidateGlooGatewayContext(context *Context) error {
+	return ValidateContext(context, validateGlooGatewayValuesManifest)
 }
 
-func (c *Context) validateValuesManifest(name string, file string) error {
+func validateGlooGatewayValuesManifest(name string, file string) error {
 	if file == "" {
 		return eris.Errorf("%s must be provided in glooGateway.Context", name)
 	}
@@ -77,3 +67,19 @@ func (c *Context) validateValuesManifest(name string, file string) error {
 	}
 	return nil
 }
+
+// ValidateContext returns an error if the provided Context is invalid
+// This accepts a manifestValidator so that it can be used by Gloo Gateway Enterprise
+func ValidateContext(context *Context, manifestValidator func(string, string) error) error {
+	// We are intentionally restrictive, and expect a ProfileValuesManifestFile to be defined.
+	// This is because we want all existing and future tests to rely on this concept
+	if err := manifestValidator("ProfileValuesManifestFile", context.ProfileValuesManifestFile); err != nil {
+		return err
+	}
+
+	if err := manifestValidator("ValuesManifestFile", context.ValuesManifestFile); err != nil {
+		return err
+	}
+
+	return nil
+}

test/testutils/helm_values.go

@@ -2,10 +2,10 @@ package testutils
 
 import (
 	"encoding/json"
-	"os"
 	"path"
 
 	"github.com/solo-io/gloo/install/helm/gloo/generate"
+	"github.com/solo-io/gloo/pkg/utils/helmutils"
 	"helm.sh/helm/v3/pkg/strvals"
 	"knative.dev/pkg/test/helpers"
 	k8syamlutil "sigs.k8s.io/yaml"
@@ -54,8 +54,21 @@ func ValidateHelmValues(unstructuredHelmValues map[string]interface{}) error {
 // BuildHelmValues reads the base values.yaml file from a Helm chart and merges it with the provided values
 // each entry in valuesArgs should look like `path.to.helm.field=value`
 func BuildHelmValues(values HelmValues) (map[string]interface{}, error) {
+	// read the chart's base values file first
+	rootDir, err := helpers.GetRootDir()
+	if err != nil {
+		return nil, err
+	}
+	chartPath := path.Join(rootDir, "install", "helm", "gloo")
+
+	return BuildHelmValuesForChart(chartPath, values)
+}
 
-	finalValues, err := readFinalValues()
+// BuildHelmValuesForChart reads the base values.yaml file from a Helm chart and merges it with the provided values
+// each entry in valuesArgs should look like `path.to.helm.field=value`
+func BuildHelmValuesForChart(chartPath string, values HelmValues) (map[string]interface{}, error) {
+	// read the chart's base values file first
+	finalValues, err := helmutils.UnmarshalValuesFile(path.Join(chartPath, "values.yaml"))
 	if err != nil {
 		return nil, err
 	}
@@ -70,62 +83,14 @@ func BuildHelmValues(values HelmValues) (map[string]interface{}, error) {
 	if values.ValuesFile != "" {
 		// these lines ripped out of Helm internals
 		// https://github.com/helm/helm/blob/release-3.0/pkg/cli/values/options.go
-		mapFromFile, err := readValuesFile(values.ValuesFile)
+		mapFromFile, err := helmutils.UnmarshalValuesFile(values.ValuesFile)
 		if err != nil {
 			return nil, err
 		}
 
 		// Merge with the previous map
-		finalValues = mergeMaps(finalValues, mapFromFile)
+		finalValues = helmutils.MergeMaps(finalValues, mapFromFile)
 	}
 
 	return finalValues, nil
 }
-
-func readFinalValues() (map[string]interface{}, error) {
-	// read the chart's base values file first
-	rootDir, err := helpers.GetRootDir()
-	if err != nil {
-		return nil, err
-	}
-	filePath := path.Join(rootDir, "install", "helm", "gloo", "values.yaml")
-	return readValuesFile(filePath)
-}
-
-func readValuesFile(filePath string) (map[string]interface{}, error) {
-	mapFromFile := map[string]interface{}{}
-
-	bytes, err := os.ReadFile(filePath)
-	if err != nil {
-		return nil, err
-	}
-
-	// NOTE: This is not the default golang yaml.Unmarshal, because that implementation
-	// does not unmarshal into a map[string]interface{}; it unmarshals the file into a map[interface{}]interface{}
-	// https://github.com/go-yaml/yaml/issues/139
-	if err := k8syamlutil.Unmarshal(bytes, &mapFromFile); err != nil {
-		return nil, err
-	}
-
-	return mapFromFile, nil
-}
-
-// mergeMaps comes from Helm internals: https://github.com/helm/helm/blob/release-3.0/pkg/cli/values/options.go#L88
-func mergeMaps(a, b map[string]interface{}) map[string]interface{} {
-	out := make(map[string]interface{}, len(a))
-	for k, v := range a {
-		out[k] = v
-	}
-	for k, v := range b {
-		if v, ok := v.(map[string]interface{}); ok {
-			if bv, ok := out[k]; ok {
-				if bv, ok := bv.(map[string]interface{}); ok {
-					out[k] = mergeMaps(bv, v)
-					continue
-				}
-			}
-		}
-		out[k] = v
-	}
-	return out
-}