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

MODULES-9447 -- Narrow dependency between removed user and group. #232

Merged
merged 2 commits into from
Jun 25, 2019

Conversation

pillarsdotnet
Copy link
Contributor

@pillarsdotnet pillarsdotnet commented Jun 24, 2019

(MODULES-9447) Unfortunately, we still have some issues with dependency cycles when removing accounts.

This line was added to ensure that we don't remove a user's primary group before removing a user:

User[$name] -> Group <| |>

This causes a dependency cycle whenever any code in the same run requires any group, and must run before the user is deleted. For instance, if an exec is used to backup a homedir before deletion, and the exec is run as user and group root, then a dependency cycle is created.

At a minimum, the dependency should be narrowed to:

User[$name] -> Group <| ensure == 'absent' |>

The included acceptance tests fails without the above change, and passes with it.

@pillarsdotnet pillarsdotnet force-pushed the modules-9447 branch 2 times, most recently from 23f42d1 to e868d02 Compare June 25, 2019 00:49
@carabasdaniel
Copy link
Contributor

Hello @pillarsdotnet,

I've ran the acceptance tests on CentOS, Debian and Ubuntu and apparently there is one test that is failing with the following error:

`

  1. accounts::user define create duplicate users with same uid runs with no errors
    On host localhost:2223' Failure/Error: apply_manifest(pp_user_with_duplicate_uid, catch_failures: true) RuntimeError: {"_error"=>{"kind"=>"puppetlabs.tasks/connect-error", "msg"=>"Authentication failed for user root@localhost", "details"=>{}, "issue_code"=>"AUTH_ERROR"}}
    Could you please fix the PR to make sure that all the acceptance tests are passing ?

Thanks.

@pillarsdotnet
Copy link
Contributor Author

... Could you please fix the PR to make sure that all the acceptance tests are passing ? ...

@carabasdaniel -- I can't replicate your error, but have modified the acceptance test slightly. Hopefully it will both demonstrate the bug and resolve your error.

@carabasdaniel
Copy link
Contributor

@pillarsdotnet Thanks for the quick reply, trying it out now.

@pillarsdotnet
Copy link
Contributor Author

pillarsdotnet commented Jun 25, 2019

@carabasdaniel -- You might want to wait until the acceptance tests pass in Travis-CI first...

@carabasdaniel
Copy link
Contributor

👍 Looks good.

@carabasdaniel carabasdaniel merged commit 370352e into puppetlabs:master Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants