Merge #451

451: Implement feature request: To detect duplicate defined environment variables r=zegl a=ryo-yamaoka

score/container: To detect duplicate defined environment variables.
    
Fixes #448

### behaviour

```
$ ./kube-score score ~/test.yaml 
apps/v1/Deployment sample-deployment                                          💥
    [CRITICAL] Environment Variable Key Duplication
        · sampleA -> Environment Variable Key Duplication
            Container environment variable key 'SAMPLE_ENV_VAR' duplicated
        · sampleB -> Environment Variable Key Duplication
            Container environment variable key 'SAMPLE_ENV_VAR1' duplicated
        · sampleB -> Environment Variable Key Duplication
            Container environment variable key 'SAMPLE_ENV_VAR2' duplicated
```

```yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: sample-deployment
  labels:
    app: sample
  annotations:
    kube-score/ignore: pod-networkpolicy,container-resources,container-image-pull-policy,container-security-context-privileged,container-security-context-user-group-id,container-security-context-readonlyrootfilesystem,container-ephemeral-storage-request-and-limit,container-image-tag
spec:
  replicas: 1
  selector:
    matchLabels:
      app: sample
  template:
    metadata:
      labels:
        app: sample
    spec:
      containers:
      - name: sampleA
        image: sample:latest
        ports:
        - containerPort: 80
        env:
        - name: SAMPLE_ENV_VAR
          value: sample1
        - name: SAMPLE_ENV_VAR
          value: sample1
      - name: sampleB
        image: sample:latest
        ports:
        - containerPort: 80
        env:
        - name: SAMPLE_ENV_VAR1
          value: sample1
        - name: SAMPLE_ENV_VAR1
          value: sample1
        - name: SAMPLE_ENV_VAR2
          value: sample2
        - name: SAMPLE_ENV_VAR2
          value: sample2
```

Co-authored-by: ryo-yamaoka <20574684+ryo-yamaoka@users.noreply.github.com>

README_CHECKS.md

@@ -33,3 +33,4 @@
 | statefulset-pod-selector-labels-match-template-metadata-labels | StatefulSet | Ensure the StatefulSet selector labels match the template metadata labels. | default |
 | label-values | all | Validates label values | default |
 | horizontalpodautoscaler-has-target | HorizontalPodAutoscaler | Makes sure that the HPA targets a valid object | default |
+| environment-variable-key-duplication | Pod | Makes sure that no duplicated environment variable keys. | default |

go.mod

@@ -3,6 +3,7 @@ module github.com/zegl/kube-score
 require (
 	github.com/eidolon/wordwrap v0.0.0-20161011182207-e0f54129b8bb
 	github.com/fatih/color v1.13.0
+	github.com/google/go-cmp v0.5.7
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.7.1
 	golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1

go.sum

@@ -51,8 +51,9 @@ github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5a
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
-github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o=
+github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g=
 github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=

score/container/container.go

@@ -1,6 +1,7 @@
 package container
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/zegl/kube-score/config"
@@ -20,6 +21,7 @@ func Register(allChecks *checks.Checks, cnf config.Configuration) {
 	allChecks.RegisterPodCheck("Container Ephemeral Storage Request and Limit", "Makes sure all pods have ephemeral-storage requests and limits set", containerStorageEphemeralRequestAndLimit)
 	allChecks.RegisterOptionalPodCheck("Container Ephemeral Storage Request Equals Limit", "Make sure all pods have matching ephemeral-storage requests and limits", containerStorageEphemeralRequestEqualsLimit)
 	allChecks.RegisterOptionalPodCheck("Container Ports Check", "Container Ports Checks", containerPortsCheck)
+	allChecks.RegisterPodCheck("Environment Variable Key Duplication", "Makes sure that duplicated environment variable keys are not duplicated", environmentVariableKeyDuplication)
 }
 
 // containerResources makes sure that the container has resource requests and limits set
@@ -280,3 +282,27 @@ func containerPortsCheck(podTemplate corev1.PodTemplateSpec, typeMeta metav1.Typ
 
 	return
 }
+
+// environmentVariableKeyDuplication checks that no duplicated environment variable keys.
+func environmentVariableKeyDuplication(podTemplate corev1.PodTemplateSpec, _ metav1.TypeMeta) (score scorecard.TestScore) {
+	pod := podTemplate.Spec
+
+	allContainers := pod.InitContainers
+	allContainers = append(allContainers, pod.Containers...)
+
+	score.Grade = scorecard.GradeAllOK
+
+	for _, container := range allContainers {
+		envs := make(map[string]struct{})
+		for _, env := range container.Env {
+			if _, duplicated := envs[env.Name]; duplicated {
+				msg := fmt.Sprintf("Container environment variable key '%s' is duplicated", env.Name)
+				score.AddComment(container.Name, "Environment Variable Key Duplication", msg)
+				score.Grade = scorecard.GradeCritical
+				continue
+			}
+			envs[env.Name] = struct{}{}
+		}
+	}
+	return
+}

score/score_test.go

@@ -4,6 +4,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 	"github.com/zegl/kube-score/config"
 	ks "github.com/zegl/kube-score/domain"
@@ -398,3 +399,42 @@ func TestPodContainerPortsOK(t *testing.T) {
 		EnabledOptionalTests: structMap,
 	}, "Container Ports Check", scorecard.GradeAllOK)
 }
+
+func TestPodEnvOK(t *testing.T) {
+	t.Parallel()
+
+	structMap := make(map[string]struct{})
+	structMap["environment-variable-key-duplication"] = struct{}{}
+
+	testExpectedScoreWithConfig(t, config.Configuration{
+		AllFiles:             []ks.NamedReader{testFile("pod-env-ok.yaml")},
+		EnabledOptionalTests: structMap,
+	}, "Environment Variable Key Duplication", scorecard.GradeAllOK)
+}
+
+func TestPodEnvDuplicated(t *testing.T) {
+	t.Parallel()
+
+	structMap := make(map[string]struct{})
+	structMap["environment-variable-key-duplication"] = struct{}{}
+
+	actual := testExpectedScoreWithConfig(t, config.Configuration{
+		AllFiles:             []ks.NamedReader{testFile("pod-env-duplicated.yaml")},
+		EnabledOptionalTests: structMap,
+	}, "Environment Variable Key Duplication", scorecard.GradeCritical)
+
+	expected := []scorecard.TestScoreComment{
+		{
+			Path:        "foobar",
+			Summary:     "Environment Variable Key Duplication",
+			Description: "Container environment variable key 'bar' is duplicated",
+		},
+		{
+			Path:        "foobar",
+			Summary:     "Environment Variable Key Duplication",
+			Description: "Container environment variable key 'baz' is duplicated",
+		},
+	}
+	diff := cmp.Diff(expected, actual)
+	assert.Empty(t, diff)
+}

score/testdata/pod-env-duplicated.yaml

@@ -0,0 +1,19 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: pod-test-1
+spec:
+  containers:
+  - name: foobar
+    image: foo/bar:latest
+    env:
+    - name: foo
+      value: foo
+    - name: bar
+      value: bar
+    - name: bar
+      value: bar
+    - name: baz
+      value: baz
+    - name: baz
+      value: baz

score/testdata/pod-env-ok.yaml

@@ -0,0 +1,13 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: pod-test-1
+spec:
+  containers:
+  - name: foobar
+    image: foo/bar:latest
+    env:
+    - name: foo
+      value: foo
+    - name: bar
+      value: bar