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

adding missing GID value when fetching Hostuser #48245

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

eriktate
Copy link
Contributor

@eriktate eriktate commented Oct 31, 2024

Fixes #48243

This fixes a regression that breaks migrating existing, unmanaged host users to Teleport. The missing value for GID when fetching a HostUser causes an error while trying to conver to an int for home directory creation. The migration flow is covered by unit tests, but since this only manifests in the linux backend it wasn't caught. I added a new host user integration test to cover this regression going forward.

changelog: Fixed an issue preventing migration of unmanaged users to Teleport host users when including teleport-keep in a role's host_groups.

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-48245.d3pp5qlev8mo18.amplifyapp.com

t.Cleanup(func() { cleanupUsersAndGroups([]string{testuser}, []string{types.TeleportKeepGroup}) })

users := srv.NewHostUsers(context.Background(), presence, "host_uuid")
expectedHome := filepath.Join("/home", testuser)
Copy link
Contributor

Choose a reason for hiding this comment

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

should we use a temp dir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We actually shouldn't be creating a directory at all, this was left over from the test I coped as a base for this one 😅 There's a few tests in here that rely on creating directories in /home that probably need to be updated, so I'll make a note to fix those in a separate PR once I'm finished with v17 testing 👍

@greedy52 greedy52 requested a review from rosstimothy October 31, 2024 21:05
@eriktate eriktate force-pushed the eriktate/48243/fix-unmanaged-user-migration branch from 90c42c0 to 0a2c5a6 Compare October 31, 2024 21:45
@eriktate eriktate requested a review from greedy52 November 4, 2024 20:46
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from ryanclark November 5, 2024 14:42
@eriktate eriktate added this pull request to the merge queue Nov 5, 2024
Merged via the queue into master with commit 83b1d23 Nov 5, 2024
40 checks passed
@eriktate eriktate deleted the eriktate/48243/fix-unmanaged-user-migration branch November 5, 2024 16:30
@public-teleport-github-review-bot

@eriktate See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Create PR
branch/v16 Create PR
branch/v17 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrating unmanaged users to Teleport fails
3 participants