Admin post certificate

[europaul]

• Added an endpoint at /admin/certs that will accept incoming POST requests
  to set Adam's signing certificate
• Changed configs to reference pathes to certificates and keys instead
  of copying
• Added utils to write files atomically

Closes #118

[milan-zededa]

@deitch Please review when you have some extra time. Paul has prepared a test which depends on this PR: lf-edge/eden#927

[giggsoff]

As I understand we try to override the variable which come from cli here. I can see that we potentially may create new file few lines above, how we will use it in case of adam's container restart?
I still cannot understand, is the approach you define here better than just manual file override. Looks like restart of adam is not required to re-read the certificate.

[europaul]

We only create a different file in case that CLI option was set to an empty string, otherwise we rewrite the certificate file. I think it's pretty unlikely that somebody unsets the CLI option explicetly, if it's not set we use the default value. So if Adam is restarted with the same CLI params it will pick up the new certificate. Only if it's restarted with --signing-cert="" it will not pick up any certificate and we'll need to set it again with the POST request.

I didn't understand how you are proposing to replace the certificate file from Eden without an API call? I thought they are only supposed to communicate through the API and we cannot assume that Adam and Eden are running on the same system, so that we can just replace the file locally.

[giggsoff]

it's pretty unlikely that somebody unsets the CLI option explicitly

well, sounds reasonable, it will look like a problem of the caller.

I didn't understand how you are proposing to replace the certificate file from Eden without an API call? I thought they are only supposed to communicate through the API and we cannot assume that Adam and Eden are running on the same system, so that we can just replace the file locally.

right now we prepare the options to start Adam container in Eden. And we generate certs on Eden side. But looks like we use environment variables to set signing cert here. So we cannot change the certificate without Adam's restart (StopAdam + StartAdam calls) in current logic and you approach looks valid. But can you please check that we will have no problems without file replacement after Adam restart (I mean, will fill correct SIGNING_CERT env), e.g. eden adam stop + eden adam start?

[europaul]

upon the start of adam server, we always take the certificate from the env - so the one that's provided by the caller. So now it looks like it's eden's responsibility to provide the new certificate / replace the certificate file etc.

[europaul]

btw, why is the adam stop command doing the exact opposite of what --adam-rm requests? and all the other stop commands as well?

[europaul]

yes, but using POST is cleaner because

1. we can use it in a scenario when the caller (like Eden) and Adam are not colocated - through the network
2. if the certificate resides in the database path (like done by default), we shouldn't probably just manipulate files there, circumventing Adam's interfaces

[deitch]

if the certificate resides in the database path (like done by default), we shouldn't probably just manipulate files there, circumventing Adam's interfaces

100%. This usage of --cert-path is just wrong. Either there should be no --cert-path option, or if there is one, it is outside the database and is a read-only space that Adam uses to get the content, never to write it.

we can use it in a scenario when the caller (like Eden) and Adam are not colocated - through the network

Don't you find it strange to change those certs over the network? No other server software of which I am aware does that.

If you do, you are saying, "cert is a database artifact". In that case:

• --cert-path must be removed; it is fixed in the database
• cert must be stored in the actual database, based on the driver (file, redis, memory, etc.), but entirely inside the database; Adam has no knowledge of what it is or where it goes, just passed it to the database driver or reads it from the database driver
• the env var must be resolved. Does it override what is in the database? Or seed it? Override seems to make sense, unless we rename it something that says very clearly, "this is the seed only, but if something is in the database I will use that". Which still is a bit odd behaviour.

Doesn't GET /certs return multiple certs? What would your proposed POST do?

[europaul]

I was talking about the POST implemented as part for this PR, just for the signing certificate

[deitch]

What does GET /certs get? Is that just the signing certificate as well?

[europaul]

I added POST /admin/certs. There is no GET /admin/certs

[deitch]

After all of this, I think I finally understand why you did what you did @europaul . You viewed the env var as the equivalent of "load cert material into the database", and --cert-path as "here is where in the database that cert material goes, and by the way, we probably shouldn't really use that path outside of adam", as if that were some internal detail that had leaked.

So the POST was a simple way of being equivalent to the env var.

We had viewed the CLI flag as definitive, like if you passed it to nginx or apache, i.e. a way to tell Adam a location to source the certificate.

[europaul]

as per the discussion above, we decided to change the approach and go with #123 instead.
closing this PR as superseded