Skip to content

Commit b6b10eb

Browse files
committed
Fix crictl config --set if the YAML defines entries multiple times
Only the first entry would be written before applying this patch. This means that a config like this: ```yaml timeout: 5 timeout: 10 timeout: 20 runtime-endpoint: "" image-endpoint: "" debug: false pull-image-on-create: false disable-pull-on-run: false ``` Would return the right value (the last one): ``` > crictl --config ./crictl.yaml config --get timeout 20 ``` But setting the config will only result in setting the first item: ``` > crictl --config ./crictl.yaml config --set timeout=30 > cat crictl.yaml timeout: 30 timeout: 10 timeout: 20 runtime-endpoint: "" image-endpoint: "" debug: false pull-image-on-create: false disable-pull-on-run: false ``` And therefore also a wrong result of `crictl config --get`: ``` > crictl --config ./crictl.yaml config --get timeout 20 ``` We now change that behavior and set all values without de-duplicating them: ``` > crictl --config ./crictl.yaml config --set timeout=30 > cat crictl.yaml timeout: 30 timeout: 30 timeout: 30 runtime-endpoint: "" image-endpoint: "" debug: false pull-image-on-create: false disable-pull-on-run: false ``` e2e test cases around that scenario have been added as well. Signed-off-by: Sascha Grunert <[email protected]>
1 parent 47503b3 commit b6b10eb

File tree

2 files changed

+39
-1
lines changed

2 files changed

+39
-1
lines changed

Diff for: pkg/common/file.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,9 @@ func setConfigOption(configName, configValue string, yamlData *yaml.Node) {
182182
for indx := 0; indx < contentLen-1; {
183183
name := yamlData.Content[0].Content[indx].Value
184184
if name == configName {
185+
// Set the value, even if we have the option defined multiple times.
185186
yamlData.Content[0].Content[indx+1].Value = configValue
186187
foundOption = true
187-
break
188188
}
189189
indx += 2
190190
}

Diff for: test/e2e/config_test.go

+38
Original file line numberDiff line numberDiff line change
@@ -87,4 +87,42 @@ pull-image-on-create: false
8787
disable-pull-on-run: false
8888
`))
8989
})
90+
91+
It("should succeed to get the right value if duplicate entries are defined", func() {
92+
_, err := configFile.WriteString(`
93+
timeout: 20
94+
timeout: 5
95+
timeout: 10
96+
`)
97+
Expect(err).NotTo(HaveOccurred())
98+
99+
t.CrictlExpectSuccess("--config "+configFile.Name()+" config --get timeout", "10")
100+
})
101+
102+
It("should succeed to set duplicate entries", func() {
103+
_, err := configFile.WriteString(`
104+
timeout: 20
105+
timeout: 5
106+
timeout: 10
107+
`)
108+
Expect(err).NotTo(HaveOccurred())
109+
110+
t.CrictlExpectSuccess("--config "+configFile.Name()+" config --set timeout=30", "")
111+
112+
cfgContent, err := os.ReadFile(configFile.Name())
113+
Expect(err).NotTo(HaveOccurred())
114+
115+
Expect(string(cfgContent)).To(Equal(
116+
`timeout: 30
117+
timeout: 30
118+
timeout: 30
119+
runtime-endpoint: ""
120+
image-endpoint: ""
121+
debug: false
122+
pull-image-on-create: false
123+
disable-pull-on-run: false
124+
`))
125+
126+
t.CrictlExpectSuccess("--config "+configFile.Name()+" config --get timeout", "30")
127+
})
90128
})

0 commit comments

Comments
 (0)