internal/pkg/scaffold/olm-catalog: use olm.targetNamespaces ann… (#1709)

* internal/pkg/scaffold/olm-catalog/csv_updaters.go: use olm.targetNamespaces annotation in WATCH_NAMESPACE, and add unit tests

* CHANGELOG.md: add CSV WATCH_NAMESPACE update

Author: estroz

CHANGELOG.md

@@ -11,6 +11,7 @@
 ### Bug Fixes
 
 - Fixes header file content validation when the content contains empty lines or centered text. ([#1544](https://github.com/operator-framework/operator-sdk/pull/1544))
+- Generated CSV's that include a deployment install strategy will be checked for a reference to `metadata.annotations['olm.targetNamespaces']`, and if one is not found a reference will be added to the `WATCH_NAMESPACE` env var for all containers in the deployment. This is a bug because any other value that references the CSV's namespace is incorrect. ([#1396](https://github.com/operator-framework/operator-sdk/pull/1396))
 
 ## v0.8.1
 

internal/pkg/scaffold/olm-catalog/csv_test.go

@@ -26,12 +26,14 @@ import (
 	"github.com/operator-framework/operator-sdk/internal/pkg/scaffold/input"
 	testutil "github.com/operator-framework/operator-sdk/internal/pkg/scaffold/internal/testutil"
 	"github.com/operator-framework/operator-sdk/internal/util/diffutil"
+	"github.com/operator-framework/operator-sdk/pkg/k8sutil"
 
 	"github.com/coreos/go-semver/semver"
 	"github.com/ghodss/yaml"
 	olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
 	olminstall "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
 	"github.com/spf13/afero"
+	appsv1 "k8s.io/api/apps/v1"
 )
 
 const testDataDir = "testdata"
@@ -185,3 +187,45 @@ func TestGetDisplayName(t *testing.T) {
 		}
 	}
 }
+
+func TestSetAndCheckOLMNamespaces(t *testing.T) {
+	depBytes, err := ioutil.ReadFile(filepath.Join(testDeployDir, "operator.yaml"))
+	if err != nil {
+		t.Fatalf("Failed to read Deployment bytes: %v", err)
+	}
+
+	// The test operator.yaml doesn't have "olm.targetNamespaces", so first
+	// check that depHasOLMNamespaces() returns false.
+	dep := &appsv1.Deployment{}
+	if err := yaml.Unmarshal(depBytes, dep); err != nil {
+		t.Fatalf("Failed to unmarshal Deployment bytes: %v", err)
+	}
+	if depHasOLMNamespaces(dep) {
+		t.Error("Expected depHasOLMNamespaces to return false, got true")
+	}
+
+	// Insert "olm.targetNamespaces" into WATCH_NAMESPACE and check that
+	// depHasOLMNamespaces() returns true.
+	setWatchNamespacesEnv(dep)
+	if !depHasOLMNamespaces(dep) {
+		t.Error("Expected depHasOLMNamespaces to return true, got false")
+	}
+
+	// Overwrite WATCH_NAMESPACE and check that depHasOLMNamespaces() returns
+	// false.
+	overwriteContainerEnvVar(dep, k8sutil.WatchNamespaceEnvVar, newEnvVar("FOO", "bar"))
+	if depHasOLMNamespaces(dep) {
+		t.Error("Expected depHasOLMNamespaces to return false, got true")
+	}
+
+	// Insert "olm.targetNamespaces" elsewhere in the deployment pod spec
+	// and check that depHasOLMNamespaces() returns true.
+	dep = &appsv1.Deployment{}
+	if err := yaml.Unmarshal(depBytes, dep); err != nil {
+		t.Fatalf("Failed to unmarshal Deployment bytes: %v", err)
+	}
+	dep.Spec.Template.ObjectMeta.Labels["namespace"] = olmTNMeta
+	if !depHasOLMNamespaces(dep) {
+		t.Error("Expected depHasOLMNamespaces to return true, got false")
+	}
+}

internal/pkg/scaffold/olm-catalog/csv_updaters.go

@@ -15,14 +15,19 @@
 package catalog
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"strings"
 
+	"github.com/operator-framework/operator-sdk/pkg/k8sutil"
+
 	"github.com/ghodss/yaml"
 	olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
 	olminstall "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
+	log "github.com/sirupsen/logrus"
 	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 )
@@ -124,11 +129,23 @@ func (store *updaterStore) AddClusterRole(yamlDoc []byte) error {
 	return nil
 }
 
+const olmTNMeta = "metadata.annotations['olm.targetNamespaces']"
+
 func (store *updaterStore) AddDeploymentSpec(yamlDoc []byte) error {
 	dep := &appsv1.Deployment{}
 	if err := yaml.Unmarshal(yamlDoc, dep); err != nil {
 		return err
 	}
+
+	setWatchNamespacesEnv(dep)
+	// Make sure "olm.targetNamespaces" is referenced somewhere in dep,
+	// and emit a warning of not.
+	if !depHasOLMNamespaces(dep) {
+		log.Warnf(`No WATCH_NAMESPACE environment variable nor reference to "%s"`+
+			` detected in operator Deployment. For OLM compatibility, your operator`+
+			` MUST watch namespaces defined in "%s"`, olmTNMeta, olmTNMeta)
+	}
+
 	depSpec := olminstall.StrategyDeploymentSpec{
 		Name: dep.ObjectMeta.Name,
 		Spec: dep.Spec,
@@ -138,6 +155,47 @@ func (store *updaterStore) AddDeploymentSpec(yamlDoc []byte) error {
 	return nil
 }
 
+// setWatchNamespacesEnv sets WATCH_NAMESPACE to olmTNString in dep if
+// WATCH_NAMESPACE exists in a pod spec container's env list.
+func setWatchNamespacesEnv(dep *appsv1.Deployment) {
+	overwriteContainerEnvVar(dep, k8sutil.WatchNamespaceEnvVar, newEnvVar(k8sutil.WatchNamespaceEnvVar, olmTNMeta))
+}
+
+func overwriteContainerEnvVar(dep *appsv1.Deployment, name string, ev corev1.EnvVar) {
+	for _, c := range dep.Spec.Template.Spec.Containers {
+		for i := 0; i < len(c.Env); i++ {
+			if c.Env[i].Name == name {
+				c.Env[i] = ev
+			}
+		}
+	}
+}
+
+func newEnvVar(name, fieldPath string) corev1.EnvVar {
+	return corev1.EnvVar{
+		Name: name,
+		ValueFrom: &corev1.EnvVarSource{
+			FieldRef: &corev1.ObjectFieldSelector{
+				FieldPath: fieldPath,
+			},
+		},
+	}
+}
+
+// OLM places the set of target namespaces for the operator in
+// "metadata.annotations['olm.targetNamespaces']". This value should be
+// referenced in either:
+//	- The Deployment's pod spec WATCH_NAMESPACE env variable.
+//	- Some other Deployment pod spec field.
+func depHasOLMNamespaces(dep *appsv1.Deployment) bool {
+	b, err := dep.Spec.Template.Marshal()
+	if err != nil {
+		// Something is wrong with the deployment manifest, not with CLI inputs.
+		log.Fatalf("marshal Deployment spec: %v", err)
+	}
+	return bytes.Index(b, []byte(olmTNMeta)) != -1
+}
+
 func (u *InstallStrategyUpdate) Apply(csv *olmapiv1alpha1.ClusterServiceVersion) (err error) {
 	// Get install strategy from csv. Default to a deployment strategy if none found.
 	var strat olminstall.Strategy

internal/pkg/scaffold/olm-catalog/testdata/deploy/olm-catalog/app-operator/0.1.0/app-operator.v0.1.0.clusterserviceversion.yaml

@@ -48,7 +48,7 @@ spec:
                 - name: WATCH_NAMESPACE
                   valueFrom:
                     fieldRef:
-                      fieldPath: metadata.namespace
+                      fieldPath: metadata.annotations['olm.targetNamespaces']
                 - name: OPERATOR_NAME
                   value: app-operator
                 image: quay.io/example-inc/operator:v0.1.0

test/test-framework/deploy/olm-catalog/memcached-operator/0.0.2/memcached-operator.v0.0.2.clusterserviceversion.yaml

@@ -84,7 +84,7 @@ spec:
                 - name: WATCH_NAMESPACE
                   valueFrom:
                     fieldRef:
-                      fieldPath: metadata.namespace
+                      fieldPath: metadata.annotations['olm.targetNamespaces']
                 - name: POD_NAME
                   valueFrom:
                     fieldRef:

test/test-framework/deploy/olm-catalog/memcached-operator/0.0.3/memcached-operator.v0.0.3.clusterserviceversion.yaml

@@ -77,7 +77,7 @@ spec:
                 - name: WATCH_NAMESPACE
                   valueFrom:
                     fieldRef:
-                      fieldPath: metadata.namespace
+                      fieldPath: metadata.annotations['olm.targetNamespaces']
                 - name: POD_NAME
                   valueFrom:
                     fieldRef: