search

Ouranosinc / PAVICS-e2e-workflow-tests

Test user-level workflow.
Apache License 2.0
0 stars 2 forks source link

Dac 468 geoserver e2e tests #125

Closed ChaamC closed 11 months ago

ChaamC commented 12 months ago

Overview

This PR adds e2e tests for the access to different Geoserver resources and checks that the different user permissions from Magpie are respected.

Changes

Related Issue / Discussion

review-notebook-app[bot] commented 12 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

fmigneault commented 11 months ago

The notebooks outputs are missing. They should be left here because the CI will do an additional check to make sure that executed notebook outputs match exactly the expected ones provided.

If some outputs contain some variable data (eg: variable instance URL, UUID, etc.), you can add the entries in the following config to parse them properly: https://github.com/Ouranosinc/PAVICS-e2e-workflow-tests/blob/master/notebooks/output-sanitize.cfg

ChaamC commented 11 months ago

@fmigneault Notebook updated, with added output. Build is succesful using a custom iac instance and using the geoserver-secured service (see jenkins build)

Ready for a new review.

tlvu commented 11 months ago

@ChaamC

Sorry why is this PR merged before I have time to take a look at it?

This test is creating and destroying (cleanup) data on the Geoserver.

I do not want this to run by default on our production instance.

Please make it like the Magpie Auth test, that it is not enabled by default so this is run on staging servers only. Please add similar comments warnings to the user that will enable this test.

See https://github.com/Ouranosinc/PAVICS-e2e-workflow-tests/blob/134b41987a0106feb369fccc9dcd36efabb2e6bc/Jenkinsfile#L21-L33

ChaamC commented 11 months ago

@tlvu Very sorry about merging this too soon. I will look into it right now.

Just to be clear on the structure of the repo, should the new geoserver notebook be transferred to the notebooks-auth or does it not belong in that directory? If the current location of the notebook is still good, I will add a specific section on the testall script for the geoserver notebook.

ChaamC commented 11 months ago

@tlvu Seems to me that the geoserver notebook just belongs with the thredds notebook in notebooks-auth, as my new notebook also relies on Magpie authorizations and permission resolutions. The geoserver notebook was actually highly inspired by the thredds one too.

tlvu commented 11 months ago

Just to be clear on the structure of the repo, should the new geoserver notebook be transferred to the notebooks-auth or does it not belong in that directory?

In another directory please, notebooks-geoserver-write would be good. Any tests that perform a write should not enabled by default to not accidentally break or leave cruft on the production server when the test fails or is interrupted without proper cleanup (power outage or human error for example).

notebooks-auth would be for other authentication and access control related tests in the future, if need be.

If the current location of the notebook is still good, I will add a specific section on the testall script for the geoserver notebook.

Only notebooks safe to run by default on the production server should be in the current notebooks folder.

tlvu commented 11 months ago

@tlvu Seems to me that the geoserver notebook just belongs with the thredds notebook in notebooks-auth, as my new notebook also relies on Magpie authorizations and permission resolutions. The geoserver notebook was actually highly inspired by the thredds one too.

Oh ! Did you also had to create and destroy magpie users? No conflicts (same names) if both notebooks run in the same run?

If you also have to create and destroy magpie users, then I guess we can keep it in notebooks-auth.

ChaamC commented 11 months ago

Oh ! Did you also had to create and destroy magpie users? No conflicts (same names) if both notebooks run in the same run? If you also have to create and destroy magpie users, then I guess we can keep it in notebooks-auth.

Yes, I am creating and using Magpie test users, to validate permission access to some Geoserver routes. Also, the test users that are created use by default a username generated with uuid4, unless a specific value was defined in the extra env vars. So by default, there shouldn't be conflicts with the other notebook (which also uses uuid4 for its test user by the way).

tlvu commented 11 months ago

Yes, I am creating and using Magpie test users, to validate permission access to some Geoserver routes. Also, the test users that are created use by default a username generated with uuid4, unless a specific value was defined in the extra env vars. So by default, there shouldn't be conflicts with the other notebook (which also uses uuid4 for its test user by the way).

Ok then just keep in same dir but update the description of that existing Jenkins checkbox mentioning it also enable Geoserver testing notebook.

And I assume in this new Geoserver notebook, all test geoserver data/workspaces and Magpie test users are cleaned up at the end?

Please ensure all those test items created are displayed in Jenkins build logs, in case we have to clean them up manually for some unexpected reasons.