Reload TLS certificates for bundle downloads

[msklosak]

Expected Behavior

When a short-lived certificate / key combination used for bundle downloads is reloaded, OPA should use the new certificate / key combination instead of the old ones.

Actual Behavior

OPA loads the certificate / key as soon as the server is created, and uses that combination for the life of the server.

Steps to Reproduce the Problem

1. Configure OPA to use a short-lived certificate to download a policy bundle from an HTTPS server
2. Reload the certificate used to download the policy bundle
3. Receive TLS errors because OPA does not pick up the reloaded certificate

Additional Info

This is critical functionality for my team, as short-lived certificates are vital for security reasons and restarting OPA upon certificate refresh is not feasible.

[tsandall]

As far as implementation goes, one option is to just re-read the certificates each time a request is made (if OPA sees a partially written cert file, it will just log an error and retry later.) Another option would be to reload the config (and any referenced files) on SIGHUP. I took a look to figure out how much work this would be.

• The status and decision log plugins support reconfiguration and don't require any changes. They obtain the rest client each time they try to upload.
• The bundle plugin implements reconfiguration for itself but not the service config that it depends on. I.e., it holds onto a Downloader object which in turn holds onto rest client that depends on service config.
• The discovery plugin does not implement reconfiguration and holds onto a Downloader object like the bundle plugin.

One option would be to modify the Downloader implementation to create rest clients lazily. Each time they try to download they would re-create the rest client. Another option would be to add a reconfigure method to the rest client. Modifying the Downloader to re-create the rest client each time is probably easier.

In either case, I noticed the rest client exports the underlying http client. I don't see a good reason for this so we should unexport it.

As fr as plumbing goes, I would update the runtime package to handle SIGHUP calls. When SIGHUP is received, the runtime package should tell the discovery plugin to reconfigure. The discovery plugin already knows how to reconfigure the plugin manger (which holds onto the rest clients) and the other plugins so it only has to be updated to reconfigure itself. I think the discovery plugin should only reconfigure itself when SIGHUP is received (i.e., don't reconfigure the discovery plugin when policy-based configuration is enabled).

@patrick-east WDYT?

[patrick-east]

Maybe for starters we just do the new client for each download attempt.

Longer term I like the idea of supporting SIGHUP for config changes (works better for like side-car usages where you maybe don't want to just kill and start and new opa), but we maybe don't need it right now.