JSON schema validation error when using variable imagePullPolicy

[matthias4217]

One of my helm charts let users configure various variable put into hull.config.specific. It works well, except for the imagePullPolicy. Below is part of the chart's value :

hull:
  config:
    specific:
      image:
        pullPolicy: Never
  objects:
    deployment:
      nginx:
        pod:
          containers:
            nginx:
              imagePullPolicy: _HT*hull.config.specific.image.pullPolicy

When doing a helm template, I've got a schema validation error :

Error: values don't meet the specifications of the schema(s) in the following chart(s):
hull:
- (root): Must validate at least one schema (anyOf)
- objects.deployment.nginx: Must validate at least one schema (anyOf)
- objects.deployment.nginx.pod.containers.nginx: Must validate at least one schema (anyOf)
- objects.deployment.nginx.pod.containers.nginx.imagePullPolicy: objects.deployment.nginx.pod.containers.nginx.imagePullPolicy must be one of the following: "Always", "IfNotPresent", "Never"
- objects.deployment.nginx.pod.containers.nginx: Must validate all the schemas (allOf)
- objects.deployment.nginx.pod: Must validate all the schemas (allOf)
- objects.deployment.nginx: Must validate all the schemas (allOf)

I've taken a look at hull's values.schema.json, and there is the following for imagePullPolicy :

"imagePullPolicy": {
    "description": "Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. Cannot be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images\n\nPossible enum values:\n - `\"Always\"` means that kubelet always attempts to pull the latest image. Container will fail If the pull fails.\n - `\"IfNotPresent\"` means that kubelet pulls if the image isn't present on disk. Container will fail if the image isn't present and the pull fails.\n - `\"Never\"` means that kubelet never pulls an image, but only uses a local image. Container will fail if the image isn't present",
    "enum": [
        "Always",
        "IfNotPresent",
        "Never"
    ],
    "anyOf": [
        {
            "$ref": "#/definitions/hull.Transformation.Pattern"
        },
        {
            "type": "string"
        }
    ]
}

I'm not 100% knowledgeable, but shouldn't the enum part be inside the anyOf ? I've tried changing this to the following, and it seems to work fine. I'll open a pull request with these changes.

"imagePullPolicy": {
    "description": "Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. Cannot be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images\n\nPossible enum values:\n - `\"Always\"` means that kubelet always attempts to pull the latest image. Container will fail If the pull fails.\n - `\"IfNotPresent\"` means that kubelet pulls if the image isn't present on disk. Container will fail if the image isn't present and the pull fails.\n - `\"Never\"` means that kubelet never pulls an image, but only uses a local image. Container will fail if the image isn't present",
    "anyOf": [
        {
            "$ref": "#/definitions/hull.Transformation.Pattern"
        },
        {
            "enum": [
                "Always",
                "IfNotPresent",
                "Never"
            ]
        }
    ]
},

[gre9ory]

To me this change looks 100% correct.

Regarding the anyOf:

• the hull.Transformation.Pattern covers all the _HT variants and _HULL_TRANSFORMATION_ prefixed strings so you can put a transformation in place of an actual concrete value.
• the other part of the anyOf (here "type": "string") covers all allowed concrete values - which in this case should be the enum values as stated. According to JSON schema the enum values can be of any type but if we put in the string values here this should make an additional "type": "string" next to the enum spec obsolete I think.

Due to the huge amount of data and initial work on the values.schema.json it is possible that similar (hopefully non-major) issues may be lurking there too.

Thanks @matthias4217 for your contribution! The tests have been successful against all Helm versions (note there is one "failing" test vidispine.hull.test but this is due to a GitHub issue and can be ignored for now - still need a way to fix this inconvenience).

Will create a batch of new releases with this fix. FYI the strategy I followed for this is to merge the changes for new releases including documentation and chart version bump into last three branches named fixes-1.xx. Then I open PRs to merge these to the corresponding release-1.xx branches hereby running all gated tests. After merges, the actual releases are created (on the Azure Dev Ops side at the moment) and lastly the main branch is updated from the latest release-1.xx branch to match the latest charts state.

[matthias4217]

Okay thanks ! So, next time I open a pull request, should I do it against the fixes-1.xx branches instead of main ?

[gre9ory]

That would for sure save me some effort so feel free if you want to but - at least for now - I am fine with someone forking and merging to main and me doing the rest. At the moment the additional effort for me is reasonable.

Btw fixed that failing AzureDevOps (dummy) test so they all get green on PRs now 👍
Also added a test to verify imagePullRequests can be used in the way you intended.

Releases:

1.23.15
1.24.10
1.25.9