Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ issues:
- goconst
- golint
text: "underscore"
- path: ^pkg\/util\/envvar
linters:
- forbidigo
- path: ^cmd\/(readiness|versionhook|manager)\/main\.go$
linters:
- forbidigo
linters:
enable:
- govet
Expand All @@ -28,10 +34,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
Expand Down
2 changes: 1 addition & 1 deletion controllers/construct/build_statefulset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion controllers/construct/mongodbstatefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions controllers/replica_set_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()).
Expand All @@ -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)
}

Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kube/container/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/kube/container/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/kube/podtemplatespec/podspec_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions pkg/readiness/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/e2eutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/setup/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
27 changes: 14 additions & 13 deletions test/e2e/setup/test_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Loading