ROX-7150: Allow overriding related images in operator (stackrox/rox#8496)

Author: misberner

.idea/codeStyles/Project.xml

@@ -35,6 +35,10 @@ image:
     fullRef: null # string
     pullPolicy: null # string
   collector:
+    slim:
+      fullRef: null # string
+    full:
+      fullRef: null # string
     registry: null # string
     name: null # string
     repository: null # string

image/templates/helm/stackrox-secured-cluster/internal/config-shape.yaml

@@ -21,6 +21,17 @@ image:
   collector:
     repository: {{ list ._rox.image.collector.registry ._rox.image.collector.name | compact | join "/" }}
 ---
+collector:
+  slimMode: {{ eq ._rox.image.collector.registry "[< .CollectorRegistry >]" }}
+---
+image:
+  collector:
+    {{- if and ._rox.collector.slimMode ._rox.image.collector.slim.fullRef }}
+    fullRef: {{ ._rox.image.collector.slim.fullRef }}
+    {{- else if and (not ._rox.collector.slimMode) ._rox.image.collector.full.fullRef }}
+    fullRef: {{ ._rox.image.collector.full.fullRef }}
+    {{- end }}
+---
 image:
   main:
     {{- if or ._rox.image.main.tag ._rox.image.main.fullRef }}
@@ -39,9 +50,6 @@ image:
     _abbrevImageRef: {{ ._rox.image.collector.repository }}
     {{- end }}
 ---
-collector:
-  slimMode: {{ eq ._rox.image.collector.registry "[< .CollectorRegistry >]" }}
----
 image:
   collector:
     {{- if ._rox.collector.slimMode }}

image/templates/helm/stackrox-secured-cluster/internal/defaults/50-images.yaml.htpl

@@ -33,8 +33,6 @@ type DeploymentSpec struct {
 	Resources      *Resources        `json:"resources,omitempty"`
 	// Customizations to apply on this deployment.
 	Customize *CustomizeSpec `json:"customize,omitempty"`
-	// TODO(ROX-7150): We do not support setting image in the CRs because they are determined by
-	// the operator version whose lifecycle is orthogonal to that of the CR.
 }
 
 // ServiceTLSSpec is just a wrapper for ServiceTLS field to make documentation available in all spots where it is used.

operator/api/common/v1alpha1/types.go

@@ -53,8 +53,6 @@ type SecuredClusterSpec struct {
 	// Customizations to apply on all secured cluster components.
 	//+operator-sdk:csv:customresourcedefinitions:type=spec
 	Customize *common.CustomizeSpec `json:"customize,omitempty"`
-	// TODO(ROX-7150): We do not support setting image in the CRs because they are determined by
-	// the operator version whose lifecycle is orthogonal to that of the CR.
 }
 
 // SensorComponentSpec defines settings for sensor.

operator/api/securedcluster/v1alpha1/securedcluster_types.go

@@ -307,6 +307,12 @@ spec:
                 - --health-probe-bind-address=:8081
                 - --metrics-bind-address=127.0.0.1:8080
                 - --leader-elect
+                env:
+                - name: RELATED_IMAGE_MAIN
+                - name: RELATED_IMAGE_SCANNER
+                - name: RELATED_IMAGE_SCANNER_DB
+                - name: RELATED_IMAGE_COLLECTOR_SLIM
+                - name: RELATED_IMAGE_COLLECTOR_FULL
                 image: docker.io/stackrox/stackrox-operator:0.0.1
                 livenessProbe:
                   httpGet:

operator/bundle/manifests/rhacs-operator.clusterserviceversion.yaml

@@ -27,6 +27,12 @@ spec:
       containers:
       - args:
         - --leader-elect
+        env:
+        - name: RELATED_IMAGE_MAIN
+        - name: RELATED_IMAGE_SCANNER
+        - name: RELATED_IMAGE_SCANNER_DB
+        - name: RELATED_IMAGE_COLLECTOR_SLIM
+        - name: RELATED_IMAGE_COLLECTOR_FULL
         image: controller:latest
         name: manager
         securityContext:

operator/config/manager/manager.yaml

@@ -0,0 +1,11 @@
+package translation
+
+import "github.com/stackrox/rox/operator/pkg/images"
+
+var (
+	imageOverrides = images.Overrides{
+		images.Main:      "central.image.fullRef",
+		images.Scanner:   "scanner.image.fullRef",
+		images.ScannerDB: "scanner.dbImage.fullRef",
+	}
+)

operator/pkg/central/values/translation/image_overrides.go

@@ -10,6 +10,7 @@ import (
 	"github.com/pkg/errors"
 	central "github.com/stackrox/rox/operator/api/central/v1alpha1"
 	"github.com/stackrox/rox/operator/pkg/values/translation"
+	"github.com/stackrox/rox/pkg/helmutil"
 	"github.com/stackrox/rox/pkg/utils"
 	"helm.sh/helm/v3/pkg/chartutil"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -45,7 +46,11 @@ func (t Translator) Translate(ctx context.Context, u *unstructured.Unstructured)
 		return nil, err
 	}
 
-	return chartutil.CoalesceTables(baseValues, valsFromCR), nil
+	imageOverrideVals, err := imageOverrides.ToValues()
+	if err != nil {
+		return nil, errors.Wrap(err, "computing image override values")
+	}
+	return helmutil.CoalesceTables(baseValues, imageOverrideVals, valsFromCR), nil
 }
 
 // translate translates a Central CR into helm values.

operator/pkg/central/values/translation/translation.go

@@ -0,0 +1,12 @@
+package images
+
+import "github.com/stackrox/rox/pkg/env"
+
+// Environment variable settings for related image overrides.
+var (
+	Main          = env.RegisterSetting("RELATED_IMAGE_MAIN")
+	Scanner       = env.RegisterSetting("RELATED_IMAGE_SCANNER")
+	ScannerDB     = env.RegisterSetting("RELATED_IMAGE_SCANNER_DB")
+	CollectorSlim = env.RegisterSetting("RELATED_IMAGE_COLLECTOR_SLIM")
+	CollectorFull = env.RegisterSetting("RELATED_IMAGE_COLLECTOR_FULL")
+)

operator/pkg/images/env.go

@@ -0,0 +1,29 @@
+package images
+
+import (
+	"github.com/pkg/errors"
+	"github.com/stackrox/rox/pkg/env"
+	"github.com/stackrox/rox/pkg/helmutil"
+	"helm.sh/helm/v3/pkg/chartutil"
+)
+
+// Overrides defines a mapping from image override environment variable settings to
+// Helm chart configuration paths.
+type Overrides map[env.Setting]string
+
+// ToValues returns a Helm chart values object that applies the override settings that are set.
+func (o Overrides) ToValues() (chartutil.Values, error) {
+	vals := chartutil.Values{}
+	for setting, configPath := range o {
+		val := setting.Setting()
+		if val == "" {
+			continue
+		}
+		newVals, err := helmutil.ValuesForKVPair(configPath, val)
+		if err != nil {
+			return nil, errors.Wrapf(err, "applying image override from %s", setting.EnvVar())
+		}
+		vals = chartutil.CoalesceTables(vals, newVals)
+	}
+	return vals, nil
+}

operator/pkg/images/overrides.go

