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

Adding "read ingest pipeline" cluster privilege #66137

Merged
merged 11 commits into from
Dec 15, 2020

Conversation

BigPandaToo
Copy link
Contributor

The new privilege will allow to run ingest tools with minimal
privilege to check whether a
pipeline exists, without being able to modify it.
This privilege also support simulate pipeline too since it is a readonly
operation.

Resolve #48028

The new privilege will allow to run ingest tools with minimal
privilege to check whether a
pipeline exists, without being able to modify it.
This privilege also support simulate pipeline too since it is a readonly
 operation.

 Resolve elastic#48028
@BigPandaToo
Copy link
Contributor Author

@elasticmachine update branch

@BigPandaToo
Copy link
Contributor Author

@elasticmachine update branch

@albertzaharovits
Copy link
Contributor

@BigPandaToo the unit test class that I alluded to in our conv is PrivilegeTests. I think we should add a new test there for this new privilege.

@BigPandaToo
Copy link
Contributor Author

@elasticmachine update branch

@albertzaharovits
Copy link
Contributor

Looking good @BigPandaToo , what it still needs is a simple unit test in PrivilegeTests or ClusterPrivilegeTests.

Copy link
Contributor

@albertzaharovits albertzaharovits left a comment

Choose a reason for hiding this comment

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

LGTM

@albertzaharovits albertzaharovits added v7.11.0 :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC >enhancement labels Dec 15, 2020
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Dec 15, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@BigPandaToo BigPandaToo merged commit 76589f0 into elastic:master Dec 15, 2020
BigPandaToo added a commit to BigPandaToo/elasticsearch that referenced this pull request Dec 15, 2020
* Adding "read ingest pipeline" cluster privilege

The new privilege will allow to run ingest tools with minimal
privilege to check whether a
pipeline exists, without being able to modify it.
This privilege also support simulate pipeline too since it is a readonly
 operation.

 Resolve elastic#48028

* Adding "read ingest pipeline" cluster privilege

Fixing tests.

 Resolve elastic#48028

* Adding "read ingest pipeline" cluster privilege

Fixing tests.

 Resolve elastic#48028

* Adding "read ingest pipeline" cluster privilege

Fixing tests.

 Resolve elastic#48028

* Adding "read ingest pipeline" cluster privilege

Resolving bwc, renaming the action

Resolve elastic#48028

* Adding "read ingest pipeline" cluster privilege

Fixing doc

Resolve elastic#48028

* Adding "read ingest pipeline" cluster privilege

Fixing test

* Adding ingest pipeline privileges test

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
BigPandaToo added a commit to BigPandaToo/elasticsearch that referenced this pull request Dec 15, 2020
BigPandaToo added a commit that referenced this pull request Dec 15, 2020
* Adding "read ingest pipeline" cluster privilege (#66137)

* Adding "read ingest pipeline" cluster privilege

The new privilege will allow to run ingest tools with minimal
privilege to check whether a
pipeline exists, without being able to modify it.
This privilege also support simulate pipeline too since it is a readonly
 operation.

 Resolve #48028

* Adding "read ingest pipeline" cluster privilege

Fixing tests.

 Resolve #48028

* Adding "read ingest pipeline" cluster privilege

Fixing tests.

 Resolve #48028

* Adding "read ingest pipeline" cluster privilege

Fixing tests.

 Resolve #48028

* Adding "read ingest pipeline" cluster privilege

Resolving bwc, renaming the action

Resolve #48028

* Adding "read ingest pipeline" cluster privilege

Fixing doc

Resolve #48028

* Adding "read ingest pipeline" cluster privilege

Fixing test

* Adding ingest pipeline privileges test

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* Adding "read ingest pipeline" cluster privilege (#66137)

Fixing resolution

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@ywangd
Copy link
Member

ywangd commented Dec 16, 2020

@BigPandaToo @albertzaharovits We have two issues (#56353, #55640) suggesting deprecating the manage_ingest_pipelines privilege to in favor of manage_pipeline. Though we have not done anything about it because of bwc concerns, I wonder whether we should name the new privilege as read_pipeline instead of read_ingest_pipelines to avoid introducing another potential deprecation concern?

@tvernum
Copy link
Contributor

tvernum commented Dec 16, 2020

I agree with @ywangd - if we've agreed (and I think we have) that manage_pipeline is the correct management privilege, then read_pipeline is the better name for the read privilege, and we should fix it.

@BigPandaToo
Copy link
Contributor Author

I plan to open a Refuctoring issue to depricate and change the action names for the following privileges form "admin" to "monitor" :
GRANT_API_KEY
READ_INGEST_PIPELINE
MONITOR_SNAPSHOT
READ_CCR
READ_ILM
READ_SLM

How about I do this privileges name change as part of this refuctoring?

@tvernum
Copy link
Contributor

tvernum commented Dec 16, 2020

I think we should just handle this change by itself. We can do a quick rename before 7.11.0 is released so that it doesn't break anything.
If we wait, we'll need to go through the deprecation cycle.

@BigPandaToo BigPandaToo deleted the Read_ingest_pipeline branch April 19, 2021 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team v7.11.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "read ingest pipeline" cluster privilege
6 participants