search

zegl / kube-score

Kubernetes object analysis with recommendations for improved reliability and security. kube-score actively prevents downtime and bugs in your Kubernetes YAML and Charts. Static code analysis for Kubernetes.
https://kube-score.com
MIT License
2.75k stars 177 forks source link

--enable-optional-test for container-memory-requests-equal-limits does not enable test #582

Closed boskoop closed 8 months ago

boskoop commented 8 months ago

Which version of kube-score are you using?

kube-score version: 1.17.0, commit: 0b3f154ca3f06a13323431a7d2199a74a1869fbc, built: 2023-07-06T07:43:29Z

What did you do?

I would like to enable the optional container-memory-requests-equal-limits test, but it seems that the test is SKIPPED nonetheless.

For demonstration, I created the following Pod descriptor. Note that the memory limits/requests differ:

pod.yaml ```yaml $ cat pod.yaml apiVersion: v1 kind: Pod metadata: name: pod-test-1 namespace: testspace labels: app: foo-all-ok spec: containers: - name: foobar image: foo/bar:123 imagePullPolicy: Always resources: requests: cpu: 1 memory: 1Gi ephemeral-storage: 500Mi limits: cpu: 1 memory: 2Gi ephemeral-storage: 500Mi readinessProbe: httpGet: path: /ready port: 8080 livenessProbe: httpGet: path: /live port: 8080 securityContext: privileged: False runAsUser: 30000 runAsGroup: 30000 readOnlyRootFilesystem: True --- apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: name: foo-all-ok-netpol namespace: testspace spec: podSelector: matchLabels: app: foo-all-ok policyTypes: - Egress - Ingress ```

What did you expect to see?

Validation should yield an error when enabling the optional test with --enable-optional-test container-memory-requests-equal-limits. Output should be something like:

$ cat pod.yaml | kube-score score - --enable-optional-test container-memory-requests-equal-limits --output-format ci
...
[CRITICAL] pod-test-1/testspace v1/Pod: (foobar) Memory requests does not match limits

What did you see instead?

Validation does not fail and it even reports the test as skipped with [SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-memory-requests-equal-limits is ignored:

$ cat pod.yaml | kube-score score - --enable-optional-test container-memory-requests-equal-limits --output-format ci
[OK] foo-all-ok-netpol/testspace networking.k8s.io/v1/NetworkPolicy
[OK] foo-all-ok-netpol/testspace networking.k8s.io/v1/NetworkPolicy
[OK] foo-all-ok-netpol/testspace networking.k8s.io/v1/NetworkPolicy
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-resource-requests-equal-limits is ignored
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-ports-check is ignored
[OK] pod-test-1/testspace v1/Pod
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-ephemeral-storage-request-equals-limit is ignored
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod: The pod is not targeted by a service, skipping probe checks.
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-seccomp-profile is ignored
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-memory-requests-equal-limits is ignored
[OK] pod-test-1/testspace v1/Pod
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-cpu-requests-equal-limits is ignored
[OK] pod-test-1/testspace v1/Pod: Pod Topology Spread Constraints

Did I miss something or is this a bug in kube-score 1.17.0?

boskoop commented 8 months ago

It seems to me, the issue was introduced in kube-score 1.15.0.

With 1.14.0, the test was working:

$ cat pod.yaml | kube-score_1.14.0 score - --enable-optional-test container-memory-requests-equal-limits --output-format ci
[OK] foo-all-ok-netpol/testspace networking.k8s.io/v1/NetworkPolicy
[OK] foo-all-ok-netpol/testspace networking.k8s.io/v1/NetworkPolicy
[OK] foo-all-ok-netpol/testspace networking.k8s.io/v1/NetworkPolicy
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[CRITICAL] pod-test-1/testspace v1/Pod: (foobar) Memory requests does not match limits
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod: The pod is not targeted by a service, skipping probe checks.
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod

With 1.15.0, it does no longer:

$ cat pod.yaml | kube-score_1.15.0 score - --enable-optional-test container-memory-requests-equal-limits --output-format ci
[OK] foo-all-ok-netpol/testspace networking.k8s.io/v1/NetworkPolicy
[OK] foo-all-ok-netpol/testspace networking.k8s.io/v1/NetworkPolicy
[OK] foo-all-ok-netpol/testspace networking.k8s.io/v1/NetworkPolicy
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-seccomp-profile is ignored
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-cpu-requests-equal-limits is ignored
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-memory-requests-equal-limits is ignored
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod: The pod is not targeted by a service, skipping probe checks.
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-resource-requests-equal-limits is ignored
[OK] pod-test-1/testspace v1/Pod
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-ports-check is ignored
[OK] pod-test-1/testspace v1/Pod
[OK] pod-test-1/testspace v1/Pod
[SKIPPED] pod-test-1/testspace v1/Pod: Skipped because container-ephemeral-storage-request-equals-limit is ignored

I'm no go programmer, but as far as I understand, the logic in https://github.com/zegl/kube-score/blob/master/scorecard/enabled.go#L9 could be flawed since it only checks for annotations. It was introduced in #489. Also see the comment on line 40:

    // Optional checks are disabled unless explicitly allowed above
    if check.Optional {
        return false
    }
zegl commented 8 months ago

Thank you for investigating and a great bug report, I'll debug and fix the bug.