additionalProperties false on top level of sub-chart

[MrFreezeDZ]

I have an umbrella helm chart. I really like the simplicity of the helm-schema executable. The only problem I am facing is, that keys on the top level in the values.yaml of a sub chart can be added because the values.schema.json does not set the additionalProperties=false for this type of values.

Example: Folder structure:

umbrella-chart
├── charts
│   └── first-sub-chart
│       ├── Chart.yaml
│       ├── values.yaml
│       └── templates
├── Chart.yaml
├── values.yaml
└── templates

values.yaml content of first-sub-chart:

someKey:
  someSubKey: someValue

Chart.yaml dependencies part of umbrella-chart:

dependencies:
  - name: first-sub-chart
    version: "1.0.0-SNAPSHOT"

Then I run helm-schema on the umbrella-chart folder to create the values.schema.json

When I call helm like this, it will not fail, but I think it should:

helm lint umbrella-chart --set first-sub-chart.someUnknownKey=someValue

When I call helm lint like this it will fail like presumed:

helm lint umbrella-chart --set first-sub-chart.someKey.someUnknownKey=someValue

[dadav]

Hi @MrFreezeDZ,

I think the problem here is that you didn't created a jsonschema for the first-sub-chart chart too..?

I tried to reproduce the issue and this only happened when I created the jsonschema for the umbrella, but not for the dependency.

[MrFreezeDZ]

Hi @dadav thank you for investigating. You are correct, I have one umbrella chart, which I maintain. The sub charts are not under my control. I wanted to use the helm-schema executable to generate the values.schema.json so that all key names of the umbrella-chart and all it's sub-charts can be validated. I wanted to use that mechanism to get informed when value keys in a sub-chart are changed so that my values would not overwrite these changed keys anymore. I thought it is a feature of helm-schema to generate the values.schema.json for an umbrella-chart and all sub charts in one invocation recursively. This already works for alle keys except the ones mentioned here in this issue. I used version 0.14.1 of helm-schema for this. And this version generated the values.schema.json for the sub-charts and combined all of the sub-chart values.schema.json into one values.schema.json in the umbrella chart. At least this is how I think it worked. Only the top level keys of the sub-charts did not have the additionalProperties=false in the values.schema.json of the umbrella-chart then.

If this should be how helm-schema should work, then please close this ticket and I have to search another way to accomplish, what I am looking for.

[yashmehrotra]

@MrFreezeDZ @dadav

I had the same usecase, had 5 subcharts who I wanted to ignore for schema validation

I modified the code a bit to get it working for me, heres the patch:

diff --git a/cmd/helm-schema/main.go b/cmd/helm-schema/main.go
index 117ad34..5632d4c 100644
--- a/cmd/helm-schema/main.go
+++ b/cmd/helm-schema/main.go
@@ -197,6 +200,11 @@ loop:
 		}
 
 		log.Debugf("Processing result for chart: %s (%s)", result.Chart.Name, result.ChartPath)
+		var depNames []string
+
+		for _, dep := range result.Chart.Dependencies {
+			depNames = append(depNames, dep.Name)
+		}
 		if !noDeps {
 			chartNameToResult[result.Chart.Name] = result
 			log.Debugf("Stored chart %s in chartNameToResult", result.Chart.Name)
@@ -262,6 +270,23 @@ loop:
 			}
 		}
 
+		oldRequired := result.Schema.Required.Strings
+		var newRequired []string
+		for _, n := range oldRequired {
+			if !slices.Contains(depNames, n) {
+				newRequired = append(newRequired, n)
+			}
+		}
+		result.Schema.Required.Strings = newRequired
+		for p := range result.Schema.Properties {
+			if slices.Contains(depNames, p) {
+				result.Schema.Properties[p] = &schema.Schema{
+					AdditionalProperties: true,
+					Type:                 []string{"object"},
+				}
+			}
+		}
+
 		jsonStr, err := result.Schema.ToJson()
 		if err != nil {
 			log.Error(err)

Let me know if you are interested in merging something like this under a cli flag, I'll clean it up and raise a PR

[xmj]

@MrFreezeDZ @dadav

I had the same usecase, had 5 subcharts who I wanted to ignore for schema validation

I modified the code a bit to get it working for me, heres the patch:

diff --git a/cmd/helm-schema/main.go b/cmd/helm-schema/main.go
index 117ad34..5632d4c 100644
--- a/cmd/helm-schema/main.go
+++ b/cmd/helm-schema/main.go
@@ -197,6 +200,11 @@ loop:
 		}
 
 		log.Debugf("Processing result for chart: %s (%s)", result.Chart.Name, result.ChartPath)
