Add cluster CA certificate into kubeconfig if one exists #36
Conversation
Codecov Report
@@ Coverage Diff @@
## master #36 +/- ##
=========================================
+ Coverage 34.51% 39.42% +4.9%
=========================================
Files 1 1
Lines 197 208 +11
=========================================
+ Hits 68 82 +14
+ Misses 123 121 -2
+ Partials 6 5 -1
Continue to review full report at Codecov.
|
kuberos.go
Outdated
| // If the cluster definition does not come with certificate-authority-data nor | ||
| // certificate-authority, then check if kuberos has access to the cluster's CA | ||
| // certificate and include it when possible. Assume all errors are non-fatal. | ||
| if len(cluster.CertificateAuthorityData) == 0 && len(cluster.CertificateAuthority) == 0 { |
There was a problem hiding this comment.
cluster.CertificateAuthority is a string, so it would be more idiomatic to test that it's the empty string (i.e. cluster.CertificateAuthority == "".
There was a problem hiding this comment.
Good call! I'll make the change.
kuberos.go
Outdated
| cluster.CertificateAuthorityData = caCert | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
https://github.com/spf13/afero
I like to use afero to make this kind of thing testable. It's pretty much a drop in replacement for the stdlib's filesystem functions, with an in-memory backend that's great for testing.
There was a problem hiding this comment.
I attempted to use a similar library (github.com/blang/vfs) for a work project, and ended up running into a lot of tiny little differences compared to stdlib.
I'll check out afero, though!
kuberos.go
Outdated
| // Use the first context as the current context | ||
| if c.CurrentContext == "" { | ||
| c.CurrentContext = name | ||
| } |
There was a problem hiding this comment.
I'm not sure about this functionality. I personally like to avoid setting a current context, because I typically have many clusters and always want to type kubectl --context=blah to make sure I never accidentally operate on my current context when I actually meant to operate on another one. (i.e. I want to make it hard to set my current context to a production cluster one day, then assume it's set to the staging cluster the next day and delete a whole namespace). That said I don't use Kuberos in production at my current job, and I think I'm a little weird in this regard, so perhaps this behaviour is what most users would prefer?
There was a problem hiding this comment.
Yeah, I considered whether or not it's worth it.
We (work) recommend keeping configs for separate clusters in separate configs and setting the KUBECONFIG env-var to switch instead. It's obviously a different workflow, so having a default context is useful.
I'll say that this behavior does seem a little magical, though. So maybe I should just remove it, haha.
ripta
force-pushed
the
ripta/ca-certificate
branch
from
4a8e072 to
3763b73
Compare
ripta
force-pushed
the
ripta/ca-certificate
branch
from
3763b73 to
f2ff301
Compare
|
@negz - I added a couple of test cases; used afero, and I really like it so far. Please take a look and LMK what you think! |
Addresses one of the ideas in #31. Instead of adding YAML templating, this PR takes a different approach:
clusters[*].clusterin the kubeconfig) to see if eithercertificate-authority-data(binary embedded PEM data) orcertificate-authority(path to client-side file) are non-empty in the template.a. If both certificate fields are empty and
/var/run/secrets/kubernetes.io/serviceaccount/ca.crtis accessible by Kuberos (which only happens when Kuberos is run in-cluster, and its pod is given a service account), then the contents of ca.crt is used as the value ofcertificate-authority-data.b. Otherwise, no behavior changes.
current-context, it's still used as the default. Otherwise,current-contextis set to the first non-empty context.I'd love feedback overall, but two things jumped out in particular:
DefaultAPITokenMountPath, because there doesn't seem to be an easy way to access the definition of it without pulling in the entirety ofk8s.io/kubernetes/plugin/pkg/admission/serviceaccount.