Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: kairos-io/kairos-sdk
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v0.0.10
Choose a base ref
...
head repository: kairos-io/kairos-sdk
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v0.0.11
Choose a head ref
  • 1 commit
  • 2 files changed
  • 1 contributor

Commits on Aug 1, 2023

  1. 🐛 Wrong deep merge when array contains maps (#38)

    * Do not try to smart merge arrays, but just append them
    
    Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com>
    
    * Fix tests
    
    Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com>
    
    * Rewording for tests
    
    Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com>
    
    * Rename function to match what is doing after refactoring
    
    Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com>
    
    ---------
    
    Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com>
    mauromorales authored Aug 1, 2023

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
    Copy the full SHA
    396c555 View commit details
Showing with 192 additions and 45 deletions.
  1. +9 −33 collector/collector.go
  2. +183 −12 collector/collector_test.go
42 changes: 9 additions & 33 deletions collector/collector.go
Original file line number Diff line number Diff line change
@@ -108,50 +108,26 @@ func (c *Config) MergeConfig(newConfig *Config) error {
return c.applyMap(cMap)
}

func deepMergeSlices(sliceA, sliceB []interface{}) ([]interface{}, error) {
func mergeSlices(sliceA, sliceB []interface{}) ([]interface{}, error) {
// We use the first item in the slice to determine if there are maps present.
// Do we need to do the same for other types?
firstItem := sliceA[0]
// If the first item is a map, we concatenate both slices
if reflect.ValueOf(firstItem).Kind() == reflect.Map {
temp := make(map[string]interface{})
union := append(sliceA, sliceB...)

// first we put in temp all the keys present in a, and assign them their existing values
for _, item := range sliceA {
for k, v := range item.(map[string]interface{}) {
temp[k] = v
}
}

// then we go through b to merge each of its keys
for _, item := range sliceB {
for k, v := range item.(map[string]interface{}) {
current, ok := temp[k]
if ok {
// if the key exists, we deep merge it
dm, err := DeepMerge(current, v)
if err != nil {
return []interface{}{}, fmt.Errorf("cannot merge %s with %s", current, v)
}
temp[k] = dm
} else {
// otherwise we just set it
temp[k] = v
}
}
}

return []interface{}{temp}, nil
return union, nil
}

// This implementation is needed because Go 1.19 does not implement compare for {}interface. Once
// FIPS can be upgraded to 1.20, we should be able to use this other code:
// // for simple slices
// For any other type, we check if the every item in sliceB is already present in sliceA and if not, we add it.
// Implementation for 1.20:
// for _, v := range sliceB {
// i := slices.Index(sliceA, v)
// if i < 0 {
// sliceA = append(sliceA, v)
// }
// }
// This implementation is needed because Go 1.19 does not implement compare for {}interface. Once
// FIPS can be upgraded to 1.20, we should be able to use the code above instead.
for _, vB := range sliceB {
found := false
for _, vA := range sliceA {
@@ -204,7 +180,7 @@ func DeepMerge(a, b interface{}) (interface{}, error) {
}

if typeA.Kind() == reflect.Slice {
return deepMergeSlices(a.([]interface{}), b.([]interface{}))
return mergeSlices(a.([]interface{}), b.([]interface{}))
}

if typeA.Kind() == reflect.Map {
195 changes: 183 additions & 12 deletions collector/collector_test.go
Original file line number Diff line number Diff line change
@@ -242,15 +242,24 @@ info:
It("merges", func() {
c, err := DeepMerge(a, b)
Expect(err).ToNot(HaveOccurred())
users := c.([]interface{})[0].(map[string]interface{})["users"]
Expect(users).To(HaveLen(1))
Expect(users).To(Equal([]interface{}{
Expect(c).To(HaveLen(2))
Expect(c).To(Equal([]interface{}{
map[string]interface{}{
"kairos": map[string]interface{}{
"passwd": "kairos",
"users": []interface{}{
map[string]interface{}{
"kairos": map[string]interface{}{
"passwd": "kairos",
},
},
},
"foo": map[string]interface{}{
"passwd": "bar",
},
map[string]interface{}{
"users": []interface{}{
map[string]interface{}{
"foo": map[string]interface{}{
"passwd": "bar",
},
},
},
},
}))
@@ -300,6 +309,158 @@ info:
})

Describe("Scan", func() {
Context("When users are created for the same stage on different files (issue kairos-io/kairos#1341)", func() {
var cmdLinePath, tmpDir1 string
var err error

BeforeEach(func() {
tmpDir1, err = os.MkdirTemp("", "config1")
Expect(err).ToNot(HaveOccurred())
err := os.WriteFile(path.Join(tmpDir1, "local_config_1.yaml"), []byte(`#cloud-config
install:
auto: true
reboot: false
poweroff: false
grub_options:
extra_cmdline: "console=tty0"
options:
device: /dev/sda
stages:
initramfs:
- users:
kairos:
groups:
- sudo
passwd: kairos
`), os.ModePerm)
Expect(err).ToNot(HaveOccurred())
err = os.WriteFile(path.Join(tmpDir1, "local_config_2.yaml"), []byte(`#cloud-config
stages:
initramfs:
- users:
foo:
groups:
- sudo
passwd: bar
`), os.ModePerm)
Expect(err).ToNot(HaveOccurred())
})

AfterEach(func() {
err = os.RemoveAll(tmpDir1)
Expect(err).ToNot(HaveOccurred())
})

It("keeps the two users", func() {
o := &Options{}
err := o.Apply(
MergeBootLine,
WithBootCMDLineFile(cmdLinePath),
Directories(tmpDir1),
)
Expect(err).ToNot(HaveOccurred())

c, err := Scan(o, FilterKeysTestMerge)
Expect(err).ToNot(HaveOccurred())

fmt.Println(c.String())
Expect(c.String()).To(Equal(`#cloud-config
install:
auto: true
grub_options:
extra_cmdline: console=tty0
poweroff: false
reboot: false
options:
device: /dev/sda
stages:
initramfs:
- users:
kairos:
groups:
- sudo
passwd: kairos
- users:
foo:
groups:
- sudo
passwd: bar
`))
})
})

Context("When a YIP if expression is contained", func() {
var cmdLinePath, tmpDir1 string
var err error

BeforeEach(func() {
tmpDir1, err = os.MkdirTemp("", "config1")
Expect(err).ToNot(HaveOccurred())
err := os.WriteFile(path.Join(tmpDir1, "local_config_1.yaml"), []byte(`#cloud-config
stages:
initramfs:
- users:
kairos:
passwd: kairos
- name: set_inotify_max_values
if: '[ ! -f /oem/80_stylus.yaml ]'
sysctl:
fs.inotify.max_user_instances: "8192"
`), os.ModePerm)
Expect(err).ToNot(HaveOccurred())
err = os.WriteFile(path.Join(tmpDir1, "local_config_2.yaml"), []byte(`#cloud-config
stages:
initramfs:
- commands:
- ln -s /etc/kubernetes/admin.conf /run/kubeconfig
sysctl:
kernel.panic: "10"
kernel.panic_on_oops: "1"
vm.overcommit_memory: "1"
`), os.ModePerm)
Expect(err).ToNot(HaveOccurred())
})

AfterEach(func() {
err = os.RemoveAll(tmpDir1)
Expect(err).ToNot(HaveOccurred())
})

It("it remains within its scope after merging", func() {
o := &Options{}
err := o.Apply(
MergeBootLine,
WithBootCMDLineFile(cmdLinePath),
Directories(tmpDir1),
)
Expect(err).ToNot(HaveOccurred())

c, err := Scan(o, FilterKeysTestMerge)
Expect(err).ToNot(HaveOccurred())

fmt.Println(c.String())
Expect(c.String()).To(Equal(`#cloud-config
stages:
initramfs:
- users:
kairos:
passwd: kairos
- if: '[ ! -f /oem/80_stylus.yaml ]'
name: set_inotify_max_values
sysctl:
fs.inotify.max_user_instances: "8192"
- commands:
- ln -s /etc/kubernetes/admin.conf /run/kubeconfig
sysctl:
kernel.panic: "10"
kernel.panic_on_oops: "1"
vm.overcommit_memory: "1"
`))
})
})

Context("duplicated configs", func() {
var cmdLinePath, tmpDir1 string
var err error
@@ -358,7 +519,7 @@ install:
Expect(err).ToNot(HaveOccurred())
})

It("should be the same as just one of them", func() {
It("remain duplicated, and are the responsibility of the user", func() {
o := &Options{}
err := o.Apply(
MergeBootLine,
@@ -379,6 +540,9 @@ install:
- rootfs_path: /usr/local/lib/extensions/kubo
targets:
- container://ttl.sh/97d4530c-df80-4eb4-9ae7-39f8f90c26e5:8h
- rootfs_path: /usr/local/lib/extensions/kubo
targets:
- container://ttl.sh/97d4530c-df80-4eb4-9ae7-39f8f90c26e5:8h
device: auto
grub_options:
extra_cmdline: foobarzz
@@ -390,9 +554,15 @@ stages:
users:
kairos:
passwd: kairos
- hostname: kairos-{{ trunc 4 .Random }}
name: Set user and password
users:
kairos:
passwd: kairos
`))
})
})

Context("Deep merge maps within arrays", func() {
var cmdLinePath, tmpDir1 string
var err error
@@ -460,14 +630,15 @@ options:
stages:
initramfs:
- users:
foo:
groups:
- sudo
passwd: bar
kairos:
groups:
- sudo
passwd: kairos
- users:
foo:
groups:
- sudo
passwd: bar
`))
})
})