Add AZURE_TARGET_NAMESPACES to restrict the namespaces the operator watches (#1559)

* Add a target namespaces config, only watch resources therein * Initial work on target namespace test * Get target namespace test working in both cases * More useful logging when creating test RG fails * Run the no-target-namespaces test in the CI pipeline This is handled in the same way as the secret naming version setting, but the more settings we add (some more are on the way), the more unwieldy it's going to be. We need to come up with a better way of making different settings testable. * Rework install- targets so they don't trample go.mod & .sum Renamed them to install-tools and install-test-tools, since they're installing binaries used in the build process rather than code dependencies. Run the `go get` commands in a temp directory and dummy module so that they don't update the ASO go.mod and .sum files with dependencies that our code doesn't actually depend on. * Use the unfiltered API reader when looking for AAD identities When target namespaces are set, there's no guarantee that the operator's namespace is included. The identity finder always needs to look in the operator namespace so pass it the API reader which bypasses the filtered cache. * Review tweaks, thanks @matthchr!
2021-06-17 03:05:17 +12:00 · 2021-06-17 03:05:17 +12:00 · a4d3a51843
--- a/38
+++ b/38
@ -57,11 +57,21 @@ generate-test-certs:
 .PHONY: test-integration-controllers
 test-integration-controllers: generate fmt vet manifests
 	TEST_RESOURCE_PREFIX=$(TEST_RESOURCE_PREFIX) TEST_USE_EXISTING_CLUSTER=false REQUEUE_AFTER=20 \
+	AZURE_TARGET_NAMESPACES=default,watched \
 	go test -v -tags "$(BUILD_TAGS)" -coverprofile=reports/integration-controllers-coverage-output.txt -coverpkg=./... -covermode count -parallel 4 -timeout 45m \
 		./controllers/... \
 		./pkg/secrets/...
 		# TODO: Note that the above test (secrets/keyvault) is not an integration-controller test... but it's not a unit test either and unfortunately the test-integration-managers target isn't run in CI either?

+# Check that when there are no target namespaces all namespaces are watched
+.PHONY: test-no-target-namespaces
+test-no-target-namespaces: generate fmt vet manifests
+	TEST_RESOURCE_PREFIX=$(TEST_RESOURCE_PREFIX) TEST_USE_EXISTING_CLUSTER=false REQUEUE_AFTER=20 \
+	AZURE_TARGET_NAMESPACES= \
+	go test -v -tags "$(BUILD_TAGS)" -coverprofile=reports/no-target-namespaces-coverage-output.txt -coverpkg=./... -covermode count -parallel 4 -timeout 45m \
+		-run TestTargetNamespaces \
+		./controllers/...
+
 # Run subset of tests with v1 secret naming enabled to ensure no regression in old secret naming
 .PHONY: test-v1-secret-naming
 test-v1-secret-naming: generate fmt vet manifests
@ -208,7 +218,7 @@ helm-chart-manifests: generate

 # Generate manifests e.g. CRD, RBAC etc.
 .PHONY: manifests
-manifests: install-dependencies
+manifests: install-tools
 	$(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
 	# update manifests to force preserveUnknownFields to false. We can't use controller-gen to set this to false because it has a bug...
 	# see: https://github.com/kubernetes-sigs/controller-tools/issues/476
@ -242,11 +252,11 @@ generate-template:
 # TODO: These kind-delete / kind-create targets were stolen from k8s-infra and
 # TODO: should be merged back together when the projects more closely align
 .PHONY: kind-delete
-kind-delete: install-test-dependencies
+kind-delete: install-test-tools
 	kind delete cluster --name=$(KIND_CLUSTER_NAME) || true

 .PHONY: kind-create
-kind-create: install-test-dependencies
+kind-create: install-test-tools
 	kind get clusters | grep -E $(KIND_CLUSTER_NAME) > /dev/null;\
 	EXISTS=$$?;\
 	if [ $$EXISTS -eq 0 ]; then \
@ -316,20 +326,28 @@ install-cert-manager:
 install-aad-pod-identity:
 	kubectl apply -f https://raw.githubusercontent.com/Azure/aad-pod-identity/master/deploy/infra/deployment-rbac.yaml

-.PHONY: install-test-dependencies
-install-test-dependencies: install-dependencies
-	go get github.com/jstemmer/go-junit-report \
+.PHONY: install-test-tools
+install-test-tools: TEST_TOOLS_MOD_DIR := $(shell mktemp -d -t goinstall_XXXXXXXXXX)
+install-test-tools: install-tools
+	cd $(TEST_TOOLS_MOD_DIR) \
+	&& go mod init fake/mod \
+	&& go get github.com/jstemmer/go-junit-report \
 	&& go get github.com/axw/gocov/gocov \
 	&& go get github.com/AlekSi/gocov-xml \
 	&& go get github.com/wadey/gocovmerge \
-	&& go get sigs.k8s.io/kind@v0.9.0 \
+	&& go get sigs.k8s.io/kind@v0.9.0
+	rm -r $(TEST_TOOLS_MOD_DIR)

-.PHONY: install-dependencies
-install-dependencies:
-	go get github.com/mikefarah/yq/v4 \
+.PHONY: install-tools
+install-tools: TEMP_DIR := $(shell mktemp -d -t goinstall_XXXXXXXXXX)
+install-tools:
+	cd $(TEMP_DIR) \
+	&& go mod init fake/mod \
+	&& go get github.com/mikefarah/yq/v4 \
 	&& go get k8s.io/code-generator/cmd/conversion-gen@v0.18.2 \
 	&& go get sigs.k8s.io/kustomize/kustomize/v3@v3.8.6 \
 	&& go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.4.0
+	rm -r $(TEMP_DIR)
    CONTROLLER_GEN=$(shell go env GOPATH)/bin/controller-gen

 # Operator-sdk release version
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@ -100,7 +100,7 @@ steps:
        arch=$(go env GOARCH)
        go mod download
        make install-kubebuilder
-        make install-test-dependencies
+        make install-test-tools
        make generate-test-certs
      workingDirectory: '$(System.DefaultWorkingDirectory)'

@ -156,6 +156,26 @@ steps:
      BUILD_ID: $(Build.BuildId)
    workingDirectory: '$(System.DefaultWorkingDirectory)'

+  # TODO: There is no way to run steps in parallel in Azure pipelines but ideally this step would run in parallel
+  # TODO: with the above testing step to reduce overall runtime
+  - script: |
+      set -e
+      export PATH=$PATH:$(go env GOPATH)/bin:$(go env GOPATH)/kubebuilder/bin
+      export KUBEBUILDER_ASSETS=$(go env GOPATH)/kubebuilder/bin
+      make test-no-target-namespaces
+    displayName: Run test for no target namespaces
+    condition: or(eq(variables['check_changes.SOURCE_CODE_CHANGED'], 'true'), eq(variables['Build.SourceBranch'], 'refs/heads/master'))
+    continueOnError: 'false'
+    env:
+      GO111MODULE: on
+      AZURE_SUBSCRIPTION_ID: $(AZURE_SUBSCRIPTION_ID)
+      AZURE_TENANT_ID: $(AZURE_TENANT_ID)
+      AZURE_CLIENT_ID: $(AZURE_CLIENT_ID)
+      AZURE_CLIENT_SECRET: $(AZURE_CLIENT_SECRET)
+      REQUEUE_AFTER: $(REQUEUE_AFTER)
+      BUILD_ID: $(Build.BuildId)
+    workingDirectory: '$(System.DefaultWorkingDirectory)'
+
  - script: |
      set -e
      export PATH=$PATH:$(go env GOPATH)/bin
--- a/config/default/manager_image_patch.yaml
+++ b/config/default/manager_image_patch.yaml
@ -61,6 +61,12 @@ spec:
                name: azureoperatorsettings
                key: AZURE_SECRET_NAMING_VERSION
                optional: true
+          - name: AZURE_TARGET_NAMESPACES
+            valueFrom:
+              secretKeyRef:
+                name: azureoperatorsettings
+                key: AZURE_TARGET_NAMESPACES
+                optional: true
          # Used along with aad-pod-identity integration, but set always
          # because it doesn't hurt
          - name: POD_NAMESPACE
--- a/controllers/suite_test.go
+++ b/controllers/suite_test.go
@ -15,36 +15,39 @@ import (
 	"time"

 	"github.com/gobuffalo/envy"
-
-	"github.com/Azure/azure-service-operator/pkg/helpers"
-	resourcemanagersqldb "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqldb"
-	"github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
-	mysqladmin "github.com/Azure/azure-service-operator/pkg/resourcemanager/mysql/aadadmin"
-	"github.com/Azure/azure-service-operator/pkg/resourcemanager/mysql/mysqlaaduser"
-
+	"k8s.io/client-go/kubernetes/scheme"
 	kscheme "k8s.io/client-go/kubernetes/scheme"
-
 	"k8s.io/client-go/rest"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/envtest"

-	k8sSecrets "github.com/Azure/azure-service-operator/pkg/secrets/kube"
-
+	azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1"
+	"github.com/Azure/azure-service-operator/api/v1alpha2"
+	"github.com/Azure/azure-service-operator/api/v1beta1"
+	"github.com/Azure/azure-service-operator/pkg/helpers"
 	resourcemanagerapimgmt "github.com/Azure/azure-service-operator/pkg/resourcemanager/apim/apimgmt"
 	resourcemanagerappinsights "github.com/Azure/azure-service-operator/pkg/resourcemanager/appinsights"
 	resourcemanagersqlaction "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlaction"
+	resourcemanagersqldb "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqldb"
 	resourcemanagersqlfailovergroup "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlfailovergroup"
 	resourcemanagersqlfirewallrule "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlfirewallrule"
 	resourcemanagersqlmanageduser "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlmanageduser"
 	resourcemanagersqlserver "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlserver"
 	resourcemanagersqluser "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqluser"
 	resourcemanagersqlvnetrule "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlvnetrule"
+	"github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
 	resourcemanagerconfig "github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
 	resourcemanagercosmosdbaccount "github.com/Azure/azure-service-operator/pkg/resourcemanager/cosmosdb/account"
 	resourcemanagercosmosdbsqldatabase "github.com/Azure/azure-service-operator/pkg/resourcemanager/cosmosdb/sqldatabase"
 	resourcemanagereventhub "github.com/Azure/azure-service-operator/pkg/resourcemanager/eventhubs"
 	resourcemanagerkeyvaults "github.com/Azure/azure-service-operator/pkg/resourcemanager/keyvaults"
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager/loadbalancer"
+	mysqladmin "github.com/Azure/azure-service-operator/pkg/resourcemanager/mysql/aadadmin"
 	mysqlDatabaseManager "github.com/Azure/azure-service-operator/pkg/resourcemanager/mysql/database"
 	mysqlFirewallManager "github.com/Azure/azure-service-operator/pkg/resourcemanager/mysql/firewallrule"
+	"github.com/Azure/azure-service-operator/pkg/resourcemanager/mysql/mysqlaaduser"
 	mysqluser "github.com/Azure/azure-service-operator/pkg/resourcemanager/mysql/mysqluser"
 	mysqlServerManager "github.com/Azure/azure-service-operator/pkg/resourcemanager/mysql/server"
 	mysqlvnetrule "github.com/Azure/azure-service-operator/pkg/resourcemanager/mysql/vnetrule"
@ -64,16 +67,8 @@ import (
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager/vmext"
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager/vmss"
 	resourcemanagervnet "github.com/Azure/azure-service-operator/pkg/resourcemanager/vnet"
+	k8sSecrets "github.com/Azure/azure-service-operator/pkg/secrets/kube"
 	telemetry "github.com/Azure/azure-service-operator/pkg/telemetry"
-
-	"k8s.io/client-go/kubernetes/scheme"
-	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/envtest"
-
-	azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1"
-	"github.com/Azure/azure-service-operator/api/v1alpha2"
-	"github.com/Azure/azure-service-operator/api/v1beta1"
 	// +kubebuilder:scaffold:imports
 )

@ -154,11 +149,19 @@ func setup() error {

 	var k8sManager ctrl.Manager

+	targetNamespaces := resourcemanagerconfig.TargetNamespaces()
+	var cacheFunc cache.NewCacheFunc
+	if targetNamespaces != nil {
+		log.Println("Restricting operator cache to namespaces", targetNamespaces)
+		cacheFunc = cache.MultiNamespacedCacheBuilder(targetNamespaces)
+	}
+
 	// +kubebuilder:scaffold:scheme
 	k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{
-		Scheme:  scheme.Scheme,
-		CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir,
-		Port:    testEnv.WebhookInstallOptions.LocalServingPort,
+		Scheme:   scheme.Scheme,
+		CertDir:  testEnv.WebhookInstallOptions.LocalServingCertDir,
+		Port:     testEnv.WebhookInstallOptions.LocalServingPort,
+		NewCache: cacheFunc,
 	})
 	if err != nil {
 		return err
@ -935,7 +938,7 @@ func setup() error {
 	if result.Response.StatusCode != 204 {
 		_, err = resourceGroupManager.CreateGroup(context.Background(), resourceGroupName, resourceGroupLocation)
 		if err != nil {
-			return fmt.Errorf("ResourceGroup creation failed")
+			return fmt.Errorf("ResourceGroup creation failed: %v", err)
 		}
 	}

--- a/controllers/targetnamespace_test.go
+++ b/controllers/targetnamespace_test.go
@ -0,0 +1,123 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+package controllers
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/Azure/go-autorest/autorest/to"
+	"github.com/gobuffalo/envy"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+
+	v1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1"
+	"github.com/Azure/azure-service-operator/pkg/helpers"
+)
+
+func TestTargetNamespaces(t *testing.T) {
+	t.Parallel()
+	defer PanicRecover(t)
+	ctx := context.Background()
+
+	// Check that resources in default and watched get reconciled
+	// successfully, but ones created in other ones don't.
+	rgName := tc.resourceGroupName
+	rgLocation := tc.resourceGroupLocation
+
+	newName := func() string {
+		return "storageacct" + helpers.RandomString(6)
+	}
+
+	createNamespaces(ctx, t, "watched", "unwatched")
+
+	configuredNamespaces := envy.Get("AZURE_TARGET_NAMESPACES", "")
+
+	instanceDefault := v1alpha1.StorageAccount{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      newName(),
+			Namespace: "default",
+		},
+		Spec: v1alpha1.StorageAccountSpec{
+			Kind:          "BlobStorage",
+			Location:      rgLocation,
+			ResourceGroup: rgName,
+			Sku: v1alpha1.StorageAccountSku{
+				Name: "Standard_LRS",
+			},
+			AccessTier:             "Hot",
+			EnableHTTPSTrafficOnly: to.BoolPtr(true),
+		},
+	}
+
+	EnsureInstance(ctx, t, tc, &instanceDefault)
+
+	// The watched namespace is also reconciled.
+	instanceWatched := instanceDefault
+	instanceWatched.ObjectMeta = metav1.ObjectMeta{
+		Name:      newName(),
+		Namespace: "watched",
+	}
+	EnsureInstance(ctx, t, tc, &instanceWatched)
+
+	// But the unwatched namespace isn't...
+	instanceUnwatched := instanceDefault
+	instanceUnwatched.ObjectMeta = metav1.ObjectMeta{
+		Name:      newName(),
+		Namespace: "unwatched",
+	}
+	require := require.New(t)
+	err := tc.k8sClient.Create(ctx, &instanceUnwatched)
+	require.Equal(nil, err)
+
+	res, err := meta.Accessor(&instanceUnwatched)
+	require.Equal(nil, err)
+	names := types.NamespacedName{Name: res.GetName(), Namespace: res.GetNamespace()}
+
+	gotFinalizer := func() bool {
+		err := tc.k8sClient.Get(ctx, names, &instanceUnwatched)
+		require.Equal(nil, err)
+		return HasFinalizer(res, finalizerName)
+	}
+
+	if configuredNamespaces == "" {
+		// The operator should be watching all namespaces.
+		require.Eventually(
+			gotFinalizer,
+			tc.timeoutFast,
+			tc.retry,
+			"instance in some namespace never got a finalizer",
+		)
+	} else {
+		// We can tell that the resource isn't being reconciled if it
+		// never gets a finalizer.
+		require.Never(
+			gotFinalizer,
+			20*time.Second,
+			time.Second,
+			"instance in unwatched namespace got finalizer",
+		)
+	}
+
+	EnsureDelete(ctx, t, tc, &instanceDefault)
+	EnsureDelete(ctx, t, tc, &instanceWatched)
+	EnsureDelete(ctx, t, tc, &instanceUnwatched)
+}
+
+func createNamespaces(ctx context.Context, t *testing.T, names ...string) {
+	for _, name := range names {
+		err := tc.k8sClient.Create(ctx, &corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: name,
+			},
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+}
--- a/main.go
+++ b/main.go
@ -18,6 +18,7 @@ import (
 	kscheme "k8s.io/client-go/kubernetes/scheme"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"

 	"github.com/Azure/azure-service-operator/api/v1alpha1"
@ -122,9 +123,24 @@ func main() {

 	ctrl.SetLogger(zap.Logger(true))

+	err := resourcemanagerconfig.ParseEnvironment()
+	if err != nil {
+		setupLog.Error(err, "unable to parse settings required to provision resources in Azure")
+		os.Exit(1)
+	}
+
+	setupLog.V(0).Info("Configuration details", "Configuration", resourcemanagerconfig.ConfigString())
+
+	targetNamespaces := resourcemanagerconfig.TargetNamespaces()
+	var cacheFunc cache.NewCacheFunc
+	if targetNamespaces != nil {
+		cacheFunc = cache.MultiNamespacedCacheBuilder(targetNamespaces)
+	}
+
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme:               scheme,
 		MetricsBindAddress:   metricsAddr,
+		NewCache:             cacheFunc,
 		LeaderElection:       enableLeaderElection,
 		LivenessEndpointName: "/healthz",
 		Port:                 9443,
@ -135,14 +151,6 @@ func main() {
 		os.Exit(1)
 	}

-	err = resourcemanagerconfig.ParseEnvironment()
-	if err != nil {
-		setupLog.Error(err, "unable to parse settings required to provision resources in Azure")
-		os.Exit(1)
-	}
-
-	setupLog.V(0).Info("Configuration details", "Configuration", resourcemanagerconfig.ConfigString())
-
 	keyvaultName := resourcemanagerconfig.GlobalCredentials().OperatorKeyvault()

 	if keyvaultName == "" {
@ -761,7 +769,10 @@ func main() {
 		os.Exit(1)
 	}

-	identityFinder := helpers.NewAADIdentityFinder(mgr.GetClient(), config.PodNamespace())
+	// Use the API reader rather than using mgr.GetClient(), because
+	// the client might be restricted by target namespaces, while we
+	// need to read from the operator namespace.
+	identityFinder := helpers.NewAADIdentityFinder(mgr.GetAPIReader(), config.PodNamespace())
 	if err = (&controllers.MySQLAADUserReconciler{
 		Reconciler: &controllers.AsyncReconciler{
 			Client:      mgr.GetClient(),
--- a/pkg/resourcemanager/config/config.go
+++ b/pkg/resourcemanager/config/config.go
@ -31,6 +31,7 @@ var (
 	baseURI                string
 	environment            *azure.Environment
 	podNamespace           string
+	targetNamespaces       []string
 	secretNamingVersion    secrets.SecretNamingVersion

 	testResourcePrefix string // used to generate resource names in tests, should probably exist in a test only package
@ -106,6 +107,11 @@ func PodNamespace() string {
 	return podNamespace
 }

+// TargetNamespaces returns the namespaces the operator should watch for resources.
+func TargetNamespaces() []string {
+	return targetNamespaces
+}
+
 // AppendRandomSuffix will append a suffix of five random characters to the specified prefix.
 func AppendRandomSuffix(prefix string) string {
 	return randname.GenerateWithPrefix(prefix, 5)
@ -125,13 +131,14 @@ func SecretNamingVersion() secrets.SecretNamingVersion {
 func ConfigString() string {
 	creds := GlobalCredentials()
 	return fmt.Sprintf(
-		"clientID: %q, tenantID: %q, subscriptionID: %q, cloudName: %q, useDeviceFlow: %v, useManagedIdentity: %v, podNamespace: %q, secretNamingVersion: %q",
+		"clientID: %q, tenantID: %q, subscriptionID: %q, cloudName: %q, useDeviceFlow: %v, useManagedIdentity: %v, targetNamespaces: %v, podNamespace: %q, secretNamingVersion: %q",
 		creds.ClientID(),
 		creds.TenantID(),
 		creds.SubscriptionID(),
 		cloudName,
 		UseDeviceFlow(),
 		creds.UseManagedIdentity(),
+		targetNamespaces,
 		podNamespace,
 		SecretNamingVersion())
 }
--- a/pkg/resourcemanager/config/env.go
+++ b/pkg/resourcemanager/config/env.go
@ -8,6 +8,7 @@ import (
 	"log"
 	"os"
 	"strconv"
+	"strings"

 	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/gobuffalo/envy"
@ -65,6 +66,7 @@ func ParseEnvironment() error {
 	if err != nil {
 		return errors.Wrapf(err, "couldn't get POD_NAMESPACE env variable")
 	}
+	targetNamespaces = ParseStringListFromEnvironment("AZURE_TARGET_NAMESPACES")

 	secretNamingVersionInt, err := ParseIntFromEnvironment("AZURE_SECRET_NAMING_VERSION")
 	if err != nil {
@ -142,3 +144,16 @@ func ParseIntFromEnvironment(variable string) (int, error) {
 	}
 	return value, nil
 }
+
+func ParseStringListFromEnvironment(variable string) []string {
+	env := envy.Get(variable, "")
+	if len(strings.TrimSpace(env)) == 0 {
+		return nil
+	}
+	items := strings.Split(env, ",")
+	// Remove any whitespace used to separate items.
+	for i, item := range items {
+		items[i] = strings.TrimSpace(item)
+	}
+	return items
+}
--- a/pkg/resourcemanager/config/env_test.go
+++ b/pkg/resourcemanager/config/env_test.go
@ -0,0 +1,34 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+package config_test
+
+import (
+	"testing"
+
+	"github.com/gobuffalo/envy"
+	"github.com/stretchr/testify/require"
+
+	"github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
+)
+
+const parseTestVar = "---TestParseStringListFromEnvironment---"
+
+func checkValue(value string) []string {
+	// Try to avoid polluting global state, although there's no way to
+	// unset a variable.
+	oldValue := envy.Get(parseTestVar, "")
+	envy.Set(parseTestVar, value)
+	result := config.ParseStringListFromEnvironment(parseTestVar)
+	envy.Set(parseTestVar, oldValue)
+	return result
+}
+
+func TestParseStringListFromEnvironment(t *testing.T) {
+	require := require.New(t)
+	require.Empty(checkValue(""))
+	require.Empty(checkValue("    "))
+	require.Equal(checkValue("a"), []string{"a"})
+	require.Equal(checkValue("a,b,c,d"), []string{"a", "b", "c", "d"})
+	require.Equal(checkValue("a , b, c "), []string{"a", "b", "c"})
+}