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

Allow overriding the "pretty" name of the OS inside the image #1330

Merged
merged 2 commits into from
Feb 20, 2021

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Feb 18, 2021

Issue number:

Fixes #1314 - thanks @diranged!

Description of changes:

Similar to #1312, this passes another field through for user customization - this time the "pretty name" that shows up in os-release, which in turn is used by k8s and other tools.

The second commit reorganizes buildsys's builder module to use argument vectors instead of strings, so we can handle arbitrary characters, rather than using "|" as a marker (and subsequently making it unavailable to names).

Testing done:

Specifying a pretty name works:

> cargo make -e BUILDSYS_PRETTY_NAME="My Pretty Name" && cargo make ami -e BUILDSYS_PRETTY_NAME="My Pretty Name"
...
bash-5.0# cat /etc/os-release
PRETTY_NAME="My Pretty Name 1.0.5"
...

Default is still OK:

> cargo make && cargo make ami
...
bash-5.0# cat /etc/os-release 
PRETTY_NAME="Bottlerocket OS 1.0.5"
...

Retested with the second (refactor) commit and found that buildsys still produced the artifacts in the same way, and the resulting AMIs were still happy and healthy. Here's one with a weird name:

> cargo make -e BUILDSYS_PRETTY_NAME="My|Pretty@Name" && cargo make ami -e BUILDSYS_PRETTY_NAME="My|Pretty@Name"
...
bash-5.0# cat /etc/os-release 
PRETTY_NAME="My|Pretty@Name 1.0.5"
...
bash-5.0# apiclient -u /os      
{"pretty_name":"My|Pretty@Name 1.0.5","variant_id":"aws-k8s-1.17","version_id":"1.0.5","build_id":"ace13fe9","arch":"x86_64"}

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.

Copy link
Contributor

@zmrow zmrow left a comment

Choose a reason for hiding this comment

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

🍰

Some arguments have internal spaces, like the list of packages in --build-arg
PACKAGES=a b c.  This was handled using a "|" marker that was later replaced by
spaces.  For other uses of --build-arg, like PRETTY_NAME, we don't want to
arbitrarily restrict the characters that can be used, in case someone wants "|"
in their OS name.

This change stores arguments directly in vectors, rather than strings that we
split on space, so that we can pass through arbitrary characters.
@tjkirch
Copy link
Contributor Author

tjkirch commented Feb 18, 2021

^ This push rebases on develop.

@tjkirch
Copy link
Contributor Author

tjkirch commented Feb 18, 2021

ace13fe

The following push (which GitHub doesn't show distinctly) adds the above commit, which reorganizes the buildsys file to use argument vecs instead of strings, so that we can handle arbitrary characters. This is intended to address @samuelkarp's concern.

@tjkirch tjkirch merged commit a5ab4c2 into bottlerocket-os:develop Feb 20, 2021
@tjkirch tjkirch deleted the pretty-name branch February 20, 2021 01:07
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.

Can we customize the "OS Image" name for our images?
5 participants