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

Clone git repository as $user #352

Merged
merged 4 commits into from
Mar 31, 2022
Merged

Clone git repository as $user #352

merged 4 commits into from
Mar 31, 2022

Conversation

smortex
Copy link
Member

@smortex smortex commented Mar 26, 2022

Instead of cloning as root and using a file resource to change files
ownership to the configured user, directly clone the repository with
this user.

This fix a warning issued by puppet:
Warning: The directory '/srv/puppetboard/puppetboard' contains 1936 entries, which exceeds the default soft limit 1000

Fixes #351

Instead of cloning as root and using a file resource to change files
ownership to the configured user, directly clone the repository with
this user.

This fix a warning issued by puppet:
Warning: The directory '/srv/puppetboard/puppetboard' contains 1936 entries, which exceeds the default soft limit 1000

Fixes #351
We python module is fragile and automatic detection of the need of
running pip install is sometimes broken.
@smortex smortex force-pushed the vcsrepo-user branch 2 times, most recently from f7ff1e8 to ee80856 Compare March 28, 2022 03:16
spec/acceptance/class_spec.rb Outdated Show resolved Hide resolved
spec/acceptance/class_spec.rb Outdated Show resolved Hide resolved
@smortex smortex force-pushed the vcsrepo-user branch 2 times, most recently from e57ea72 to 58f4a98 Compare March 28, 2022 16:46
spec/acceptance/support/puppetdb.rb Show resolved Hide resolved
spec/setup_acceptance_node.pp Outdated Show resolved Hide resolved
@@ -138,7 +138,7 @@
vcsrepo { "${basedir}/puppetboard":
ensure => present,
provider => git,
owner => $user,
user => $user,
Copy link
Member

Choose a reason for hiding this comment

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

will that break existing installations? was that maybe setup like that on purpose, so the user that runs it has no permissions / less permissions?

Copy link
Member Author

@smortex smortex Mar 28, 2022

Choose a reason for hiding this comment

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

The only case I can see is if the user cannot traverse one of the parent directories.

Because of #349 (review) I would also want to change this user to root:root by default, but this will be a breaking change.

No puppetserver6 / puppetdb6 packages are available for Debian 11.
@smortex smortex force-pushed the vcsrepo-user branch 2 times, most recently from 50edee9 to 33cc4e6 Compare March 28, 2022 17:01
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Feel free to merge or address the open comments. They're rather small style guides.

Comment on lines +6 to +9
return false if ENV['BEAKER_PUPPET_COLLECTION'] == 'puppet6' && host_inventory['facter']['os']['release']['major'] == '11'
end

true
Copy link
Member

Choose a reason for hiding this comment

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

You can consider this style as well.

Suggested change
return false if ENV['BEAKER_PUPPET_COLLECTION'] == 'puppet6' && host_inventory['facter']['os']['release']['major'] == '11'
end
true
ENV['BEAKER_PUPPET_COLLECTION'] != 'puppet6' && host_inventory['facter']['os']['release']['major'] != '11'
else
true
end

Copy link
Member Author

Choose a reason for hiding this comment

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

The && should also be changed to || but it feels less readable in my opinion so for now I will merge as is so that other PR can benefit from the fixes for Debian / Ubuntu.

@smortex smortex merged commit 091eaf8 into master Mar 31, 2022
@smortex smortex deleted the vcsrepo-user branch March 31, 2022 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants