Adding logic for registering and updating consumers with client certs #28
Conversation
| with self.subTest(comment='check response body'): | ||
| self.assertEqual(result['display_name'], body['delta']['display_name']) | ||
| self.assertEqual(result['notes'], body['delta']['notes']) | ||
| self.assertEqual(result['description'], body['delta']['description']) |
There was a problem hiding this comment.
How about putting each of these three assertions into separate subtests?
| """Show that one can `register and update a consumer`_. | ||
|
|
||
| The call to `consumer register api` should return a x.509 certificate, | ||
| that should be useful in updating a consumer and for other actions. |
| class RegisterAndUpdateConsumerTestCase(unittest.TestCase): | ||
| """Show that one can `register and update a consumer`_. | ||
|
|
||
| The call to `consumer register api` should return a x.509 certificate, |
There was a problem hiding this comment.
This isn't how to create a link:
My `link`.
This is:
My `link`_.
| consumer = client.post(CONSUMERS_PATH, {'id': utils.uuid4()}).json() | ||
| self.addCleanup(client.delete, consumer['consumer']['_href']) | ||
| with self.subTest(comment='check register call returns with certificate'): | ||
| self.assertIn('certificate', consumer) |
There was a problem hiding this comment.
I see this code below:
certificate = consumer['certificate']So why is this test continuing even when "certificate" isn't in the "consumer" dict? Drop the surrounding subTest.
| tmp_cert_file = tempfile.NamedTemporaryFile(delete=False) | ||
| self.addCleanup(os.unlink, tmp_cert_file.name) | ||
| with open(tmp_cert_file.name, 'w') as file_writer: | ||
| file_writer.write(certificate) |
There was a problem hiding this comment.
Don't mangle my bare-metal host! This creates files on the host that's running Pulp Smash, not the host under test.
| @@ -0,0 +1,29 @@ | |||
| # coding=utf-8 | |||
| """Utility functions for Platform tests.""" | |||
|
|
|||
There was a problem hiding this comment.
Nitpick: drop this blank line.
| # Step 2 | ||
| certificate = consumer['certificate'] | ||
| tmp_cert_file = tempfile.NamedTemporaryFile(delete=False) | ||
| self.addCleanup(os.unlink, tmp_cert_file.name) |
There was a problem hiding this comment.
Don't mangle my bare-metal host!
There was a problem hiding this comment.
Hi @Ichimonji10,
This is a client-side certificate. A certificate file is needed for testing this test case. The certificate is created as a temp file which is cleaned up as soon as the test completes by the self.addCleanup , thus the file wont exist once the test completes (either pass/fail). Is there any other workaround rather than creating a file?
ragabala
force-pushed
the
test_issue_1007
branch
from
01cee91
to
830e266
Compare
| from pulp_smash import api | ||
|
|
||
|
|
||
| def make_client_with_cert_auth(cfg, cert=None): |
There was a problem hiding this comment.
Why is cert an optional parameter?
| class RegisterAndUpdateConsumerTestCase(unittest.TestCase): | ||
| """Show that one can `register and update a consumer`_. | ||
|
|
||
| The call to ``consumer register api`` should return a x.509 certificate, |
There was a problem hiding this comment.
Is "consumer register api" code?
| client = api.Client(cfg) | ||
|
|
||
| # Step 1 | ||
| consumer = client.post(CONSUMERS_PATH, {'id': utils.uuid4()}).json() |
There was a problem hiding this comment.
We should add a gen_consumer function and use it in cases like this. It doesn't need to be done as part of this PR.
| file_writer.write(certificate) | ||
|
|
||
| # step 3 | ||
| new_client = make_client_with_cert_auth(cfg, cert=tmp_cert_file.name) |
There was a problem hiding this comment.
Rather than making a new client, how about mutating the existing client object?
| * `Pulp Smash #1007 <https://github.com/PulpQE/pulp-smash/issues/1007>`_ | ||
| """ | ||
| cfg = config.get_config() | ||
| client = api.Client(cfg) |
There was a problem hiding this comment.
How about passing api.json_handler as the response handler?
This commit adds a new testcase for registering and updating a consumer. The Registering happens with basic auth url and the update is through the client certificate that is in the previous response. See pulp/pulp-smash#1007
ragabala
force-pushed
the
test_issue_1007
branch
from
830e266
to
73338c0
Compare
This commit adds a new testcase for registering and updating a consumer.
The Registering happens with basic auth url and the update is through
the client certificate that is in the previous response.
See pulp/pulp-smash#1007