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

cmd/initContainer: Simplify removing the user's password #1349

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

debarshiray
Copy link
Member

@debarshiray debarshiray commented Aug 15, 2023

It's one less invocation of an external command, which is good because
spawning a new process is generally expensive.

One positive side-effect of this is that on some Active Directory
set-ups, the entry point no longer fails with:

  Error: failed to remove password for user login@company.com: failed
      to invoke passwd(1)

... because of:

  # passwd --delete login@company.com
  passwd: Libuser error at line: 210 - name contains invalid char `@'.

This is purely an accident, and isn't meant to be an intential change to
support Active Directory. Tools like useradd(8) and usermod(8) from
Shadow aren't meant to work with Active Directory users, and, hence, it
can still break in other ways. For that, one option is to expose $USER
from the host operating system to the Toolbx container through a Varlink
interface that can be used by nss-systemd inside the container.

Based on an idea from Si.

#585

@debarshiray debarshiray marked this pull request as draft August 15, 2023 18:12
@debarshiray debarshiray marked this pull request as ready for review August 15, 2023 18:36
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/0038398cc57a4aa2a2ecdf775d39a8e5

unit-test RETRY_LIMIT in 34s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 33s
unit-test-restricted RETRY_LIMIT in 34s
system-test-fedora-rawhide RETRY_LIMIT in 33s
✔️ system-test-fedora-38 SUCCESS in 29m 09s
✔️ system-test-fedora-37 SUCCESS in 28m 20s

@debarshiray debarshiray changed the title [WIP] Support Active Directory cmd/initContainer: Simplify removing the user's password Aug 22, 2023
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/2b23914c714f4899b41ab68d43269aa0

unit-test RETRY_LIMIT in 33s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 18s
unit-test-restricted RETRY_LIMIT in 30s
system-test-fedora-rawhide RETRY_LIMIT in 31s
✔️ system-test-fedora-38 SUCCESS in 28m 30s
✔️ system-test-fedora-37 SUCCESS in 27m 57s

@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/272a98df4cf6405583f72575fe0984ab

unit-test NODE_FAILURE Node request 200-0006306328 failed in 0s
unit-test-migration-path-for-coreos-toolbox NODE_FAILURE Node request 200-0006306329 failed in 0s
unit-test-restricted NODE_FAILURE Node request 200-0006306330 failed in 0s
system-test-fedora-rawhide NODE_FAILURE Node request 200-0006306331 failed in 0s
system-test-fedora-38 NODE_FAILURE Node request 200-0006306332 failed in 0s
system-test-fedora-37 NODE_FAILURE Node request 200-0006306333 failed in 0s

It's one less invocation of an external command, which is good because
spawning a new process is generally expensive.

One positive side-effect of this is that on some Active Directory
set-ups, the entry point no longer fails with:
  Error: failed to remove password for user login@company.com: failed
      to invoke passwd(1)

... because of:
  # passwd --delete login@company.com
  passwd: Libuser error at line: 210 - name contains invalid char `@'.

This is purely an accident, and isn't meant to be an intential change to
support Active Directory.  Tools like useradd(8) and usermod(8) from
Shadow aren't meant to work with Active Directory users, and, hence, it
can still break in other ways.  For that, one option is to expose $USER
from the host operating system to the Toolbx container through a Varlink
interface that can be used by nss-systemd inside the container.

Based on an idea from Si.

containers#585
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/324eed88ed8544c28849d7cbf56ab9af

✔️ unit-test SUCCESS in 8m 26s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 09s
✔️ unit-test-restricted SUCCESS in 7m 40s
system-test-fedora-rawhide FAILURE in 33m 58s
✔️ system-test-fedora-38 SUCCESS in 30m 31s
✔️ system-test-fedora-37 SUCCESS in 29m 20s

@debarshiray
Copy link
Member Author

There are still some test failures on Fedora Rawhide. For example:

fedora-rawhide | not ok 3 help: Run command 'help' in 145ms
fedora-rawhide | # (from function `assert_line' in file test/system/libs/bats-assert/src/assert.bash, line 479,
fedora-rawhide | #  in test file test/system/002-help.bats, line 45)
fedora-rawhide | #   `assert_line --index 0 --partial "toolbox(1)"' failed
fedora-rawhide | # /usr/bin/man
fedora-rawhide | #
fedora-rawhide | # -- line does not contain substring --
fedora-rawhide | # index     : 0
fedora-rawhide | # substring : toolbox(1)
fedora-rawhide | # line      : troff:<standard input>:33: warning: cannot select font 'C'
fedora-rawhide | # --

I believe these are because of changes in various other components in Fedora 39, which we need to track down one by one and work out a fix.

In the mean time, I am going to temporarily override these failures.

@debarshiray debarshiray merged commit b1b1d45 into containers:main Sep 14, 2023
2 checks passed
@debarshiray debarshiray deleted the wip/rishi/issue-585 branch September 14, 2023 11:22
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.

1 participant