Add the ability to configure the client id and client secret using environment variables for the OpenShift connector.

[onkarbhat]

In a k8s deployment that uses Dex, this will allow us to define environment variables using a k8s secret that contains the client ID and client secret instead of showing it in plaintext in a configmap.

This partially meets requirements in #1099.

/cc @sagikazarmark @sabre1041 @nabokihms

[sagikazarmark]

Damn, another PR with ENV config keys in it. 😄 We really need a better strategy for this.

[onkarbhat]

Hi @sagikazarmark . Thanks for taking a look at this PR.

According to this update here - #1099 (comment) , it looked like creating a separate field in the config such as hashFromFile or hashFromEnv was the plan. #1601 and #1664 were implemented based on this recommendation .

The ClientSecretFromEnv in this PR was also based on that. Please let me know if there is any other thread/discussion about a different strategy or if you have something in mind .

fyi - This is another PR that I created for BindPWFromEnv for the ldap connector - #1797

[onkarbhat]

@sagikazarmark - Here is another solution:

How about ClientSecretFromFile and ClientIDFromFile instead of ClientSecretFromEnv/ClientIDFromEnv ?

The value for these fields will be a path to a file that contains the client-id/client-secret, example: /var/run/secrets/openshift/client-secret . This will allow us to mount a K8s secrets at /var/run/secrets/openshift/ to setup the client id and secret.

Would you also mind explaining why the env variable approach is not preferred here? Thanks.

[sagikazarmark]

Generally, the current config file is a mess, with different structures for each connector and these weird env var config keys.

It just feels wrong and I'm desperately trying to find a better solution, but at the same time all these PRs are coming with features that people want to use IMMEDIATELY. 😄

Obviously, finding The Right Solution (tm) takes time, but we keep merging these PRs and ultimately messing up the config even more, making our job a lot harder.

Feel free to ignore me, it's just maintainer mumbling. 😄

[sagikazarmark]

Please check my comment in #1099, it would probably provide an immediate solution to this problem without waiting for us to merge and release your PR (which we might not do, if we can settle on a different solution for loading values from env vars)

[sagikazarmark]

The default image can now template the configuration file before running Dex. However, it requires a writable /tmp directory which might not work on OpenShift.