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

Dac 472 update geoserver post magpie integration #35

Merged
merged 90 commits into from
Jul 10, 2023

Conversation

ChaamC
Copy link
Contributor

@ChaamC ChaamC commented Apr 19, 2023

Resolves DAC-472

This PR adds permission synchronization between Magpie's permissions and Geoserver files permissions :

  • If a new shapefile is added to the Geoserver user folder, its file permissions is synchronized with Magpie, creating the required resources and corresponding permissions in Magpie.

  • If a permissions is changed on Magpie, the event is sent to Cowbird, to update the actual file permissions in the Geoserver user folder.

  • If any Magpie permission from the LAYER_READ_PERMISSIONS is allowed for a user, the corresponding file/folder is made readable. Same thing for any Magpie permission from the LAYER_WRITE_PERMISSIONS, which makes the file/folder writable.

  • If any of the different files from a shapefile is readable and/or writable for the user, all the files of the shapefile are considered readable and/or writable and are updated as so. The corresponding resource on Magpie should have all the readable and/or writable permissions allowed for the user, if the shapefile is readable and/or writable.

  • For this PR, the workspace name is assumed to be the name of a user. To be determined in the future if we want to support other type of workspaces.

  • For this PR, permission changes on a group do not do anything, since workspaces are assumed to be organized per user only.

  • For now, nothing is done if a folder is manually created (see TODO in geoserver.py::Geoserver::on_created()). We assume having a workspace (folder) per user, and the workspaces are already created and added to Geoserver upon the user creation. Also, shapefiles are assumed to always in the same directory, at the same level, as it was already the case before this PR.

Relates to bird-house/birdhouse-deploy#323 which adds support to the new added cases here.
Relates to bird-house/birdhouse-deploy#120

@github-actions github-actions bot added api Issue related to API functionalities. tests Modifications to tests or problems related to test suite. labels Apr 19, 2023
@github-actions github-actions bot added the doc Updates to documentation. label Apr 19, 2023
@codecov
Copy link

codecov bot commented Apr 19, 2023

Codecov Report

Patch coverage: 92.58% and project coverage change: +1.55 🎉

Comparison is base (c448908) 84.73% compared to head (92b6329) 86.28%.

❗ Current head 92b6329 differs from pull request most recent head 1256bde. Consider uploading reports for the commit 1256bde to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #35      +/-   ##
==========================================
+ Coverage   84.73%   86.28%   +1.55%     
==========================================
  Files          40       40              
  Lines        2528     2793     +265     
  Branches      376      429      +53     
==========================================
+ Hits         2142     2410     +268     
+ Misses        282      269      -13     
- Partials      104      114      +10     
Impacted Files Coverage Δ
cowbird/config.py 80.62% <ø> (ø)
cowbird/monitoring/fsmonitor.py 100.00% <ø> (ø)
cowbird/monitoring/monitor.py 88.46% <66.66%> (+0.30%) ⬆️
cowbird/handlers/impl/magpie.py 88.70% <89.18%> (+11.32%) ⬆️
cowbird/monitoring/monitoring.py 91.11% <90.90%> (-0.38%) ⬇️
cowbird/handlers/impl/geoserver.py 91.02% <93.75%> (+12.18%) ⬆️
cowbird/permissions_synchronizer.py 95.95% <96.55%> (+0.47%) ⬆️
cowbird/api/exception.py 85.36% <100.00%> (ø)
cowbird/api/requests.py 80.48% <100.00%> (+0.48%) ⬆️
cowbird/api/webhooks/views.py 98.52% <100.00%> (+0.06%) ⬆️
... and 3 more

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions github-actions bot added the ci Updates to the CI pipelines and related utilities. label Apr 19, 2023
CHANGES.rst Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
cowbird/api/requests.py Outdated Show resolved Hide resolved
cowbird/api/webhooks/views.py Outdated Show resolved Hide resolved
cowbird/constants.py Outdated Show resolved Hide resolved
tests/test_geoserver.py Outdated Show resolved Hide resolved
tests/test_geoserver.py Show resolved Hide resolved
tests/test_permissions_synchronizer.py Outdated Show resolved Hide resolved
cowbird/handlers/impl/magpie.py Show resolved Hide resolved
cowbird/handlers/impl/magpie.py Outdated Show resolved Hide resolved
@ChaamC
Copy link
Contributor Author

ChaamC commented Jun 19, 2023

