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

Test ipa-join using different options #4182

Closed
wants to merge 2 commits into from

Conversation

jayesh-garg
Copy link
Contributor

test ipa-join on client with passing differnt options
like unenroll,hostname,bindpw.

Signed-off-by: Jayesh Garg jgarg@redhat.com

@jayesh-garg jayesh-garg force-pushed the ipa-join branch 3 times, most recently from 1c34d07 to 8e8ee23 Compare January 31, 2020 10:55
Copy link
Contributor

@mrizwan93 mrizwan93 left a comment

Choose a reason for hiding this comment

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

Hello @jayesh-garg .
Thanks for the patch.

ipatests/prci_definitions/nightly_latest.yaml Outdated Show resolved Hide resolved
@jayesh-garg jayesh-garg force-pushed the ipa-join branch 7 times, most recently from 581bca6 to d5f078e Compare February 3, 2020 06:42
test ipa-join on client with passing differnt options
like unenroll,hostname,bindpw.

Signed-off-by: Jayesh Garg <jgarg@redhat.com>
Signed-off-by: Jayesh Garg <jgarg@redhat.com>
Copy link
Contributor

@mrizwan93 mrizwan93 left a comment

Choose a reason for hiding this comment

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

Hello @jayesh-garg Please find inline comments.

test_suite: test_integration/test_installation.py::TestIpaJoin
template: *ci-master-latest
timeout: 10800
topology: *ad_master_2client
Copy link
Contributor

Choose a reason for hiding this comment

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

This test does not involve AD. Please change the topology.


class TestIpaJoin(IntegrationTest):

num_clients = 2
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we not use single client here?
First un-enrolling the client then enrolling the same with bad password and correct password?

if "Random password" in line:
valid_password = line.split()[2]
break
result = self.clients[0].run_command(['ipa-join',
Copy link
Contributor

@mrizwan93 mrizwan93 Feb 3, 2020

Choose a reason for hiding this comment

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

Should self.clients[0] be self.clients[1] ?. Please consider use of single client.

@freeipa-pr-ci freeipa-pr-ci added the needs rebase Pull Request cannot be automatically merged - needs to be rebased label Feb 4, 2020
# checking unenroll option
result = self.clients[0].run_command(['ipa-join', '-u',
self.clients[0].hostname])
assert "Unenrollment successful." in result.stderr_text
Copy link
Contributor

Choose a reason for hiding this comment

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

This verifies that the command returns success, not that it did what it said it did.

ipa-join -u does a kinit using the host principal then calls the ipa host-disable command. You could, for example, do a host-show to ensure that it has no kerberos keys. To really stretch it any services on the host should also be disabled.

It is probably worth calling join -u twice, expecting success the first and failure the second..

# checking hostname option
result = self.clients[0].run_command(['ipa-join', '-h',
self.clients[0].hostname])
assert "Keytab successfully retrieved" in result.stderr_text
Copy link
Contributor

Choose a reason for hiding this comment

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

The keytab should be verified. Perhaps this would be sufficient:

self.clients[0].run_command(['kinit', '-kt', '/etc/krb5.keytab'])
self.clients[0].run_command(['ipa', 'host-show', self.clients[0].hostname)

ensure that has_keytab is Yes.

'-h', self.clients[1].hostname,
'-w', invalid_password],
raiseonerr=False)
assert "Bind failed:" in result.stderr_text
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd call ipa-join -u first to un-enroll the client like you did with the previous test.

You only use invalid_password once AFAICT so there is no need to have it in a variable IMHO.

valid_password = ''
self.master.run_command(['ipa', 'host-del', self.clients[1].hostname])
cmd = self.master.run_command(['ipa', 'host-add', '--random'],
stdin_text=self.clients[1].hostname)
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be simpler if you set a fixed password instead of using a random one. You are just testing that ipa-join works with a password, not that random passwords work.

result = self.clients[0].run_command(['ipa-join',
'-h', self.clients[1].hostname,
'-w', valid_password])
assert "Keytab successfully retrieved" in result.stderr_text
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd add similar keytab validation code as above, in a separate function would be best I suppose.

@stale
Copy link

stale bot commented May 6, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Stale PR [Bot] label May 6, 2020
@stale
Copy link

stale bot commented May 21, 2020

This issue has been automatically closed as stale it has not had recent activity.

@stale stale bot closed this May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase Pull Request cannot be automatically merged - needs to be rebased stale Stale PR [Bot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants