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

Document sudo policy for gitfs post-recieve hook #33900

Merged
merged 2 commits into from
Jun 14, 2016

Conversation

amendlik
Copy link
Contributor

@amendlik amendlik commented Jun 9, 2016

What does this PR do?

Update documentation for creating a git hook to update gitfs.

What issues does this PR fix or reference?

Because the hook is run as the user performing the push,
salt-call will typically fail.

Previous Behavior

Git pushes not done by root would fail to run the git
post-receive hook

New Behavior

Updates the documentation to use sudo for salt-call, and provides
a sudo policy that permits any user to run the post-recieve hook.

Tests written?

No

Because the hook is run as the user performing the push,
salt-call will typically fail. This documentation change
uses sudo for salt-call, and provides a sudo policy that
permits any user to run the post-recieve hook.
@terminalmage
Copy link
Contributor

I think we need more information on why we are telling people to use the sudoers change. Also, salt may not be running as root, so we should clarify a few points:

  1. This sudoers change needs to be made in cases where the user on the git server initiating the push is not the same as the user under which salt is running.
  2. The policy should instead be ALL ALL=(root) NOPASSWD: SALT_GIT_HOOK to be more restrictive.
  3. A clarification should be added noting that where the root user is mentioned (in the hook itself, and in point number 2 above), root should be the user under which salt is running (which in the vast majority of cases will be root, but in some cases may be a different user).

@cachedout cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jun 10, 2016
@amendlik
Copy link
Contributor Author

Thank you for the feedback, @terminalmage. I agree with everything you've said and will make those changes.

@terminalmage
Copy link
Contributor

Awesome, thanks! And thanks for contributing.

@cachedout cachedout merged commit a400f6a into saltstack:2016.3 Jun 14, 2016
@amendlik amendlik deleted the gitfs-hook-doc branch May 12, 2017 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants