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

(PUP-6241) Acceptance unreadable environment #5247

Merged
merged 3 commits into from
Jul 14, 2017
Merged

(PUP-6241) Acceptance unreadable environment #5247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4d6d853
(PUP-6241) Acceptance unreadable environment
Aug 30, 2016
478a557
(PUP-6241) Acceptance unreadable environment site.pp
Sep 2, 2016
533b05c
(PUP-6241) Acceptance unreadable environment
May 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 58 additions & 0 deletions acceptance/tests/environment/negative/agent_run_should_fail_if_env_unreadable.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
test_name "C97899 - Agent run should fail if environment is unreadable" do
skip_test 'requires a puppetserver master for managing the environment' if hosts_with_role(hosts, 'master').length == 0 or not @options[:is_puppetserver]

testdir = ''
env_path = ''
test_env = ''

step 'setup environments' do
testdir = create_tmpdir_for_user master, 'c97899_unreadable_envdir'
env_path = "#{testdir}/environments"
test_env = "#{env_path}/testing"

apply_manifest_on(master, <<-MANIFEST, :catch_failures => true)
File {
ensure => directory,
mode => "0770",
owner => #{master.puppet['user']},
group => #{master.puppet['group']},
}
file {
'#{env_path}':;
'#{env_path}/production':;
'#{test_env}':;
'#{test_env}/manifests':;
'#{test_env}/modules':;
}
file { '#{test_env}/manifests/site.pp':
ensure => file,
mode => "0640",
content => 'node default { notify { "Hello agent": } }',
}
MANIFEST
end

step 'change permissions of envdir to 644' do
on(master, "chmod 644 #{test_env}")
Copy link

Choose a reason for hiding this comment

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

There's two possible cases here: the environment folder has overly restrictive permissions; and the environment folder manifest has correct permissions but site.pp has overly restrictive permissions.
This code seems to restrict both permissions but will only really be testing the folder permissions. Perhaps this test file (or a separate test file) should test the case of the folder having correct permissions and site.pp having restrictive permissions?

(I imagine we could also have cases where both the folder and site.pp have correct permissions but then other manifest files/folders are restricted - I think that should be a separate ticket though)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@james-stocks , I am unable to reproduce the case where the site.pp file is too restrictive. I have added another test case to this PR for that scenario. It currently, fails the expect_failure condition when I run it against a recent master puppet-agent (SHA=e15c8b567fee6846f06aa8f0e74a07dd8fe90441).

end

step 'verify environment fails with puppet agent run' do
master_opts = {
'main' => {
'environmentpath' => env_path,
}
}
with_puppet_running_on master, master_opts, testdir do
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this starts a 'puppet master' and not a puppet server. The problem is only reproduced using a puppet server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If run with the ci:test:aio rake task, the with_puppet_running_on uses the deployed puppetserver rather than a passenger based master. We could throttle the test to only run in the context of a type == aio run. If we want this test to be added to the regression suite, I would recommend that we run it without such a restriction.

Copy link
Contributor

Choose a reason for hiding this comment

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

i agree with @johnduarte here

Copy link
Contributor

Choose a reason for hiding this comment

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

there are better ways to inject an environment into the codedir without a requirement to HUP the server. we might want to do this with more tests so that we can remove with_puppet_running_on once we can assume the server is running.
e.g.: https://github.com/puppetlabs/puppet/blob/master/acceptance/lib/puppet/acceptance/environment_utils.rb#L378

Copy link
Contributor

Choose a reason for hiding this comment

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

The master in with_puppet_running_on master is actually the host with the master role defined in the beaker host configuration. It's confusing because beaker also supports methods like on(host, puppet("master ...")) which really does mean run the webrick master...

Also, ci:test:aio always uses puppetserver, but ci:test:git uses webrick, and we no longer support or test passenger packages. Since the failure only occurs under puppetserver, I think that means the overall test will fail when run in ci:test:git, e.g. due to the expect_failure below? If that's true, then I think we should confine to aio only.

Copy link
Contributor

Choose a reason for hiding this comment

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

@johnduarte can you confirm if the test still passes with ci:test:git?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joshcooper it looks like one of the pre-suite tasks for ci:test:git is currently broken on master. So, I am not able to successfully test that task.

Errored Tests Cases:
  Test Case setup/git/pre-suite/000_EnvSetup.rb reported: #<Beaker::Host::CommandFailure: Host 'ychcq6ueup73370.delivery.puppetlabs.net' exited with 1 running:
 cd /opt/puppet-git-repos/puppet-win32-ruby; cp -r ruby/* /
Last 10 lines of output were:
        bash: line 0: cd: /opt/puppet-git-repos/puppet-win32-ruby: No such file or directory
        cp: cannot stat 'ruby/*': No such file or directory>
    Test line: setup/git/pre-suite/000_EnvSetup.rb:119:in `block (2 levels) in run_test'

Filed PUP-7376

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fyi: PUP-7376 is a dupe of PUP-7009

agents.each do |agent|
on(agent, puppet("agent --test --server #{master} --environment testing"), :accept_all_exit_codes => true) do |result|
refute_equal(2, result.exit_code, 'agent run should not apply changes')
expect_failure('expected to fail until PUP-6241 is resolved') do
refute_equal(0, result.exit_code, 'agent run should not succeed')
refute_empty(result.stderr, 'an appropriate error is expected')
end
end
end
end
end

end
52 changes: 52 additions & 0 deletions acceptance/tests/environment/negative/agent_run_should_fail_if_site_pp_unreadable.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
test_name "C98160 - Agent run should fail if an environment's site.pp is unreadable" do
skip_test 'requires a master for managing the environment' if hosts_with_role(hosts, 'master').length == 0

testdir = ''
env_path = ''
test_env = ''

step 'setup environments' do
testdir = create_tmpdir_for_user master, 'c97899_unreadable_envdir'
env_path = "#{testdir}/environments"
test_env = "#{env_path}/testing"

apply_manifest_on(master, <<-MANIFEST, :catch_failures => true)
File {
ensure => directory,
mode => "0770",
owner => #{master.puppet['user']},
group => #{master.puppet['group']},
}
file {
'#{env_path}':;
'#{env_path}/production':;
'#{test_env}':;
'#{test_env}/manifests':;
'#{test_env}/modules':;
}
file { '#{test_env}/manifests/site.pp':
ensure => file,
mode => "0000", # <- NOTE: this is unreadable
content => 'node default { notify { "Hello agent": } }',
}
MANIFEST
end

step 'verify environment fails with puppet agent run' do
master_opts = {
'main' => {
'environmentpath' => env_path,
}
}
with_puppet_running_on master, master_opts, testdir do
agents.each do |agent|
on(agent, puppet("agent --test --server #{master} --environment testing"), :accept_all_exit_codes => true) do |result|
refute_equal(2, result.exit_code, 'agent run should not apply changes')
refute_equal(0, result.exit_code, 'agent run should not succeed')
refute_empty(result.stderr, 'an appropriate error is expected')
end
end
end
end

end