Always use the certs from the quadlet machine in the tests - #703
Conversation
| @pytest.fixture(scope="module") | ||
| def curl_request(server, certificates, server_fqdn): | ||
| def curl_request(server, certificates, quadlet_client_certificate, server_fqdn): | ||
| cert, key = quadlet_client_certificate |
There was a problem hiding this comment.
I didn't check if there are cases where we actually want the proxy cert here, but we should do that before merging
There was a problem hiding this comment.
If you want to test server APIs that are reserved for proxy auth, for example fact upload.
There was a problem hiding this comment.
Sure, but we don't have such a test here today.
evgeni
changed the title
|
Upgrade failure unrelated, fix in #705 |
| @pytest.fixture(scope="module") | ||
| def curl_request(server, certificates, server_fqdn): | ||
| def curl_request(server, certificates, quadlet_client_certificate, server_fqdn): | ||
| cert, key = quadlet_client_certificate |
There was a problem hiding this comment.
If you want to test server APIs that are reserved for proxy auth, for example fact upload.
|
|
||
| @pytest.fixture(scope="module") | ||
| def curl_request(server, certificates, server_fqdn): | ||
| def curl_request(server, certificates, quadlet_client_certificate, server_fqdn): |
There was a problem hiding this comment.
| def curl_request(server, certificates, quadlet_client_certificate, server_fqdn): | |
| def curl_request(server, certificates, client_certificate=quadlet_client_certificate, server_fqdn): |
This will enable a sane default with ability to properly override
There was a problem hiding this comment.
Those are Pytest fixtures, not normal Python args, I don't think you can override them like this.
There was a problem hiding this comment.
Yeah, tests say that doesn't work.
There was a problem hiding this comment.
Sorry, for pytest you can do it like this:
@pytest.fixture
def client_certificate
return quadlet_client_certificateThis will be the default, and later, if you want to change the value, you will do something like:
@pytest.mark.parameterize("client_certificate", ["/path/to/host/cert"])
def test_facts(curl_request)
# ...There was a problem hiding this comment.
Tests still say no:
TypeError: cannot unpack non-iterable FixtureFunctionDefinition object
Can we use my working code and worry about possible overrides later when we actually need them?
There was a problem hiding this comment.
ACK, let's get a working version out and worry about the overrides later
There was a problem hiding this comment.
Dropped the last commit again.
pr-processor
Bot
added
Needs re-review
and removed
Waiting on contributor
Needs re-review
labels
|
Given @ShimShtein is out today, and the override was his only concern, I am going to merge this, to unblock #706 There was also a concern raised by @jeremylenz about the hard-coded "quadlet" name here, which I will follow up in a separate PR. |
3 participants
Why are you introducing these changes? (Problem description, related links)
Our tests talk to the Smart Proxy API (see #519 and the BMC change in this PR), which requires auth. In the case of an external proxy deployment, the certs that were used were of the proxy, but that is not allowed to auth against itself.
What are the changes introduced in this pull request?
How to test this pull request
Steps to reproduce:
Checklist