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 disabling of elastic user. #7723

Merged
merged 11 commits into from
Apr 25, 2024
Merged

Conversation

naemono
Copy link
Contributor

@naemono naemono commented Apr 17, 2024

resolves #7719

Primary changes

This change allows a user to disable the elastic user from being created upon Elasticsearch creation. A use case for this would be when an organization/user would prefer to manage all users/roles via SSO (SAML/IDP/LDAP/etc).

This adds a new field to the Elasticsearch CRD:

apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: elasticsearch-sample
spec:
  auth:
    disableElasticUser: true

This also updates the documentation to reference the new change in multiple sections where the elastic user is mentioned.

Additional changes

Since one of the primary uses of the elastic user is to allow our ECK diagnostics to run, a new user with minimal privileges has been created to allow this functionality: elastic-internal-diagnostics.

Testing done

  1. Simple elasticsearch cluster creation
  2. Full stack creation using ES/Kibana/Fleet server/Agents with file-realm user added; Kibana login/functionality worked without issues, along with Fleet server and all agents worked perfectly.
  3. ECK diagnostics run using this version of ECK.

Open issues

  • The eck-diagnostics call to /api/exception_lists/items/_find?list_id=endpoint_host_isolation_exceptions&namespace_type=agnostic is still non-functional. I'm working with the relevant team to try and find the right permissions to adjust.

TODO:

  • Documentation updates
  • e2e testing; unit testing seems sufficient for this, as we check for the non-existence of the elastic secret.
  • Fix unit tests
  • Fix e2e smoke tests

Unit tests.
CRD changes.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@botelastic botelastic bot added the triage label Apr 17, 2024
@thbkrkr thbkrkr added the >enhancement Enhancement of existing functionality label Apr 18, 2024
@botelastic botelastic bot removed the triage label Apr 18, 2024
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono changed the title WIP: Allow disabling of elastic user. Allow disabling of elastic user. Apr 22, 2024
@naemono naemono marked this pull request as ready for review April 22, 2024 14:31
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Copy link
Contributor

@kvalliyurnatt kvalliyurnatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from a code perspective, have not tested it myself.

@naemono naemono added the >docs Documentation label Apr 22, 2024
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I was trying to think if the disableElasticUser attribute is the right name but could not come up with something more succinct 👍

pkg/controller/elasticsearch/user/reconcile.go Outdated Show resolved Hide resolved
// that are required for the ECK diagnostics. In the future we should try again to
// generate a more fine-grained role and not use "*".
Privileges: []string{
"*",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you still investigating this or is this the least privileges we can give?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm going to take another shot at this, as one of the items referenced in the readme (/api/exception_lists/items/_find?list_id=endpoint_host_isolation_exceptions&namespace_type=agnostic) is still under investigation. The rest of the pieces here are ready for review. This could change, and I'll note it here if it does.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pebrc update: I've found that some of these endpoints require platinum licenses, which is why I'm getting unauthorized errors when attempting to call them, even with privileges that should allow me to call them, as I'm running a dev version of ECK make go-run which doesn't support enterprise features.

I'm going to try and test this a bit differently and update.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok @pebrc with the current roles, which are as limited as I can get them, I can successfully run eck-diagnostics with no issues. This should be ready for final reviews. An upcoming PR to eck-diagnostics will be upcoming once this is merged.

Move conditional logic into the `reconcileElasticUser` func.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM code wise. Did a quick test disabling/enabling the user. I also got the errors on the api/exception_lists in the Kibana diagnostics, I am approving nonetheless assuming you will update the permissions as needed.

@naemono
Copy link
Contributor Author

naemono commented Apr 25, 2024

LGTM code wise. Did a quick test disabling/enabling the user. I also got the errors on the api/exception_lists in the Kibana diagnostics, I am approving nonetheless assuming you will update the permissions as needed.

I'm taking one more look at this...

@naemono
Copy link
Contributor Author

naemono commented Apr 25, 2024

LGTM code wise. Did a quick test disabling/enabling the user. I also got the errors on the api/exception_lists in the Kibana diagnostics, I am approving nonetheless assuming you will update the permissions as needed.

@pebrc just ran the diagnostics using the elastic superuser, and I get a 404 on this endpoint as well, so this is expected, or at least in line with current behavior:

15:08:52.552 [main] INFO  co.elastic.support.diagnostics.commands.BaseQuery - kibana_security_endpoint_host_isolation_1   /api/exception_lists/items/_find?list_id=endpoint_host_isolation_exceptions&namespace_type=agnostic&page=1&per_page=100  failed. Bypassing
15:08:52.552 [main] INFO  co.elastic.support.diagnostics.commands.BaseQuery - See archived diagnostics.log for more detail. Status: 404  Reason: Endpoint does not exist.
15:08:52.578 [main] INFO  co.elastic.support.diagnostics.commands.BaseQuery - kibana_security_endpoint_event_filters_1   /api/exception_lists/items/_find?list_id=endpoint_event_filters&namespace_type=agnostic&page=1&per_page=100  failed. Bypassing
15:08:52.578 [main] INFO  co.elastic.support.diagnostics.commands.BaseQuery - See archived diagnostics.log for more detail. Status: 404  Reason: Endpoint does not exist.

@naemono naemono merged commit 8420c9c into elastic:main Apr 25, 2024
5 checks passed
@naemono naemono deleted the remove-elastic-user branch April 25, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs Documentation >enhancement Enhancement of existing functionality v2.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potentially allow disabling the creation of the elastic user.
4 participants