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

Configuration version and kind are not validated #1237

Closed
kke opened this issue Nov 2, 2021 · 6 comments
Closed

Configuration version and kind are not validated #1237

kke opened this issue Nov 2, 2021 · 6 comments
Labels
bug Something isn't working

Comments

@kke
Copy link
Contributor

kke commented Nov 2, 2021

The k0s.yaml "header" fields apiVersion and kind are not checked.

$ cat k0s.yaml
apiVersion: hessu
kind: hopo
spec:
  api:
    address: 10.0.0.1
$ k0s validate config -c k0s.yaml
# no error
$ k0s controller -c k0s.yaml
# no error
@kke kke added the bug Something isn't working label Nov 2, 2021
@mviitane
Copy link
Contributor

mviitane commented Nov 3, 2021

Also these kind of spelling mistakes go through the validation without any error:

spec:
  api:
    addressWRONG: 172.1.1.1
    sansWRONG:
    - 172.x.x.x
  storageWRONG:
    type: etcd
    etcd:
      peerAddress: 172.x.x.x
  network:
    podCIDR__WRONG: 10.x.0.0/16

@kke
Copy link
Contributor Author

kke commented Nov 3, 2021

That's odd, because the config is read using the strict yaml parser with DisallowUnknownFields enabled 🤔

@kke
Copy link
Contributor Author

kke commented Nov 4, 2021

The field validation, btw, if it would work, would say json instead of yaml, because the yaml parser from google converts the yaml to json and the decodes it again (for reasons): error unmarshaling JSON: while decoding JSON: json: unknown field "stringB"'

@kke
Copy link
Contributor Author

kke commented Nov 4, 2021

I don't understand why the field validation does not work.

All config loading is done via ConfigFromString:

func ConfigFromString(yml string, dataDir string) (*ClusterConfig, error) {
	config := DefaultClusterConfig(dataDir)
	err := strictyaml.YamlUnmarshalStrictIgnoringFields([]byte(yml), config, "interval")
	if err != nil {
		return config, err
	}
	if config.Spec == nil {
		config.Spec = DefaultClusterSpec(dataDir)
	}
	return config, nil
}

The strictyaml thing has tests and it does catch unknown fields.

A test like this however fails:

func TestUnknownFieldValidation(t *testing.T) {
	_, err := ConfigFromString(`
apiVersion: k0s.k0sproject.io/v1beta1
kind: ClusterConfig
unknown: 1`, dataDir)

	assert.Error(t, err)
}
    clusterconfig_types_test.go:41:
        	Error Trace:	clusterconfig_types_test.go:41
        	Error:      	An error is expected but got nil.
        	Test:       	TestUnknownFieldValidation

@kke
Copy link
Contributor Author

kke commented Nov 4, 2021

Aha! Possibly because of the UnmarshalJSON function in ClusterConfig which uses json.Unmarshal.

@kke
Copy link
Contributor Author

kke commented Nov 25, 2021

Fixed by #1246

@kke kke closed this as completed Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants