Update Thredds to supported version #413
Conversation
github-actions
bot
added
component/THREDDS
|
FYI, for this change, the automated pipeline test is not enough. We have already attempted to upgrade Thredds on Ouranos side and we have |
|
Thanks for the info. I mostly created this PR to test the pipeline with the updated version to see what happens. I can close this if you're working on it on the ouranos side. |
|
Oh keep this PR, we have not yet opened a formal PR on our side, we simply override in |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2366/Result : failure BIRDHOUSE_DEPLOY_BRANCH : update-thredds-5.4 DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-20.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1482/NOTEBOOK TEST RESULTS |
|
A few problems found related to Thredds 5.4 Notebook changes required: |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2420/Result : failure BIRDHOUSE_DEPLOY_BRANCH : update-thredds-5.4 DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-46.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1498/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2421/Result : failure BIRDHOUSE_DEPLOY_BRANCH : update-thredds-5.4 DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-69.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1499/NOTEBOOK TEST RESULTS |
|
FYI @mishaschwartz we still have not done fixing the issues on our side. Besides there is a performce problem with 5.4 (Unidata/tds#406) and we hope to be fixed in 5.5 so this PR is not ready (5.5 is not released). |
…antic versioning tag
|
FYI, we found more problems with version 5. NCML, UDDC, ISO and NetcdfSubset link do not work with v5. Have not had time to investigate. Opendap and other links work fine. Ex: click on the NCML, UDDC, ISO and NetcdfSubset link here https://pavics.ouranos.ca/testthredds/catalog/testdatasets/testdata/catalog.html?dataset=testdatasets/testdata/ta_Amon_MRI-CGCM3_decadal1980_r1i1p1_199101-200012.nc |
Related to Unidata/thredds-docker#310 |
|
NCML, UDDC, ISO links fixed by adding the missing jar, see Unidata/thredds-docker#310 (comment) No other clues found for broken NetcdfSubset link, opened an issue Unidata/tds#544 |
github-actions
bot
added
the
component/magpie
| @@ -33,6 +33,7 @@ providers: | |||
| dodsC, | |||
| wcs, | |||
| wms, | |||
| ncss, | |||
There was a problem hiding this comment.
Leave the old location.
THREDDS_VERSION can be overridden, so a server could still employ the older variant.
Given that, that will most probably cause a conflict in the data_type resolution order if all 3 are defined, so maybe alternate paths or a dynamic variable resolution will be needed here.
There was a problem hiding this comment.
ugh... you're right. Ok I'll figure something out.
| ncss/grid, | ||
| ncss/point, |
There was a problem hiding this comment.
Did you test if this works?
I do not know if Magpie/Twitcher handles this properly.
The data types were not planned to include a /, so I'm not sure if a url.split("/") might happen somewhere in the code leading to misinterpretation of the targeted data_type.
There was a problem hiding this comment.
Seemed to be fine but I didn't rigorously test by trying to read the two options with different directory permissions I guess. I'll look into that.
BUT... if these prefixes can't handle a / we should change the service definitions to:
<service name="ncssGrid" serviceType="NetcdfSubset" base="${TWITCHER_PROTECTED_PATH}/thredds/ncss-grid/" />
<service name="ncssPoint" serviceType="NetcdfSubset" base="${TWITCHER_PROTECTED_PATH}/thredds/ncss-point/" />or similar and that way we don't have to worry about that at all
There was a problem hiding this comment.
Ok maybe we can't change the path... the URL construction docs for this service look like point or grid needs to be a subpath under ncss:
https://docs.unidata.ucar.edu/tds/current/userguide/netcdf_subset_service_ref.html#url-construction
There was a problem hiding this comment.
The data types were not planned to include a /, so I'm not sure if a url.split("/") might happen somewhere in the code leading to misinterpretation of the targeted data_type.
Yup magpie splits it up (see here) so our only option at this point is to keep the magpie config as it is and treat grid/ and point/ as directory paths if we want to differentiate their magpie permissions.
That's going to cause other confusion though...
Right now we have URL paths like:
${FQDN}/twitcher/ows/proxy/thredds/ncss/datasets/...${FQDN}/twitcher/ows/proxy/thredds/dodsC/datasets/...
So if we want to set specific permissions on the datasets/ directory we can do that by setting one directory permission rule on the datasets/ subdirectory and it will work for all services.
But if we instead have:
${FQDN}/twitcher/ows/proxy/thredds/ncss/point/datasets/...${FQDN}/twitcher/ows/proxy/thredds/dodsC/datasets/...
Then we would need to set the same permission rule on datasets/ as well as point/datasets/. If we don't set the same rule on point/datasets/ then the rule won't apply to resources accessed by the ncss/point service.
Please let me know if I'm interpreting this correctly @fmigneault
If I'm right then I think we need to update Magpie to handle this use case.
There was a problem hiding this comment.
I'm going to revert my changes for now until we figure something out.
There was a problem hiding this comment.
@mishaschwartz Yes, you can create the issue.
@tlvu Magpie cannot handle this case currently because it wasn't designed for this. It assumes THREDDS "services" don't contain a /, so it could rely on {Twitcher-prefix}/{THREDDS_name}/{THREDDS_service}/{rest-as-dir/file}.
There was a problem hiding this comment.
@tlvu Magpie cannot handle this case currently because it wasn't designed for this. It assumes THREDDS "services" don't contain a
/, so it could rely on{Twitcher-prefix}/{THREDDS_name}/{THREDDS_service}/{rest-as-dir/file}.
@fmigneault I am still lost. What case are you talking about?
Just to be clear, in the current state, with Misha's proposed Magpie config change rollback, do we have any problems?
Been running some Jenkins on the new Thredds and I am having some weird errors. Not sure if it's my test system or the code. So if you foresee some problem, please let me know.
There was a problem hiding this comment.
The ncssGrid and ncssPoint services won't be protected like other services do (ie: a common browse/read/write set for all services) since the paths are not handled.
For the moment, if you want them protected, you need to duplicate the permission hierarchy.
i.e.:
A file accessed by ncssGrid via ${FQDN}/twitcher/ows/proxy/thredds/ncss/point/datasets/sub/file.nc will actually go through ncss service, and point will be considered by Magpie as if it was any other directory (like datasets or sub).
Therefore, the same datasets/sub/file.nc would need 3 sets of permissions:
- datasets/sub/file.nc => (browse,read,write) for anything currently handled
- grid/datasets/sub/file.nc => (read) only if accessing via ncssGrid
- point/datasets/sub/file.nc => (read) only if accessing via ncssPoint
and those (read) need to be duplicated for every user/group/dir/file combination applicable
There was a problem hiding this comment.
The
ncssGridandncssPointservices won't be protected like other services do (ie: a common browse/read/write set for all services) since the paths are not handled.
@fmigneault I am surprised. If we "Deny or Allow" everything under ncss path, isn't both of those variant grid/datasets/sub/file.nc and point/datasets/sub/file.nc will be properly denied or allowed, because they are all under ncss/ path?
Under ncss is read only no write anyways, since there is no write for that kind of path.
There was a problem hiding this comment.
@tlvu
Yes.
A deny top-level will restrict everything lower. But usually, a deny is "undone" by an allow on a lower specific resource we want to grant access. The same issue also happens even when working the other way around -- allow recursive followed by restricted deny lower (see below example).
With the current implementation of THREDDS that checks the service [accses-mode] (dodC, ncss, etc.) between Twitcher-proxy-path and the rest of the dir/file path, those resources would be at different "level" for the same file reference from the point of view of Magpie. So you would need to duplicate your 'allows'/'denies' across [accses-mode].
RESOURCE PERMISSION APPLIED / RESULT
===========================================
THREDDS allow-recursive
ncss [access-mode] (not a resource, cannot have permissions directly)
datasets deny-recursive
sub allow-recursve (all but sub's contents are blocked)
public allow-recursive
secure deny-recursive
secret allow-match only for user-1, maybe group "manager" also, others...
point <-- THREDDS sees this as another [access-mode], but Magpie thinks its a RESOURCE!
datasets
sub <==== OH! OH! undesired full open-access (it's not under the denied "/datasets")
public allow-recursive (but because on THREDDS)
secure if you forget to replicate above 'secure', this is still full open acces
grid (repeat again, another set of duplicat permissions, for each group/user combination)
...
Overview
Unidata has dropped support for TDS versions < 5.x. This updates Thredds to version 5.4.
Changes
Non-breaking changes
Breaking changes
Related Issue / Discussion
Additional Information
Links to other issues or sources.
birdhouse_daccs_configs_branch: master
birdhouse_skip_ci: false