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 does not recognize vpa UpdateMode = Off as string #820

Closed itaydj closed 1 year ago

itaydj commented 1 year ago

Describe the bug We are trying to deploy a vpa resource from a helm chart we manage but even though we specify 'Off' as the value for updateMode, Datree check return the following error:

k8s schema validation error: For field spec.updatePolicy.updateMode: Invalid type. Expected: string, given: boolean

To Reproduce Steps to reproduce the behavior:

  1. deploy a vpa resource from a helm chart, use this vpa resource in the chart:

{{- $fullName := include "project.fullname" . -}} apiVersion: "autoscaling.k8s.io/v1" kind: VerticalPodAutoscaler metadata: name: {{ $fullName }} labels: app: {{ .Chart.Name }} release: {{ .Release.Name }} heritage: {{ .Release.Service }} spec: resourcePolicy: containerPolicies:


use the following values.yaml file:
```yaml

vpa:
  updateMode: Off
  maxAllowed:
    cpu: 12
    memory: 25Gi
  minAllowed:
    cpu: 50m
    memory: 256Mi
  1. run Datree against this chart:
    
    helm datree test chart_ name 


**Expected behavior**
Datree will approve the resource with no issues

**Desktop (please complete the following information):**
 - OS: Ubuntu

**Datree version (run `datree version`):**
 - Version: 1.5.30

**Client ID (`cat ~/.datree/config.yaml`):**
 - client_id: 3cskkU34PXkAsYK9pV4M7T
eyarz commented 1 year ago

Hi @itaydj, this is not a bug, this is actually a real misconfiguration. Unquoted Off is interpreted as a boolean by YAML:

If you try to apply this config, it will be rejected by your cluster. Just add quotes to make it a string ("Off"), and you're all set :)

itaydj commented 1 year ago

Hi Eyar, Thanks for the quick reply. before I issued this issue I tried a few settings, one of which was the following:


  updatePolicy:
    updateMode: {{ .Values.vpa.updateMode | quote }}

while the values.yaml file is:


vpa:
  updateMode: Off

This returns the following error:

[V] YAML validation
[X] Kubernetes schema validation
❌  k8s schema validation error: For field spec.updatePolicy.updateMode: spec.updatePolicy.updateMode must be one of the following: "Off", "Initial", "Recreate", "Auto"
[?] Policy check didn't run for this file
(Summary)

Hope this adds more information and thanks again

eyarz commented 1 year ago

This is strange 🤔 Can you use the helm template command and check if the manifest is rendered with Off as a string?

itaydj commented 1 year ago

it does... off_yaml

eyarz commented 1 year ago

What is your helm version? I used helm 3.8.2 & 3.10.1 (latest) to render the manifest, and this is what I got:

...
  updatePolicy:
    updateMode: "false"

so when I use updateMode: {{ .Values.vpa.updateMode | quote }} (template) & updateMode: Off (values), datree returns the following error (which is correct!):

k8s schema validation error: For field spec.updatePolicy.updateMode: spec.updatePolicy.updateMode must be one of the following: "Off", "Initial", "Recreate", "Auto"
itaydj commented 1 year ago

I understand, any suggestions on how I pass the schema validation and have this value set to Off? if we didnt have the Datree tool in our pipeline than i wouldnt even know something is wrong for now I just set it to Off in the template itself (hardcoded the Off value)

eyarz commented 1 year ago

If you pass "Off" in the Values files, the template will be generated correctly. So, now that we know it's a real misconfiguration and not false-positive, why not just leave it as is (updateMode: {{ .Values.vpa.updateMode }})?

If someone uses an invalid value or will not add quotes in the values file, Datree will catch it...

itaydj commented 1 year ago

Thanks again @eyarz

I tried your suggestion but sadly it didnt pass the datree validation and my pipeline failed again with the following message:

[X] Kubernetes schema validation
❌  k8s schema validation error: For field spec.updatePolicy.updateMode: Invalid type. Expected: string, given: boolean

the vpa.yaml updateMode entry is:

updatePolicy:
    updateMode: {{ .Values.vpa.updateMode }}

the values file has this values:


vpa:
  updateMode: "Off"
eyarz commented 1 year ago

It works on my machine 😅 Which helm version are you using? my version is 3.10.1 (latest) image

itaydj commented 1 year ago

haha

we use helm 3.7 in our pipeline we run helm plugin install https://github.com/datreeio/helm-datree and than helm datree test $chartname

eyarz commented 1 year ago

can you try using helm 3.10.1? this is the only thing that I think of that causing the discrepancy between our results...

itaydj commented 1 year ago

yes, I will try on Sunday and update here, I dont want to break the pipeline on a Thursday :-)

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

lesh366 commented 1 year ago

Just posting a workaround since it still gets interpolated as a boolean instead of a string when setting the Off option (no matter which Template Functions and Pipelines transformations we tried)

In our case, we want to have default Off if .Values.vpa.updateMode wasn't specified .. This is how we did it:

{{- $mode := "Off" | quote -}} 
{{ if .Values.vpa.updateMode }}
{{- $mode = .Values.vpa.updateMode | quote -}}
{{ end}}
updateMode: {{ $mode }}