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

Use the workaround if we can't determine the server's version #581

Merged
merged 7 commits into from
Sep 15, 2023

Conversation

bartbroere
Copy link
Contributor

This is related to issue #580

In some scenarios (a limited-scope API key, or maybe a corporate environment) a user of eland might not have enough permissions to determine the host's Elasticsearch version.

In that case, we could default to using the workaround, to be sure.

@elasticmachine
Copy link

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@pquentin
Copy link
Member

pquentin commented Sep 6, 2023

buildkite test this please

@pquentin
Copy link
Member

pquentin commented Sep 6, 2023

buildkite test this please

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

I'm happy to do this is it unblocks you, but note that there are other places where we look at the ES version in the same way that would break similarly.

eland/field_mappings.py Outdated Show resolved Hide resolved
Co-authored-by: Quentin Pradet <quentin.pradet@gmail.com>
@bartbroere
Copy link
Contributor Author

I'm happy to do this is it unblocks you, but note that there are other places where we look at the ES version in the same way that would break similarly.

Thanks! I noticed too that it was used in some other places. This one is probably in the place someone is most likely to encounter it. You get to this code by instantiating an eland DataFrame I believe.

Of course I'm still trying to find out if we could grant monitor/main privileges to all users in our organisation, but for now this would be very helpful.

@pquentin pquentin merged commit 5c5ef63 into elastic:main Sep 15, 2023
1 check passed
@droberts195
Copy link
Contributor

Of course I'm still trying to find out if we could grant monitor/main privileges to all users in our organisation

cluster:monitor/main literally just gives access to the / endpoint, which I find hard to believe is a security risk to allow for any user who legitimately has access to the cluster to do anything else. We probably ought to make it easier to grant access to this endpoint alone instead of requiring an obscure action name to be granted. But until then I don’t think it would be a security risk to add it manually to some of the other roles you have defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants