overwrite only backstage and init-dynamic-plugins image. (#295)

* set images on certain containers only

* fixes

controllers/backstage_controller_test.go

@@ -1354,7 +1354,7 @@ spec:
 			}, time.Minute, time.Second).Should(Succeed())
 
 			By("Checking that the image was set on all containers in the Pod Spec")
-			model.VisitContainers(&found.Spec.Template.Spec, func(container *corev1.Container) {
+			VisitContainers(&found.Spec.Template.Spec, func(container *corev1.Container) {
 				By(fmt.Sprintf("Checking Image in the Backstage Deployment - container: %q", container.Name), func() {
 					Expect(container.Image).Should(Equal(imageName))
 				})
@@ -1572,3 +1572,19 @@ func getSecretName(containers []corev1.Container, name string, secretName string
 	}
 	return ""
 }
+
+// ContainerVisitor is called with each container
+type ContainerVisitor func(container *corev1.Container)
+
+// visitContainers invokes the visitor function for every container in the given pod template spec
+func VisitContainers(podTemplateSpec *corev1.PodSpec, visitor ContainerVisitor) {
+	for i := range podTemplateSpec.InitContainers {
+		visitor(&podTemplateSpec.InitContainers[i])
+	}
+	for i := range podTemplateSpec.Containers {
+		visitor(&podTemplateSpec.Containers[i])
+	}
+	for i := range podTemplateSpec.EphemeralContainers {
+		visitor((*corev1.Container)(&podTemplateSpec.EphemeralContainers[i].EphemeralContainerCommon))
+	}
+}

pkg/model/deployment.go

@@ -18,6 +18,8 @@ import (
 	"fmt"
 	"os"
 
+	"k8s.io/utils/ptr"
+
 	corev1 "k8s.io/api/core/v1"
 
 	bsv1alpha1 "redhat-developer/red-hat-developer-hub-operator/api/v1alpha1"
@@ -77,12 +79,7 @@ func (b *BackstageDeployment) addToModel(model *BackstageModel, _ bsv1alpha1.Bac
 	// override image with env var
 	// [GA] TODO Do we need this feature?
 	if os.Getenv(BackstageImageEnvVar) != "" {
-		b.deployment.Spec.Template.Spec.Containers[0].Image = os.Getenv(BackstageImageEnvVar)
-		// exactly the same image for initContainer and want it to be overriden
-		// the same way as Backstage's one
-		for i := range b.deployment.Spec.Template.Spec.InitContainers {
-			b.deployment.Spec.Template.Spec.InitContainers[i].Image = os.Getenv(BackstageImageEnvVar)
-		}
+		b.setImage(ptr.To(os.Getenv(BackstageImageEnvVar)))
 	}
 
 	return true, nil
@@ -155,13 +152,17 @@ func (b *BackstageDeployment) setReplicas(replicas *int32) {
 // sets container image name of Backstage Container
 func (b *BackstageDeployment) setImage(image *string) {
 	if image != nil {
-		// TODO this is a workaround for RHDH/Janus configuration
+		b.container().Image = *image
+		// this is a workaround for RHDH/Janus configuration
 		// it is not a fact that all the containers should be updated
-		// in general case need something smarter (probably annotation based)
+		// in general case need something smarter
 		// to mark/recognize containers for update
-		VisitContainers(b.podSpec(), func(container *corev1.Container) {
-			container.Image = *image
-		})
+		if len(b.podSpec().InitContainers) > 0 {
+			i, ic := dynamicPluginsInitContainer(b.podSpec().InitContainers)
+			if ic != nil {
+				b.podSpec().InitContainers[i].Image = *image
+			}
+		}
 	}
 }
 
@@ -190,19 +191,3 @@ func (b *BackstageDeployment) setImagePullSecrets(pullSecrets []string) {
 			corev1.LocalObjectReference{Name: ps})
 	}
 }
-
-// ContainerVisitor is called with each container
-type ContainerVisitor func(container *corev1.Container)
-
-// visitContainers invokes the visitor function for every container in the given pod template spec
-func VisitContainers(podTemplateSpec *corev1.PodSpec, visitor ContainerVisitor) {
-	for i := range podTemplateSpec.InitContainers {
-		visitor(&podTemplateSpec.InitContainers[i])
-	}
-	for i := range podTemplateSpec.Containers {
-		visitor(&podTemplateSpec.Containers[i])
-	}
-	for i := range podTemplateSpec.EphemeralContainers {
-		visitor((*corev1.Container)(&podTemplateSpec.EphemeralContainers[i].EphemeralContainerCommon))
-	}
-}

pkg/model/deployment_test.go

@@ -16,7 +16,6 @@ package model
 
 import (
 	"context"
-	"os"
 	"testing"
 
 	"k8s.io/utils/ptr"
@@ -60,23 +59,38 @@ func TestSpecs(t *testing.T) {
 
 }
 
+func TestSpecImagePullSecrets(t *testing.T) {
+	bs := *deploymentTestBackstage.DeepCopy()
+
+	bs.Spec.Application.ImagePullSecrets = nil //[]string{}
+
+	testObj := createBackstageTest(bs).withDefaultConfig(true).
+		addToDefaultConfig("deployment.yaml", "janus-deployment.yaml")
+
+	model, err := InitObjects(context.TODO(), bs, testObj.externalConfig, true, true, testObj.scheme)
+	assert.NoError(t, err)
+
+	assert.Equal(t, 0, len(model.backstageDeployment.deployment.Spec.Template.Spec.ImagePullSecrets))
+	//assert.Equal(t, "my-secret", model.backstageDeployment.deployment.Spec.Template.Spec.ImagePullSecrets[0].Name)
+
+}
+
 // It tests the overriding image feature
-// [GA] if we need this (and like this) feature
-// we need to think about simple template engine
-// for substitution env vars instead.
-// Janus image specific
 func TestOverrideBackstageImage(t *testing.T) {
 
 	bs := *deploymentTestBackstage.DeepCopy()
 
 	testObj := createBackstageTest(bs).withDefaultConfig(true).
-		addToDefaultConfig("deployment.yaml", "janus-deployment.yaml")
+		addToDefaultConfig("deployment.yaml", "sidecar-deployment.yaml")
 
-	_ = os.Setenv(BackstageImageEnvVar, "dummy")
+	t.Setenv(BackstageImageEnvVar, "dummy")
 
 	model, err := InitObjects(context.TODO(), bs, testObj.externalConfig, true, false, testObj.scheme)
 	assert.NoError(t, err)
 
+	assert.Equal(t, 2, len(model.backstageDeployment.podSpec().Containers))
 	assert.Equal(t, "dummy", model.backstageDeployment.container().Image)
+	assert.Equal(t, "dummy", model.backstageDeployment.podSpec().InitContainers[0].Image)
+	assert.Equal(t, "busybox", model.backstageDeployment.podSpec().Containers[1].Image)
 
 }

pkg/model/dynamic-plugins.go

@@ -54,7 +54,7 @@ func addDynamicPlugins(spec v1alpha1.BackstageSpec, deployment *appsv1.Deploymen
 		return nil
 	}
 
-	if dynamicPluginsInitContainer(deployment.Spec.Template.Spec.InitContainers) == nil {
+	if _, ic := dynamicPluginsInitContainer(deployment.Spec.Template.Spec.InitContainers); ic == nil {
 		return fmt.Errorf("deployment validation failed, dynamic plugin name configured but no InitContainer %s defined", dynamicPluginInitContainerName)
 	}
 
@@ -106,7 +106,7 @@ func (p *DynamicPlugins) updatePod(deployment *appsv1.Deployment) {
 	//it creates a volume with dynamic-plugins ConfigMap (there should be a key named "dynamic-plugins.yaml")
 	//and mount it to the dynamic-plugin initContainer's WorkingDir (what if not specified?)
 
-	initContainer := dynamicPluginsInitContainer(deployment.Spec.Template.Spec.InitContainers)
+	_, initContainer := dynamicPluginsInitContainer(deployment.Spec.Template.Spec.InitContainers)
 	if initContainer == nil {
 		// it will fail on validate
 		return
@@ -121,7 +121,7 @@ func (p *DynamicPlugins) updatePod(deployment *appsv1.Deployment) {
 // ConfigMap name must be the same as (deployment.yaml).spec.template.spec.volumes.name.dynamic-plugins-conf.ConfigMap.name
 func (p *DynamicPlugins) validate(model *BackstageModel, _ v1alpha1.Backstage) error {
 
-	initContainer := dynamicPluginsInitContainer(model.backstageDeployment.deployment.Spec.Template.Spec.InitContainers)
+	_, initContainer := dynamicPluginsInitContainer(model.backstageDeployment.deployment.Spec.Template.Spec.InitContainers)
 	if initContainer == nil {
 		return fmt.Errorf("failed to find initContainer named %s", dynamicPluginInitContainerName)
 	}
@@ -142,11 +142,11 @@ func (p *DynamicPlugins) setMetaInfo(backstageName string) {
 
 // returns initContainer supposed to initialize DynamicPlugins
 // TODO consider to use a label to identify instead
-func dynamicPluginsInitContainer(initContainers []corev1.Container) *corev1.Container {
-	for _, ic := range initContainers {
+func dynamicPluginsInitContainer(initContainers []corev1.Container) (int, *corev1.Container) {
+	for i, ic := range initContainers {
 		if ic.Name == dynamicPluginInitContainerName {
-			return &ic
+			return i, &ic
 		}
 	}
-	return nil
+	return -1, nil
 }

pkg/model/testdata/janus-deployment.yaml

@@ -28,14 +28,6 @@ spec:
             defaultMode: 420
             optional: true
             secretName: dynamic-plugins-npmrc
-#        - name: dynamic-plugins-conf
-#          configMap:
-#            name: default-dynamic-plugins
-#            optional: true
-#            items:
-#              - key: dynamic-plugins.yaml
-#                path: dynamic-plugins.yaml
-
       initContainers:
         - command:
             - ./install-dynamic-plugins.sh
@@ -53,9 +45,6 @@ spec:
               name: dynamic-plugins-npmrc
               readOnly: true
               subPath: .npmrc
-#            - mountPath: /opt/app-root/src/dynamic-plugins.yaml
-#              subPath: dynamic-plugins.yaml
-#              name: dynamic-plugins-conf
           workingDir: /opt/app-root/src
 
       containers:

pkg/model/testdata/sidecar-deployment.yaml

@@ -0,0 +1,25 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name:  <to_be_replaced> # placeholder for 'backstage-<cr-name>'
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      rhdh.redhat.com/app:  # placeholder for 'backstage-<cr-name>'
+  template:
+    metadata:
+      labels:
+        rhdh.redhat.com/app:  # placeholder for 'backstage-<cr-name>'
+    spec:
+      initContainers:
+         - image: 'quay.io/janus-idp/backstage-showcase:next'
+           name: install-dynamic-plugins
+      containers:
+        - name: backstage-backend    # placeholder for 'backstage-backend'
+          image: quay.io/janus-idp/backstage-showcase:next
+        - name: sidecar
+          image: busybox
+
+
+