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

fix GID & UID lookup #19

Closed
wants to merge 1 commit into from
Closed

fix GID & UID lookup #19

wants to merge 1 commit into from

Conversation

kotowicz
Copy link

@kotowicz kotowicz commented Dec 2, 2016

GID is not necessarily the same as UID - invoke '/usr/bin/id' to find out both.
This fixes the following two problems with (non-local) user accounts for me:

  • directory owner & group:

Googleauthenticator::User::Systemwide[my-user]/File[/etc/google-authenticator/my-user]/owner) change from root to my-user failed: Could not find user my-user

  • file owner & group:
Googleauthenticator::User::Systemwide[my-user]/Googleauthenticator::User[my-user]/File[/etc/google-authenticator/my-user/google_authenticator]/owner) change from root to my-user failed: Could not find user my-user
Googleauthenticator::User::Systemwide[my-user]/Googleauthenticator::User[my-user]/File[/etc/google-authenticator/my-user/google_authenticator]/group) change from root to my-user failed: Could not find group my-user

@@ -65,8 +65,8 @@

file {$real_file:
ensure => $ensure,
owner => $real_user,
group => $real_group,
owner => chomp(generate("/usr/bin/id", "-u", $real_user)),
Copy link
Member

Choose a reason for hiding this comment

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

the generate function will run a command on the puppet master, not on the agent.

@alexjfisher
Copy link
Member

I don't think this is the correct fix, but the original code does contain a bug I think. https://github.com/voxpupuli/puppet-googleauthenticator/pull/5/files#r91200384

@juniorsysadmin juniorsysadmin added needs-work not ready to merge just yet tests-fail labels Jan 1, 2017
@ffrank
Copy link

ffrank commented Apr 11, 2017

Agree with @alexjfisher's assessment.

@kotowicz would you be interested in replacing your implementation with something that addresses Alex's linked comment?

@bastelfreak
Copy link
Member

Hi. Thanks for raising this PR, but I'm going to close it due to inactivity. Please reopen it if you're still interested in getting this merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-work not ready to merge just yet tests-fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants