crossplane / crossplane

The Cloud Native Control Plane
https://crossplane.io
Apache License 2.0
9.5k stars 955 forks source link

`validate` should handle Kubernetes validation libraries evolving over time #5377

Closed jbw976 closed 2 weeks ago

jbw976 commented 8 months ago

What problem are you facing?

For our crossplane validate command, we are importing upstream kubernetes api-server validation logic and CEL validation support. This raises the question of how we can best keep the crossplane validate command up to date with changes in upstream Kubernetes validation logic and avoid a confusing user experience for valdidate that shows differing results from the users real control plane that may be using a different version of Kubernetes.

We want to avoid scenarios for example where folks find that “this is valid according to crossplane vX, which imports Kubernetes v1.27, but it won’t be valid for my control plane, which is running Kubernetes v1.29”.

We suspect this could be a problem for us because of the compatibility guarantees in the upstream K8s CEL validation logic: https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go#L35

How could Crossplane help solve your problem?

Some options for approaching this problem:

github-actions[bot] commented 5 months ago

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

jbw976 commented 5 months ago

/fresh

enesonus commented 4 months ago

Our current code is using the upstream kubernetes validations. We probably will get some updates from the upstream by using a newer version of packages. For this issue I think the highest value is on CEL validation version selection since the other parts like schema validation are unlikely to get an important update that we wont have in our code (Version availability table of CEL). I think @jbw976’s --kubernetes-version suggestion is pretty straightforward and easy to use in a DevEx sense but there are some trade-offs.

As far as I understood from it’s code and docs kubectl-validate command’s --version flag is only gets used for the CRDs it downloads and not for CEL validation version (I didnt see any mention about CEL validation at all at kubectl-validate). see: https://github.com/kubernetes-sigs/kubectl-validate/blob/23e1b98a9977d61cbf4270df3f423867b3115638/pkg/cmd/validate.go#L180

Current CEL validation which we generate via cel.NewValidator() function does not take any parameters for version but it uses environment.DefaultCompatibilityVersion() under the hood for determining the version while compiling the rules, resulting us to use the minimum compatible version by default (currently 1.29). With the current state of the cel package we need to write our own NewValidator() function that compiles the rules with the given kubernetes version if we want to enable this. This would of course decouple us a bit from the upstream at this particular feature.

In my opinion decoupling from the upstream is not a favorable thing to do for CEL validation version determination. I think giving an error message when we have a kubernetes version different than the environment.DefaultCompatibilityVersion() would be enough for compatibility differencies due to kubernetes version.

github-actions[bot] commented 1 month ago

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.