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

Tests: Remove gen_cert util #14449

Merged
merged 4 commits into from
Nov 12, 2024
Merged

Conversation

markylaing
Copy link
Contributor

The gen_cert util had a weird side-effect if I was concurrently running a LXD snap with OIDC configured. (I realise that there can be some test failures when doing this related to networks etc. but I do it regularly when running a single suite as I'm often using the snapped LXD for something else). I've put more details in the commit messages.

Additionally simplifies the user_is_server_admin function in the fine-grained auth tests.

@tomponline
Copy link
Member

The gen_cert util had a weird side-effect

Please don't keep us in suspense, what was it? :)

@tomponline
Copy link
Member

I've put more details in the commit messages.

ah ok will read those :)

tomponline
tomponline previously approved these changes Nov 12, 2024
@markylaing
Copy link
Contributor Author

Hmmm test failures. Weird because the auth tests passed locally. Will look into it.

This test was unnecessarily moving certificates around to test that a
server administrator can create and edit certificates. This is
simplified by using a separate LXD_CONF directory.

Additionally, now use `gen_cert_and_key` instead of `gen_cert`.

Signed-off-by: Mark Laing <[email protected]>
This util did not specify the `--auth-type` flag when adding the dummy
remote. This caused issues if another LXD is running on the host machine
while listening on :8443 with OIDC configured. The `remote add` command
would fail and get the OIDC headers, then attempt to start the device
authorization grant flow with the IdP configured on the other LXD. With
`BROWSER=curl` set for use with the `mini-oidc` test util, it was causing
`lxc` to attempt to curl the code verification page.

Since we have a simpler and more efficient equivalent in
`gen_cert_and_key`, I've opted to remove it instead of setting the
`--auth-type` flag to avoid this behaviour.

Signed-off-by: Mark Laing <[email protected]>
@markylaing
Copy link
Contributor Author

Hmmm test failures. Weird because the auth tests passed locally. Will look into it.

Should be sorted now. The value of LXD_CONF2 was set in another function and that's were the tls remote was defined. When I ran the test locally I had named it TMP_LXD_CONF and I didn't think the rename would matter. Sorry for the wasted CI time 😢

@markylaing markylaing changed the title Tests: Remove gen_cert util. Tests: Remove gen_cert util Nov 12, 2024
@tomponline tomponline merged commit 1e036d5 into canonical:main Nov 12, 2024
25 checks passed
hamistao pushed a commit to hamistao/lxd that referenced this pull request Nov 12, 2024
The `gen_cert` util had a weird side-effect if I was concurrently
running a LXD snap with OIDC configured. (I realise that there can be
some test failures when doing this related to networks etc. but I do it
regularly when running a single suite as I'm often using the snapped LXD
for something else). I've put more details in the commit messages.

Additionally simplifies the `user_is_server_admin` function in the
fine-grained auth tests.

Signed-off-by: hamistao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants