Skip to content
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

netdog: Add initial networkd config structs and associated Display macro #3134

Merged
merged 3 commits into from
Jun 14, 2023

Conversation

zmrow
Copy link
Contributor

@zmrow zmrow commented May 22, 2023

Issue number:
Related to #2449

Description of changes:
This is the first in a series of changes to netdog that add the ability to generate systemd-networkd config files. This PR adds the initial set of structs that represent the two kinds of systemd-networkd config files we currently use: .network and .netdev. These structs contain the config entries we currently will make use of; it is expected they will grow over time.

These structs must be serialized to file. Though a serde library exists for INI, systemd doesn't strictly follow INI spec as it allows for duplicate sections and duplicate keys within a section. Rather than hack around the library, I opted to write a proc macro that generates Display implementations in the systemd unit format for the structs so the user can serialize the structs to string/file. The proc macro allows for annotation of structs that represent config file sections, as well as fields that represent entries within the sections.

This allows us to write the following:

// Very abbreviated example
#[derive(SystemdUnit)]
struct NetworkConfig {
    match_section: Option<MatchSection>
}

#[derive(SystemdUnitSection)
#[systemd(section = "Match")] // The name of the networkd config section
struct MatchSection {
    #[systemd(entry = "Name")] // The name of the networkd config entry
    name: Option<String>
}

which would represent the following section of a .network file:

[Match]
Name=...

Testing done:

  • All unit tests continue to pass
    Additional integration tests to come as the code is added to build these structs.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@zmrow zmrow requested review from bcressey, webern and yeazelm May 22, 2023 17:47
@zmrow zmrow changed the title netdog: Add initial networkd config structs and associated display macro netdog: Add initial networkd config structs and associated Display macro May 22, 2023
Copy link
Contributor

@yeazelm yeazelm left a comment

Choose a reason for hiding this comment

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

I didn't deeply review the proc_macro or darling usage but the Networkd specifics all look right!

sources/api/netdog/networkd-derive/Cargo.toml Outdated Show resolved Hide resolved
sources/api/netdog/src/networkd/config/network.rs Outdated Show resolved Hide resolved
sources/api/netdog/src/networkd/config/network.rs Outdated Show resolved Hide resolved
sources/api/netdog/src/networkd/config/network.rs Outdated Show resolved Hide resolved
sources/api/netdog/networkd-derive/README.md Outdated Show resolved Hide resolved
sources/api/netdog/networkd-derive/README.md Outdated Show resolved Hide resolved
@zmrow
Copy link
Contributor Author

zmrow commented Jun 8, 2023

^ Addresses all the comments...

(Ugh, I rebased onto develop and didn't push before I added my updates... 😢 Sorry about that)

@@ -0,0 +1,129 @@
# systemd-derive
Copy link
Contributor

Choose a reason for hiding this comment

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

I may benefit from this crate. Would it make sense to make it its own lib in sources out of api? We could name the crate systemd-lib and extend it as needed. I already know I want to have a function to receive the file descriptors passed by systemd, and I was planning to have this as a standalone lib that may be used by other “dogs”. We will use this both in EFS utils replacement, and when we decide to implement socket activation for API server.

Copy link
Contributor Author

@zmrow zmrow Jun 13, 2023

Choose a reason for hiding this comment

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

I don't mind if the macros move out of netdog, but I'd rather not conflate these macros as part of a larger systemd-related "library" crate.

Copy link
Contributor

@bcressey bcressey left a comment

Choose a reason for hiding this comment

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

LGTM. Just a few documentation nits.

sources/api/netdog/systemd-derive/README.md Outdated Show resolved Hide resolved
sources/api/netdog/systemd-derive/Cargo.toml Outdated Show resolved Hide resolved
sources/api/netdog/systemd-derive/README.md Outdated Show resolved Hide resolved
sources/api/netdog/systemd-derive/README.md Outdated Show resolved Hide resolved

impl ToTokens for SystemdUnitSectionField {
fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) {
let struct_field_name = self.ident.as_ref().expect("Should always have a name");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an attribute that ensures this property?

Copy link
Contributor Author

@zmrow zmrow Jun 13, 2023

Choose a reason for hiding this comment

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

Yes, but indirectly :)

The attribute supports(struct_named) on the containing struct SystemdUnitSection ensures the macro only supports structs with named fields. This means it does not support tuple or newtype structs.

SystemdUnitSectionField derives FromField, meaning it can only be "deserialized" from a struct field, and given the above struct_named attribute is guaranteed to have an ident/name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regardless - if the developer attempted to do "the wrong thing" here, it would be a compile-time issue printing out the expect message.

This commit adds a macro that generates Display implementations for
structs representing `systemd` unit files.  This allows serializing the
structs to a string suitable for writing to file.
This commit adds the initial set of structs that represent the netdev
and network configuration files for systemd-networkd.
@zmrow
Copy link
Contributor Author

zmrow commented Jun 13, 2023

Fixed up @bcressey 's requests

@zmrow zmrow merged commit d966ae0 into bottlerocket-os:develop Jun 14, 2023
@zmrow zmrow deleted the generate-networkd branch June 14, 2023 16:25
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.

5 participants