Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow existingSecret parameter for initial 'elastic' superuser #5460

Closed
brsolomon-deloitte opened this issue Mar 10, 2022 · 8 comments
Closed
Labels
>feature Adds or discusses adding a feature to the product

Comments

@brsolomon-deloitte
Copy link

Chart such as https://github.com/bitnami/charts/tree/master/bitnami/elasticsearch offer an existingSecret parameter that can point to an existing secret containing credentials which, when specified, will take precedence and prevent an auto-generated secret.

Does the ECK CRD expose this functionality for the initial elastic user in any way, and if so, what would a minimal example look like?

I do realize It is not recommended to use elastic user credentials for production use cases - but, that aside, it would still be nice to control the initial credentials if possible. For example, a common pattern is to use ExternalSecrets that point to a ground source of truth in somewhere like AWS Secrets Manager.

Environment

ECK 1.9.1, Elastic stack 7.17.1, K8s 1.21 on EKS

@botelastic botelastic bot added the triage label Mar 10, 2022
@brsolomon-deloitte
Copy link
Author

After speaking with Elastic support, it looks like this is possible by pre-creating a Secret with the name {{ cluster.name }}-es-elastic-user, which ECK will look for to use as an existing Secret.

While this appears to work, it is sub-optimal for several reasons:

  • It is implicit rather than explicit. Relying on an undocumented naming convention that occurs in Go code is prone to error and confusion. It would be better practice to allow the admin to explicitly point to an existingSecret.
  • It is a GitOps anti-pattern. With the current design, there is no evidence as far as IaC goes of the k8s admin pointing to the existing Secret rather than allowing one to be auto-created
  • It is (possibly) subject to a race condition. An example is with using ExternalSecret objects from kubernetes-external-secrets. Namely, ECK will "race" against the ExternalSecret first-creation of the {{ cluster.name }}-es-elastic-user Secret, with both fighting for control over the same secret.

@brsolomon-deloitte brsolomon-deloitte changed the title Use existingSecret for initial 'elastic' superuser? Allow existingSecret parameter for initial 'elastic' superuser Mar 15, 2022
@pebrc pebrc added the >feature Adds or discusses adding a feature to the product label Mar 15, 2022
@botelastic botelastic bot removed the triage label Mar 15, 2022
@pebrc
Copy link
Collaborator

pebrc commented Mar 18, 2022

As you said yourself we discourage the use of the elastic user for any productive use. For some use cases it also no longer holds the necessary permissions (e.g. stack monitoring).

We already have a declarative and predictable (and thus automatable) way to setup users through the file realm as documented here: https://www.elastic.co/guide/en/cloud-on-k8s/master/k8s-users-and-roles.html#k8s_file_realm
It has the advantage that you can also specify the roles the user should have and is therefore encouraging the use of minimal privileges.

I do understand however that the fact that you would have to provide the bcrypt hash of the passwords in the secret that we use as the source for creating these users poses a burden for the GitOps/ ExternalSecrets approach you have mentioned in your post. We discussed this a bit out of band and I am wondering that if we were to make the creation of users through the file realm easier by allowing you to specify simple key/value pairs in the secret where the key is the username and the value is the clear text password it would also address your use case.

This was already discussed back in the day when we first implemented support for the file realm in the ECK operator as possible next step in building out that feature. Consider this example for how this could potentially look in YAML:

apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: elasticsearch-sample
spec:
  version: 8.1.0
  auth:
    fileRealm:
    - secretName: es-users-file-realm
  nodeSets:
  - name: default
    count: 1
---
kind: Secret
apiVersion: v1
metadata:
  name: es-users-file-realm
stringData:
  # any unknown key/value pair will be internally transformed into 'users' by ECK, with a hash of the provided passwords
  # known key/value pairs are `users_roles` and `users` which contain raw file realm configuration as documented today. 
 my-user: cleartextpassword
 users_roles: |-
    admin:my-user

@pebrc
Copy link
Collaborator

pebrc commented Mar 21, 2022

Related #3056

I suggest to track the work on this feature on #3056 because it is more on topic than this one.

@brsolomon-deloitte
Copy link
Author

@pebrc to be clear on something and close the loop here, the elastic user is not supported through file realm user:hash secret, correct?

@pebrc
Copy link
Collaborator

pebrc commented Mar 21, 2022

No, it is not. But you would be able to create your own users through that mechanism, with more restricted permissions than superuser

@pebrc
Copy link
Collaborator

pebrc commented Apr 14, 2022

Coming back to the question of whether the elasticuser would be supported through the file realm. We think we should make it so that this is supported. If a user configures a user called elastic through the file realm mechanism with a clear text password as proposed in #3056 the password set through this mechanism should be also propagated to the secrets we create today for the elastic user.

There is an open question as to what should happen if the elastic user is configured through the raw user:hash configuration (which is already possible today). In this case the internally generated secrets would hold the incorrect password and we would not be able to correct that because in the raw file ream configuration we don't have the password available. One option would be to not generate the related secrets in this case. I believe this is last scenario is already possible today as any user provided configuration takes precedence over configuration generated by the operator.

@brsolomon-deloitte
Copy link
Author

Coming back to my original point here, in my opinion the heart of the matter is "explicit is better than implicit". It's currently undocumented behavior and not part of IaC that ECK will use an existing Secret if it follows a specific naming convention. Making this relationship clearer by providing an existingSecret parameter is more sound and doesn't necessarily imply that the cluster admin will actually use the elastic user for normal operations--just that they prefer to control that user's initial credentials in a known state.

@pebrc
Copy link
Collaborator

pebrc commented May 11, 2022

We merged a new feature which allows setting up the elastic user through the file realm. It should match the requirements formulated here. A preview of the documentation can be seen here https://www.elastic.co/guide/en/cloud-on-k8s/master/k8s-users-and-roles.html#k8s_file_realm

The elastic user can be specified in an secret that can be managed through an external secret store mechanism like the external-secrets operator.

It is explicit rather than implicit. It will give users control over the initial password set for this user. As a side note the feature is not restricted to the elastic user but can be used for any user one wants to create.

It will be part of the next ECK operator release. I am closing this issue therefore for now.

@pebrc pebrc closed this as completed May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature Adds or discusses adding a feature to the product
Projects
None yet
Development

No branches or pull requests

2 participants