From b6b10ebf20395992a2a640e162037f378bc44306 Mon Sep 17 00:00:00 2001
From: Sascha Grunert <sgrunert@redhat.com>
Date: Mon, 28 Oct 2024 09:25:28 +0100
Subject: [PATCH] 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 <sgrunert@redhat.com>
---
 pkg/common/file.go      |  2 +-
 test/e2e/config_test.go | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/pkg/common/file.go b/pkg/common/file.go
index e57e58d0da..8637fc4850 100644
--- a/pkg/common/file.go
+++ b/pkg/common/file.go
@@ -182,9 +182,9 @@ func setConfigOption(configName, configValue string, yamlData *yaml.Node) {
 	for indx := 0; indx < contentLen-1; {
 		name := yamlData.Content[0].Content[indx].Value
 		if name == configName {
+			// Set the value, even if we have the option defined multiple times.
 			yamlData.Content[0].Content[indx+1].Value = configValue
 			foundOption = true
-			break
 		}
 		indx += 2
 	}
diff --git a/test/e2e/config_test.go b/test/e2e/config_test.go
index 822b9c9fc4..8c9043fa8e 100644
--- a/test/e2e/config_test.go
+++ b/test/e2e/config_test.go
@@ -87,4 +87,42 @@ pull-image-on-create: false
 disable-pull-on-run: false
 `))
 	})
+
+	It("should succeed to get the right value if duplicate entries are defined", func() {
+		_, err := configFile.WriteString(`
+timeout: 20
+timeout: 5
+timeout: 10
+`)
+		Expect(err).NotTo(HaveOccurred())
+
+		t.CrictlExpectSuccess("--config "+configFile.Name()+" config --get timeout", "10")
+	})
+
+	It("should succeed to set duplicate entries", func() {
+		_, err := configFile.WriteString(`
+timeout: 20
+timeout: 5
+timeout: 10
+`)
+		Expect(err).NotTo(HaveOccurred())
+
+		t.CrictlExpectSuccess("--config "+configFile.Name()+" config --set timeout=30", "")
+
+		cfgContent, err := os.ReadFile(configFile.Name())
+		Expect(err).NotTo(HaveOccurred())
+
+		Expect(string(cfgContent)).To(Equal(
+			`timeout: 30
+timeout: 30
+timeout: 30
+runtime-endpoint: ""
+image-endpoint: ""
+debug: false
+pull-image-on-create: false
+disable-pull-on-run: false
+`))
+
+		t.CrictlExpectSuccess("--config "+configFile.Name()+" config --get timeout", "30")
+	})
 })