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

[Backport 2.x] Command cat/indices will filter results per the do not fail on forbidden setting #3258

Merged
merged 11 commits into from
Aug 29, 2023

Conversation

derek-ho
Copy link
Collaborator

@derek-ho derek-ho commented Aug 29, 2023

Description

Backport 4c095d2 of #3236

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Returns the error message instead of the validation message because
validation message should already have been shown in above case
statements.

Fix: opensearch-project#3055

Is this a backport? If so, please add backport PR # and/or commits #

[Please provide details of testing done: unit testing, integration
testing and manual testing]

- [ ] New functionality includes testing
- [ ] New functionality has been documented
- [ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
(cherry picked from commit 847f911)
Fix merge conflicts

Signed-off-by: Derek Ho <[email protected]>
Fix merge conflicts

Signed-off-by: Derek Ho <[email protected]>
Fix final merge conflict

Signed-off-by: Derek Ho <[email protected]>
…idden setting (opensearch-project#3236)

This change allows for DNFOF behavior on the _cat/_indices API. It adds
the required index permissions into the DNFOF regex to be picked up in
the DNFOF code path. Previously it was being skipped/returning 403,
since the index permissions were not in the regex.

Fix: opensearch-project#1815

Is this a backport? If so, please add backport PR # and/or commits #

[Please provide details of testing done: unit testing, integration
testing and manual testing]

- [ ] New functionality includes testing
- [ ] New functionality has been documented
- [ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Derek Ho <[email protected]>
(cherry picked from commit 4c095d2)
Signed-off-by: Derek Ho <[email protected]>
willyborankin
willyborankin previously approved these changes Aug 29, 2023
@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Merging #3258 (8ac16ad) into 2.x (8c3c9c5) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #3258      +/-   ##
============================================
+ Coverage     62.35%   62.37%   +0.02%     
- Complexity     3292     3294       +2     
============================================
  Files           252      252              
  Lines         19456    19457       +1     
  Branches       3308     3308              
============================================
+ Hits          12132    12137       +5     
  Misses         5704     5704              
+ Partials       1620     1616       -4     
Files Changed Coverage Δ
...earch/security/privileges/PrivilegesEvaluator.java 73.20% <100.00%> (+0.08%) ⬆️

... and 5 files with indirect coverage changes

@peternied peternied changed the title [MANUAL BACKPORT 2.x] Command cat/indices will filter results per the do not fail on forbidden setting [Backport 2.x] Command cat/indices will filter results per the do not fail on forbidden setting Aug 29, 2023
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.opensearch.security.privileges.PrivilegesEvaluator.isClusterPerm;
import static org.opensearch.security.privileges.PrivilegesEvaluator.*;
Copy link
Member

Choose a reason for hiding this comment

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

Can star import be removed here?

Copy link
Member

Choose a reason for hiding this comment

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

Its in the original PR on main, checkstyle missed it? What do you think about handling this out of band - want to create an issue?

Copy link
Member

Choose a reason for hiding this comment

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

OpenSearch refuses wildcard imports: https://github.com/opensearch-project/OpenSearch/blob/main/gradle/formatting.gradle#L89-L94

We should update the spotless formatting guidelines in this repo to also refuse wildcard imports

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Raised an issue in the repo for this: #3260
Will create a follow up PR in main to fix the original violation and add the checkstyle. For now I just fixed it in this manual backport as well.

Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
@cwperks cwperks merged commit 8197431 into opensearch-project:2.x Aug 29, 2023
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.

4 participants