-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kubeconform fails for object value=null #123
Comments
Can you provide an example of a manifest with |
Gosh @eyarz you're fast 🤣 💯 Same question ;) |
Hi, yes of course i have an example ;) claim.yml
xrd.json
command:
result:
|
"compositionRevisionRef": {
"properties": {
"name": {
"type": "string"
}
},
"required": [
"name" According to the JSON schema you attached, the key Therefore, it's the expected behavior that Kubconform will fail when |
@eyarz are you sure? further this is example is accepted by kubernetes and kubectl without any issues imho a valid violation would be:
|
I'm not a JSON schema expert, but I know that the keyword But we need to clarify something, Kubeconfrom checks manifest against a provided schema(s). If you think If you provide me with the CRD, I can look into why |
@eyarz i guess we have a misunderstanding here
but
and but with e.g. |
Thank you for the clarification, now we are synced :) The problem is that your schema states that So the issue is still that the manifest doesn't match the schema - you can also check it out here: |
Hi, okay.. i have seen that when i would change the json to be like this
its working.. but now its getting curious :P so maybe this script needs to be updated? |
It's not my script so I can't take credit for it 😅 The interesting question is if this is also how it's defined in the CRD or if it's something that is happening when translating the openAPI into JSON schema. If you give a link to the CRD itself, I can look into it :) |
crd.yml
|
So it's the same definition in the openAPI spec: compositionRevisionRef:
properties:
name:
type: string
required:
- name Therefore you can fix it locally or open a PR in the project that provides this CRD (crossplane?). |
yep that snippet you have posted is correct.. but the problem is that please do not reference the name property everytime.. this is fine.. its all about the |
@yannh sorry for the late response but i have tested the script from @tricktron and its producing the same schema and thus will also not work. its related directly to "how is a yaml applied" and not the "result" itself.. so guess the best way to fix this is to have the schema generator to add |
@yannh i got good news.. i got it fixed by checking the script code and the upstream and have seen that you have might missed an introduction while porting the script. in the upstream the following method is called but this is not called in your script, so adding the following will fix it
maybe you can add the allow_null_optional_fields to your hosted script :) |
@eloo-abi Why do you have/want to have optional null values in your crd? Can't you just remove them? I want to understand your use case🙃. |
@tricktron to delete a previous set or default key by setting the value to null |
@eloo-abi I see. I mostly use kustomize and helm in a gitops style and thus hardly ever need to remove an already set key. That means that every optional field should allow to be null. I am not sure though if the provided |
we do use this as well but from time to time helm chart or requirements are changing and thus something need to be disabled again ;) so far i have not seen any issues in our case but i also don't have checked it that deep.
but thats from the "patching" perspective this is true and must also be considered. |
HI, I bumped into this while validating Prometheus CRDs where there are multiple properties (e.g.: serviceMonitorSelector, serviceMonitorNamespaceSelector, podMonitorSelector, etc.) that could be either
The example's CRD is defined here, so the datree's jsonschema is correct. Is there any workaround for kubeconform to pass this scenario? |
Hi,
just have seen that
key: null
is validated correctly.Also in kubeval there was similar issue where it is indicated that
key: null
is a validobject
or at least should be expectedinstrumenta/kubeval#168
The text was updated successfully, but these errors were encountered: