add the service serving cert signer ca to SA token secret #9044
Conversation
| glog.Fatalf("Error parsing ca file for Service Serving Certificate Signer: %s: %v", c.Options.ControllerConfig.ServiceServingCert.Signer.CertFile, err) | ||
| } | ||
|
|
||
| // if we have a rootCA bundle add that too. The rootCA will be used when hitting the default master service, since those are signed |
There was a problem hiding this comment.
@liggitt decided if you think is true. With that key I'd be able to spoof the master, rewrite traffic to the controller keeping the ca.crt in sync, and substitute a ca.crt of my choosing into any pod I wanted. It's also seems unlikely/wrong for the rootCA key to used to sign a conflicting set of internal dns names by accident.
|
[testextended][extended:cmd] |
|
This is effectively API since the ca cert is in a known location. I marked it alpha to match the service account flags. |
|
Approved |
| @@ -41,6 +41,8 @@ import ( | |||
| "k8s.io/kubernetes/pkg/watch" | |||
| ) | |||
|
|
|||
| const ServiceServingCASecretKey = "alpha.service-serving-cert-ca.crt" | |||
There was a problem hiding this comment.
simplify the name to something like service-ca.crt, then LGTM
deads2k
force-pushed
the
service-signer-ca-02
branch
from
716135f to
0fd084a
Compare
Updated. waiting for green extended test before merge. @stevekuznetsov about my desire for bash based conformance tests.... any reason not to? |
|
Evaluated for origin testextended up to 0fd084a |
|
continuous-integration/openshift-jenkins/testextended SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin_extended/207/) (Extended Tests: cmd) |
|
The primary reason is to have a portable binary for conducting
conformance tests. Something that leans heavily on bash is probably
not conformance, but related.
|
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4456/) (Image: devenv-rhel7_4316) |
|
Evaluated for origin merge up to 0fd084a |
|
[Test]ing while waiting on the merge queue |
|
Evaluated for origin test up to 0fd084a |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4443/) |
|
@deads2k can't we have the carry merged upstream? |
|
@Kargakis No. It's nonsensical without a service cert signing system |
|
I think the point was whether the carry would be necessary if the
generation of the secret itself can be manipulated. Ie passing a
generator function in rather than embedding the generator. That's
probably a better factoring for other reasons.
|
|
@smarterclayton I can't think of a good reason to do it that way except for fragmentation. It's a controller made to create a secret of a fixed shape. If you want an upstream solution, I'd go back to my auto-mounted secrets specified on service accounts from way back. |
|
I don't consider that a very good argument - the controller
establishes a default secret shape, but the exact shape isn't fully
defined, and making the secret construction being an encapsulated step
from a set of inputs isn't really an unusual thing to do. It reduces
the size of the carry, separated two parts of function, and aligns
with our change. Arguing that we shouldn't divide up those two bits
is like arguing we shouldn't subdivide big methods into smaller
methods, or abstract certain bits of classes for testability or
function. I don't think it's that dramatic a request.
|
The shape is fully defined. It's API. The intent is "things I need to know to work in this cluster", but to be effective, the shape is defined. The argument of "I want to have a plug point to change what goes into this secret" makes no sense when that secret is effectively an API and they've decided what goes into it. |
|
Except we're changing the API with your patch in a backwards
compatible way by adding a field. So the argument that this is a
perfect unchanging API falls a bit flat.
|
Adds the service serving cert signer ca to the SA token secret so it gets automounted into every pod.
Doing this in the upstream controller makes sure that new secrets always have this before the pod starts and old secrets also get updated.
I did create bundle of the rootCA and the serviceSignerCA to mount. The rootCA key is more closely guarded and if you compromise that, you can change the CA cert in the mountpoint, so you're effectively trusting it anyway.
Updated the existing extended test for proof. I owe a conformance test, but those appear to busted in jenkins at the moment.
@liggitt @smarterclayton