Add option to disable TTY autologin in grub

[dalees]

Change description

Flatcar defaults tty autologin to true[1], and the core user logged in has passwordless sudo.

To allow disabling of this, we add an ansible variable 'flatcar_disable_autologin' that defaults to false to disable this during the image build[2].

This does not change the autologin during the image build process but secures the console during use within Kubernetes clusters.

[1] flatcar/Flatcar#396
[2] https://www.flatcar.org/docs/latest/setup/customization/other-settings/#enable-flatcar-container-linux-autologin

Related issues

None

Additional context

Links included above.

[k8s-ci-robot]

Hi @dalees. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign mboersma for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[AverageMarcus]

/ok-to-test

[AverageMarcus]

Thanks @dalees this looks great!

Do you have an example of how to set this value when running image-builder? I can never remember how to override the Ansible variables at runtime. 🙈

[dalees]

I'd encourage defaulting flatcar_disable_autologin to true, but I want to provide the least offensive option initially.

The implications of VM console access leading to a trivial root shell without interrupting the host doesn't seem like a great default for Flatcar, or the CAPI image builder.

With this proposed option as false it's a gotcha unless you know about it.

[AverageMarcus]

Also the linting is failing because of the use of sed rather than something like https://docs.ansible.com/ansible/latest/collections/ansible/builtin/replace_module.html

If you feel sed is more appropriate (thats fine) then please also run make lint-ignore to include this entry in our ignores file. :)

[dalees]

Thanks @dalees this looks great!

Do you have an example of how to set this value when running image-builder? I can never remember how to override the Ansible variables at runtime. 🙈

Yeah, certainly. On the ansible-playbook command line add --extra-vars "flatcar_disable_autologin=true". See also [1].

[1] https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_variables.html#defining-variables-at-runtime

[AverageMarcus]

I'd encourage defaulting flatcar_disable_autologin to true, but I want to provide the least offensive option initially.

100% agree. I might actually be ok with us doing that by default now actually. I think it's best to encourage best practices when it comes to security. Especially as we've had a couple CVEs we've had to deal with recently it'd be good to have a stricter default on these things.

@mboersma @jsturtevant @drew-viles any objections to making this the new default?

[AverageMarcus]

Thanks @dalees this looks great!
Do you have an example of how to set this value when running image-builder? I can never remember how to override the Ansible variables at runtime. 🙈

Yeah, certainly. On the ansible-playbook command line add --extra-vars "flatcar_disable_autologin=true". See also [1].

[1] docs.ansible.com/ansible/latest/playbook_guide/playbooks_variables.html#defining-variables-at-runtime

I meant as a user running image-builder via make XXXX can I provide a way to override that ansible variable without changing files. This is required for those running image-builder from the container image as is our recommended approach.

EDIT: Ah! Found it! We support setting ansible_extra_vars as a Packer user variable that then gets passed though! I knew we had something just couldn't remember! 😆

[dalees]

Also the linting is failing because of the use of sed rather than something like https://docs.ansible.com/ansible/latest/collections/ansible/builtin/replace_module.html

If you feel sed is more appropriate (thats fine) then please also run make lint-ignore to include this entry in our ignores file. :)

Ok thanks; I can change the sed into ansible.builtin.replace.

[drew-viles]

I don't have any objections. Looking at it, it seems sensible BUT I don't use nor have I used flatcar directly so can't say with 100% knowledge on the matter! 😆