datreeio / datree

Prevent Kubernetes misconfigurations from reaching production (again 😤 )! From code to cloud, Datree provides an E2E policy enforcement solution to run automatic checks for rule violations. See our docs: https://hub.datree.io
https://datree.io
Apache License 2.0
6.39k stars 363 forks source link

'datree kustomize test' is getting false positive error - Incorrect value for key `replicas` #636

Closed HariSekhon closed 2 years ago

HariSekhon commented 2 years ago

Describe the bug Getting an error complaining about incorrect value for replicas key when it is set to 3

To Reproduce

$ git clone https://github.com/HariSekhon/Kubernetes-configs k8s

$ cd k8s

$ datree version
1.4.26

$ datree kustomize test .
>>  File: ../../../../var/folders/30/kxjrq3fj5tqdhsvbj3p9m2fh0000gq/T/datree_kustomize_289484019.yaml

[V] YAML validation
[V] Kubernetes schema validation

[X] Policy check

[X]  Ensure Deployment has more than one replica configured  [1 occurrence]
    — metadata.name: NAME (kind: Deployment)
[*]  Incorrect value for key `replicas` - running 2 or more replicas will increase the availability of the service

(Summary)

- Passing YAML validation: 1/1

- Passing Kubernetes (1.19.0) schema validation: 1/1

- Passing policy check: 0/1

+-----------------------------------+-----------------------+
| Enabled rules in policy “Default” | 20                    |
| Configs tested against policy     | 5                     |
| Total rules evaluated             | 20                    |
| Total rules skipped               | 0                     |
| Total rules failed                | 1                     |
| Total rules passed                | 19                    |
| See all rules in policy           | https://hub.datree.io |
+-----------------------------------+-----------------------+

$ grep replicas: deployment.yaml
  replicas: 3  # XXX: don't define replicas if using HPA, otherwise gets overridden and reset to this number every deploy which could cause load issues

Expected behavior Expected it to pass

Desktop (please complete the following information):

Datree version (run datree version):

HariSekhon commented 2 years ago

Moving the comment suffix to another line made no difference either, and either way it should strip comments before processing (I do this in my code a lot).

eyarz commented 2 years ago

I wonder if the issue is with how we build the manifest with Kustomize or in the (replica) rule logic...

Can you try and run grep replicas: against the temp file that is created by datree? The path for this file is printed before the yaml validation check (e.g. ../../../../var/folders/30/kxjrq3fj5tqdhsvbj3p9m2fh0000gq/T/datree_kustomize_289484019.yaml)

HariSekhon commented 2 years ago

I ran the grep replicas: against the temp file immediately after a new run but the temp file was already gone and I can't see a switch to retain it:


18:54:08 ~/github/k8s $ datree kustomize test .
>>  File: ../../../../var/folders/30/kxjrq3fj5tqdhsvbj3p9m2fh0000gq/T/datree_kustomize_1862986846.yaml

[V] YAML validation
[V] Kubernetes schema validation

[X] Policy check

[X]  Ensure Deployment has more than one replica configured  [1 occurrence]
    — metadata.name: NAME (kind: Deployment)
[*]  Incorrect value for key `replicas` - running 2 or more replicas will increase the availability of the service

(Summary)

- Passing YAML validation: 1/1

- Passing Kubernetes (1.19.0) schema validation: 1/1

- Passing policy check: 0/1

+-----------------------------------+-----------------------+
| Enabled rules in policy “Default” | 20                    |
| Configs tested against policy     | 5                     |
| Total rules evaluated             | 20                    |
| Total rules skipped               | 0                     |
| Total rules failed                | 1                     |
| Total rules passed                | 19                    |
| See all rules in policy           | https://hub.datree.io |
+-----------------------------------+-----------------------+

18:54:13 ~/github/k8s $ grep replicas: ../../../../var/folders/30/kxjrq3fj5tqdhsvbj3p9m2fh0000gq/T/datree_kustomize_1862986846.yaml
grep: ../../../../var/folders/30/kxjrq3fj5tqdhsvbj3p9m2fh0000gq/T/datree_kustomize_1862986846.yaml: No such file or directory
Ben-Zaad commented 2 years ago

@HariSekhon hey I’m Ben from Datree’s Dev team, just wanted to let you know that we are taking care of this bug, and it’ll be fixed within the next few days 😄

dimabru commented 2 years ago

Hi @HariSekhon, This looks like a bug in kustomize. I clone the repo https://github.com/HariSekhon/Kubernetes-configs and ran the following: kustomize build Kubernetes-configs > out.yaml

the content of the file out.yaml can be found here notice line 52 (replicas: 1) that does not exist in the original deployment.yaml file

Since datree is running kusomize command before starting the invocation this seems like an issue in kustomize.

Can you confirm this behavior exists in your end as well?

HariSekhon commented 2 years ago

Debugging the kustomize build output has exposed the cause - the replicas in the out.yaml was indeed set to 1 due to being overridden by a patch.

I've tweaked the patch to 2 to avoid the policy warning which is genuine.

Let's close this, it's a feature not a bug, my apologies.