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
vsphere: Add support for passing ignition user-data #458
vsphere: Add support for passing ignition user-data #458
Conversation
/hold Needs further testing. |
Still open question: Ignition uses guestinfo keys specific to Ignition, so this only supports Ignition as-is. I guess we're only interested in supporting Ignition in OpenShift, so should we maybe rename the |
} | ||
|
||
// EncodeIgnitionConfig attempts to decode the given data until it looks to be | ||
// plain-text, then returns a base64 encoded version of that plain-text. |
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: this is not ignition specific, could be may be called base64DataToString
, sanitiseBase64
or similar.
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 implementation happens to not be Ignition specific, but the function is meant to encode Ignition data for inclusion in guestinfo variables, so I think the name should reflect that.
for { | ||
decoded, err := base64.StdEncoding.DecodeString(string(data)) | ||
if err != nil { | ||
break |
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: may be log something here
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.
Like what? This is expected to break. It breaks when the input looks like plain-text, i.e. can no longer be successfully decoded. And the input is considered secret, so it shouldn't be logged.
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.
actually don't we want to return err here?
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.
No, we want to break and then re-encode as base64. This function ensures that you can pass either plain-text, or already encoded data (e.g. straight from a k8s secret) to IgnitionConfig()
and have it do the right thing.
This adds support for fetching the user-data secret referenced in the ProviderSpec of a Machine using the vsphere provider, and passing that data as ignition config via guestinfo variables.
This is working now. We just need to decide if we call the field |
/test unit |
/hold cancel |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/refresh |
This adds support for fetching the user-data secret referenced in the
ProviderSpec of a Machine using the vsphere provider, and passing that
data as ignition config via guestinfo variables.