Enable support for multiple issuers per namespace #2103
Conversation
This introduces the new Origin.Exports.IssuerUrls param for populating the exports with a list of issuer URLs, but the commit does not yet plumb the rest of this into other relevant places (scitokens config, origin ads, etc). For now, if no issuer URLs are provided, the server will still create a list for the export using the result of `config.GetServerIssuerURL()` as a fallback. This makes the code backwards compatible.
This handles creation of the correct scitokens config for the Origin, and sends the relevant information to the Director. That I can tell, both from unit tests and a few hand-crafted checks, the information that arrives in the Director is sufficient for caches to turn around and construct their own correct scitokens config for multi-issuer Origins.
Not sure what the original need for this test was, but it doesn't look like it actually does anything we don't do already with every fed test (other than using the test file transfer implementation, which has its own unit tests). Rather than update the test to work with the new multi- issuer setup, let's just excise the cruft.
Because issuer configuration now happens on instantiation of the Origin exports, we really do need to know the ports ahead of time. Unfortunately this breaks some of our testing infrastructure, which relied on a feature in XRootD that turned a port of '0' into a random, available port handed over by the OS. However, setting '0' as the port causes many of the default issuers to use this port, both for the issuer proper and the server's audience URL in scitokens config. Manually correcting ports in this file would be complicated and would require waiting for XRootD to sit around long enough that it decides to re-load the file. Instead, this pre-allocates the ports by asking the OS for a few that are available. There's a small race condition in testing environments where the ports we get back from the OS might become bound before our Pelican servers start. Hopefully this doesn't make testing super flaky, but I'm assuming the odds of that happening are currently relatively minimal.
jhiemstrawisc
added
enhancement
turetske
left a comment
turetske
left a comment
There was a problem hiding this comment.
I foresee a lot of merge conflicts between this and my PR involving scopes because we both touched a lot of the same code, but that's a problem for future me to rebase and deal with after this has been merged in.
I have a few questions and comments, otherwise, before I approve.
There was a problem hiding this comment.
I've confirmed that the changes here are independent of the origin's built-in OA4MP issuer.
@jhiemstrawisc I did observe one oddity during testing, though.
Suppose we set Origin.Exports[].IssuerUrls to two URLs. We then attempt a pelican object put to that export when the client has no previously saved token. The client will attempt to send the user to two URLs: the first element of Origin.Exports[].IssuerUrls, followed by Server.IssuerUrl.
I find it odd that that the second element of Origin.Exports[].IssuerUrls is ignored.
Alternatively, I find it odd that any elements of Origin.Exports[].IssuerUrls are considered by the client given that the so-called "token generation" URL in the director UI is actually Server.IssuerUrl.
jhiemstrawisc
force-pushed
the
issue-2040
branch
from
1f9edd6 to
01acd5a
Compare
4 participants
This lets origin administrators do the following in their config file:
I accomplished this by treating issuers as part of each OriginExport object, which changes a few of our assumptions about setting things up (see commit RE pre-allocating ports for XRootD). Luckily, we were forward thinking enough to design server/origin/namespace ads such that they already take a list of issuers, meaning there are no backwards compatibility issues to be concerned about there.
Otherwise, I'm punting slightly on re-defining some of the other issuer types (Origin server issuer as opposed to xrootd data issuers). Once we're sure this is working as intended, it'll let us stop registering services in Topology, which we're still doing to handle this multi-issuer case.