canonical / lxd-ui

Easy and accessible container and virtual machine management. A browser interface for LXD
GNU General Public License v3.0
242 stars 30 forks source link

chore: add oidc login tests [WD-9461] #781

Open mas-who opened 1 month ago

mas-who commented 1 month ago

Done

QA

  1. Run the LXD-UI:
    • On the demo server via the link posted by @webteam-app below. This is only available for PRs created by collaborators of the repo. Ask @mas-who or @edlerd for access.
    • With a local copy of this branch, run as described here.
  2. Perform the following QA steps:
    • Run tests locally. Make sure everything passes and also check that tls cert is still added.
    • Ensure CI passes.
webteam-app commented 1 month ago

Demo

Jenkins

demos.haus

mas-who commented 1 month ago

There is something weird when I tested this on my fork. It seems like the oidc user doesn't have the right permissions to view all the volumes, see video. When I run the same test locally, it works, so it feels like there might be a lxd config difference between CI and local environment. Still trying to figure out what's going on so keeping this PR as draft.

Edit: I think I may have found a bug. After updating my lxd snap to the 5.21/edge channel, when logged in as an oidc user I can no longer see custom volumes created. Will investigate a bit more to see what's going on.

mas-who commented 1 month ago

Another issue is that repo secrets or variables cannot be read in a workflow triggered by a forked PR. See this discussion. I will have to keep testing on my fork.

mas-who commented 1 month ago

There is an issue in LXD where the permission check for storage volumes is not taking into consideration the location of a volume in a clustered setup. This is relevant for testing with OIDC identities in a cluster environment since TLS identities currently do not go through permission checks.

mas-who commented 1 month ago

@edlerd also updated the workflows to use repo secrets instead of repo variables.

mas-who commented 1 month ago

The workflow seems to be failing, this might be because the secrets access is not available in this PR originating from a fork.

Do you have a link to a passing workflow run in your fork?

Sure here's the action running on my fork. I had to rebase main quick to get around errors in the screenshot tests. Once it's done, you will likely see failing storage volume tests for the latest/edge and 5.21/edge tests due to the permission bug which should be fixed with this PR. We should follow up on that PR though once merged though, since it would be immediately available in the latest/edge channel but not the 5.21/edge channel.