WrenSecurity / wrenidm

Community‐developed identity management system with a flexible data model, multiple extension points and scripting support, including JavaScript and Groovy.
https://wrensecurity.org/
Other
40 stars 19 forks source link

OPENIDM-8521: Create an EnvironmentVariablePropertyAccessor #57

Open GuyPaddock opened 6 years ago

GuyPaddock commented 6 years ago

As a devops engineer using Docker or another container platform, I'd love it if we had the option to use environment variables to provide configuration instead of just properties files, so that we don't have to translate environment variables into configuration files or command-line arguments within the container.

For reference, FR implemented this under this ticket: https://bugster.forgerock.org/jira/browse/OPENIDM-8521

Unfortunately, that's after the point the trunk became closed off.

siepkes commented 6 years ago

Is there currently already some level of abstraction in IDM for getting configuration options? If there isn't we should probably abstract it and ensure we can write implementations for getting config options from different systems (also in the future); Environmental variables, Zookeeper, etcd, Consul, etc.

pavelhoral commented 6 years ago

@siepkes Properties are being obtained exclusively through IdentityServer and the abstraction you are asking about is PropertyAccessor.

Btw. if we ever want a source of inspiration, I suggest to look into Spring Boot.

pavelhoral commented 6 years ago

FYI you can pass environment variables to IDM via OPENIDM_OPTS variable as this is used in startup.sh:

OPENIDM_OPTS="-Xmx1024m -Xms1024m -Dmy.custom.property=$MY_CUSTOM_VARIABLE" ./startup.sh

Just remember to add the default arguments as well.

GuyPaddock commented 6 years ago

Thanks, @pavelhoral!

We are currently doing exactly as you proposed -- passing them in via startup vars -- but we're having to transpose underscores to periods in a bash script first since periods in variable names are not friends with BASH :(. It would be awesome if the startup script we're using for our containers didn't have to do any munging of environment variables and we could just make them available to IDM (like FR is doing in OPENIDM-8521). If I have some time, I might be able to code-up this feature and close out my own ticket.

First I need to circle back to @pavelhoral and @Kortanul's feedback in the other PR from me -- that's way overdue!

GuyPaddock commented 6 years ago

@pavelhoral We've run into a case that's a real pain in the ass -- properties that have spaces in the values. For example, in our containers, we're passing in the username for the LDAP connector in as a variable. Java expects the command line to look like this:

java -Dopenidm.connector.ldap.username="cn=Directory manager" rest...of...command...line

But if the variable contains spaces, it ends up coming out like this when passed through to Java:

java '-Dopenidm.connector.ldap.username="cn=Directory' 'manager"' 'rest...of...command...line'

As in, the property gets split on the space. And so far absolutely no combination of double-quotes, single quotes, slash-escaping, etc works properly when args are passed via the OPENIDM_OPTS env var like this. For this reason, the solution FR implemented in the released version of 5.5 -- of natively supporting env vars -- is ideal.

pavelhoral commented 3 years ago

The way property substitution works in Felix might be relevant. They use env: prefix to resolve stuff from System.getenv. I am upgrading the framework at the moment so maybe this can be addressed afterwards as I am getting familiar with it #56.