Skip to content

Conversation

fmount
Copy link
Contributor

@fmount fmount commented Sep 24, 2025

This patch introduces a new INI manipulation utility with the ability to process/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, nor does it add a new section if one does not already exist in the specified configuration. This design preserves the original purpose of customServiceConfig as user input.

Note: Since we already process customServiceConfig in storage operators, lib-common seems to be a good fit for the use case.

Related: https://issues.redhat.com/browse/OSPRH-14309

Copy link
Contributor

@konan-abhi konan-abhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!!

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, nor does it add a new section if one does not
already exist in the specified configuration. This design preserves the
original purpose of customServiceConfig as user input.

Related: https://issues.redhat.com/browse/OSPRH-14309

Signed-off-by: Francesco Pantano <[email protected]>
// Check if key already exists in target section
if token == customServiceConfigExtend.Key && sectionIndex > -1 &&
sectionName == customServiceConfigExtend.Section {
errMsg := fmt.Errorf("%w: key %s in section %s", ErrKeyAlreadyExists, token, sectionName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the openstack config is not strictly speaking ini config where you always have only once a parameter (key) in a section. there are services where it is valid to have multiple keys with the same name. e.g. in nova for pci passthrough, like the pci_passthrough_alias parameter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe IniOption should have a parameter to allow multiple of the same kind?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants