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

[boskoop]

Which version of kube-score are you using?

kube-score version: 1.17.0, commit: 0b3f154, 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

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

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]

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