Skip to content

fix(config): use underscores for config keys/env vars, hyphens for flags - #24

Merged
synackd merged 3 commits into
mainfrom
fix/config-file-read
Jul 20, 2026
Merged

fix(config): use underscores for config keys/env vars, hyphens for flags #24
synackd merged 3 commits into
mainfrom
fix/config-file-read

Conversation

@synackd

@synackd synackd commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Description

A conflict in Viper naming schemas was causing a bug where a config file
using underscore-separated config keys (which is correct according to
the documentation) would get overridden by the default values, which use
hyphen-separated keys. This could be fixed by using hyphen-separated
keys in the config file, but this is neither idiomatic for YAML keys nor
consistent with the config documentation.

This is due to ambiguity in the way Viper config key aliases are used.
The config struct uses underscores for mapstructure tags and flags use
hyphens. Flags are aliased to config keys via
viper.RegisterAlias("config_key", "config-key"), but this causes the
unintentional side effect of hyphen-separated values being able to
override underscore-separated values in an ambiguous way, such as the
default values not being overridden by config file values.

The intended behavior is strictly that:

  • CLI flags only use hyphen separation
  • config file and environment variable keys use underscore separation

Therefore, this change removes the viper alias registration and adds a
bindFlagsWithUnderscoreKeys() helper that dynamically binds
hyphen-separated flags to underscore-separated config values. This
ensures the rules stated above.

Implements OpenCHAMI/fabrica#75

Checklist

  • My code follows the style guidelines of this project
  • I have added/updated comments where needed
  • I have added tests that prove my fix is effective or my feature works
  • I have run make test (or equivalent) locally and all tests pass
  • DCO Sign-off: All commits are signed off (git commit -s) with my real name and email
  • REUSE Compliance:
    • Each new/modified source file has SPDX copyright and license headers
    • Any non-commentable files include a <filename>.license sidecar
    • All referenced licenses are present in the LICENSES/ directory

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

For more info, see Contributing Guidelines.

synackd added 3 commits July 17, 2026 10:08
A conflict in Viper naming schemas was causing a bug where a config file
using underscore-separated config keys (which is correct according to
the documentation) would get overridden by the default values, which use
hyphen-separated keys. This could be fixed by using hyphen-separated
keys in the config file, but this is neither idiomatic for YAML keys nor
consistent with the config documentation.

This is due to ambiguity in the way Viper config key aliases are used.
The config struct uses underscores for mapstructure tags and flags use
hyphens. Flags are aliased to config keys via
viper.RegisterAlias("config_key", "config-key"), but this causes the
unintentional side effect of hyphen-separated values being able to
override underscore-separated values in an ambiguous way, such as the
default values not being overridden by config file values.

The intended behavior is strictly that:

- CLI flags only use hyphen separation
- config file and environment variable keys use underscore separation

Therefore, this change removes the viper alias registration and adds a
bindFlagsWithUnderscoreKeys() helper that dynamically binds
hyphen-separated flags to underscore-separated config values. This
ensures the rules stated above.

Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>

@seantronsen seantronsen left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

Note

Reviewed code changes and tested by booting a compute vm. Had to change from --tokensmith_url to --tokensmith-url, though that was already expected.

[rocky@head boot-service.container.d]$ cat 10-override.conf
[Container]
Image=ghcr.io/openchami/boot-service:pr-24

Exec=serve --enable-legacy-api=false --enable-auth=true --tokensmith-url=http://tokensmith:8080 --hsm-url=http://smd:27779 --tokensmith-target-service smd --port 8081

@synackd

synackd commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

@seantronsen Can you verify that a basic config file using underscores works?

enable_legacy_api: false
enable_auth: true
tokensmith_url: http://tokensmith:8080
hsm_url: http://smd:27779
tokensmith_target_service: smd
port: 8081

You can remove the entire Exec line in the quadlet for this to apply. Mount config at /etc/boot-service/config.yaml in the container.

@seantronsen

Copy link
Copy Markdown

@seantronsen Can you verify that a basic config file using underscores works?

enable_legacy_api: false
enable_auth: true
tokensmith_url: http://tokensmith:8080
hsm_url: http://smd:27779
tokensmith_target_service: smd
port: 8081

You can remove the entire Exec line in the quadlet for this to apply. Mount config at /etc/boot-service/config.yaml in the container.

Works.

@seantronsen
seantronsen self-requested a review July 20, 2026 20:41

@seantronsen seantronsen left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Underscore config file tested successfully. Approved.

@synackd
synackd merged commit 3e4c01f into main Jul 20, 2026
6 checks passed
@synackd
synackd deleted the fix/config-file-read branch July 20, 2026 20:47
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.

2 participants