Skip to content
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

Validate config apiVersion and kind #1245

Closed
wants to merge 1 commit into from

Conversation

kke
Copy link
Contributor

@kke kke commented Nov 4, 2021

Signed-off-by: Kimmo Lehto klehto@mirantis.com

Fixes #1237

Errors out if configuration header apiVersion and kind fields do not have the expected values k0s.k0sproject.io/v1beta1 and ClusterConfig.

@kke kke requested a review from a team as a code owner November 4, 2021 14:21
@kke kke force-pushed the validate-config-header branch 3 times, most recently from b7fc1b2 to 362356c Compare November 9, 2021 14:05
@jnummelin
Copy link
Collaborator

Smoke test for CRD based config changes appears to be broken in some way, does not seem to pass even with few restarts

@mikhail-sakhnov
Copy link
Contributor

looks like actions are stuck? might be rebasing over main will help, we once already had that behavior.

@kke
Copy link
Contributor Author

kke commented Dec 2, 2021

I'm not sure why the config change smoke is failing. Is it building a config that does not have the fields?

@mikhail-sakhnov
Copy link
Contributor

The controller0 logs have ```
time="2021-11-26 08:14:11" level=error msg="cluster-config reconcile failed: failed to validate config: [expected apiVersion: k0s.k0sproject.io/v1beta1 but found expected kind: ClusterConfig but found ]" component=clusterConfig-reconciler


so looks like it generates wrong config

@kke
Copy link
Contributor Author

kke commented Dec 3, 2021

Hmm, in pkg/component/controller/clusterConfig.go there was:

resourceType = v1.TypeMeta{APIVersion: "k0s.k0sproject.io/v1beta1", Kind: "clusterconfigs"}

I wonder if that was intentional. I also wonder what happens if that changes when upgrading.

@kke
Copy link
Contributor Author

kke commented Dec 3, 2021

The client-gen thing does have some plural override flag but I'm not sure if it should be used.

@kke
Copy link
Contributor Author

kke commented Dec 7, 2021

Maybe there should be different config kind for the crd without the node level stuff.

@kke kke marked this pull request as draft December 9, 2021 15:13
Signed-off-by: Kimmo Lehto <klehto@mirantis.com>
@kke kke closed this Jan 10, 2022
@kke kke deleted the validate-config-header branch January 10, 2022 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuration version and kind are not validated
3 participants