+		var depNames []string
+
+		for _, dep := range result.Chart.Dependencies {
+			depNames = append(depNames, dep.Name)
+		}
 		if !noDeps {
 			chartNameToResult[result.Chart.Name] = result
 			log.Debugf("Stored chart %s in chartNameToResult", result.Chart.Name)
@@ -262,6 +270,23 @@ loop:
 			}
 		}
 
+		oldRequired := result.Schema.Required.Strings
+		var newRequired []string
+		for _, n := range oldRequired {
+			if !slices.Contains(depNames, n) {
+				newRequired = append(newRequired, n)
+			}
+		}
+		result.Schema.Required.Strings = newRequired
+		for p := range result.Schema.Properties {
+			if slices.Contains(depNames, p) {
+				result.Schema.Properties[p] = &schema.Schema{
+					AdditionalProperties: true,
+					Type:                 []string{"object"},
+				}
+			}
+		}
+
 		jsonStr, err := result.Schema.ToJson()
 		if err != nil {
 			log.Error(err)

Let me know if you are interested in merging something like this under a cli flag, I'll clean it up and raise a PR

Hi, I'd be interested in this.

Was recursively checking all dependencies' schemas a design choice?

[MrFreezeDZ]

@yashmehrotra that looks good to me.

If I understood it correct, your code adds

AdditionalProperties: true, Type: []string{"object"},

to every result.Schema.Properties[p] where p is the name of a sub-chart. The name of the sub-chart also belongs to a key in the umbrella-chart values.

So if the last comma in the line Type: []string{"object"}, is no problem and the AdditionalProperties: true results in a "additionalProperties": false in the values.schema.json in our desired place, this is what I am interested in, too.

[dadav]

So, to summarize, @MrFreezeDZ would like to have AdditionalProperties set to false, so that the validation fails if the subcharts values.yaml file changed somehow. On the otherside @yashmehrotra wants to ignore the validation of additional subchart values.

Sounds like we need to add a flag for this :)

[dadav]

Was recursively checking all dependencies' schemas a design choice?

Well, yes. In my case I had a helm chart with lots of dependencies which were all under my control. I see that this is not always the case and needs some reconsideration.

[dadav]

@MrFreezeDZ @dadav I had the same usecase, had 5 subcharts who I wanted to ignore for schema validation I modified the code a bit to get it working for me, heres the patch: diff --git a/cmd/helm-schema/main.go b/cmd/helm-schema/main.go index 117ad34..5632d4c 100644 --- a/cmd/helm-schema/main.go +++ b/cmd/helm-schema/main.go @@ -197,6 +200,11 @@ loop: }

	log.Debugf("Processing result for chart: %s (%s)", result.Chart.Name, result.ChartPath)

•   var depNames []string
  

•   for _, dep := range result.Chart.Dependencies {
  

•   	depNames = append(depNames, dep.Name)
  

•   }
    if !noDeps {
    	chartNameToResult[result.Chart.Name] = result
    	log.Debugf("Stored chart %s in chartNameToResult", result.Chart.Name)
  

@@ -262,6 +270,23 @@ loop: } }

•   oldRequired := result.Schema.Required.Strings
  

•   var newRequired []string
  

•   for _, n := range oldRequired {
  

•   	if !slices.Contains(depNames, n) {
  

•   		newRequired = append(newRequired, n)
  

•   	}
  

•   }
  

•   result.Schema.Required.Strings = newRequired
  

•   for p := range result.Schema.Properties {
  

•   	if slices.Contains(depNames, p) {
  

•   		result.Schema.Properties[p] = &schema.Schema{
  

•   			AdditionalProperties: true,
  

•   			Type:                 []string{"object"},
  

•   		}
  

•   	}
  

•   }
  

•   jsonStr, err := result.Schema.ToJson()
    if err != nil {
    	log.Error(err)
  

Let me know if you are interested in merging something like this under a cli flag, I'll clean it up and raise a PR

Please do :) Looks like a good solution to me.