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

[Transform] Fix privileges check failures by adding allow_restricted_indices flag #95187

Merged
merged 2 commits into from
Apr 12, 2023

Conversation

przemekwitek
Copy link
Contributor

This PR adds allow_restricted_indices flag to the HasPrivilegesRequests issued by TransformPrivilegeChecker.
This allows having restricted indices as transforms' source indices so cases like Fleet are supported.

Relates #93259

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@elasticsearchmachine
Copy link
Collaborator

Hi @przemekwitek, I've created a changelog YAML for you.

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

I am happy to merge this to fix the immediate problem.

It wouldn't surprise me if another problem emerges in this area though. The docs for _has_privileges say that allow_restricted_indices is an option for that endpoint to stop spurious failures from indices that users might not know exist. For example, before if a user had *agents* as their source index pattern and it was matching my-own-agents-1 and my-own-agents-2 then the previous check would have passed but the new one will fail because it will also match .fleet-agents-1 and the user won't have access to that. Hopefully what will help us avoid this is that most index patterns don't begin with a wildcard, and all system indices begin with a dot. So hopefully the change in this PR won't fix one situation but break a load of others.

@kevinlog
Copy link
Contributor

Thanks for putting this up!

I checked it out and tested against Kibana and we're now able to install our integration correctly.

image

Our transforms get installed:
image

And our transform dependent feature is working:
image

LGTM from a functionality POV

@przemekwitek
Copy link
Contributor Author

LGTM from a functionality POV

Thanks for verifying this fix!

@przemekwitek przemekwitek merged commit e832e63 into elastic:main Apr 12, 2023
@przemekwitek przemekwitek deleted the allow_restricted_indices branch April 12, 2023 14:13
kevinlog added a commit to elastic/kibana that referenced this pull request Apr 12, 2023
## Summary

Resolves #154740
#154741

These tests were skipped due to an ES promotion block. This PR:
elastic/elasticsearch#95187 was added on the ES
side and another snapshot was promoted which should allow us to un skip
these tests.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
saarikabhasi pushed a commit to saarikabhasi/kibana that referenced this pull request Apr 19, 2023
…4863)

## Summary

Resolves elastic#154740
elastic#154741

These tests were skipped due to an ES promotion block. This PR:
elastic/elasticsearch#95187 was added on the ES
side and another snapshot was promoted which should allow us to un skip
these tests.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :ml/Transform Transform Team:ML Meta label for the ML team v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants