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.

Sign up for GitHub

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

Jump to bottom

Clone git repository as $user #352

Merged
merged 4 commits into from
Mar 31, 2022
Merged

Clone git repository as $user #352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1935f97
Clone git repository as $user
smortex Mar 26, 2022
56cd9ac
Add workaround for resource ordering
smortex Mar 28, 2022
c1da63a
Skip acceptance tests on Puppet 6 / Debian 11
smortex Mar 28, 2022
7366b4e
Fix acceptance tests on Ubuntu 18.04 with Puppet 6
smortex Mar 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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.

source => $git_source,
revision => $revision,
require => [
Expand All @@ -148,12 +148,7 @@
before => [
File[$settings_file],
],
}

file { "${basedir}/puppetboard":
owner => $user,
recurse => true,
require => Vcsrepo["${basedir}/puppetboard"],
notify => Python::Requirements["${basedir}/puppetboard/requirements.txt"],
}

$pyvenv_proxy_env = $python_proxy ? {
Expand Down
4 changes: 3 additions & 1 deletion spec/acceptance/class_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

require 'spec_helper_acceptance'

describe 'puppetboard class' do
require_relative 'support/puppetdb'

describe 'puppetboard class', if: has_puppetdb do
case fact('os.family')
when 'RedHat'
apache_conf_file = '/etc/httpd/conf.d/25-puppetboard.conf'
Expand Down
10 changes: 10 additions & 0 deletions spec/acceptance/support/puppetdb.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# frozen_string_literal: true

def has_puppetdb
case host_inventory['facter']['os']['name']
when 'Debian'
return false if ENV['BEAKER_PUPPET_COLLECTION'] == 'puppet6' && host_inventory['facter']['os']['release']['major'] == '11'
end

true
Comment on lines +6 to +9
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.

end
smortex marked this conversation as resolved.
Show resolved Hide resolved
1 change: 0 additions & 1 deletion spec/classes/puppetboard_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
it { is_expected.to contain_package('py38-puppetboard') }
else
it { is_expected.to contain_file('/srv/puppetboard/puppetboard/settings.py') }
it { is_expected.to contain_file('/srv/puppetboard/puppetboard') }
it { is_expected.to contain_file('/srv/puppetboard') }
it { is_expected.to contain_python__pyvenv('/srv/puppetboard/virtenv-puppetboard') }
it { is_expected.to contain_vcsrepo('/srv/puppetboard/puppetboard') }
Expand Down
8 changes: 8 additions & 0 deletions spec/setup_acceptance_node.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
if $facts['os']['name'] == 'Ubuntu' {
# Needed for facter to fetch facts used by the postgresql module
if versioncmp($facts['facterversion'], '4.0.0') <= 0 {
package{ 'lsb-release':
ensure => present,
}
}
}