Generic association proposal #3144
Conversation
There was a problem hiding this comment.
I think it's a very interesting idea.
I left several comments to discuss other approaches.
As you outlined in the problem statement there are several problems that can be solved independently:
- creating an additional secret with a well-defined format that contains everything you need to access Elasticsearch (to also include in ES namespace itself?) vs. the current situation where you need to mount the password + mount the certificates + "guess" what the url is
- injecting the content of this secret into user Pods automatically
- rotating content from a mounted secret (especially when using env vars)
- syncing secrets across multiple namespaces (should this be the responsibility of ECK?)
- higher-level declarative user creation
|
|
||
| - [Secrets](https://kubernetes.io/docs/tasks/inject-data-application/distribute-credentials-secure/) _(not mutually exclusive with environment variables)_, they naturally address the case of the certificates, but they can also contain the variables exposed as environment variables. | ||
|
|
||
| - [PodPreset](https://kubernetes.io/docs/tasks/inject-data-application/podpreset/), it is an interesting solution, designed to solve the problem described here. Unfortunately this is a recent addition in the Kubernetes API and it's not widely available yet. |
There was a problem hiding this comment.
TIL!
Interestingly the alpha landed in 1.6 but there's still no plans for beta.
| elasticsearchName: monitor | ||
| kibanaName: monitor | ||
| # apmServerName: monitor | ||
|
|
There was a problem hiding this comment.
To avoid the need for an extra CRD, could this also be set in the existing stack CRDs?
eg. something like:
apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
name: quickstart
spec:
version: 7.7.0
nodeSets:
- name: default
count: 1
syncCredentials:
- secretRef:
namespace: ns-a
name: my-elastic-creds
user: elastic
- secretRef:
namespace: ns-b
name: my-elastic-creds-b
user: my-custom-userThere was a problem hiding this comment.
The rational here was to allow a quick review of the existing associations (for example in case of a security audit) and to be allow to have an alpha release and improve this feature over the time (decoupled lifecycle with the stack applications). But this could be discussed.
| # symbols: false | ||
| # ttl: "24h" | ||
| # Or just reference a password | ||
| # secretName: secret-with-user-and-password |
There was a problem hiding this comment.
it feels like we need to solve/simplify user management (#728 and #3056) before being able to make progress on cross-namespace custom users sync.
However your proposal is a very interesting input for that issue, I think. I like the concept of having a standard secret format to express username + password + tls certs + url at once.
| metadata: | ||
| name: my-application | ||
| annotations: | ||
| association.k8s.elastic.co/name: "monitor-ns/monitor" # namespaced name of the association CR |
There was a problem hiding this comment.
I think we'd need to differentiate the various stack components.
I may want to access Elasticsearch, but not APMServer for example?
annotations:
elasticsearch.k8s.elastic.co/inject-credentials: "monitor-ns/my-es"or since we may already know the secret (in the current namespace) we want to inject:
annotations:
elasticsearch.k8s.elastic.co/inject-credentials: "es-creds-synced-secret"^ at this point we're pretty close to the effort of the user mounting the secret themselves directly (adding env vars is more verbose though).
There was a problem hiding this comment.
In the original approach an association is a top level component which already allows to select which components are accessed by the remote application. That being said your idea is still relevant in that case because it would allow to have a better granularity from the application point of view.
| * `password`: clear text version of the password, the hashed version is maintained, reconciled and is made available by the generic association controller to the Elasticsearch controller. | ||
| * `elasticsearch-host`: hostname of the Elasticsearch cluster | ||
| * `elasticsearch-port`: port of the Elasticsearch cluster | ||
| * `elasticsearch-ca`: Certificate of the Elasticsearch CA |
There was a problem hiding this comment.
In a way, it makes it simpler to access Elasticsearch in a different namespace, than accessing it in its own namespace where there is currently a combination of multiple secrets (certs, password) + implicit information (service name, port).
I guess we probably want to have the same in the stack application's own namespace.
|
|
||
| ### The Association Secret | ||
|
|
||
| The proposal here is to define the content of the association secret. This `Secret` is managed by a generic association controller. According to what the user specify in the generic association custom resource this `Secret` may be filled with the following items: |
There was a problem hiding this comment.
This only works in situations where ECK is managing the target namespace.
vs. a situation where ECK manages resources in namespaceA but the end-user application runs in namespaceB.
In that case, a cluster-wide secret-sync operator, decoupled from ECK, could make more sense? (something like https://github.com/IBM-Cloud/kube-samples/tree/master/secret-sync-operator)
There was a problem hiding this comment.
True if we only want to share a Secret (and I agree it could be a first step). But if we want to update/mutate some resources we must manage the target namespace anyway.
| * The configuration checksum can be injected in the pod template to allow an automatic restart of the `Pods` | ||
| * ECK can inject and manage some "well-known" environment variables, some good examples are the ones used in the context of the APM Server (`ELASTIC_APM_SERVER_URLS`, `ELASTIC_APM_SECRET_TOKEN`...) | ||
| * Cons: | ||
| * It could be considered as "intrusive" |
There was a problem hiding this comment.
++ I don't like much the idea of ECK updating the spec of other Pods/Deployments/StatefulSets outside the scope of a MutatingAdmissionWebhook.
There was a problem hiding this comment.
I have the same feeling, it sounds odd. But I'm not sure I have a technical argument against it 🤷♂️
| * Pros: | ||
| * Less intrusive than updating the parent resource | ||
| * Cons: | ||
| * Cannot handle the lifecycle of the Secret content. The associated application must detect than something has changed in the `Secret`. In case of an environment variable the application must handle a `Pod` restart itself. |
There was a problem hiding this comment.
Would it make sense to optionally (opt-in annotation) inject a liveness probe that would fail if the secret gets updated? Based on an additional "checksum" entry in the secret maybe.
| * Pros: | ||
| * Less intrusive than updating the parent resource | ||
| * Cons: | ||
| * Cannot handle the lifecycle of the Secret content. The associated application must detect than something has changed in the `Secret`. In case of an environment variable the application must handle a `Pod` restart itself. |
There was a problem hiding this comment.
I was looking at how https://www.vaultproject.io/docs/platform/k8s/injector deals with this problem. Looks like they inject files, not environment variables. Maybe for that particular reason 😄
It's a general problem with k8s secrets.
|
@sebgl thanks for your feedback I'll try to split this proposal into smaller issues. |
The goal of this PR is to describe and discuss a way to make it easier for a user application (i.e. not managed by ECK) to establish a trusted connection with an Elastic stack application.
Sorry for reviving this directory, but I was not sure about the right way to discuss the ideas I have in mind for a "generic" association controller.
I used to find the PR format convenient to discuss some architectural choices, but I'm happy to create an issue if you think it is more convenient (or once we are done with this one).