sparkles: Integrate schema validation (#853)

* Change ValidationError to return the actual error Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Add validate command Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Warn validation errors when scanning configs Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Lint Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Add schema command to print config json schema Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Add strict-validations flag Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Lint and remove focus Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Rename command schema to print-schema Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Fix issue by reading originalData Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Lint Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Remove test from removed feature Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Add comments to exported functions Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Lint Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Add test for validate.go Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Remove focus Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Add more tests for root schema Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> * Add more tests Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> --------- Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com> Co-authored-by: Itxaka <itxaka.garcia@spectrocloud.com>
2025-09-18 16:06:58 +00:00 · 2023-02-14 16:15:13 +01:00
parent a09db09db7
commit 5c57dcebdf
8 changed files with 238 additions and 120 deletions
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -12,6 +12,7 @@ import (
 	retry "github.com/avast/retry-go"
 	"github.com/imdario/mergo"
 	"github.com/itchyny/gojq"
+	schema "github.com/kairos-io/kairos/pkg/config/schemas"
 	"github.com/kairos-io/kairos/pkg/machine"
 	"github.com/kairos-io/kairos/sdk/bundles"
 	"github.com/kairos-io/kairos/sdk/unstructured"
@@ -123,6 +124,8 @@ func (c Config) Query(s string) (res string, err error) {
 	s = fmt.Sprintf(".%s", s)
 	jsondata := map[string]interface{}{}

+	// c.String() takes the original data map[string]interface{} and Marshals into YAML, then here we unmarshall it again?
+	// we should be able to use c.originalData and copy it to jsondata
 	err = yaml.Unmarshal([]byte(c.String()), &jsondata)
 	if err != nil {
 		return
@@ -151,6 +154,11 @@ func (c Config) Query(s string) (res string, err error) {
 	return
 }

+// HasConfigURL returns true if ConfigURL has been set and false if it's empty.
+func (c Config) HasConfigURL() bool {
+	return c.ConfigURL != ""
+}
+
 func allFiles(dir []string) []string {
 	files := []string{}
 	for _, d := range dir {
@@ -182,35 +190,10 @@ func Scan(opts ...Option) (c *Config, err error) {
 		}
 	}

-	if c.ConfigURL != "" {
-		var body []byte
-
-		err := retry.Do(
-			func() error {
-				resp, err := http.Get(c.ConfigURL)
-				if err != nil {
-					return err
-				}
-				defer resp.Body.Close()
-
-				body, err = io.ReadAll(resp.Body)
-				if err != nil {
-					return err
-				}
-
-				return nil
-			},
-		)
-
+	if c.HasConfigURL() {
+		err = c.fetchRemoteConfig()
 		if err != nil {
-			return c, fmt.Errorf("could not merge configs: %w", err)
-		}
-
-		yaml.Unmarshal(body, c)               //nolint:errcheck
-		yaml.Unmarshal(body, &c.originalData) //nolint:errcheck
-
-		if exists, header := HasHeader(string(body), ""); exists {
-			c.header = header
+			return c, err
 		}
 	}

@@ -218,6 +201,32 @@ func Scan(opts ...Option) (c *Config, err error) {
 		c.header = DefaultHeader
 	}

+	finalYAML, err := yaml.Marshal(c.originalData)
+	if !o.NoLogs && err != nil {
+		fmt.Printf("WARNING: %s\n", err.Error())
+	}
+
+	kc, err := schema.NewConfigFromYAML(string(finalYAML), schema.RootSchema{})
+	if err != nil {
+		if !o.NoLogs && !o.StrictValidation {
+			fmt.Printf("WARNING: %s\n", err.Error())
+		}
+
+		if o.StrictValidation {
+			return c, fmt.Errorf("ERROR: %s", err.Error())
+		}
+	}
+
+	if !kc.IsValid() {
+		if !o.NoLogs && !o.StrictValidation {
+			fmt.Printf("WARNING: %s\n", kc.ValidationError.Error())
+		}
+
+		if o.StrictValidation {
+			return c, fmt.Errorf("ERROR: %s", kc.ValidationError.Error())
+		}
+	}
+
 	return c, nil
 }

@@ -379,3 +388,37 @@ func parseConfig(dir []string, nologs bool) *Config {

 	return c
 }
+
+func (c *Config) fetchRemoteConfig() error {
+	var body []byte
+
+	err := retry.Do(
+		func() error {
+			resp, err := http.Get(c.ConfigURL)
+			if err != nil {
+				return err
+			}
+			defer resp.Body.Close()
+
+			body, err = io.ReadAll(resp.Body)
+			if err != nil {
+				return err
+			}
+
+			return nil
+		},
+	)
+
+	if err != nil {
+		return fmt.Errorf("could not merge configs: %w", err)
+	}
+
+	yaml.Unmarshal(body, c)               //nolint:errcheck
+	yaml.Unmarshal(body, &c.originalData) //nolint:errcheck
+
+	if exists, header := HasHeader(string(body), ""); exists {
+		c.header = header
+	}
+
+	return nil
+}
--- a/pkg/config/config_test.go
+++ b/pkg/config/config_test.go
@@ -55,7 +55,7 @@ var _ = Describe("Config", func() {
 			err := os.WriteFile(filepath.Join(d, "b"), []byte(`zz.foo="baa" options.foo=bar`), os.ModePerm)
 			Expect(err).ToNot(HaveOccurred())

-			c, err := Scan(MergeBootLine, WithBootCMDLineFile(filepath.Join(d, "b")))
+			c, err := Scan(MergeBootLine, WithBootCMDLineFile(filepath.Join(d, "b")), NoLogs, StrictValidation(false))
 			Expect(err).ToNot(HaveOccurred())
 			headerCheck(c)
 			Expect(c.Options["foo"]).To(Equal("bar"))
@@ -80,7 +80,7 @@ c: d
 			err = os.WriteFile(filepath.Join(d, "b.yaml"), []byte(c2), os.ModePerm)
 			Expect(err).ToNot(HaveOccurred())

-			c, err := Scan(Directories(d))
+			c, err := Scan(Directories(d), NoLogs, StrictValidation(false))
 			Expect(err).ToNot(HaveOccurred())
 			Expect(c).ToNot(BeNil())
 			providerCfg := &TConfig{}
@@ -115,7 +115,7 @@ kairos:
 `), os.ModePerm)
 			Expect(err).ToNot(HaveOccurred())

-			c, err := Scan(Directories(d))
+			c, err := Scan(Directories(d), NoLogs, StrictValidation(false))
 			Expect(err).ToNot(HaveOccurred())
 			Expect(c).ToNot(BeNil())
 			providerCfg := &TConfig{}
@@ -148,7 +148,7 @@ bb:
 			err = os.WriteFile(filepath.Join(d, "b"), []byte(`zz.foo="baa" options.foo=bar`), os.ModePerm)
 			Expect(err).ToNot(HaveOccurred())

-			c, err := Scan(Directories(d), MergeBootLine, WithBootCMDLineFile(filepath.Join(d, "b")))
+			c, err := Scan(Directories(d), MergeBootLine, WithBootCMDLineFile(filepath.Join(d, "b")), NoLogs, StrictValidation(false))
 			Expect(err).ToNot(HaveOccurred())
 			Expect(c.Options["foo"]).To(Equal("bar"))

@@ -170,7 +170,7 @@ config_url: "https://gist.githubusercontent.com/mudler/ab26e8dd65c69c32ab2926857
 			err := os.WriteFile(filepath.Join(d, "test.yaml"), []byte(cc), os.ModePerm)
 			Expect(err).ToNot(HaveOccurred())

-			c, err := Scan(Directories(d))
+			c, err := Scan(Directories(d), NoLogs, StrictValidation(false))
 			Expect(err).ToNot(HaveOccurred())
 			Expect(c).ToNot(BeNil())
 			Expect(len(c.Bundles)).To(Equal(1))
@@ -187,7 +187,7 @@ config_url: "https://gist.githubusercontent.com/mudler/7e3d0426fce8bfaaeb2644f83
 			err := os.WriteFile(filepath.Join(d, "test.yaml"), []byte(cc), os.ModePerm)
 			Expect(err).ToNot(HaveOccurred())

-			c, err := Scan(Directories(d))
+			c, err := Scan(Directories(d), NoLogs, StrictValidation(false))
 			Expect(err).ToNot(HaveOccurred())
 			Expect(c).ToNot(BeNil())
 			Expect(len(c.Bundles)).To(Equal(1))
--- a/pkg/config/options.go
+++ b/pkg/config/options.go
@@ -5,6 +5,7 @@ type Options struct {
 	BootCMDLineFile  string
 	MergeBootCMDLine bool
 	NoLogs           bool
+	StrictValidation bool
 }

 type Option func(o *Options) error
@@ -40,3 +41,11 @@ func Directories(d ...string) Option {
 		return nil
 	}
 }
+
+// StrictValidation sets the strict validation option to true or false.
+func StrictValidation(b bool) Option {
+	return func(o *Options) error {
+		o.StrictValidation = b
+		return nil
+	}
+}
--- a/pkg/config/schemas/install_schema_test.go
+++ b/pkg/config/schemas/install_schema_test.go
@@ -14,7 +14,7 @@ var _ = Describe("Install Schema", func() {
 	var yaml string

 	JustBeforeEach(func() {
-		config, err = NewConfigFromYAML(yaml, "#cloud-config", InstallSchema{})
+		config, err = NewConfigFromYAML(yaml, InstallSchema{})
 		Expect(err).ToNot(HaveOccurred())
 	})

@@ -49,7 +49,7 @@ device: foobar`
 		It("errors", func() {
 			Expect(config.IsValid()).NotTo(BeTrue())
 			Expect(
-				strings.Contains(config.ValidationError(),
+				strings.Contains(config.ValidationError.Error(),
 					"does not match pattern '^(auto|/|(/[a-zA-Z0-9_-]+)+)$'",
 				),
 			).To(BeTrue())
@@ -66,7 +66,7 @@ poweroff: true`

 		It("errors", func() {
 			Expect(config.IsValid()).NotTo(BeTrue())
-			Expect(config.ValidationError()).To(MatchRegexp("value must be false"))
+			Expect(config.ValidationError.Error()).To(MatchRegexp("value must be false"))
 		})
 	})

--- a/pkg/config/schemas/p2p_schema_test.go
+++ b/pkg/config/schemas/p2p_schema_test.go
@@ -3,7 +3,6 @@ package config_test
 import (
 	"strings"

-	. "github.com/kairos-io/kairos/pkg/config"
 	. "github.com/kairos-io/kairos/pkg/config/schemas"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -15,7 +14,7 @@ var _ = Describe("P2P Schema", func() {
 	var yaml string

 	JustBeforeEach(func() {
-		config, err = NewConfigFromYAML(yaml, DefaultHeader, P2PSchema{})
+		config, err = NewConfigFromYAML(yaml, P2PSchema{})
 		Expect(err).ToNot(HaveOccurred())
 	})

@@ -63,8 +62,8 @@ network_token: "b3RwOgogIGRoYWdlX3NpemU6IDIwOTcxNTIwCg=="`
 		})

 		It("errors", func() {
-			Expect(config.ValidationError()).To(MatchRegexp(`value must be one of "master", "worker", "none"`))
 			Expect(config.IsValid()).NotTo(BeTrue())
+			Expect(config.ValidationError.Error()).To(MatchRegexp(`value must be one of "master", "worker", "none"`))
 		})
 	})

@@ -79,7 +78,7 @@ auto:
 		It("errors", func() {
 			Expect(config.IsValid()).NotTo(BeTrue())
 			Expect(
-				strings.Contains(config.ValidationError(), `value must be true`),
+				strings.Contains(config.ValidationError.Error(), `value must be true`),
 			).To(BeTrue())
 		})
 	})
@@ -95,7 +94,7 @@ auto:
 		It("Fails", func() {
 			Expect(config.IsValid()).NotTo(BeTrue())
 			Expect(
-				strings.Contains(config.ValidationError(),
+				strings.Contains(config.ValidationError.Error(),
 					"length must be >= 1, but got 0",
 				),
 			).To(BeTrue())
@@ -127,7 +126,7 @@ auto:

 		It("errors", func() {
 			Expect(config.IsValid()).NotTo(BeTrue())
-			Expect(config.ValidationError()).To(MatchRegexp("(length must be >= 1, but got 0|value must be true)"))
+			Expect(config.ValidationError.Error()).To(MatchRegexp("(length must be >= 1, but got 0|value must be true)"))
 		})
 	})

@@ -144,7 +143,7 @@ auto:

 		It("fails", func() {
 			Expect(config.IsValid()).NotTo(BeTrue())
-			Expect(config.ValidationError()).To(MatchRegexp("must be >= 1 but found 0"))
+			Expect(config.ValidationError.Error()).To(MatchRegexp("must be >= 1 but found 0"))
 		})
 	})

--- a/pkg/config/schemas/root_schema.go
+++ b/pkg/config/schemas/root_schema.go
@@ -2,7 +2,6 @@ package config

 import (
 	"encoding/json"
-	"fmt"
 	"strings"

 	"github.com/santhosh-tekuri/jsonschema/v5"
@@ -26,33 +25,49 @@ type RootSchema struct {

 // KConfig is used to parse and validate Kairos configuration files.
 type KConfig struct {
-	source          string
+	Source          string
 	parsed          interface{}
-	validationError error
+	ValidationError error
 	schemaType      interface{}
-	header          string
 }

-func (kc *KConfig) validate() {
+// GenerateSchema takes the given schema type and builds a JSON Schema out of it
+// if a URL is passed it will also add it as the $schema key, which is useful when
+// defining a version of a Root Schema which will be available online.
+func GenerateSchema(schemaType interface{}, url string) (string, error) {
 	reflector := jsonschemago.Reflector{}

-	generatedSchema, err := reflector.Reflect(kc.schemaType)
+	generatedSchema, err := reflector.Reflect(schemaType)
 	if err != nil {
-		kc.validationError = err
+		return "", err
+	}
+	if url != "" {
+		generatedSchema.WithSchema(url)
 	}

 	generatedSchemaJSON, err := json.MarshalIndent(generatedSchema, "", " ")
 	if err != nil {
-		kc.validationError = err
+		return "", err
+	}
+
+	return string(generatedSchemaJSON), nil
+}
+
+func (kc *KConfig) validate() {
+	generatedSchemaJSON, err := GenerateSchema(kc.schemaType, "")
+	if err != nil {
+		kc.ValidationError = err
+		return
 	}

 	sch, err := jsonschema.CompileString("schema.json", string(generatedSchemaJSON))
 	if err != nil {
-		kc.validationError = err
+		kc.ValidationError = err
+		return
 	}

 	if err = sch.Validate(kc.parsed); err != nil {
-		kc.validationError = err
+		kc.ValidationError = err
 	}
 }

@@ -60,36 +75,29 @@ func (kc *KConfig) validate() {
 func (kc *KConfig) IsValid() bool {
 	kc.validate()

-	return kc.validationError == nil
+	return kc.ValidationError == nil
 }

-// ValidationError returns one of the errors of an invalid schemam rule, when the configuration is valid, then it returns an empty string.
-func (kc *KConfig) ValidationError() string {
-	kc.validate()
+// HasHeader returns true if the config has one of the valid headers.
+func (kc *KConfig) HasHeader() bool {
+	var found bool

-	if kc.validationError != nil {
-		return kc.validationError.Error()
+	availableHeaders := []string{"#cloud-config", "#kairos-config", "#node-config"}
+	for _, header := range availableHeaders {
+		if strings.HasPrefix(kc.Source, header) {
+			found = true
+		}
 	}
-
-	return ""
-}
-
-func (kc *KConfig) hasHeader() bool {
-	return strings.HasPrefix(kc.source, kc.header)
+	return found
 }

 // NewConfigFromYAML is a constructor for KConfig instances. The source of the configuration is passed in YAML and if there are any issues unmarshaling it will return an error.
-func NewConfigFromYAML(s, h string, st interface{}) (*KConfig, error) {
+func NewConfigFromYAML(s string, st interface{}) (*KConfig, error) {
 	kc := &KConfig{
-		source:     s,
-		header:     h,
+		Source:     s,
 		schemaType: st,
 	}

-	if !kc.hasHeader() {
-		return kc, fmt.Errorf("missing %s header", kc.header)
-	}
-
 	err := yaml.Unmarshal([]byte(s), &kc.parsed)
 	if err != nil {
 		return kc, err
--- a/pkg/config/schemas/root_schema_test.go
+++ b/pkg/config/schemas/root_schema_test.go
@@ -67,60 +67,120 @@ func structFieldsContainedInOtherStruct(left, right interface{}) {
 }

 var _ = Describe("Schema", func() {
-	var config *KConfig
-	var err error
-	var yaml string
+	Context("NewConfigFromYAML", func() {
+		var config *KConfig
+		var err error
+		var yaml string

-	JustBeforeEach(func() {
-		config, err = NewConfigFromYAML(yaml, DefaultHeader, RootSchema{})
-	})
+		JustBeforeEach(func() {
+			config, err = NewConfigFromYAML(yaml, RootSchema{})
+		})

-	Context("While the new Schema is not the single source of truth", func() {
-		structFieldsContainedInOtherStruct(Config{}, RootSchema{})
-	})
-	Context("While the new InstallSchema is not the single source of truth", func() {
-		structFieldsContainedInOtherStruct(Install{}, InstallSchema{})
-	})
-	Context("While the new BundleSchema is not the single source of truth", func() {
-		structFieldsContainedInOtherStruct(Bundle{}, BundleSchema{})
-	})
+		Context("While the new Schema is not the single source of truth", func() {
+			structFieldsContainedInOtherStruct(Config{}, RootSchema{})
+		})
+		Context("While the new InstallSchema is not the single source of truth", func() {
+			structFieldsContainedInOtherStruct(Install{}, InstallSchema{})
+		})
+		Context("While the new BundleSchema is not the single source of truth", func() {
+			structFieldsContainedInOtherStruct(Bundle{}, BundleSchema{})
+		})

-	Context("With invalid YAML syntax", func() {
-		BeforeEach(func() {
-			yaml = `#cloud-config
+		Context("With invalid YAML syntax", func() {
+			BeforeEach(func() {
+				yaml = `#cloud-config
 this is:
 - invalid
 yaml`
+			})
+
+			It("errors", func() {
+				Expect(err.Error()).To(MatchRegexp("yaml: line 4: could not find expected ':'"))
+			})
 		})

-		It("errors", func() {
-			Expect(err.Error()).To(MatchRegexp("yaml: line 4: could not find expected ':'"))
-		})
-	})
-
-	Context("With the wrong header", func() {
-		BeforeEach(func() {
-			yaml = `---
-users:
- name: "kairos"
-  passwd: "kairos"`
-		})
-
-		It("errors", func() {
-			Expect(err.Error()).To(MatchRegexp("missing #cloud-config header"))
-		})
-	})
-
-	Context("When `users` is empty", func() {
-		BeforeEach(func() {
-			yaml = `#cloud-config
+		Context("When `users` is empty", func() {
+			BeforeEach(func() {
+				yaml = `#cloud-config
 users: []`
+			})
+
+			It("errors", func() {
+				Expect(err).ToNot(HaveOccurred())
+				Expect(config.IsValid()).NotTo(BeTrue())
+				Expect(config.ValidationError.Error()).To(MatchRegexp("minimum 1 items required, but found 0 items"))
+			})
 		})

-		It("errors", func() {
-			Expect(err).ToNot(HaveOccurred())
-			Expect(config.IsValid()).NotTo(BeTrue())
-			Expect(config.ValidationError()).To(MatchRegexp("minimum 1 items required, but found 0 items"))
+		Context("without a valid header", func() {
+			BeforeEach(func() {
+				yaml = `---
+users:
+  - name: kairos
+    passwd: kairos`
+			})
+
+			It("is successful but HasHeader returns false", func() {
+				Expect(err).ToNot(HaveOccurred())
+				Expect(config.HasHeader()).To(BeFalse())
+			})
 		})
+
+		Context("With a valid config", func() {
+			BeforeEach(func() {
+				yaml = `#cloud-config
+users:
+  - name: kairos
+    passwd: kairos`
+			})
+
+			It("is successful", func() {
+				Expect(err).ToNot(HaveOccurred())
+				Expect(config.HasHeader()).To(BeTrue())
+			})
+		})
+	})
+
+	Context("GenerateSchema", func() {
+		var url string
+		var schema string
+		var err error
+
+		type TestSchema struct {
+			Key interface{} `json:"key,omitemtpy" required:"true"`
+		}
+
+		JustBeforeEach(func() {
+			schema, err = GenerateSchema(TestSchema{}, url)
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		It("does not include the $schema key by default", func() {
+			Expect(strings.Contains(schema, `$schema`)).To(BeFalse())
+		})
+
+		It("can use any type of schma", func() {
+			wants := `{
+ "required": [
+  "key"
+ ],
+ "properties": {
+  "key": {}
+ },
+ "type": "object"
+}`
+			Expect(schema).To(Equal(wants))
+		})
+
+		Context("with a URL", func() {
+			BeforeEach(func() {
+				url = "http://foobar"
+			})
+
+			It("appends the $schema key", func() {
+				Expect(strings.Contains(schema, `$schema": "http://foobar"`)).To(BeTrue())
+			})
+		})
+
 	})
 })
--- a/pkg/config/schemas/users_schema_test.go
+++ b/pkg/config/schemas/users_schema_test.go
@@ -3,7 +3,6 @@ package config_test
 import (
 	"strings"

-	. "github.com/kairos-io/kairos/pkg/config"
 	. "github.com/kairos-io/kairos/pkg/config/schemas"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -15,7 +14,7 @@ var _ = Describe("Users Schema", func() {
 	var yaml string

 	JustBeforeEach(func() {
-		config, err = NewConfigFromYAML(yaml, DefaultHeader, UserSchema{})
+		config, err = NewConfigFromYAML(yaml, UserSchema{})
 		Expect(err).ToNot(HaveOccurred())
 	})

@@ -27,7 +26,7 @@ passwd: foobar`

 		It("errors", func() {
 			Expect(config.IsValid()).NotTo(BeTrue())
-			Expect(config.ValidationError()).To(MatchRegexp("missing properties: 'name'"))
+			Expect(config.ValidationError.Error()).To(MatchRegexp("missing properties: 'name'"))
 		})
 	})

@@ -41,7 +40,7 @@ passwd: "bond"`
 		It("errors", func() {
 			Expect(config.IsValid()).NotTo(BeTrue())
 			Expect(
-				strings.Contains(config.ValidationError(),
+				strings.Contains(config.ValidationError.Error(),
 					"does not match pattern '([a-z_][a-z0-9_]{0,30})'",
 				),
 			).To(BeTrue())