Add TLS on Keycloak server

[npalaska]

• Run Keycloak containers with SSL support
• Make necessary changes in the pbench-server to connect to the Keycloak running on SSL.
• Note: run-pbench-in-a-can script creates self signed pbench-server certificate and the same certificate is used for Keycloak configuration.

PBENCH-1138

[dbutenhof]

Cool. My big concerns are:

1. Do we support http urls or not? (Critically, do our intlab services support https, and do we have Opensearch credentials to use... i.e., can we actually stage this?)
2. If not, I'd like some better diagnosis here when the new parameters are omitted. (And if we do support http backend services, we need some more conditionalized code, and additional testing.)

[npalaska]

1. Do we support http urls or not? (Critically, do our intlab services support https, and do we have Opensearch credentials to use... i.e., can we actually stage this?)

True, with this change, we won't be able to support HTTP connections and we can not actually stage this until we have the required credentials to use in INTLAB. I am not sure if I know the answer to do our intlab services support https? That is something we have to get some more clarity on when we do a sit-down with Intlab. As far as I know, I don't think Intlab has to deploy certs for all the services until they move to DMZ, Right? but I may be wrong.

2. If not, I'd like some better diagnosis here when the new parameters are omitted. (And if we do support http backend services, we need some more conditionalized code, and additional testing.)

I agree if we do support HTTP then we need more conditionalized code in our source but as I said I don't know the answer and maybe we should hold off until we know the answer. I would like to think adding support for both HTTP and HTTPS and running tests with different scenarios would not be necessary if we know what is supported in the Intlab.

[dbutenhof]

I agree if we do support HTTP then we need more conditionalized code in our source but as I said I don't know the answer and maybe we should hold off until we know the answer. I would like to think adding support for both HTTP and HTTPS and running tests with different scenarios would not be necessary if we know what is supported in the Intlab.

I think we need to find out the intent and timeframe of TLS within intlab. This is a great P.O.C., but I really don't want to get into a situation where main isn't deployable.

To make this more concrete, I think either:

1. We require TLS backend, and we clearly diagnose missing configuration (certificates, credentials, a backend service specified as http instead of https, etc.), or
2. We dynamically adapt to either https with credentials and certs or http without.

(In theory we can support credentials without TLS, as we do now for Postgresql, as well as TLS without credentials, as we expect for the relay server, but I'm less concerned about those twists as long as we end up with code that we can deploy in intlab. In theory I like the idea of being general; in practice I'm not sure we'll ever need to care about anything but our intlab service.)

[webbnh]

I second Dave's comment: we should not merge a change which makes the main branch undeployable.

So, we either need code which supports both HTTPS and HTTP (and, then, we'll need to test both...), or we need to pick one. And, if we don't have support from the IntLab for HTTPS, then I think it's clear which one we need to pick.

In the meantime, I've found a number of things to consider, below.

[dbutenhof]

The server's last words are "Waiting for OIDC server to become available."

[webbnh]

This looks like a good update. However,

• Please update the PR description to reflect the current scope of the work.
• The OIDC client code around the cert validation and the CA location needs tweaking.
• There are a number of places where we are supplying default values for environment variables; I think we should be consistent in the values of the defaults at each level, unless there is a good reason for them to diverge, otherwise someone is likely to fall into some serious confusion.
• Since you've added localhost to the TLS certificate, we should use localhost to address the server(s) (assuming that it works) rather than the hostname -I value (which is likely to have a few challenges). (On the other hand, if using localhost doesn't work, why did you add it to the cert?)
• Rather than using --cacert on the curl commands, consider using the CURL_CA_BUNDLE environment variable instead (which run-server-functional-tests already sets up). (The Pbench Client also uses it...you might want to do something similar in the ODIC client code.)

And, there are a bunch of nits and smaller things.

[webbnh]

Nikhil, this looks great, but there are a couple of details left to attend to:

• The biggest item is how we make the Pbench CA cert available to inside the Server container: I think it should be copied in when the container is built, rather than mapped in when it is invoked, because this is workable and better reflects an actual deployment.
• The default value for PB_DEPLOY_FILES in server/pbenchinacan/load_keycloak.sh isn't (quite) consistent with the default from run-pbench-in-a-can (prev).

In addition, I think these are a good idea:

• The user should probably be able to override the value of CURL_CA_BUNDLE which is set in load_keycloak.sh.
• The default for CURL_CA_BUNDLE in lib/pbench/client/oidc_admin.py should probably be True.
• And, we might want a code comment.

[dbutenhof]

Don't we user container-build.sh both for the functional tests and for the "real" containers we'll eventually deploy to staging/production? I'd really prefer not to be packaging our fake CA in staging much less production even though we need it for functional tests. This probably needs to be conditionalized somehow.

[webbnh]

Yeah, Dave, we do (and I think should) use the same container image for functional testing and for deployment, but I concluded that adding the CA to the container image is the least of evils.

In the Staging/Production case, it's just another file, and a small one at that (1.3Kb). It's not a private key or anything, it's just a cert. We would give it away to anyone who wants to verify the TLS connection to a development server, so I'm not concerned about having it in the container.

And, I can't think of any way of conditionalizing this which doesn't impact Staging and Production.

[npalaska]

Yeah, you have a point. I am not sure what is the best way forward, mounting it or copying it at the build time. @webbnh do you want to weigh in?

[webbnh]

@webbnh do you want to weigh in?

Yes. 😁

[dbutenhof]

I really don't like putting our development fake CA bundle into the product container. It may be expedient, but it's wrong and there's got to be a better way.

[webbnh]

We use the config file, which we map in already, to decide whether to use the private CA or not. (It's not fake....) If we map the CA in, then we cannot use the same command/script in both development and staging/production (or, at least, we have to add conditionalization such that the container invocation will be (slightly) different in the two cases...or we have to map in a dummy file....).

Instead, for the cost of a tiny increment in size, we avoid the need for conditionalization or divergence in the invocations.

[webbnh]

Looks great (assuming the tests pass this time...).

Just two nits.

[webbnh]

This definition doesn't seem to be needed anymore.

[webbnh]

I would wrap this line:

        REQUESTS_CA_BUNDLE=${PWD}/server/pbenchinacan/etc/pki/tls/certs/pbench_CA.crt \
            SQLALCHEMY_SILENCE_UBER_WARNING=1 \
            PYTHONUNBUFFERED=True \
            PBENCH_SERVER=${server_arg} \
            KEEP_DATASETS="${keep_datasets}" \
                pytest --tb=native -v -s -rs --pyargs \
                    ${posargs} pbench.test.functional.server

[npalaska]

Yeah, but I don't think it's worth another commit though.

[dbutenhof]

I dislike the container build hack, and it'll continue to bother me: but we also need to get Keycloak TLS into main and I'm going to hold my nose and approve just to move on for now.

[dbutenhof]

I really don't like putting our development fake CA bundle into the product container. It may be expedient, but it's wrong and there's got to be a better way.