-
Notifications
You must be signed in to change notification settings - Fork 115
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
Add test coverage for BZ:2192939 #14025
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -983,7 +983,7 @@ def update_known_hosts(self, ssh_key_name, host, user=None): | |
f'Failed to put hostname in ssh known_hosts files:\n{result.stderr}' | ||
) | ||
|
||
def configure_puppet(self, proxy_hostname=None): | ||
def configure_puppet(self, proxy_hostname=None, run_puppet_agent=True): | ||
"""Configures puppet on the virtual machine/Host. | ||
:param proxy_hostname: external capsule hostname | ||
:return: None. | ||
|
@@ -1023,12 +1023,14 @@ def configure_puppet(self, proxy_hostname=None): | |
self.execute('/opt/puppetlabs/bin/puppet agent -t') | ||
proxy_host = Host(hostname=proxy_hostname) | ||
proxy_host.execute(f'puppetserver ca sign --certname {cert_name}') | ||
# This particular puppet run would create the host entity under | ||
# 'All Hosts' and let's redirect stderr to /dev/null as errors at | ||
# this stage can be ignored. | ||
result = self.execute('/opt/puppetlabs/bin/puppet agent -t 2> /dev/null') | ||
if result.status: | ||
raise ContentHostError('Failed to configure puppet on the content host') | ||
|
||
if run_puppet_agent: | ||
# This particular puppet run would create the host entity under | ||
# 'All Hosts' and let's redirect stderr to /dev/null as errors at | ||
# this stage can be ignored. | ||
result = self.execute('/opt/puppetlabs/bin/puppet agent -t 2> /dev/null') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using
The |
||
if result.status: | ||
raise ContentHostError('Failed to configure puppet on the content host') | ||
|
||
def execute_foreman_scap_client(self, policy_id=None): | ||
"""Executes foreman_scap_client on the vm to create security audit report. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ | |
import random | ||
|
||
import pytest | ||
from wait_for import wait_for | ||
|
||
from robottelo.exceptions import CLIReturnCodeError | ||
|
||
|
@@ -82,11 +83,69 @@ def test_positive_install_configure_host( | |
for client, puppet_proxy in zip(content_hosts, puppet_infra_host, strict=True): | ||
client.configure_puppet(proxy_hostname=puppet_proxy.hostname) | ||
report = session_puppet_enabled_sat.cli.ConfigReport.list( | ||
{'search': f'host~{client.hostname},origin=Puppet'} | ||
{'search': f'host~{client.hostname}'} | ||
Gauravtalreja1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
assert len(report) | ||
assert report[0]['origin'] == 'Puppet' | ||
facts = session_puppet_enabled_sat.cli.Fact.list({'search': f'host~{client.hostname}'}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not search specifically for the |
||
assert len(facts) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a rather useless assertion. There's no guarantee that the |
||
assert [f for f in facts if f['fact'] == 'puppetmaster_fqdn'][0][ | ||
'value' | ||
] == puppet_proxy.hostname | ||
Comment on lines
+92
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd write this as: assert [f['value'] for f in facts if f['fact'] == 'puppetmaster_fqdn'] == [puppet_proxy.hostname] This won't fail with an |
||
session_puppet_enabled_sat.cli.ConfigReport.delete({'id': report[0]['id']}) | ||
with pytest.raises(CLIReturnCodeError): | ||
session_puppet_enabled_sat.cli.ConfigReport.info({'id': report[0]['id']}) | ||
|
||
|
||
@pytest.mark.e2e | ||
@pytest.mark.rhel_ver_match('[^6]') | ||
def test_positive_run_puppet_agent_generate_report_when_no_message( | ||
session_puppet_enabled_sat, rhel_contenthost | ||
): | ||
"""Verify that puppet-agent can be installed from the sat-client repo | ||
and configured to report back to the Satellite, and contains the origin | ||
:id: 07777fbb-4f2e-4fab-ba5a-2b698f9b9f39 | ||
:setup: | ||
1. Satellite and Capsule with enabled puppet plugin. | ||
2. Blank RHEL content host | ||
:steps: | ||
1. Configure puppet on the content host. This creates sat-client repository, | ||
installs puppet-agent, configures it, runs it to create the puppet cert, | ||
signs in on the Satellite side and reruns it. | ||
2. Assert that Config report created in the Satellite for the content host, | ||
and has Puppet origin | ||
3. Assert that Facts were reported for the content host. | ||
:expectedresults: | ||
1. Configuration passes without errors. | ||
2. Config report is created and contains Puppet origin | ||
3. Facts are acquired. | ||
:customerscenario: true | ||
:BZ: 2192939, 2257327, 2257314 | ||
:parametrized: yes | ||
""" | ||
Gauravtalreja1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
sat = session_puppet_enabled_sat | ||
client = rhel_contenthost | ||
client.configure_puppet(proxy_hostname=sat.hostname, run_puppet_agent=False) | ||
# Run either 'puppet agent --detailed-exitcodes' or 'systemctl restart puppet' | ||
# to generate Puppet config report for host without any messages | ||
assert client.execute('/opt/puppetlabs/bin/puppet agent --detailed-exitcodes').status == 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This starts a daemon. You want to use |
||
wait_for( | ||
lambda: sat.cli.ConfigReport.list({'search': f'host~{client.hostname}'}) != [], | ||
timeout=300, | ||
delay=30, | ||
) | ||
report = sat.cli.ConfigReport.list({'search': f'host~{client.hostname}'}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a list of reports while the name implies it is a singular entity. I'd name it |
||
assert len(report) | ||
assert report[0]['origin'] == 'Puppet' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will pass if there a report with the origin puppet. How about asserting every report has an origin instead? assert None not in [report['origin'] for report in reports] |
||
facts = sat.cli.Fact.list({'search': f'host~{client.hostname}'}) | ||
assert len(facts) | ||
assert [f for f in facts if f['fact'] == 'puppetmaster_fqdn'][0]['value'] == sat.hostname | ||
sat.cli.ConfigReport.delete({'id': report[0]['id']}) | ||
with pytest.raises(CLIReturnCodeError): | ||
sat.cli.ConfigReport.info({'id': report[0]['id']}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nowadays (since Puppet 6) if you want to bootstrap you use
puppet ssl bootstrap
. Then you can sign it. You may want to pass--waitforcert 0
as well to prevent it waiting for the server to sign it.