[v7r2] HTTPs services within DIRAC by chaen · Pull Request #4677 · DIRACGrid/DIRAC

chaen · 2020-07-13T15:00:09Z

This PR replaces #3815 and is mostly based on the work of @louisjdmartin

It should be totally transparent for the existing, as the existing code was mostly left untouched. Most of it is well documented and tested, but a lot of clean up still needs to be done.

This initial PR:

implements the basic building blocks
Implements a few examples (in particular the CS and the FileCatalog)
Contains a fair bunch of documentation, but clearly not yet ready for administrators/users
Adds an extra element in the integration tests matrix, where every services that has a Tornado equivalent is replaced with the https version.

When originally designed (2 years ago), a lot of tests were written in order to compare the behaviour of DISET and HTTPs server, as well as performance tests. These tests are still included here in case of need, but no effort has been made to automate it.

The aim of the approach here is that the replacement of the protocol should be as transparent as possible. However, there are known problems, well detailed in various presentations and tasks (links in the docs) that may make it tricky in some places, in particular for global variables.

Having to rebase that two year old extensively tested code was quite some work. Thus I hope that this PR is not going to sit long before being merged.

BEGINRELEASENOTES

*Tests
FIX: Ignore tornado services and test DBs
NEW: new integration tests for Tornado based services

Core:
NEW: X509Chain returns DN entry
NEW: introduction of Tornado based services

ENDRELEASENOTES

andresailer

Basically random observations, mostly typos.

fstagni

Don't you want to run the "dummy" integration tests of Tornado in the CI?

TaykYoku · 2020-07-26T15:43:01Z

+        self._request(retry=retry + 1, stream=stream, outputFile=outputFile, **kwargs)
+
+      errStr = "%s: %s" % (str(e), rawText)
+      return S_ERROR(errStr)


It may be worth here returning S_ERROR with the response status code or describe the http codes in the DErrno and return them according to the received status code.

Hum, I think I understand what you mean. I'll think of it, but I do not think it is a good idea to re-map all the HTTP status code to a DErrno error code. However you are correct that we should distinguish the cases where we get really an HTTP error, or if we get another exception (network for example). I'll try to come up with something

fstagni · 2020-08-06T15:21:31Z

There are some conflicts, please rebase/fix.

chaen · 2020-08-06T15:33:03Z

@fstagni I think there is again a cockup with the required tests. This is constantly a source of problem. Can we either have them all or none as required ?

fstagni · 2020-08-06T15:42:29Z

I will just put None as required (the interface is pretty silly...)

fstagni

Mostly comments and a few requests.

chaen · 2020-08-27T13:29:21Z

OK I think this time it's good to go. @andresailer @chrisburr @fstagni anyone care to do the final review ?

chaen · 2020-08-28T13:00:46Z

@andresailer done !

chrisburr

Just a few uses additions of io.open that should be reverted.

There will be a handful more Python 3 issues but I'll fix them after this is merged rather than making more conflicts with my Python 3 PRs.

…ector

Co-authored-by: Andre Sailer <andre.philippe.sailer@cern.ch>

…tornado patch repo

andresailer · 2020-10-14T15:47:24Z

+- Auth attributes are still there (``auth_yourMethod``).
+
+The interface of the DISET request handler was preserved, in particular:
+* ``getCSOption``


Suggested change

* ``getCSOption``

* ``getCSOption``

I think this needs a newline before the bullet list

andresailer · 2020-10-14T15:52:32Z

+************
+
+Two special python packages are needed:
+* git+https://github.com/DIRACGrid/tornado.git@iostreamConfigurable : in place of the standard tornado. This adds configurable feature to tornado


Suggested change

* git+https://github.com/DIRACGrid/tornado.git@iostreamConfigurable : in place of the standard tornado. This adds configurable feature to tornado

* git+https://github.com/DIRACGrid/tornado.git@iostreamConfigurable : in place of the standard tornado. This adds configurable feature to tornado

Another newline needed here

andresailer · 2020-10-14T15:56:16Z

+
+From the client side, no change is needed since :py:meth:`DIRAC.Core.Tornado.Client.TornadoClient.TornadoClient.receiveFile` keeps the interface
+
+This procedure is not optimized server side (see commented ``export_streamToClient`` implementation in :py:class`DIRAC.Core.Tornado.Server.TornadoService.TornadoService`). 


Suggested change

This procedure is not optimized server side (see commented ``export_streamToClient`` implementation in :py:class`DIRAC.Core.Tornado.Server.TornadoService.TornadoService`).

This procedure is not optimized server side (see commented ``export_streamToClient`` implementation in :py:class:`DIRAC.Core.Tornado.Server.TornadoService.TornadoService`).

andresailer · 2020-10-14T15:58:51Z

+
+- `Presentation of HTTPS in DIRAC  <https://docs.google.com/presentation/d/1t0hVpceXgV8W8R0ef5raMK3sUgXWnKdCmJUrG_5LsT4/edit?usp=sharing>`_.
+- `Presentation of HTTPS migration <https://docs.google.com/presentation/d/1NZ8iKRv3c0OL1_RTXL21hP6YsAUXcKSCqDL2uhkf8Oc/edit?usp=sharing>`_.
+- `Summary presentation (latest) <https://indico.cern.ch/event/945474/>`.


Suggested change

- `Summary presentation (latest) <https://indico.cern.ch/event/945474/>`.

- `Summary presentation (latest) <https://indico.cern.ch/event/945474/>`_.

andresailer

I was wrong about the :py:class; that was fine. Only the one missing colon was giving a bad display. There might be some packages missing now for the readthedocs.org documentation, so this maybe needs a fix PR right after

chaen requested review from andresailer, atsareg and fstagni as code owners July 13, 2020 15:00

andresailer reviewed Jul 15, 2020

View reviewed changes

fstagni reviewed Jul 15, 2020

View reviewed changes

andresailer reviewed Jul 15, 2020

View reviewed changes

Comment thread ConfigurationSystem/private/Refresher.py Outdated

TaykYoku reviewed Jul 26, 2020

View reviewed changes

This was referenced Jul 29, 2020

Services integration with DIRAC Workload Manager #4623

Closed

[v7r3][WIP] OAuth2/OIDC AuthN/AuthZ mechanism (Full version) #4650

Closed

chrisburr reviewed Jul 30, 2020

View reviewed changes

chaen force-pushed the rel-v7r2_FEAT_HTTPS_2 branch from 051c31e to a10cbc4 Compare August 5, 2020 14:33

TaykYoku mentioned this pull request Aug 5, 2020

What RESTful endpoints are used for the new OAuth2 mechanism? #4703

Closed

chaen force-pushed the rel-v7r2_FEAT_HTTPS_2 branch from 1ddc171 to 7facb5d Compare August 6, 2020 15:36

fstagni reviewed Aug 10, 2020

View reviewed changes

chrisburr reviewed Aug 11, 2020

View reviewed changes

chaen force-pushed the rel-v7r2_FEAT_HTTPS_2 branch 4 times, most recently from 2e4cce1 to 284183d Compare August 26, 2020 13:54

andresailer reviewed Aug 27, 2020

View reviewed changes

Comment thread ConfigurationSystem/Client/ConfigurationClient.py Outdated

andresailer reviewed Aug 27, 2020

View reviewed changes

Comment thread Core/Tornado/Client/TornadoClient.py Outdated

chaen force-pushed the rel-v7r2_FEAT_HTTPS_2 branch from b2491d5 to 9ab2e64 Compare August 28, 2020 09:21

chaen mentioned this pull request Aug 28, 2020

[v7r2] Functional DIRAC client for Python 3 #4731

Closed

chrisburr reviewed Aug 28, 2020

View reviewed changes

chrisburr previously approved these changes Aug 28, 2020

View reviewed changes

chaen and others added 15 commits October 5, 2020 18:13

Tests: ignore Tornado services when doing installation in CI

d021d5d

Component installer can install Tornado services

30fd649

Add TornadoFileCatalogHandler

c500558

Make DFC client compatible with Tornado service

adcf718

Run FileCatalog tests with https

449b477

Move Tornado deprecated tests

de91e64

Addapt a bit the doc

3b8d5ef

Implement minor review comments

bb8c03a

refactor the stream

b2a6edf

Implement minor review comments and py3 compatibility

0d903b0

Better use of HTTPError

62a900f

Factorize

ed821bb

Tornado: make service initialization thread safe

4233c58

Factorize RPCClientSelector and TransferClientSelector into ClientSel…

624bfac

…ector

Typo in comment

a0230d7

Co-authored-by: Andre Sailer <andre.philippe.sailer@cern.ch>

chaen force-pushed the rel-v7r2_FEAT_HTTPS_2 branch from 4a1ab16 to a0230d7 Compare October 5, 2020 16:16

fstagni mentioned this pull request Oct 6, 2020

[LATER] Integration of HTTPS with Tornado #3815

Closed

Point to DIRACGrid instead of chaen repo

a0012a7

fstagni reviewed Oct 8, 2020

View reviewed changes

Comment thread environment.yml Outdated

Update environments.yml and requirements.txt to point to the correct …

916ced6

…tornado patch repo

fstagni approved these changes Oct 9, 2020

View reviewed changes

atsareg approved these changes Oct 14, 2020

View reviewed changes

andresailer reviewed Oct 14, 2020

View reviewed changes

andresailer approved these changes Oct 14, 2020

View reviewed changes

atsareg merged commit 1ca14a6 into DIRACGrid:integration Oct 14, 2020

TaykYoku mentioned this pull request Mar 18, 2021

[v7r3] Tokens auth #5045

Closed

chaen deleted the rel-v7r2_FEAT_HTTPS_2 branch August 11, 2022 12:57

	* git+https://github.com/DIRACGrid/tornado.git@iostreamConfigurable : in place of the standard tornado. This adds configurable feature to tornado

	* git+https://github.com/DIRACGrid/tornado.git@iostreamConfigurable : in place of the standard tornado. This adds configurable feature to tornado


		From the client side, no change is needed since :py:meth:`DIRAC.Core.Tornado.Client.TornadoClient.TornadoClient.receiveFile` keeps the interface

		This procedure is not optimized server side (see commented ``export_streamToClient`` implementation in :py:class`DIRAC.Core.Tornado.Server.TornadoService.TornadoService`).

	- `Summary presentation (latest) <https://indico.cern.ch/event/945474/>`.
	- `Summary presentation (latest) <https://indico.cern.ch/event/945474/>`_.

Uh oh!

Conversation

chaen commented Jul 13, 2020

Uh oh!

andresailer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fstagni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TaykYoku Jul 26, 2020

Choose a reason for hiding this comment

Uh oh!

chaen Aug 4, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fstagni commented Aug 6, 2020

Uh oh!

chaen commented Aug 6, 2020

Uh oh!

fstagni commented Aug 6, 2020

Uh oh!

fstagni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chaen commented Aug 27, 2020

Uh oh!

Uh oh!

Uh oh!

chaen commented Aug 28, 2020

Uh oh!

chrisburr left a comment