Skip to content

Conversation

@simonostendorf
Copy link
Contributor

Change description

This PR disables usb devices for Flatcar as described in their hardening guide.

Default value is false to keep the current behavior. But a user can disable usb devices in flatcar by setting disable_flatcar_usb to true.

  • Is this change including a new Provider or a new OS? (y/n) n
  • If yes, has the Provider/OS matrix been updated in the readme? (y/n) -
  • If adding a new provider, are you a representative of that provider? (y/n) -

Related issues

none

Additional context

none

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 16, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @simonostendorf. 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 k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 16, 2025
@AndiDog
Copy link
Contributor

AndiDog commented Oct 20, 2025

@mboersma What do you think of disabling USB by default (breaking change for very few users or none, I assume)?

/ok-to-test
/lgtm

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 20, 2025
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 20, 2025
@mboersma
Copy link
Contributor

What do you think of disabling USB by default

It's safer to leave it enabled by default of course. But disabling it by default is probably safe to do: it seems unlikely that USB resources are required in a Kubernetes environment.

@simonostendorf the linter has these complaints:

WARNING Listing 2 violation(s) that are fatal
command-instead-of-module: sed used in place of template, replace or lineinfile module
ansible/roles/node/tasks/flatcar.yml:26 Task/Handler: Set default HOME_MODE in login.defs
no-changed-when: Commands should not change things if nothing needs doing.
ansible/roles/node/tasks/flatcar.yml:26 Task/Handler: Set default HOME_MODE in login.defs

@simonostendorf simonostendorf force-pushed the feat/flatcar-disable-usb branch from 2774e55 to b0097bf Compare October 21, 2025 08:27
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 21, 2025
@simonostendorf
Copy link
Contributor Author

/retest

@AverageMarcus
Copy link
Member

If we're disabling USB by default can we please make sure there are some docs on how to re-enable it. Thinking about the on-prem folks here that might be using Kuberentes in things like factories.

@simonostendorf
Copy link
Contributor Author

If we're disabling USB by default can we please make sure there are some docs on how to re-enable it. Thinking about the on-prem folks here that might be using Kuberentes in things like factories.

Good point, I can do that.

Can you help me with the ci? What do I have to change to get the pull-ova-all job green? :D

@AverageMarcus
Copy link
Member

/retest

It might be a flake. Let's see if it works this time. 🙂

@mboersma
Copy link
Contributor

2025/10/22 13:32:52 packer-plugin-ansible_v1.1.3_x5.0_linux_amd64 plugin: 2025/10/22 13:32:52 [INFO] 0 bytes written for 'stdin'
�[0;32m vsphere-iso.vsphere:�[0m
�[0;32m vsphere-iso.vsphere: TASK [node : Blacklist usb-storage module] *************************************�[0m
�[0;32m vsphere-iso.vsphere: fatal: [default]: FAILED! => {"changed": false, "checksum": "956ad786624ff9c9d06cfc3e78996595024049dc", "msg": "Destination directory /etc/modprobe.d does not exist"}�[0m
�[0;32m vsphere-iso.vsphere:�[0m

@simonostendorf simonostendorf force-pushed the feat/flatcar-disable-usb branch 2 times, most recently from 6cd29ed to 1f735b8 Compare October 23, 2025 09:03
@simonostendorf
Copy link
Contributor Author

I suggest to add the following code to the sed command because it was implemented like this before and flatcar needs the sed command instead of the lineinfile because of the read-only filesystem in the /etc folder:

  args:
    warn: false
  tags:
    - skip_ansible_lint

What do you think?

@mboersma
Copy link
Contributor

@simonostendorf I think that's fine to skip the linting in this case.

@simonostendorf simonostendorf force-pushed the feat/flatcar-disable-usb branch 2 times, most recently from 50495bf to 7615e7e Compare October 23, 2025 19:17
@simonostendorf
Copy link
Contributor Author

next try, hopefully now :/

@simonostendorf
Copy link
Contributor Author

linting and pull job works now, I will add docs and I guess then its ready to merge :D

@simonostendorf simonostendorf force-pushed the feat/flatcar-disable-usb branch from 7615e7e to bb55893 Compare October 23, 2025 20:04
@simonostendorf
Copy link
Contributor Author

/retest

@simonostendorf
Copy link
Contributor Author

/retest

Copy link
Contributor

@mboersma mboersma left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 24, 2025
@mboersma mboersma requested a review from drew-viles October 24, 2025 17:06
Copy link
Contributor

@drew-viles drew-viles left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: drew-viles

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 24, 2025
@k8s-ci-robot k8s-ci-robot merged commit 7701754 into kubernetes-sigs:main Oct 24, 2025
10 checks passed
@simonostendorf simonostendorf deleted the feat/flatcar-disable-usb branch October 24, 2025 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants