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 non-root build and signing for rpm packages #47545

Merged
merged 4 commits into from May 17, 2018

Conversation

dmurphy18
Copy link
Contributor

What does this PR do?

Restores the ability to build and sign packages for rpm builds as a non-root user
This ability was changed in the Nitrogen release.

What issues does this PR fix or reference?

vmware-archive/salt-pack#545

Previous Behavior

Could only perform builds and signing for rpm packages as root since 2017.7.0

New Behavior

Restores ability to perform builds and signing for rpm packages as non-root user, for example: builder, with Salt minions as prior to the 2017.7.0 release.

Tests written?

No, built and signed Centos 7 packages.

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@dmurphy18 dmurphy18 force-pushed the fix_nonroot_build branch 2 times, most recently from 9bd6292 to 45fa27a Compare May 8, 2018 21:11
@rallytime
Copy link
Contributor

re-run py2

Copy link
Contributor

@terminalmage terminalmage left a comment

Choose a reason for hiding this comment

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

One small problem here. Also, I thought we standardized on using user instead of runas everywhere else beside cmdmod.py.

srpms = os.path.join(tree_base, 'SRPMS')
ret = []
if not os.path.isdir(dest_dir):
os.makedirs(dest_dir)
__salt__['file.chown'](path=dest_dir, user=runas, group='mock')
Copy link
Contributor

Choose a reason for hiding this comment

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

This appears to be chown'ing a directory that doesn't exist, since you have removed the os.makedirs(dest_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.

Thanks for the catch, that looks like it should have been makedirs_perms, will fix

@dmurphy18
Copy link
Contributor Author

@terminalmage There is a change coming from a contributor see #47056, so minimizing merge (user vs runas) when that is delivered. Post merge can update both debbuild.py and rpmbuild.py, as the contributor is also updating debbuild.py.

Also reluctant to change preexisting code for new naming scheme in minor release, fix up in develop after merge.

Copy link
Contributor

@terminalmage terminalmage left a comment

Choose a reason for hiding this comment

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

Was not aware that runas was already used elsewhere in the module. That's fine, then. I just didn't want to be introducing new usage of runas if we were moving away from it.

@dmurphy18
Copy link
Contributor Author

re-run py2

@rallytime rallytime merged commit dbf12f9 into saltstack:2018.3 May 17, 2018
@dmurphy18 dmurphy18 deleted the fix_nonroot_build branch August 31, 2018 17:49
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.

None yet

3 participants