Add TLS package and helper methods to runtime

[phillebaba]

This change is meant to standardize how certificates are read from secrets by the controllers. This logic is currently shared by notification-controller and image-reflector-controller, there might be more.

The original logic is taken from image-reflector-controller.
https://github.com/fluxcd/image-reflector-controller/blob/de3de64adddf8bb00276e12aa0436968bc94ff39/controllers/imagerepository_controller.go#L278-L309

I did however make a change to the behavior so that the functions will return an error if the secret does not contain a ca or cert. Or when the client cert is set but not client key and vice versa. My opinion is that this behavior is more logical than the previous one.

[phillebaba]

@squaremo do you have any opinion about the change in behavior compared to image-reflector-controller?

[squaremo]

@squaremo do you have any opinion about the change in behavior compared to image-reflector-controller?

Seems reasonable to me. I can't think of a situation in which you'd want a client cert without the key, or vice versa.

[squaremo]

I would usually put mutually exclusive conditions using the same variables like these in a switch, because it looks a bit like a logic table:

switch {
  case clientCertOk && !clientKeyOk:
    // ...
  case !clientCertOk && clientKeyOk:
    // ...
  case clientCertOk && clientKeyOk:
    // ...
}

Modulo the error message, all you care about is that they are both present or neither, so you could also do this:

if clientKeyOk != clientCertOk {
  return nil, fmt.Errorf("found one of %s or %s, and expected both or neither", ClientCert, ClientKey)
}
if clientKeyOk && clientCertOk {
  // ...
}

If you don't think either of these an improvement, your formulation is totally fine -- just fine-tuning :-)

[phillebaba]

You are right and I think the second option is smart.

[hiddeco]

I think this util can also be used in the source-controller for e.g. the Helm repository indexes.

[stefanprodan]

Please add _test to this file, we don't want these certs in flux binary or the controllers.

[hiddeco]

It may be even better to simply use the fixtures from https://cs.opensource.google/go/x/crypto/+/master:ssh/testdata/keys.go

[phillebaba]

Ok I will remove the certs from this PR.

[phillebaba]

@hiddeco I had a look at the testdata and I am a bit confused about how that can be used to replace the test certificate currently in the PR.

[hiddeco]

@phillebaba this PR is still waiting on an update. Couple of additional comments:

• Due to all the efforts in Add conditions and patch helpers, and docs to runtime #101, the introduction of the package requires a doc.go file to outline the package
• Comments are now up to 120 characters in length
• Given it only deals with TLS, I would maybe also call the package tls and not certs (this would match crypto/tls)