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

Correct the documentation for create_doc privilege #47784

Merged
merged 11 commits into from
Nov 19, 2019

Conversation

bizybot
Copy link
Contributor

@bizybot bizybot commented Oct 9, 2019

The documentation was added in #47584 but the behavior was changed which did not reflect in the documentation. This commit removes the note that is not needed anymore.

Preview: http://elasticsearch_47784.docs-preview.app.elstc.co/guide/en/elasticsearch/reference/master/security-privileges.html

@bizybot bizybot added >docs General docs changes :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v8.0.0 v7.5.0 labels Oct 9, 2019
@bizybot bizybot requested a review from ywelsch October 9, 2019 10:36
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (>docs)

@elasticmachine
Copy link
Collaborator

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

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

Perhaps we should still keep the note that this privilege requires the op_type create on the index / bulk API?

@bizybot bizybot requested a review from ywelsch October 9, 2019 10:51
Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

Can you have a look as well, @lcawl?

@ywelsch ywelsch requested a review from lcawl October 9, 2019 10:54
@tvernum tvernum self-requested a review October 9, 2019 12:13
@bizybot
Copy link
Contributor Author

bizybot commented Oct 10, 2019

@elasticmachine run elasticsearch-ci/2

@bizybot bizybot closed this Oct 10, 2019
@bizybot bizybot reopened this Oct 10, 2019
@bizybot bizybot requested a review from tvernum October 15, 2019 04:40
`create`.
[NOTE]
====
This privilege relies on the `op_type` of indexing requests (<<docs-index_>> and
Copy link
Contributor

Choose a reason for hiding this comment

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

This privilege relies on....

It seems to me that the implication here is that when you have only create_doc privilege, you cannot use the index or bulk APIs successfully unless you use one of the three options below. If that's true, I don't think we're making that dependency clear enough. I'd suggest changing "the op_type can be set to create..." to something like "...you must set the op_type to create..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you index a document (with no '_id'), then the 'op_type' is set to 'create' internally. I think the implication is correct here but open for other suggestions. Thank you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps something more like you must ensure that the op_type is set to create ?

@bizybot bizybot requested a review from lcawl October 24, 2019 10:11
Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd like to give @lcawl a chance to have another look.

@tvernum tvernum self-assigned this Oct 31, 2019
@jimczi jimczi added v7.6.0 and removed v7.5.0 labels Nov 12, 2019
Copy link
Contributor

@lcawl lcawl left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@tvernum tvernum added the v7.5.0 label Nov 19, 2019
@tvernum
Copy link
Contributor

tvernum commented Nov 19, 2019

@elasticmachine update branch

@tvernum
Copy link
Contributor

tvernum commented Nov 19, 2019

Merging despite CLA status, as the author was an Elastic employee when the development work was done

@tvernum tvernum merged commit 68870ac into elastic:master Nov 19, 2019
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Nov 20, 2019
The documentation was added in elastic#47584 but those docs did not reflect the up-to-date behavior of the feature.

Backport of: elastic#47784
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Nov 20, 2019
The documentation was added in elastic#47584 but those docs did not reflect the up-to-date behavior of the feature.

Backport of: elastic#47784
tvernum added a commit that referenced this pull request Nov 29, 2019
The documentation was added in #47584 but those docs did not reflect the up-to-date behavior of the feature.

Backport of: #47784
tvernum added a commit that referenced this pull request Nov 29, 2019
The documentation was added in #47584 but those docs did not reflect the up-to-date behavior of the feature.

Backport of: #47784
@ywelsch
Copy link
Contributor

ywelsch commented Feb 27, 2020

The backport PR seems to have been merged. I'm therefore removing the backport pending label here. Please shout if this is incorrect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v7.5.0 v7.6.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants