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

prairiedog: treat value-less key differently than "empty" values #2565

Merged
merged 1 commit into from
Nov 11, 2022

Conversation

etungsten
Copy link
Contributor

@etungsten etungsten commented Nov 9, 2022

Issue number:

Resolves #2359

Description of changes:

    prairiedog: treat value-less key differently than "empty" values
    
    Before this change, prairedog did no special handling for value-less
    bootconfig keys and would just write out 'key ='. The kernel is
    insensitive to whitespace so it would grab the next k/v from the
    bootconfig and substitute it for the value.
    
    With this change, prairiedog now differentiates between value-less keys
    and keys with empty values (e.g. 'key = ""') both in parsing an existing
    bootconfig file as well as when it generates the bootconfig initrd from
    existing settings.
    
    When the key is value-less, prariedog will just write out the key
    without '=' for its line entry in the bootconfig (e.g. just 'key').

Testing done:
The new unit test passes.
Built aws-k8s-1.23 and passed the following user-data:

[settings.boot.kernel-parameters]
"console" = [
    "tty0",
    "ttyS1,115200n8",
]
"crashkernel" = [
    "2G-:256M",
]
"slub_debug" = [
    "options,slabs",
]
"usbcore.quirks" = [
    "0781:5580:bk",
    "0a5c:5834:gij",
]
[settings.boot.init-parameters]
"log_level" = ["debug"]
"splash" = []

Host comes up fine. Prairiedog writes out the bootconfig as expected. Note how init.splash is value-less:

bash-5.1# cat /var/lib/bottlerocket/bootconfig.data                                                      
kernel.crashkernel = "2G-:256M"                                                                          
kernel.slub_debug = "options,slabs"                                                                      
kernel.usbcore.quirks = "0781:5580:bk", "0a5c:5834:gij"                                                  
kernel.console = "tty0", "ttyS1,115200n8"                                                                
init.log_level = "debug"                                                                                 
init.splash          

After a reboot, kernel interprets the bootconfig correctly:

bash-5.1# cat /proc/bootconfig 
kernel.console = "tty0", "ttyS1,115200n8"
kernel.crashkernel = "2G-:256M"
kernel.slub_debug = "options,slabs"
kernel.usbcore.quirks = "0781:5580:bk", "0a5c:5834:gij"
init.splash = ""
init.log_level = "debug"

The kernel commandline is written out as expected:

bash-5.1# cat /proc/cmdline 
crashkernel="2G-:256M" slub_debug="options,slabs" usbcore.quirks="0781:5580:bk" usbcore.quirks="0a5c:5834:gij" console="tty0" console="ttyS1,115200n8" BOOT_IMAGE=(hd0,gpt3)/vmlinuz console=tty0 console=ttyS0,115200n8 net.ifnames=0 netdog.default-interface=eth0:dhcp4,dhcp6? quiet bootconfig root=/dev/dm-0 rootwait ro raid=noautodetect random.trust_cpu=on selinux=1 enforcing=1 dm_verity.max_bios=-1 dm_verity.dev_wait=1 "dm-mod.create=root,,,ro,0 1884160 verity 1 PARTUUID=e00ffc4b-0ffd-4c54-96da-179dcf308bb0/PARTNROFF=1 PARTUUID=e00ffc4b-0ffd-4c54-96da-179dcf308bb0/PARTNROFF=2        4096 4096 235520 1 sha256 5370126d558ff5a2c1b100c3efb1da864dc293e4f314216647743fd96c50e70b 227c3407308c594bb0790cdfa7b322d04ec509ceaa32ace7198c23e285a5817f        2 restart_on_corruption ignore_zero_blocks" -- systemd.log_target=journal-or-kmsg systemd.log_color=0 systemd.show_status=true log_level="debug" splash

Note how on the kernel command line, splash is just written out as splash for the init param.

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

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

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

Great having test coverage!

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.

🧑‍🔧

Nice work!

@bcressey
Copy link
Contributor

bcressey commented Nov 10, 2022

According to the boot config docs:

There can be a key which doesn’t have value or has an empty value. Those keys are used for checking if the key exists or not (like a boolean).

I would expect to have a way to render just splash onto the kernel command line, rather than splash="". Is there a syntax in the boot config format like one of these that would work?

splash =
splash

@etungsten
Copy link
Contributor Author

splash = is what prompted the linked issue. The kernel apparently is insensitive to whitespace/newlines when parsing the bootconfig from initrd so it grabs the next line as the value for splash. I'm trying out just splash to see if it likes it better.

@etungsten etungsten changed the title prairiedog: serialize value-less bootconfig keys correctly prairiedog: treat value-less key differently than "empty" values Nov 10, 2022
@etungsten
Copy link
Contributor Author

Push above changes approach to value-less keys. Prairiedog now differentiates between value-less keys and keys with empty values. Value-less keys are always written out to the bootconfig with just their key.

Updated PR description to include new testing details. I verified the kernel no longer misinterprets value-less keys like splash and treats it as its own bootconfig entry.

Before this change, prairedog did no special handling for value-less
bootconfig keys and would just write out 'key ='. The kernel is
insensitive to whitespace so it would grab the next k/v from the
bootconfig and substitute it for the value.

With this change, prairiedog now differentiates between value-less keys
and keys with empty values (e.g. 'key = ""') both in parsing an existing
bootconfig file as well as when it generates the bootconfig initrd from
existing settings.

When the key is value-less, prariedog will just write out the key
without '=' for its line entry in the bootconfig (e.g. just 'key').
@etungsten
Copy link
Contributor Author

Push above adds to the unit test to show that key = is still parse-able by prairiedog

Copy link
Contributor

@mchaker mchaker left a comment

Choose a reason for hiding this comment

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

Thank you so much for this!

@etungsten etungsten merged commit c2d1bf8 into bottlerocket-os:develop Nov 11, 2022
@etungsten etungsten deleted the value-less-key branch November 11, 2022 01:02
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.

Prairiedog and the kernel may disagree about parsing boot config keys without values
7 participants