@@ -0,0 +1,38 @@
+package images
+
+import (
+	"testing"
+
+	"github.com/stackrox/rox/pkg/testutils/envisolator"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"helm.sh/helm/v3/pkg/chartutil"
+)
+
+func TestToValues(t *testing.T) {
+	testOverrides := Overrides{
+		Main:      "central.image.fullRef",
+		Scanner:   "scanner.image.fullRef",
+		ScannerDB: "scanner.dbImage.fullRef",
+	}
+
+	ei := envisolator.NewEnvIsolator(t)
+	defer ei.RestoreAll()
+
+	ei.Setenv(Main.EnvVar(), "override-main")
+	ei.Unsetenv(Scanner.EnvVar())
+	ei.Setenv(ScannerDB.EnvVar(), "")
+
+	vals, err := testOverrides.ToValues()
+	require.NoError(t, err)
+
+	expectedVals := chartutil.Values{
+		"central": map[string]interface{}{
+			"image": map[string]interface{}{
+				"fullRef": "override-main",
+			},
+		},
+	}
+
+	assert.Equal(t, expectedVals, vals)
+}

operator/pkg/images/overrides_test.go

@@ -0,0 +1,11 @@
+package translation
+
+import "github.com/stackrox/rox/operator/pkg/images"
+
+var (
+	imageOverrides = images.Overrides{
+		images.Main:          "image.main.fullRef",
+		images.CollectorSlim: "image.collector.slim.fullRef",
+		images.CollectorFull: "image.collector.full.fullRef",
+	}
+)

operator/pkg/securedcluster/values/translation/image_overrides.go

@@ -12,6 +12,7 @@ import (
 	securedcluster "github.com/stackrox/rox/operator/api/securedcluster/v1alpha1"
 	"github.com/stackrox/rox/operator/pkg/values/translation"
 	"github.com/stackrox/rox/pkg/buildinfo"
+	"github.com/stackrox/rox/pkg/helmutil"
 	"helm.sh/helm/v3/pkg/chartutil"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -53,7 +54,17 @@ func (t Translator) Translate(ctx context.Context, u *unstructured.Unstructured)
 		return nil, err
 	}
 
-	return t.translate(ctx, sc)
+	valsFromCR, err := t.translate(ctx, sc)
+	if err != nil {
+		return nil, err
+	}
+
+	imageOverrideVals, err := imageOverrides.ToValues()
+	if err != nil {
+		return nil, errors.Wrap(err, "computing image override values")
+	}
+
+	return helmutil.CoalesceTables(imageOverrideVals, valsFromCR), nil
 }
 
 // Translate translates a SecuredCluster CR into helm values.
@@ -73,7 +84,6 @@ func (t Translator) translate(ctx context.Context, sc securedcluster.SecuredClus
 	v.AddAllFrom(translation.GetImagePullSecrets(sc.Spec.ImagePullSecrets))
 
 	// TODO(ROX-7178): support explicit env.openshift and env.istio setting
-	// TODO(ROX-7150): support setting/overriding images
 
 	customize := translation.NewValuesBuilder()
 

operator/pkg/securedcluster/values/translation/translation.go

@@ -0,0 +1,48 @@
+name: "test explicit image reference specified for slim or full mode (Operator use only)"
+defs: |
+  def collector:
+    container(.daemonsets.collector; "collector");
+values:
+  image:
+    collector:
+      registry: docker.io/stackrox
+      name: collector
+tests:
+- name: "slim mode"
+  set:
+    collector.slimMode: true
+  tests:
+  - name: "default image is used if full ref is specified for full image only"
+    set:
+      image.collector.full.fullRef: "registry.redhat.io/rh-acs/collector:1.2.3-latest"
+    expect: |
+      collector | .image | assertThat(startswith("docker.io/stackrox/collector:") and endswith("-slim"))
+  - name: "override image is used if full ref is specified"
+    set:
+      image.collector.slim.fullRef: "registry.redhat.io/rh-acs/collector:1.2.3-slim"
+    expect: |
+      collector | .image | assertThat(. == "registry.redhat.io/rh-acs/collector:1.2.3-slim")
+    tests:
+    - name: "for slim image only"
+    - name: "for both images"
+      set:
+        image.collector.full.fullRef: "registry.redhat.io/rh-acs/collector:1.2.3-latest"
+- name: "full mode"
+  set:
+    collector.slimMode: false
+  tests:
+  - name: "default image is used if full ref is specified for slim image only"
+    set:
+      image.collector.slim.fullRef: "registry.redhat.io/rh-acs/collector:1.2.3-slim"
+    expect: |
+      collector | .image | assertThat(startswith("docker.io/stackrox/collector:") and endswith("-latest"))
+  - name: "override image is used if full ref is specified"
+    set:
+      image.collector.full.fullRef: "registry.redhat.io/rh-acs/collector:1.2.3-latest"
+    expect: |
+      collector | .image | assertThat(. == "registry.redhat.io/rh-acs/collector:1.2.3-latest")
+    tests:
+    - name: "for full image only"
+    - name: "for both images"
+      set:
+        image.collector.slim.fullRef: "registry.redhat.io/rh-acs/collector:1.2.3-slim"

pkg/charts/tests/securedclusterservices/testdata/helmtest/collector-slimfull-image-overrides.test.yaml

@@ -6,23 +6,19 @@ import (
 	"testing"
 
 	"github.com/pkg/errors"
+	"github.com/stackrox/rox/pkg/helmutil"
 	"github.com/stackrox/rox/pkg/pointers"
 	"helm.sh/helm/v3/pkg/chartutil"
 )
 
 // applySetOptions takes the values specified in the `set` stanza and merges them into the otherwise defined values.
 func (t *Test) applySetOptions() error {
 	for keyPathStr, val := range t.Set {
-		keyPath := strings.Split(keyPathStr, ".")
-		if len(keyPath) == 0 {
-			return errors.New("empty key in 'set'")
+		vals, err := helmutil.ValuesForKVPair(keyPathStr, val)
+		if err != nil {
+			return errors.Wrap(err, "in 'set'")
 		}
-
-		mapForSet := map[string]interface{}{keyPath[len(keyPath)-1]: val}
-		for i := len(keyPath) - 2; i >= 0; i-- {
-			mapForSet = map[string]interface{}{keyPath[i]: mapForSet}
-		}
-		t.Values = chartutil.CoalesceTables(mapForSet, t.Values)
+		t.Values = chartutil.CoalesceTables(vals, t.Values)
 	}
 	t.Set = nil // no longer used, but make sure this is idempotent.
 

pkg/helmtest/test.go

@@ -0,0 +1,16 @@
+package helmutil
+
+import "helm.sh/helm/v3/pkg/chartutil"
+
+// CoalesceTables is a variadic version of chartutil.CoalesceTables from the official Helm libraries.
+// It combines an arbitrary number of tables, modifying the first argument (`dst`) and giving preference
+// to arguments in left-to-right order.
+// Hence, `CoalesceTables(dst, src1, src2, ..., srcN)` is equivalent to calling
+//   CoalesceTables(...CoalesceTables(CoalesceTables(dst, src1), src2)..., srcN)
+func CoalesceTables(dst map[string]interface{}, srcs ...map[string]interface{}) map[string]interface{} {
+	res := dst
+	for _, src := range srcs {
+		res = chartutil.CoalesceTables(res, src)
+	}
+	return res
+}

pkg/helmutil/tables.go

@@ -0,0 +1,40 @@
+package helmutil
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCoalesceTables_LeftToRight(t *testing.T) {
+	dst := map[string]interface{}{
+		"foo": map[string]interface{}{
+			"bar": "baz",
+		},
+	}
+	src1 := map[string]interface{}{
+		"foo": map[string]interface{}{
+			"bar": "nope",
+			"qux": "quux",
+		},
+	}
+	src2 := map[string]interface{}{
+		"foo": map[string]interface{}{
+			"bar": "nope nope",
+			"qux": "NOPE",
+		},
+		"quuz": "corge",
+	}
+
+	result := CoalesceTables(dst, src1, src2)
+
+	expected := map[string]interface{}{
+		"foo": map[string]interface{}{
+			"bar": "baz",
+			"qux": "quux",
+		},
+		"quuz": "corge",
+	}
+
+	assert.Equal(t, expected, result)
+}