-
Notifications
You must be signed in to change notification settings - Fork 707
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
Conversation
Unit tests. CRD changes. Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
There was a problem hiding this 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.
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
There was a problem hiding this 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 👍
docs/orchestrating-elastic-stack-applications/security/users-and-roles.asciidoc
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{ | ||
"*", |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>
There was a problem hiding this 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.
I'm taking one more look at this... |
@pebrc just ran the diagnostics using the
|
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:
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
Open issues
/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:
e2e testing; unit testing seems sufficient for this, as we check for the non-existence of the elastic secret.