From 404ee450ef6ad27d0136d297a158a7c2bd52775f Mon Sep 17 00:00:00 2001 From: Francesco Pantano Date: Wed, 24 Sep 2025 12:21:58 +0200 Subject: [PATCH] Add INI config utility to common/util module This patch introduces a new INI manipulation utility with the ability to extend service configurations (customServiceConfig) by adding key-value pairs to specific sections. It includes comprehensive test coverage for various scenarios, including section detection, key validation, and edge cases. This utility does not replace existing keys if they are already specified via user input, and it optionally adds - based on the Unique parameter - keys of the same kind for a given section. This design preserves the original purpose of customServiceConfig as user input. Related: https://issues.redhat.com/browse/OSPRH-14309 Signed-off-by: Francesco Pantano --- modules/common/util/ini.go | 104 +++++++++++++++ modules/common/util/ini_test.go | 221 ++++++++++++++++++++++++++++++++ 2 files changed, 325 insertions(+) create mode 100644 modules/common/util/ini.go create mode 100644 modules/common/util/ini_test.go diff --git a/modules/common/util/ini.go b/modules/common/util/ini.go new file mode 100644 index 00000000..44e7349d --- /dev/null +++ b/modules/common/util/ini.go @@ -0,0 +1,104 @@ +/* +Copyright 2025 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "errors" + "fmt" + "strings" +) + +// IniOption - +type IniOption struct { + Section string + Key string + Value string + Unique bool +} + +// Define static errors +var ( + ErrKeyAlreadyExists = errors.New("key already exists in section") + ErrCouldNotPatchSection = errors.New("could not patch target section") +) + +// repr - print key: value in .ini format +func (i *IniOption) repr() string { + return fmt.Sprintf("%s = %s", i.Key, i.Value) +} + +// ExtendCustomServiceConfig - customServiceConfig is tokenized and parsed in a +// loop where we keep track of two indexes: +// - index: keep track of the current extracted token +// - sectionIndex: when we detect a [
] within a token, we save the index +// and we update it with the next section when is detected. This way we can +// make sure to evaluate only the keys of the target section +// +// when an invalid case is detected, we return the customServiceConfig string +// unchanged, otherwise the new key=value is appended as per the IniOption struct +func ExtendCustomServiceConfig( + iniString string, + customServiceConfigExtend IniOption, +) (string, error) { + // customServiceConfig is empty + if len(iniString) == 0 { + return iniString, nil + } + // Position where insert new option (-1 = target section not found) + index := -1 + // Current section header position (-1 = no section found) + sectionIndex := -1 + svcConfigLines := strings.Split(iniString, "\n") + sectionName := "" + for idx, rawLine := range svcConfigLines { + line := strings.TrimSpace(rawLine) + token := strings.TrimSpace(strings.SplitN(line, "=", 2)[0]) + + if token == "" || strings.HasPrefix(token, "#") { + // Skip blank lines and comments + continue + } + if strings.HasPrefix(token, "[") && strings.HasSuffix(token, "]") { + // Note the section name before looking for a backend_name + sectionName = strings.Trim(token, "[]") + sectionIndex = idx + // increment the index (as an offset) only when a section is found + if sectionName == customServiceConfigExtend.Section { + index = idx + 1 + } + } + // Check if key already exists in target section + if customServiceConfigExtend.Unique && token == customServiceConfigExtend.Key && sectionIndex > -1 && + sectionName == customServiceConfigExtend.Section { + errMsg := fmt.Errorf("%w: key %s in section %s", ErrKeyAlreadyExists, token, sectionName) + return iniString, errMsg + } + } + // index didn't progress during the customServiceConfig scan: + // return unchanged, but no error + if index == -1 { + errMsg := fmt.Errorf("%w: %s", ErrCouldNotPatchSection, customServiceConfigExtend.Section) + return iniString, errMsg + } + // index has a valid value and it is used as a pivot to inject the new ini + // option right after the section + var svcExtended []string + svcExtended = append(svcExtended, svcConfigLines[:index]...) + svcExtended = append(svcExtended, []string{customServiceConfigExtend.repr()}...) + svcExtended = append(svcExtended, svcConfigLines[index:]...) + return strings.Join(svcExtended, "\n"), nil +} diff --git a/modules/common/util/ini_test.go b/modules/common/util/ini_test.go new file mode 100644 index 00000000..c7a33252 --- /dev/null +++ b/modules/common/util/ini_test.go @@ -0,0 +1,221 @@ +package util // nolint:revive + +import ( + "testing" + + . "github.com/onsi/gomega" // nolint:revive +) + +var ( + defaultTestIniOption = IniOption{ + Section: "foo", + Key: "s3_store_cacert", + Value: "/etc/pki/tls/certs/ca-bundle.crt", + Unique: true, + } + + aliasKeyIniOption = IniOption{ + Section: "pci", + Key: "alias", + Value: "{ \"device_type\": \"type-VF\", \"resource_class\": \"CUSTOM_A16_16A\", \"name\": \"A16_16A\" }", + Unique: false, + } + + deviceSpecIniOption = IniOption{ + Section: "pci", + Key: "device_spec", + Value: "{ \"vendor_id\": \"10de\", \"product_id\": \"25b6\", \"address\": \"0000:25:00.6\", \"resource_class\": \"CUSTOM_A16_8A\", \"managed\": \"yes\" }", + Unique: false, + } +) + +var tests = []struct { + name string + input string + option IniOption + expected string + err string +}{ + { + name: "empty customServiceConfig", + input: "", + option: defaultTestIniOption, + expected: "", + err: "", + }, + { + name: "field to non-existing section", + input: `[DEFAULT] +debug=true +enabled_backends = backend:s3 +[foo] +bar = bar +foo = foo +[backend] +option1 = value1`, + option: IniOption{ + Section: "bar", + Key: "foo", + Value: "foo", + }, + expected: `[DEFAULT] +debug=true +enabled_backends = backend:s3 +[foo] +bar = bar +foo = foo +[backend] +option1 = value1`, + err: "could not patch target section: bar", + }, + { + name: "add new ini line to a section in the middle", + input: `[DEFAULT] +debug=true +enabled_backends = backend:s3 +[foo] +bar = bar +foo = foo +[backend] +option1 = value1`, + option: defaultTestIniOption, + expected: `[DEFAULT] +debug=true +enabled_backends = backend:s3 +[foo] +s3_store_cacert = /etc/pki/tls/certs/ca-bundle.crt +bar = bar +foo = foo +[backend] +option1 = value1`, + err: "", + }, + { + name: "section is not found, return it unchanged", + input: `[DEFAULT] +debug=true +enabled_backends = backend:s3 +[backend] +s3_store_cacert = /etc/pki/tls/certs/ca-bundle.crt `, + option: defaultTestIniOption, + expected: `[DEFAULT] +debug=true +enabled_backends = backend:s3 +[backend] +s3_store_cacert = /etc/pki/tls/certs/ca-bundle.crt `, + err: "could not patch target section: foo", + }, + { + name: "Add option to a section at the very beginning of customServiceConfig", + input: `[foo] +bar = bar +foo = foo +[backend] +option1 = value1 `, + option: defaultTestIniOption, + expected: `[foo] +s3_store_cacert = /etc/pki/tls/certs/ca-bundle.crt +bar = bar +foo = foo +[backend] +option1 = value1 `, + err: "", + }, + { + name: "Add option to a section at the very bottom of customServiceConfig", + input: `[DEFAULT] +debug=true +enabled_backends = backend:s3 +[backend] +# this is a comment +option1 = value1 +[foo] +# this is a comment +bar = bar +foo = foo`, + option: defaultTestIniOption, + expected: `[DEFAULT] +debug=true +enabled_backends = backend:s3 +[backend] +# this is a comment +option1 = value1 +[foo] +s3_store_cacert = /etc/pki/tls/certs/ca-bundle.crt +# this is a comment +bar = bar +foo = foo`, + err: "", + }, + { + name: "Add option to an empty target section", + input: `[DEFAULT] +debug=true +[foo]`, + option: defaultTestIniOption, + expected: `[DEFAULT] +debug=true +[foo] +s3_store_cacert = /etc/pki/tls/certs/ca-bundle.crt`, + err: "", + }, + { + name: "key/value already present in the target section", + input: `[DEFAULT] +debug=true +[foo] +s3_store_cacert = /my/custom/path/ca-bundle.crt`, + option: defaultTestIniOption, + expected: `[DEFAULT] +debug=true +[foo] +s3_store_cacert = /my/custom/path/ca-bundle.crt`, + err: "key already exists in section: key s3_store_cacert in section foo", + }, + { + name: "add new ini line anyway even though a section contains the same key ", + input: `[pci] +device_spec = { "vendor_id": "10de", "product_id": "25b6", "address": "0000:25:00.4", "resource_class": "CUSTOM_A16_16A", "managed": "no" } +device_spec = { "vendor_id": "10de", "product_id": "25b6", "address": "0000:25:00.5", "resource_class": "CUSTOM_A16_8A", "managed": "no" } +alias = { "device_type": "type-VF", "resource_class": "CUSTOM_A16_8A", "name": "A16_8A" }`, + option: aliasKeyIniOption, + expected: `[pci] +alias = { "device_type": "type-VF", "resource_class": "CUSTOM_A16_16A", "name": "A16_16A" } +device_spec = { "vendor_id": "10de", "product_id": "25b6", "address": "0000:25:00.4", "resource_class": "CUSTOM_A16_16A", "managed": "no" } +device_spec = { "vendor_id": "10de", "product_id": "25b6", "address": "0000:25:00.5", "resource_class": "CUSTOM_A16_8A", "managed": "no" } +alias = { "device_type": "type-VF", "resource_class": "CUSTOM_A16_8A", "name": "A16_8A" }`, + err: "", + }, + { + name: "add new ini line anyway even though a section contains the same key ", + input: `[pci] +device_spec = { "vendor_id": "10de", "product_id": "25b6", "address": "0000:25:00.4", "resource_class": "CUSTOM_A16_16A", "managed": "no" } +device_spec = { "vendor_id": "10de", "product_id": "25b6", "address": "0000:25:00.5", "resource_class": "CUSTOM_A16_8A", "managed": "no" } +alias = { "device_type": "type-VF", "resource_class": "CUSTOM_A16_16A", "name": "A16_16A" } +alias = { "device_type": "type-VF", "resource_class": "CUSTOM_A16_8A", "name": "A16_8A" }`, + option: deviceSpecIniOption, + expected: `[pci] +device_spec = { "vendor_id": "10de", "product_id": "25b6", "address": "0000:25:00.6", "resource_class": "CUSTOM_A16_8A", "managed": "yes" } +device_spec = { "vendor_id": "10de", "product_id": "25b6", "address": "0000:25:00.4", "resource_class": "CUSTOM_A16_16A", "managed": "no" } +device_spec = { "vendor_id": "10de", "product_id": "25b6", "address": "0000:25:00.5", "resource_class": "CUSTOM_A16_8A", "managed": "no" } +alias = { "device_type": "type-VF", "resource_class": "CUSTOM_A16_16A", "name": "A16_16A" } +alias = { "device_type": "type-VF", "resource_class": "CUSTOM_A16_8A", "name": "A16_8A" }`, + err: "", + }, +} + +func TestExtendCustomServiceConfig(t *testing.T) { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + output, err := ExtendCustomServiceConfig(tt.input, tt.option) + g.Expect(output).To(Equal(tt.expected)) + if err != nil { + // check the string matches the expected error message + g.Expect(err.Error()).To(Equal(tt.err)) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +}