🐛 Do not merge all cmdline arguments to generic config (#1256)

* Do not merge all cmdline arguments to generic config Instead allow only specific Kairos config ones, this should not be known by the collector but doing it this way as a temporary hack to release 2.0 and then we can do properly Signed-off-by: Mauro Morales <[email protected]> * Remove fmt.Println Signed-off-by: Mauro Morales <[email protected]> * Lint Signed-off-by: Mauro Morales <[email protected]> * imports Signed-off-by: Mauro Morales <[email protected]> * Filter using a cloud config structure Signed-off-by: Mauro Morales <[email protected]> * Pass a filter function Signed-off-by: Mauro Morales <[email protected]> * Exclude collector config from config.Config yaml Signed-off-by: Mauro Morales <[email protected]> * Fix issue with test now that a yaml tag gets ignored Signed-off-by: Mauro Morales <[email protected]> * cleanup FilterKeys func Signed-off-by: Mauro Morales <[email protected]> * Add comment Signed-off-by: Mauro Morales <[email protected]> --------- Signed-off-by: Mauro Morales <[email protected]>
kairos-io · Apr 6, 2023 · 0967f7c · 0967f7c
1 parent 4d16878
commit 0967f7c
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 42 deletions.
diff --git a/pkg/config/collector/collector.go b/pkg/config/collector/collector.go
@@ -12,11 +12,11 @@ import (
 	"time"
 	"unicode"
 
+	"github.com/kairos-io/kairos-sdk/machine"
+
 	"github.com/avast/retry-go"
-	"github.com/google/shlex"
 	"github.com/imdario/mergo"
 	"github.com/itchyny/gojq"
-	"github.com/kairos-io/kairos-sdk/unstructured"
 	"gopkg.in/yaml.v1"
 )
 
@@ -93,13 +93,13 @@ func (cs Configs) Merge() (*Config, error) {
 	return result, nil
 }
 
-func Scan(o *Options) (*Config, error) {
+func Scan(o *Options, filter func(d []byte) ([]byte, error)) (*Config, error) {
 	configs := Configs{}
 
 	configs = append(configs, parseFiles(o.ScanDir, o.NoLogs)...)
 
 	if o.MergeBootCMDLine {
-		cConfig, err := ParseCmdLine(o.BootCMDLineFile)
+		cConfig, err := ParseCmdLine(o.BootCMDLineFile, filter)
 		o.SoftErr("parsing cmdline", err)
 		if err == nil { // best-effort
 			configs = append(configs, cConfig)
@@ -201,41 +201,24 @@ func listFiles(dir string) ([]string, error) {
 
 // ParseCmdLine reads options from the kernel cmdline and returns the equivalent
 // Config.
-func ParseCmdLine(file string) (*Config, error) {
-	result := &Config{}
-
-	if file == "" {
-		file = "/proc/cmdline"
-	}
-	dat, err := os.ReadFile(file)
+func ParseCmdLine(file string, filter func(d []byte) ([]byte, error)) (*Config, error) {
+	result := Config{}
+	dotToYAML, err := machine.DotToYAML(file)
 	if err != nil {
-		return result, err
+		return &result, err
 	}
 
-	d, err := unstructured.ToYAML(stringToConfig(string(dat)))
+	filteredYAML, err := filter(dotToYAML)
 	if err != nil {
-		return result, err
+		return &result, err
 	}
-	err = yaml.Unmarshal(d, &result)
 
-	return result, err
-}
-
-func stringToConfig(s string) Config {
-	v := Config{}
-
-	splitted, _ := shlex.Split(s)
-	for _, item := range splitted {
-		parts := strings.SplitN(item, "=", 2)
-		value := "true"
-		if len(parts) > 1 {
-			value = strings.Trim(parts[1], `"`)
-		}
-		key := strings.Trim(parts[0], `"`)
-		v[key] = value
+	err = yaml.Unmarshal(filteredYAML, &result)
+	if err != nil {
+		return &result, err
 	}
 
-	return v
+	return &result, nil
 }
 
 // ConfigURL returns the value of config_url if set or empty string otherwise.

diff --git a/pkg/config/collector/collector_test.go b/pkg/config/collector/collector_test.go
@@ -6,6 +6,8 @@ import (
 	"path"
 	"path/filepath"
 
+	"github.com/kairos-io/kairos/v2/pkg/config"
+
 	. "github.com/kairos-io/kairos/v2/pkg/config/collector"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -273,12 +275,12 @@ options:
 				)
 				Expect(err).ToNot(HaveOccurred())
 
-				c, err := Scan(o)
+				c, err := Scan(o, config.FilterKeys)
 				Expect(err).ToNot(HaveOccurred())
 
-				config_url, ok := (*c)["config_url"].(string)
+				configURL, ok := (*c)["config_url"].(string)
 				Expect(ok).To(BeTrue())
-				Expect(config_url).To(MatchRegexp("remote_config_2.yaml"))
+				Expect(configURL).To(MatchRegexp("remote_config_2.yaml"))
 
 				k := (*c)["local_key_1"].(string)
 				Expect(k).To(Equal("local_value_1"))
@@ -301,7 +303,7 @@ options:
 				Expect(options["remote_option_2"]).To(Equal("remote_option_value_2"))
 
 				player := (*c)["player"].(map[interface{}]interface{})
-				Expect(player["name"]).To(Equal("Dimitris"))
+				Expect(player["name"]).NotTo(Equal("Toad"))
 				Expect(player["surname"]).To(Equal("Bros"))
 			})
 		})
@@ -358,7 +360,7 @@ remote_key_2: remote_value_2`), os.ModePerm)
 				err := o.Apply(Directories(tmpDir), NoLogs)
 				Expect(err).ToNot(HaveOccurred())
 
-				c, err := Scan(o)
+				c, err := Scan(o, config.FilterKeys)
 				Expect(err).ToNot(HaveOccurred())
 
 				Expect((*c)["local_key_2"]).To(BeNil())
@@ -426,7 +428,7 @@ some:
 			)
 			Expect(err).ToNot(HaveOccurred())
 
-			c, err := Scan(o)
+			c, err := Scan(o, config.FilterKeys)
 			Expect(err).ToNot(HaveOccurred())
 
 			v, err := c.Query("local_key_1")
@@ -446,7 +448,6 @@ func createRemoteConfigs(serverDir string, port int) string {
 
 config_url: http://127.0.0.1:%d/remote_config_2.yaml
 player:
-  name: Dimitris
 remote_key_1: remote_value_1
 `, port)), os.ModePerm)
 	Expect(err).ToNot(HaveOccurred())
@@ -460,7 +461,7 @@ remote_key_2: remote_value_2
 
 	cmdLinePath := filepath.Join(serverDir, "cmdline")
 	// We put the cmdline in the same dir, it doesn't matter.
-	cmdLine := fmt.Sprintf(`config_url="http://127.0.0.1:%d/remote_config_1.yaml" player.name="Mario" options.foo=bar`, port)
+	cmdLine := fmt.Sprintf(`config_url="http://127.0.0.1:%d/remote_config_1.yaml" player.name="Toad" options.foo=bar`, port)
 	err = os.WriteFile(cmdLinePath, []byte(cmdLine), os.ModePerm)
 	Expect(err).ToNot(HaveOccurred())
 

diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -36,8 +36,8 @@ type Install struct {
 }
 
 type Config struct {
-	Install *Install `yaml:"install,omitempty"`
-	collector.Config
+	Install          *Install `yaml:"install,omitempty"`
+	collector.Config `yaml:"-"`
 	// TODO: Remove this too?
 	ConfigURL          string            `yaml:"config_url,omitempty"`
 	Options            map[string]string `yaml:"options,omitempty"`
@@ -95,6 +95,22 @@ func (c Config) HasConfigURL() bool {
 	return c.ConfigURL != ""
 }
 
+// FilterKeys is used to pass to any other pkg which might want to see which part of the config matches the Kairos config.
+func FilterKeys(d []byte) ([]byte, error) {
+	cmdLineFilter := Config{}
+	err := yaml.Unmarshal(d, &cmdLineFilter)
+	if err != nil {
+		return []byte{}, err
+	}
+
+	out, err := yaml.Marshal(cmdLineFilter)
+	if err != nil {
+		return []byte{}, err
+	}
+
+	return out, nil
+}
+
 func Scan(opts ...collector.Option) (c *Config, err error) {
 	result := &Config{}
 
@@ -103,7 +119,7 @@ func Scan(opts ...collector.Option) (c *Config, err error) {
 		return result, err
 	}
 
-	genericConfig, err := collector.Scan(o)
+	genericConfig, err := collector.Scan(o, FilterKeys)
 	if err != nil {
 		return result, err
 

diff --git a/pkg/config/schemas/root_schema_test.go b/pkg/config/schemas/root_schema_test.go
@@ -16,6 +16,10 @@ func getTagName(s string) string {
 		return ""
 	}
 
+	if s == "-" {
+		return ""
+	}
+
 	f := func(c rune) bool {
 		return c == '"' || c == ','
 	}