CLOUDP-302068: Add a linter to forbid env var usage

.golangci.yml

@@ -16,6 +16,9 @@ issues:
       - goconst
       - golint
       text: "underscore"
+    - path: pkg/util/envvar/
+      linters:
+        - forbidigo
 linters:
   enable:
     - govet
@@ -28,10 +31,25 @@ linters:
     - rowserrcheck
     - gosec
     - unconvert
+    - forbidigo
 linters-settings:
   gosec:
     excludes:
       - G115
+  forbidigo:
+    forbid:
+      - p: os\.(Getenv|LookupEnv|Environ|ExpandEnv)
+        pkg: os
+        msg: "Reading environemnt variables here is prohibited. Please read environment variables in the main package."
+      - p: os\.(Clearenv|Unsetenv|Setenv)
+        msg: "Modifying environemnt variables is prohibited."
+        pkg: os
+      - p: envvar\.(Read.*?|MergeWithOverride|GetEnvOrDefault)
+        pkg: github.com/mongodb/mongodb-kubernetes-operator/pkg/util/envvar
+        msg: "Using this envvar package here is prohibited. Please work with environment variables in the main package."
+    # Rules with the `pkg` depend on it
+    analyze-types: true
+
 run:
   modules-download-mode: mod
   # timeout for analysis, e.g. 30s, 5m, default is 1m

cmd/manager/main.go

