Skip to content

Conversation

@synackd
Copy link
Collaborator

@synackd synackd commented Dec 9, 2025

Pull Request Template

Thank you for your contribution! Please ensure the following before submitting:

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

Description

BREAKING CHANGE: The coresmd and bootloop CoreDHCP plugins have a switched config format that can be migrated to in a straightforward manner,

Switch to a key-value configuration format instead of a positional-based configuration format. This means CoreDHCP configurations such as:

plugins:
    - coresmd: https://foobar.openchami.cluster http://172.16.0.253:8081 /root_ca/root_ca.crt 30s 1h false
    - bootloop: /tmp/coredhcp.db default 5m 172.16.0.156 172.16.0.200

Would become:

plugins:
    - coresmd:
      - svc_base_uri=https://foobar.openchami.cluster
      - ipxe_base_uri=http://172.16.0.253:8081
      - ca_cert=/root_ca/root_ca.crt
      - cache_valid=30s
      - lease_time=1h
      - single_port=false
    - bootloop:
      - lease_file=/tmp/coredhcp.db
      - script_path=default
      - lease_time=5m
      - ipv4_start=172.16.0.156
      - ipv4_end=172.16.0.200

The configurations can still be on the same line instead of a YAML array, but the latter allows easier file reading and better tracking of what changed when using version control and so it is recommended.

Additional functionality that was added is the ability to specify the TFTP port (tftp_port) and TFTP server root directory (tftp_dir), which default to 69 and /tftpboot, respectively, in alignment with the previous behavior.

READMEs have been updated/added to document the new format.

Other than a change of config format (and therefore file parsing, as well as some initialization log messages), previous behavior has been preserved.

Fixes #19

Testing

Test log output:

Using the old format to generate errors:

time="2025-12-04T23:38:14Z" level=info msg="initializing coresmd/coresmd 0.4.1-SNAPSHOT-bb1f115 (dirty), built 2025-12-04T23:35:29Z" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=error msg="arg 0: invalid format 'https://redondo.usrc.newmexicoconsortium.org:8443', should be 'key=val' (skipping)" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=error msg="arg 1: invalid format 'http://172.16.0.254:8081', should be 'key=val' (skipping)" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=error msg="arg 2: invalid format '/root_ca/root_ca.crt', should be 'key=val' (skipping)" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=error msg="arg 3: invalid format '30s', should be 'key=val' (skipping)" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=error msg="arg 4: invalid format '1h', should be 'key=val' (skipping)" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=error msg="arg 5: invalid format 'false', should be 'key=val' (skipping)" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=warning msg="ca_cert unset, TLS certificates will not be validated" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=warning msg="cache_valid unset, defaulting to 30s" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=warning msg="lease_time unset, defaulting to 1h" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=warning msg="tftp_dir unset, defaulting to " prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=error msg="svc_base_uri is required" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=error msg="ipxe_base_uri is required" prefix="plugins/coresmd"
time="2025-12-04T23:38:14Z" level=fatal msg="2 fatal errors occurred, exiting" prefix=main

Successful logs using new format:

time="2025-12-05T00:00:23Z" level=info msg="DHCPv4: found plugin `coresmd` with 6 args: [svc_base_uri=https://redondo.usrc.newmexicoconsortium.org:8443 ipxe_base_uri=http://172.16.0.254:8081 ca_cert=/root_ca/root_ca.crt cache_valid=30s lease_time=1h single_port=false]" prefix=config
...
time="2025-12-05T00:00:23Z" level=info msg="initializing coresmd/coresmd 0.4.1-SNAPSHOT-4472b34 (dirty), built 2025-12-04T23:59:34Z" prefix="plugins/coresmd"
time="2025-12-05T00:00:23Z" level=warning msg="tftp_port unset (0), defaulting to 69" prefix="plugins/coresmd"
time="2025-12-05T00:00:23Z" level=warning msg="tftp_dir unset, defaulting to /tftpboot" prefix="plugins/coresmd"
time="2025-12-05T00:00:23Z" level=info msg="coresmd config: svc_base_uri=https://redondo.usrc.newmexicoconsortium.org:8443 ipxe_base_uri=http://172.16.0.254:8081 ca_cert=/root_ca/root_ca.crt cache_valid=30s lease_time=1h0m0s single_port=false tftp_dir=/tftpboot tftp_port=69" prefix="plugins/coresmd"
time="2025-12-05T00:00:23Z" level=info msg="initiating cache refresh loop" prefix="plugins/coresmd"
time="2025-12-05T00:00:23Z" level=info msg="refreshing cache every duration: 30s" prefix="plugins/coresmd"
time="2025-12-05T00:00:23Z" level=info msg="initiating cache refresh" prefix="plugins/coresmd"
time="2025-12-05T00:00:23Z" level=info msg="Cache updated with 14 EthernetInterfaces and 14 Components" prefix="plugins/coresmd"
time="2025-12-05T00:00:23Z" level=info msg="starting TFTP server on port 69 with directory /tftpboot" prefix="plugins/coresmd"
time="2025-12-05T00:00:23Z" level=info msg="coresmd plugin initialized with base URL https://redondo.usrc.newmexicoconsortium.org:8443 and validity duration 30s" prefix="plugins/coresmd"
time="2025-12-05T00:00:23Z" level=info msg="Starting DHCPv4 server" prefix=server
time="2025-12-05T00:00:23Z" level=info msg="Listen 0.0.0.0:67" prefix=server

Type of Change

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

For more info, see Contributing Guidelines.

@synackd synackd added the needs testing Requires additional testing before merge label Dec 9, 2025
@synackd
Copy link
Collaborator Author

synackd commented Dec 9, 2025

Getting this out there, still adding unit tests.

@synackd synackd force-pushed the feature/key-value-config branch 2 times, most recently from 8e2117c to b0743d2 Compare December 9, 2025 21:38
@synackd
Copy link
Collaborator Author

synackd commented Dec 9, 2025

Unit tests added. This should be ready for review.

@synackd synackd removed the needs testing Requires additional testing before merge label Dec 9, 2025
@synackd synackd force-pushed the feature/key-value-config branch from b0743d2 to eb315a7 Compare December 10, 2025 21:51
Instead of relying on the position of arguments to determine which
values get assigned to which configuration keys for the coresmd CoreDHCP
plugin, parse using key=value. For example:

server4:
  plugins:
    - coresmd: svc_base_uri=https://foobar.openchami.cluster ipxe_base_uri=http://172.16.0.253:8081 ca_cert=/root_ca/root_ca.crt cache_valid=30s lease_time=1h single_port=false

or:

server4:
  plugins:
    - coresmd:
      - svc_base_uri=https://foobar.openchami.cluster
      - ipxe_base_uri=http://172.16.0.253:8081
      - ca_cert=/root_ca/root_ca.crt
      - cache_valid=30s
      - lease_time=1h
      - single_port=false

The benefits of this are:

- argument order does not matter
- optional arguments can be omitted entirely

New configuration options added:

- tftp_dir (default /tftpboot): choose TFTP server root directory
- tftp_port (default 69): TFTP server bind port

BREAKING-CHANGE: Old positional-argument-based format will not work
  anymore.

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>
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>
Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
@synackd synackd force-pushed the feature/key-value-config branch from eb315a7 to 3853e17 Compare December 10, 2025 22:34
@synackd
Copy link
Collaborator Author

synackd commented Dec 10, 2025

Rebasing to allow new unit tests to run.

@synackd
Copy link
Collaborator Author

synackd commented Dec 10, 2025

Checks are passing, received successful integration test feedback, and received out-of-band approval to merge.

@synackd synackd merged commit a161d78 into main Dec 10, 2025
4 checks passed
@synackd synackd deleted the feature/key-value-config branch December 10, 2025 23:05
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.

[DEV] Use key=value in plugin arguments to avoid confusion

2 participants