-
Notifications
You must be signed in to change notification settings - Fork 510
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
dogtag: implement imds and reverse dns hostname tools #3898
Conversation
486804f
to
ee767de
Compare
Removed unused mockito |
Addressed PR comments |
Updated to use walkdir and fixed documentation |
a78233f
to
bf78405
Compare
Fix embarassing build issue |
8948a57
to
26b1de6
Compare
|
||
dogtag resolves the hostname of a bottlerocket server/instance. It's used to generate settings.network.hostname. To accomplish this, it uses a set of standalone binaries in /var/bottlerocket/dogtag that resolve the hostname via different methods. | ||
|
||
Currently, bottlerocket ships with two hostname resolver binaries: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not ship the IMDS resolver on non-aws variants.
/// Implements a hostname lookup tool by fetching the public hostname | ||
/// from the instance metadata via IMDS. It will interface with IMDS | ||
/// via: | ||
/// | ||
/// * Check for IPv6, default to IPv4 if not available | ||
/// * Check for IMDSv2, fallback to IMDSv1 if not enabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think these comments apply; imdsclient
always uses IMDSv2 and the IPv4 address (which is still valid on IPV6-only ENIs).
pub(super) enum Error { | ||
#[snafu(display("failed to fetch hostname from IMDS: {}", source))] | ||
Imds { source: imdsclient::Error }, | ||
#[snafu(display("no hostname returned by imds"))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "IMDS"
for helper in hostname_helpers.iter() { | ||
let output = process::Command::new(helper) | ||
.output() | ||
.map(Some) | ||
.unwrap_or(None); | ||
if let Some(output) = output.as_ref() { | ||
// Read the std output | ||
if output.status.success() { | ||
let hostname = String::from_utf8_lossy(output.stdout.as_slice()).to_string(); | ||
return Ok(hostname.trim().to_string()); | ||
} | ||
} | ||
} | ||
Err(error::Error::NoHelper {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original logic in ab3b3e6 had a fallback where the host's IP address was used in a slightly cleaned up way (dots replaced with dashes) in these two cases:
lookup_addr()
returns the IP address rather than a name- no host is found at all
That logic was later removed in 0de2d11 since in some cases (EKS-A clusters) we want the hostname to exactly match the IP.
0b9658f fixed a different bug related to direct use of an IPv6 address, where colons needed to be replaced with dashes.
Is that handled in netdog
? It might be cleaner to do it here for separation of concerns, so netdog only has to put in an IP address and it gets back a hostname (from IMDS, reverse DNS, IPv4 address, or sanitized IPv6 address).
let output = process::Command::new(helper) | ||
.output() | ||
.map(Some) | ||
.unwrap_or(None); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see where we are passing the ip_address
argument, which at least the reverse DNS helper needs or else it will immediately error out.
Issue number: 3871
Closes #3871
Description of changes:
Implements two hostname discovery helpers imds and reverse dns. These small programs will be used by netdog to discover a node's hostname. This PR only covers the small implementation of the dogtag helpers. Once this is merged I can adjust the netdog PR to utilize them.
Testing done:
Built both helpers currently and tested on instances to validate they have returned the appropriate and correct hostnames.
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.