Skip to content
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

Implement feature request: To detect duplicate defined environment variables #451

Merged
merged 5 commits into from
Mar 28, 2022

Conversation

ryo-yamaoka
Copy link
Contributor

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
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

Comment on lines +413 to +414
diff := cmp.Diff(expected, actual)
assert.Empty(t, diff)
Copy link
Contributor Author

@ryo-yamaoka ryo-yamaoka Mar 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because assert.ElementsMatch() is so difficult to understand when error.

@ryo-yamaoka ryo-yamaoka marked this pull request as ready for review March 27, 2022 16:01
Copy link
Owner

@zegl zegl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I just have two smaller suggestions to the text/copy 👍

score/container/container.go Outdated Show resolved Hide resolved
score/container/container.go Outdated Show resolved Hide resolved
ryo-yamaoka and others added 4 commits March 28, 2022 10:21
Accept suggestion: fix English

Co-authored-by: Gustav Westling <gustav@westling.dev>
Accept suggestion: fix English

Co-authored-by: Gustav Westling <gustav@westling.dev>
@ryo-yamaoka
Copy link
Contributor Author

@zegl @atombrella Thanks for review, I fixed those.

Copy link
Owner

@zegl zegl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 28, 2022

Build succeeded:

@bors bors bot merged commit bda4991 into zegl:master Mar 28, 2022
@ryo-yamaoka ryo-yamaoka deleted the impl-detect-duped-envvars branch March 29, 2022 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Detect duplicate environment variables defined
3 participants