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

Pluto settings models #89

Merged
merged 3 commits into from
Aug 14, 2024
Merged

Conversation

cbgbt
Copy link
Contributor

@cbgbt cbgbt commented Aug 13, 2024

Issue number:
bottlerocket-os/bottlerocket#4135

Description of changes:
Pluto requests an initial set of settings from the Bottlerocket API as dependencies needed to generate additional kubernetes settings.

There was a bug in pluto in which no-proxy settings were attempted to be deserialized into a String when the value in the model is stored as a Vec. This updates pluto to use the settings models from the settings SDK to deserialize the settings returned from the API.

Testing done:

  • unit tests pass
  • test with proxy enabled
  • test cluster dns IP properly set in a cluster
  • test max pods properly set
  • test provider_id properly set
  • test node_name properly set

I enabled https-proxy pointing to a tinyproxy that I configured, then tested instance startup with various no-proxy settings:

  • Unset no-proxy: Noted traffic in my proxy logs and also that the Cloudtrail noted the IP address of my proxy as the source for EKS DescribeCluster traffic
  • no-proxy: ["*"] bypassed the proxy when communicating with the EKS API
  • No proxy settings: all EKS settings still set as usual

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.

@cbgbt cbgbt force-pushed the pluto-settings-models branch 2 times, most recently from 7f5dbee to 4a56be1 Compare August 13, 2024 23:18
@cbgbt
Copy link
Contributor Author

cbgbt commented Aug 13, 2024

^ Force push minor readability fix.

Comment on lines 34 to 41
pub fn read(&self) -> &SettingsView {
&self.readonly
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe this? read sounds like it will read all settings, and someone not using the macros might call it by mistake:

Suggested change
pub fn read(&self) -> &SettingsView {
&self.readonly
}
pub fn initial(&self) -> &SettingsView {
&self.initial
}

@cbgbt cbgbt force-pushed the pluto-settings-models branch 3 times, most recently from 4a56be1 to 140eea2 Compare August 14, 2024 17:31
@cbgbt
Copy link
Contributor Author

cbgbt commented Aug 14, 2024

Sorry for churn. Forgot to push after rebasing to make the inter-diffs cleaner.

^ Rebases onto develop

@cbgbt cbgbt marked this pull request as ready for review August 14, 2024 19:08
@cbgbt
Copy link
Contributor Author

cbgbt commented Aug 14, 2024

^ updated to include metadata for new settings models release.

@cbgbt cbgbt merged commit f028b32 into bottlerocket-os:develop Aug 14, 2024
2 checks passed
@cbgbt cbgbt deleted the pluto-settings-models branch August 14, 2024 20:33
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.

3 participants