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

Plugin configuration should be conformant #1016

Closed
abh3 opened this issue Jul 3, 2019 · 15 comments
Closed

Plugin configuration should be conformant #1016

abh3 opened this issue Jul 3, 2019 · 15 comments
Assignees

Comments

@abh3
Copy link
Member

abh3 commented Jul 3, 2019

Currently, many plugin are configured by adding additional parameters after the plugin specification (e.g. xrootd.fslib []). This leads, many times, to inconsistent syntax and tortuously long lines (e.g. gsi) that are pretty opaque. There should be a standard way that plug-in writers can hook into the standard xrootd configuration file syntax mechanism.

@abh3 abh3 self-assigned this Jul 3, 2019
@abh3
Copy link
Member Author

abh3 commented Mar 31, 2020

So, I've looked at this in several angles and he only way to do this is for plugin writers to use the standard interface to get to read the configuration file and parse out what is of interest. I suppose we could make it slightly easier but it would reduce the options available. I'll open it up to suggestions of what plugin writers would like to have and after a little while, if I don't get any feedback, close this issue as one that is not particularly interesting.

@bbockelm
Copy link
Contributor

This would be fantastic - the most problematic plugin one is the one mentioned in the ticket (GSI plugin). I would still be interested in exporting an alternate entry point that receives the configuration file (maintaining backward compatibility by still exporting the existing entry point).

@abh3
Copy link
Member Author

abh3 commented Apr 15, 2020

So, the question I posed in another thread remains on the table. Is it acceptable to simply have a "blob parameter" creator. That is, if you don't specify the gobbledygook on the plugin line you can use he standard format (i.e. "gsi.xxx") and we will construct that blob from those parameters and feed them into the plugin. It's not ideal as any error messages will be delayed and won't necessarily be obviously tied to the parameter you may have specified. However, this is the fastest way of getting this done with the least amount of error.

@bbockelm
Copy link
Contributor

This will help with the standard gsi.xxx options you mention - but what about the authz plugin parameters?

For example, what happens to authzfun and authzfunparms here:

sec.protocol /home/xrootd-build/release_dir/lib64 gsi \
  -certdir:/etc/grid-security/certificates \
  -cert:/etc/grid-security/hostcert.pem \
  -key:/etc/grid-security/hostkey.pem \
  -crl:1 \
  -authzfun:libXrdLcmaps.so \
  -authzfunparms:no-authz \
  -gmapopt:0 \
  -authzto:3600

Will plugin writers still need to squash all parameter setting into a single line without spaces?

@abh3
Copy link
Member Author

abh3 commented Apr 17, 2020

That's just naturally falls out where gsi.authzfun gets converted to "-authzfun:". As for long lines, there is no need and, in fact, not allowed. Each "-:" specification would be entered on a separate line prefixed by "gsi." using the scheme . All of them would be blobbed and passed onward.

@abh3
Copy link
Member Author

abh3 commented Apr 18, 2020

Ah, I see what you are saying. The authzfunparms itself might be an incomprehensible blob, much like vomsfunparms. This is more complicated simply because each of these plugins uses an arbitrary syntax, for no particularly good reason. The answer is we could given a template. So,I see that

-vomsfunparms:certfmt=pem|vos=atlas,cms,dteam|grps=/atlas,/cms,/dteam|grpopt=10|dbg

is sort of an ungodly mess. Here we know that '=' is the key-value separator (as opposed to colon) and the option separator is '|' (as opposed to space) and there is no leading character (as opposed to dash). So, yes, if I were told that's the blobbing template I could apply it when you see multiple gsi.authzfunparms directives to construct that blob. The question is where does the template come from? The config file? A builtin template? From the protocol itself? If it's a builtin template then there would need to be a way to override it if you wanted to use another plugin that has some other kind of template going on. If it comes from the protocol then the protocol would have to instantiate a blobber itself and provide the template but then each plugin would have to do the same. authz and voms plugins really use different syntax based on who wrote the plugin? Do we have multiple instances of this going on already? If not, we can kill this divergence now before it starts I think.

@bbockelm
Copy link
Contributor

Hi @abh3 -

I'm sure that all plugins use a different syntax - as you suggest, it's likely from plugin writers working independently (and, I'd add, not having access to the configuration subsystem).

However, setting up a class that generates the input string seems to be quite complicated (example question: if I specify a configuration variable multiple times, is it additive or does it replace earlier invocations) compared to simply providing access to the config system like other plugins do.

No?

Brian

@abh3
Copy link
Member Author

abh3 commented Apr 20, 2020 via email

@bbockelm
Copy link
Contributor

Yup - agreed that there are higher priority things out there!

I'm aware that XrdOucStream.hh is public; I think all we really need here is the configuration file name! I guess what I was envisioning was a new plugin invocation that includes both the config parameters (-vomsfunparms:certfmt=pem|vos=atlas,cms,dteam|grps=/atlas,/cms,/dteam|grpopt=10|dbg above) and is also given the config filename.

@xrootd-dev
Copy link

xrootd-dev commented Apr 23, 2020 via email

@bbockelm
Copy link
Contributor

Ah, good to know! I think that might allow me to do what I want.

For now, let's put this ticket on pause until I can track down the XRDCONFIGFN route.

Brian

@abh3
Copy link
Member Author

abh3 commented Apr 24, 2020 via email

@bbockelm
Copy link
Contributor

You mean the sort of thing you summarized in #1016 (comment)? If so, I don't think that's necessary - once I know how to get the config filename, I've done the parsing in several other plugins.

I do think a simplified config file interface would be fantastic - that's a separate discussion, however.

@abh3
Copy link
Member Author

abh3 commented Apr 24, 2020

If you are referring to an std::vector of config line lines, yes that's what I mean. Otherwise, look at how XrdHttpProtocol.cc sets it up. As for a simplified interface, suggestions are welcomed. If you'd like, start a new ticket for that.

@abh3
Copy link
Member Author

abh3 commented Feb 17, 2022

We have a new method to extract relevant directives from a config file making this a trivial process for new plugins. We will not be going back and fixing old plugins as the labor cost is excessive.

@abh3 abh3 closed this as completed Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants