Skip to content

config: Unify configuration for pull and push agents #1037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 7, 2025

Conversation

ansasaki
Copy link
Contributor

@ansasaki ansasaki commented Jul 1, 2025

Refactor the configuration code, removing the wrapper structure KeylimeConfig and keeping only the internal configuration structure AgentConfig.

The AgentConfig structure holds the configuration options for both pull and push agents.

A new crate keylime-macros is introduced, with the implementation of the define_trait_view procedural macro.

The define_config_view procedural macro allows annotating a structure
with the attribute #[define_view_trait(for_struct = "SomeStructure")].

The annotated structure serves as a template to create a Trait which
provides a view of the referred structure, restricting the accessible
fields through getter methods. The created Trait is named as the
annotated name followed by "Trait".

For types that are Copy primitives (e.g. u16, u32, etc.), the getter
method will return a copy of the value. For other types, the getter
method will return a reference to the value, unless it is annotated with
the #[copy] attribute. Only types that implement the Copy trait can be
annotated with the #[copy] attribute.

The annoted structure fields can also be annotated with the attribute
#[transform(using = custom_transform_fn, error = CustomError)].
The annotated fields getters are generated using the provided transform
function which may return an error of the given type.

struct KeylimeConfig {
    node_id: String,
    port: u16,
    api_versions: String,
    info: CustomCopyType,
}

#[define_view_trait(for_struct = KeylimeConfig)]
struct KeylimeView {
    node_id: String,
    port: u16,
    #[transform(using = parse_string_list, error = ParsingError)]
    api_versions: Vec<String>,
    #[copy]
    info: CustomCopyType,
}

Expands into:

pub trait KeylimeViewTrait {
   fn node_id(&self) -> &str;
   fn port(&self) -> u16;
   fn api_versions(&self) -> Result<Vec<String>, ParsingError>;
   fn info(&self) -> CustomCopyType;
}

impl KeylimeViewTrait for KeylimeConfig {
   fn node_id(&self) -> &str { &self.node_id }
   fn port(&self) -> u16 { self.port }
   fn api_versions(&self) -> Result<Vec<String>, ParsingError> {
       parse_string_list(&self.api_versions)
   }
   fn info(&self) -> CustomCopyType { self.info }
}

The PushModelConfigTrait is implemented using the define_view_trait macro, replacing the previously existing implementation. The PushModelConfig is also removed in favor of using the unified AgentConfig, whith the view restricted by the imported trait.

Finally, the AgentConfig is used in the push model client prototype, adding support to the whole configuration via the agent.conf file, overrides using configuration snippets, and overrides with environment variables, in the same way it is currently supported by the pull model agent.

@ansasaki ansasaki marked this pull request as draft July 1, 2025 13:37
Copy link

codecov bot commented Jul 1, 2025

Codecov Report

Attention: Patch coverage is 63.42282% with 109 lines in your changes missing coverage. Please review.

Project coverage is 59.53%. Comparing base (d72ef88) to head (ee7ddf5).

Files with missing lines Patch % Lines
keylime-agent/src/main.rs 0.00% 62 Missing ⚠️
keylime-push-model-agent/src/main.rs 39.13% 14 Missing ⚠️
keylime-push-model-agent/src/registration.rs 60.00% 10 Missing ⚠️
keylime-push-model-agent/src/struct_filler.rs 37.50% 10 Missing ⚠️
keylime/src/config/file_config.rs 92.95% 5 Missing ⚠️
keylime/src/config/base.rs 95.06% 4 Missing ⚠️
keylime/src/config/push_model.rs 75.00% 2 Missing ⚠️
keylime/src/context_info.rs 33.33% 2 Missing ⚠️
Additional details and impacted files
Flag Coverage Δ
e2e-testsuite 59.53% <63.42%> (+0.29%) ⬆️
upstream-unit-tests 59.53% <63.42%> (+0.29%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
keylime-agent/src/keys_handler.rs 69.85% <ø> (+0.29%) ⬆️
keylime-agent/src/payloads.rs 81.44% <100.00%> (ø)
keylime-agent/src/revocation.rs 67.98% <ø> (ø)
keylime/src/agent_data.rs 100.00% <ø> (ø)
keylime/src/config/env.rs 84.21% <ø> (ø)
keylime/src/config/push_model.rs 60.00% <75.00%> (+3.70%) ⬆️
keylime/src/context_info.rs 53.50% <33.33%> (-0.27%) ⬇️
keylime/src/config/base.rs 85.91% <95.06%> (+3.73%) ⬆️
keylime/src/config/file_config.rs 92.95% <92.95%> (ø)
keylime-push-model-agent/src/registration.rs 46.55% <60.00%> (-8.36%) ⬇️
... and 3 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ansasaki ansasaki force-pushed the unify_config branch 6 times, most recently from 701b6a7 to 24deda0 Compare July 3, 2025 08:23
@ansasaki
Copy link
Contributor Author

ansasaki commented Jul 3, 2025

/packit retest-failed

ansasaki added 4 commits July 4, 2025 11:36
Drop unnecessary options:
- DEFAULT_MEASUREDBOOT_ML_PATH
- DEFAULT_UEFI_LOGS_BINARY_PATH

Drop unnecessary lazy initialization of constants.

Organize the default values in base.rs depending on its usage (shared,
push-model only, or pull-model only)

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Remove KeylimeConfig structure, making AgentConfig the only
configuration structure.  The KeylimeConfig structure was just a wrapper
around AgentConfig.

This also moves the loading of configuration files to the file_config.rs
file.

A test for overriding options using environment variables is added as a
integration test. This is necessary to make it to run in a separate
process and not affect other tests.

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
The keylime-macros crate provide helper macros.

The define_config_view procedural macro allows annotating a structure
with the attribute #[define_view_trait(for_struct = "SomeStructure")].

The annotated structure serves as a template to create a Trait which
provides a view of the referred structure, restricting the accessible
fields through getter methods. The created Trait is named as the
annotated name followed by "Trait".

For types that are Copy primitives (e.g. u16, u32, etc.), the getter
method will return a copy of the value. For other types, the getter
method will return a reference to the value, unless it is annotated with
the #[copy] attribute. Only types that implement the Copy trait can be
annotated with the #[copy] attribute.

The annoted structure fields can also be annotated with the attribute
`#[transform(using = custom_transform_fn, error = CustomError)]`.
The annotated fields getters are generated using the provided transform
function which may return an error of the given type.

For example:

```rust
struct KeylimeConfig {
    node_id: String,
    port: u16,
    api_versions: String,
    info: CustomCopyType,
}

\#[define_view_trait(for_struct = KeylimeConfig)]
struct KeylimeView {
    node_id: String,
    port: u16,
    #[transform(using = parse_string_list, error = ParsingError)]
    api_versions: Vec<String>,
    #[copy]
    info: CustomCopyType,
}
```

Expands into:

```rust
pub trait KeylimeViewTrait {
    fn node_id(&self) -> &str;
    fn port(&self) -> u16;
    fn api_versions(&self) -> Result<Vec<String>, ParsingError>;
    fn info(&self) -> CustomCopyType;
}

impl KeylimeViewTrait for KeylimeConfig {
    fn node_id(&self) -> &str { &self.node_id }
    fn port(&self) -> u16 { self.port }
    fn api_versions(&self) -> Result<Vec<String>, ParsingError> {
        parse_string_list(&self.api_versions)
    }
    fn info(&self) -> CustomCopyType { self.info }
}
```

Assisted-by: Gemini 2.5 Pro
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Use the procedural macro 'define_view_trait' to create a view limited by
a trait for the AgentConfig instead of creating a new structure.

This removes the PushModelConfig structure. The AgentConfig should be
used directly instead, which has the PushModelConfigTrait implemented
using the procedural macro.

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
@ansasaki
Copy link
Contributor Author

ansasaki commented Jul 4, 2025

/packit test

@ansasaki ansasaki marked this pull request as ready for review July 4, 2025 14:45
Copy link
Contributor

@sergio-correia sergio-correia left a comment

Choose a reason for hiding this comment

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

Nice job, LGTM!

sarroutbi

This comment was marked as outdated.

@keylime-bot keylime-bot assigned ansasaki and unassigned ueno and sarroutbi Jul 4, 2025
@keylime-bot keylime-bot assigned ueno and unassigned ansasaki Jul 7, 2025
@ansasaki ansasaki merged commit 3d72de8 into keylime:master Jul 7, 2025
12 checks passed
@ansasaki ansasaki deleted the unify_config branch July 7, 2025 14:08
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.

4 participants