@fmigneault Updated with latest feedback, or responses added to some comments. Should be ready for a new review.

There are errors from snyk and pyup for which I am not sure what to do.

For snyk, it found 6 issues, it says they are introduced from magpie 0.1.0 and come from tornado and markdown2. Not sure I know what to do with those. I don't see those dependencies in my magpie environment though, and I'm not sure why it mentions the version 0.1.0 here.

For pyup, it mentions a security issue in celery 5.2.7, which I had to use specifically for testing with py3.7. This is because later versions require the use of kombu>=5.3.0, but kombu 5.3.0 seems to have dropped support for python 3.7 as it is reaching end-of-life next week (ref here). Not sure how to fix this. I'm wondering if we should just leave the security issue warning for now. The security issue seems to be related to the use of github workflows on their repo, so I'm not sure if this really has an impact on us (related PR)

@ChaamC ChaamC requested a review from fmigneault June 19, 2023 16:09
mishaschwartz added a commit to bird-house/birdhouse-deploy that referenced this pull request Jul 4, 2023
…vice types (#336)

## Overview

In the cowbird `config.yml.template` file: the `wps_outputs` and
`weaver` keys are supposed to be Magpie service types, not magpie
service names. This causes all cowbird hooks to fail with:

```
cowbird.config.ConfigErrorInvalidServiceKey: Service `weaver` used in sync config is not valid since it was not found in Magpie services (['access', 'api', 'geoserver', 'geoserverwfs', 'geoserverwms', 'geoserverwps', 'ncwms', 'thredds', 'wfs', 'wps']).
```

This fix is already implemented in #323 but since that is waiting on
Ouranosinc/cowbird#35 to get pulled in, this PR
factors out the bugfix that isn't dependent on the cowbird PR.

This allows us to fix this configuration bug in the meantime

## Changes

**Non-breaking changes**

None (bug fix)

**Breaking changes**

None

## Related Issue / Discussion

## Additional Information
Copy link
Collaborator

@fmigneault fmigneault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comment to address, but feature-wise seems to be ready.

For the Snyk analysis, it references magpie:0.1.0 (not sure how it resolves that since you added @3.34.0). Does this actually work? I think with a git+https, you might need to add #egg=magpie at the end to help guide it with the package name. If it this raise, then we will just ignore the irrelevant Snyk issues.

For pyup, it indicates a security issue that would require celery[mongodb]>5.2.7,<6.
This PR is big enough, so it can be done in a separate one.

@fmigneault fmigneault mentioned this pull request Jul 6, 2023
2 tasks
@ChaamC ChaamC requested a review from fmigneault July 7, 2023 15:24
@ChaamC
Copy link
Contributor Author

ChaamC commented Jul 7, 2023

Ready for a new review.

Also, I tried adding the #egg=magpie for the snyk issues, but it created problems during the installation on github, and the issues were still triggered on snyk. Might be better to explore this in another PR.

@fmigneault
Copy link
Collaborator

@ChaamC
Thanks for the great work and keeping up with my multiple change requests 😅

@ChaamC ChaamC merged commit 55d6fdb into master Jul 10, 2023
14 checks passed
@ChaamC ChaamC deleted the DAC-472-update-geoserver-post-magpie-integration branch July 10, 2023 12:02
ChaamC pushed a commit to bird-house/birdhouse-deploy that referenced this pull request Jul 10, 2023
PR to add support to the new cases added to `Cowbird`, relates to
[cowbird PR #35](Ouranosinc/cowbird#35).

## Changes

- Add Magpie webhook definitions for permission creation and deletion
cases to be processed by Cowbird.
- Add `USER_WORKSPACE_UID` and `USER_WORKSPACE_GID` env variables to manage ownership of the user workspaces used by Cowbird, JupyterHub and others.
- Update `magpie` service from [3.31.0](https:/Ouranosinc/Magpie/tree/3.31.0)
  to [3.34.0](https:/Ouranosinc/Magpie/tree/3.34.0)
- Update `cowbird` service from [1.1.1](https:/Ouranosinc/cowbird/tree/1.1.1)
  to [1.2.0](https:/Ouranosinc/cowbird/tree/1.2.0)

## Fixes

- Fixes #120
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issue related to API functionalities. ci Updates to the CI pipelines and related utilities. doc Updates to documentation. tests Modifications to tests or problems related to test suite.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants