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

providers: Add "akamai" provider #1062

Merged
merged 1 commit into from
Apr 20, 2024
Merged

providers: Add "akamai" provider #1062

merged 1 commit into from
Apr 20, 2024

Conversation

nesv
Copy link
Contributor

@nesv nesv commented Apr 1, 2024

The "akamai" provider adds support for retrieving configuration from Akamai Connected Cloud's (a.k.a. Linode) Metadata Service.

References: flatcar/Flatcar#1404
References: coreos/fedora-coreos-tracker#1701
References: coreos/ignition#1841

Note

I'll push any requested changes as fixup commits, just so that the Github comment threads don't get messed up. If this PR is approved, I will squash these fixup commits out prior to merge.

@prestist
Copy link
Contributor

prestist commented Apr 8, 2024

@nesv Awesome, thank you for doing this!

Took a quick look at it, and overall it looks sane, I need to do some double checking on some of the meat of the mod.rs but so far so good!

@nesv
Copy link
Contributor Author

nesv commented Apr 8, 2024

@nesv Awesome, thank you for doing this!

Took a quick look at it, and overall it looks sane, I need to do some double checking on some of the meat of the mod.rs but so far so good!

Thanks a bunch, @prestist!

I misunderstood how serde handles deserializing untagged enums. I had hoped I could use an enum to enumerate all possible values for a field, but it would seem that isn't possible; I have removed the InterfacePurpose enum, in favour of an Option<String>, but I am not too enthused with that tradeoff. It feels like a loss in precision, though it does keep things flexible should the API change in the future.

@nesv
Copy link
Contributor Author

nesv commented Apr 8, 2024

I was hoping to provide an as-complete-as-possible MetadataProvider implementation, but I don't think there is enough networking information that comes from Linode's Metadata Service to be able to implement the MetadataProvider::networks method.

@nesv
Copy link
Contributor Author

nesv commented Apr 10, 2024

The latest patches have been checked, and they work! All of the fixup commits have been squashed and rebased onto main.

Copy link
Contributor

@tormath1 tormath1 left a comment

Choose a reason for hiding this comment

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

FWIW, same as the Ignition PR (coreos/ignition#1841) it has been tested on Flatcar and it works as expected on Akamai/Linode from a functional aspect.

src/providers/akamai/mod.rs Outdated Show resolved Hide resolved
src/providers/akamai/mod.rs Outdated Show resolved Hide resolved
src/providers/akamai/mod.rs Outdated Show resolved Hide resolved
src/providers/akamai/mod.rs Outdated Show resolved Hide resolved
src/providers/akamai/mod.rs Outdated Show resolved Hide resolved
src/providers/akamai/mod.rs Outdated Show resolved Hide resolved
src/providers/akamai/mod.rs Outdated Show resolved Hide resolved
src/providers/akamai/mod.rs Outdated Show resolved Hide resolved
@nesv
Copy link
Contributor Author

nesv commented Apr 19, 2024

@jlebon Requested changes have been made! Thank you for taking the time to review this PR.

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

LGTM overall, though looks like there are a few lints to address. While we're here, can you squash your fixup commits together? We don't "Squash and merge" in this repo.

@nesv
Copy link
Contributor Author

nesv commented Apr 19, 2024

LGTM overall, though looks like there are a few lints to address.

Gah, oversights on my part. I'll get those fixed up.

While we're here, can you squash your fixup commits together? We don't "Squash and merge" in this repo.

Absolutely! I was planning on squashing my own fixup commits out anyways. 😄

The "akamai" provider adds support for retrieving configuration from
Akamai Connected Cloud's (a.k.a. Linode) [Metadata
Service][metadata-service].

References: flatcar/Flatcar#1404
References: coreos/fedora-coreos-tracker#1701
References: coreos/ignition#1841

[metadata-service]: https://www.linode.com/docs/products/compute/compute-instances/guides/metadata/
@nesv
Copy link
Contributor Author

nesv commented Apr 19, 2024

@jlebon Fixup commits squashed, and I've applied the linting suggestions.

@jlebon jlebon enabled auto-merge April 20, 2024 21:08
@jlebon jlebon merged commit f918f4c into coreos:main Apr 20, 2024
9 checks passed
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