@@ -44,7 +44,7 @@ func configureLogger() (*zap.Logger, error) {
 func hasRequiredVariables(logger *zap.Logger, envVariables ...string) bool {
 	allPresent := true
 	for _, envVariable := range envVariables {
-		if _, envSpecified := os.LookupEnv(envVariable); !envSpecified {
+		if _, envSpecified := os.LookupEnv(envVariable); !envSpecified { // nolint:forbidigo
 			logger.Error(fmt.Sprintf("required environment variable %s not found", envVariable))
 			allPresent = false
 		}
@@ -70,7 +70,7 @@ func main() {
 	}
 
 	// Get watch namespace from environment variable.
-	namespace, nsSpecified := os.LookupEnv(WatchNamespaceEnv)
+	namespace, nsSpecified := os.LookupEnv(WatchNamespaceEnv) // nolint:forbidigo
 	if !nsSpecified {
 		log.Sugar().Fatal("No namespace specified to watch")
 	}
@@ -110,12 +110,12 @@ func main() {
 	// Setup Controller.
 	if err = controllers.NewReconciler(
 		mgr,
-		os.Getenv(construct.MongodbRepoUrlEnv),
-		os.Getenv(construct.MongodbImageEnv),
-		envvar.GetEnvOrDefault(construct.MongoDBImageTypeEnv, construct.DefaultImageType),
-		os.Getenv(construct.AgentImageEnv),
-		os.Getenv(construct.VersionUpgradeHookImageEnv),
-		os.Getenv(construct.ReadinessProbeImageEnv),
+		os.Getenv(construct.MongodbRepoUrlEnv), // nolint:forbidigo
+		os.Getenv(construct.MongodbImageEnv),   // nolint:forbidigo
+		envvar.GetEnvOrDefault(construct.MongoDBImageTypeEnv, construct.DefaultImageType), // nolint:forbidigo
+		os.Getenv(construct.AgentImageEnv),                                                // nolint:forbidigo
+		os.Getenv(construct.VersionUpgradeHookImageEnv),                                   // nolint:forbidigo
+		os.Getenv(construct.ReadinessProbeImageEnv),                                       // nolint:forbidigo
 	).SetupWithManager(mgr); err != nil {
 		log.Sugar().Fatalf("Unable to create controller: %v", err)
 	}

cmd/readiness/main.go

@@ -180,7 +180,7 @@ func performCheckOMMode(health health.Status) bool {
 }
 
 func isHeadlessMode() bool {
-	return os.Getenv(headlessAgent) == "true"
+	return os.Getenv(headlessAgent) == "true" // nolint:forbidigo
 }
 
 func kubernetesClientset() (kubernetes.Interface, error) {

cmd/versionhook/main.go

@@ -32,7 +32,7 @@ func main() {
 
 	logger.Info("Running version change post-start hook")
 
-	if statusPath := os.Getenv(agentStatusFilePathEnv); statusPath == "" {
+	if statusPath := os.Getenv(agentStatusFilePathEnv); statusPath == "" { // nolint:forbidigo
 		logger.Fatalf(`Required environment variable "%s" not set`, agentStatusFilePathEnv)
 		return
 	}
@@ -124,7 +124,7 @@ func waitForAgentHealthStatus() (agent.Health, error) {
 // getAgentHealthStatus returns an instance of agent.Health read
 // from the health file on disk
 func getAgentHealthStatus() (agent.Health, error) {
-	f, err := os.Open(os.Getenv(agentStatusFilePathEnv))
+	f, err := os.Open(os.Getenv(agentStatusFilePathEnv)) // nolint:forbidigo
 	if err != nil {
 		return agent.Health{}, err
 	}
@@ -150,7 +150,7 @@ func readAgentHealthStatus(reader io.Reader) (agent.Health, error) {
 }
 
 func getHostname() string {
-	return os.Getenv("HOSTNAME")
+	return os.Getenv("HOSTNAME") // nolint:forbidigo
 }
 
 // shouldDeletePod returns a boolean value indicating if this pod should be deleted

controllers/construct/build_statefulset_test.go

@@ -109,7 +109,7 @@ func assertStatefulSetIsBuiltCorrectly(t *testing.T, mdb mdbv1.MongoDBCommunity,
 	assert.Len(t, sts.Spec.Template.Spec.Containers[0].Env, 4)
 	assert.Len(t, sts.Spec.Template.Spec.Containers[1].Env, 1)
 
-	managedSecurityContext := envvar.ReadBool(podtemplatespec.ManagedSecurityContextEnv)
+	managedSecurityContext := envvar.ReadBool(podtemplatespec.ManagedSecurityContextEnv) // nolint:forbidigo
 	if !managedSecurityContext {
 		assert.NotNil(t, sts.Spec.Template.Spec.SecurityContext)
 		assert.Equal(t, podtemplatespec.DefaultPodSecurityContext(), *sts.Spec.Template.Spec.SecurityContext)

controllers/construct/mongodbstatefulset.go

@@ -417,7 +417,7 @@ func collectEnvVars() []corev1.EnvVar {
 	})
 
 	addEnvVarIfSet := func(name string) {
-		value := os.Getenv(name)
+		value := os.Getenv(name) // nolint:forbidigo
 		if value != "" {
 			envVars = append(envVars, corev1.EnvVar{
 				Name:  name,

controllers/replica_set_controller.go

@@ -219,7 +219,7 @@ func (r ReplicaSetReconciler) Reconcile(ctx context.Context, request reconcile.R
 	}
 
 	res, err := status.Update(ctx, r.client.Status(), &mdb, statusOptions().
-		withMongoURI(mdb.MongoURI(os.Getenv(clusterDomain))).
+		withMongoURI(mdb.MongoURI(os.Getenv(clusterDomain))). // nolint:forbidigo
 		withMongoDBMembers(mdb.AutomationConfigMembersThisReconciliation()).
 		withStatefulSetReplicas(mdb.StatefulSetReplicasThisReconciliation()).
 		withStatefulSetArbiters(mdb.StatefulSetArbitersThisReconciliation()).
@@ -232,7 +232,7 @@ func (r ReplicaSetReconciler) Reconcile(ctx context.Context, request reconcile.R
 		return res, err
 	}
 
-	if err := r.updateConnectionStringSecrets(ctx, mdb, os.Getenv(clusterDomain)); err != nil {
+	if err := r.updateConnectionStringSecrets(ctx, mdb, os.Getenv(clusterDomain)); err != nil { // nolint:forbidigo
 		r.log.Errorf("Could not update connection string secrets: %s", err)
 	}
 
@@ -515,8 +515,8 @@ func (r ReplicaSetReconciler) ensureAutomationConfig(mdb mdbv1.MongoDBCommunity,
 }
 
 func buildAutomationConfig(mdb mdbv1.MongoDBCommunity, isEnterprise bool, auth automationconfig.Auth, currentAc automationconfig.AutomationConfig, modifications ...automationconfig.Modification) (automationconfig.AutomationConfig, error) {
-	domain := getDomain(mdb.ServiceName(), mdb.Namespace, os.Getenv(clusterDomain))
-	arbiterDomain := getDomain(mdb.ServiceName(), mdb.Namespace, os.Getenv(clusterDomain))
+	domain := getDomain(mdb.ServiceName(), mdb.Namespace, os.Getenv(clusterDomain))        // nolint:forbidigo
+	arbiterDomain := getDomain(mdb.ServiceName(), mdb.Namespace, os.Getenv(clusterDomain)) // nolint:forbidigo
 
 	zap.S().Debugw("AutomationConfigMembersThisReconciliation", "mdb.AutomationConfigMembersThisReconciliation()", mdb.AutomationConfigMembersThisReconciliation())
 
@@ -557,7 +557,7 @@ func buildAutomationConfig(mdb mdbv1.MongoDBCommunity, isEnterprise bool, auth a
 }
 
 func guessEnterprise(mdb mdbv1.MongoDBCommunity, mongodbImage string) bool {
-	overrideAssumption, err := strconv.ParseBool(os.Getenv(construct.MongoDBAssumeEnterpriseEnv))
+	overrideAssumption, err := strconv.ParseBool(os.Getenv(construct.MongoDBAssumeEnterpriseEnv)) // nolint:forbidigo
 	if err == nil {
 		return overrideAssumption
 	}

pkg/kube/container/container_test.go

@@ -146,7 +146,7 @@ func TestMergeEnvs(t *testing.T) {
 		},
 	}
 
-	merged := envvar.MergeWithOverride(existing, desired)
+	merged := envvar.MergeWithOverride(existing, desired) // nolint:forbidigo
 
 	t.Run("EnvVars should be sorted", func(t *testing.T) {
 		assert.Equal(t, "A_env", merged[0].Name)

pkg/kube/container/containers.go

@@ -129,7 +129,7 @@ func WithLifecycle(lifeCycleMod lifecycle.Modification) Modification {
 // WithEnvs ensures all of the provided envs exist in the container
 func WithEnvs(envs ...corev1.EnvVar) Modification {
 	return func(container *corev1.Container) {
-		container.Env = envvar.MergeWithOverride(container.Env, envs)
+		container.Env = envvar.MergeWithOverride(container.Env, envs) // nolint:forbidigo
 	}
 }
 

pkg/kube/podtemplatespec/podspec_template.go

@@ -297,7 +297,7 @@ func FindContainerByName(name string, podTemplateSpec *corev1.PodTemplateSpec) *
 }
 
 func WithDefaultSecurityContextsModifications() (Modification, container.Modification) {
-	managedSecurityContext := envvar.ReadBool(ManagedSecurityContextEnv)
+	managedSecurityContext := envvar.ReadBool(ManagedSecurityContextEnv) // nolint:forbidigo
 	configureContainerSecurityContext := container.NOOP()
 	configurePodSpecSecurityContext := NOOP()
 	if !managedSecurityContext {

pkg/readiness/config/config.go

@@ -43,15 +43,15 @@ func BuildFromEnvVariables(clientSet kubernetes.Interface, isHeadless bool, file
 	var namespace, automationConfigName, hostname string
 	if isHeadless {
 		var ok bool
-		namespace, ok = os.LookupEnv(podNamespaceEnv)
+		namespace, ok = os.LookupEnv(podNamespaceEnv) // nolint:forbidigo
 		if !ok {
 			return Config{}, fmt.Errorf("the '%s' environment variable must be set", podNamespaceEnv)
 		}
-		automationConfigName, ok = os.LookupEnv(automationConfigSecretEnv)
+		automationConfigName, ok = os.LookupEnv(automationConfigSecretEnv) // nolint:forbidigo
 		if !ok {
 			return Config{}, fmt.Errorf("the '%s' environment variable must be set", automationConfigSecretEnv)
 		}
-		hostname, ok = os.LookupEnv(hostNameEnv)
+		hostname, ok = os.LookupEnv(hostNameEnv) // nolint:forbidigo
 		if !ok {
 			return Config{}, fmt.Errorf("the '%s' environment variable must be set", hostNameEnv)
 		}
@@ -85,7 +85,7 @@ func readinessProbeLogFilePath() string {
 }
 
 func GetEnvOrDefault(envVar, defaultValue string) string {
-	value := strings.TrimSpace(os.Getenv(envVar))
+	value := strings.TrimSpace(os.Getenv(envVar)) // nolint:forbidigo
 	if value == "" {
 		return defaultValue
 	}

test/e2e/e2eutil.go

@@ -36,7 +36,7 @@ func TestAnnotations() map[string]string {
 }
 
 func TestDataDir() string {
-	return envvar.GetEnvOrDefault(testDataDirEnv, "/workspace/testdata")
+	return envvar.GetEnvOrDefault(testDataDirEnv, "/workspace/testdata") // nolint:forbidigo
 }
 
 func TlsTestDataDir() string {

test/e2e/setup/setup.go

@@ -42,7 +42,7 @@ const (
 )
 
 func Setup(ctx context.Context, t *testing.T) *e2eutil.TestContext {
-	testCtx, err := e2eutil.NewContext(ctx, t, envvar.ReadBool(performCleanupEnv))
+	testCtx, err := e2eutil.NewContext(ctx, t, envvar.ReadBool(performCleanupEnv)) // nolint:forbidigo
 
 	if err != nil {
 		t.Fatal(err)
@@ -57,7 +57,7 @@ func Setup(ctx context.Context, t *testing.T) *e2eutil.TestContext {
 }
 
 func SetupWithTLS(ctx context.Context, t *testing.T, resourceName string, additionalHelmArgs ...HelmArg) (*e2eutil.TestContext, TestConfig) {
-	textCtx, err := e2eutil.NewContext(ctx, t, envvar.ReadBool(performCleanupEnv))
+	textCtx, err := e2eutil.NewContext(ctx, t, envvar.ReadBool(performCleanupEnv)) // nolint:forbidigo
 
 	if err != nil {
 		t.Fatal(err)
@@ -76,7 +76,7 @@ func SetupWithTLS(ctx context.Context, t *testing.T, resourceName string, additi
 }
 
 func SetupWithTestConfig(ctx context.Context, t *testing.T, testConfig TestConfig, withTLS, defaultOperator bool, resourceName string) *e2eutil.TestContext {
-	testCtx, err := e2eutil.NewContext(ctx, t, envvar.ReadBool(performCleanupEnv))
+	testCtx, err := e2eutil.NewContext(ctx, t, envvar.ReadBool(performCleanupEnv)) // nolint:forbidigo
 
 	if err != nil {
 		t.Fatal(err)

test/e2e/setup/test_config.go

@@ -34,18 +34,19 @@ type TestConfig struct {
 
 func LoadTestConfigFromEnv() TestConfig {
 	return TestConfig{
-		Namespace:               envvar.GetEnvOrDefault(testNamespaceEnvName, "mongodb"),
-		CertManagerNamespace:    envvar.GetEnvOrDefault(testCertManagerNamespaceEnvName, "cert-manager"),
-		CertManagerVersion:      envvar.GetEnvOrDefault(testCertManagerVersionEnvName, "v1.5.3"),
-		OperatorImage:           envvar.GetEnvOrDefault(operatorImageEnvName, "quay.io/mongodb/community-operator-dev:latest"),
-		MongoDBImage:            envvar.GetEnvOrDefault(construct.MongodbImageEnv, "mongodb-community-server"),
-		MongoDBRepoUrl:          envvar.GetEnvOrDefault(construct.MongodbRepoUrlEnv, "quay.io/mongodb"),
-		VersionUpgradeHookImage: envvar.GetEnvOrDefault(construct.VersionUpgradeHookImageEnv, "quay.io/mongodb/mongodb-kubernetes-operator-version-upgrade-post-start-hook:1.0.2"),
-		AgentImage:              envvar.GetEnvOrDefault(construct.AgentImageEnv, "quay.io/mongodb/mongodb-agent-ubi:10.29.0.6830-1"), // TODO: better way to decide default agent image.
-		ClusterWide:             envvar.ReadBool(clusterWideEnvName),
-		PerformCleanup:          envvar.ReadBool(performCleanupEnvName),
-		ReadinessProbeImage:     envvar.GetEnvOrDefault(construct.ReadinessProbeImageEnv, "quay.io/mongodb/mongodb-kubernetes-readinessprobe:1.0.3"),
-		HelmChartPath:           envvar.GetEnvOrDefault(helmChartPathEnvName, "/workspace/helm-charts/charts/community-operator"),
-		LocalOperator:           envvar.ReadBool(LocalOperatorEnvName),
+		Namespace:               envvar.GetEnvOrDefault(testNamespaceEnvName, "mongodb"),                                                                                           // nolint:forbidigo
+		CertManagerNamespace:    envvar.GetEnvOrDefault(testCertManagerNamespaceEnvName, "cert-manager"),                                                                           // nolint:forbidigo
+		CertManagerVersion:      envvar.GetEnvOrDefault(testCertManagerVersionEnvName, "v1.5.3"),                                                                                   // nolint:forbidigo
+		OperatorImage:           envvar.GetEnvOrDefault(operatorImageEnvName, "quay.io/mongodb/community-operator-dev:latest"),                                                     // nolint:forbidigo
+		MongoDBImage:            envvar.GetEnvOrDefault(construct.MongodbImageEnv, "mongodb-community-server"),                                                                     // nolint:forbidigo
+		MongoDBRepoUrl:          envvar.GetEnvOrDefault(construct.MongodbRepoUrlEnv, "quay.io/mongodb"),                                                                            // nolint:forbidigo
+		VersionUpgradeHookImage: envvar.GetEnvOrDefault(construct.VersionUpgradeHookImageEnv, "quay.io/mongodb/mongodb-kubernetes-operator-version-upgrade-post-start-hook:1.0.2"), // nolint:forbidigo
+		// TODO: better way to decide default agent image.
+		AgentImage:          envvar.GetEnvOrDefault(construct.AgentImageEnv, "quay.io/mongodb/mongodb-agent-ubi:10.29.0.6830-1"),                 // nolint:forbidigo
+		ClusterWide:         envvar.ReadBool(clusterWideEnvName),                                                                                 // nolint:forbidigo
+		PerformCleanup:      envvar.ReadBool(performCleanupEnvName),                                                                              // nolint:forbidigo
+		ReadinessProbeImage: envvar.GetEnvOrDefault(construct.ReadinessProbeImageEnv, "quay.io/mongodb/mongodb-kubernetes-readinessprobe:1.0.3"), // nolint:forbidigo
+		HelmChartPath:       envvar.GetEnvOrDefault(helmChartPathEnvName, "/workspace/helm-charts/charts/community-operator"),                    // nolint:forbidigo
+		LocalOperator:       envvar.ReadBool(LocalOperatorEnvName),                                                                               // nolint:forbidigo
 	}
 }