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] Support security config updates on the REST API using permission (#3264) #3459

Merged

Conversation

willyborankin
Copy link
Collaborator

Backport e4a1b77 from #3264

@peternied
Copy link
Member

That BWC failure looks like it was a network hiccup, but could you take a look just to confirm? It was on linux which classically hasn't had those kinds of problems.

…nsearch-project#3264)

Instead of setting
`SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION`
settings to update security configuration using `PATCH` or `PUT` a new
permission was added: `restapi:admin/config/update`.

So far I decided to keep this flag as it is due to a backward
compatibility and log a deprecation message that these settings will be
removed in the future. Maybe it is better to remove it completely.

Besides, added the missed test for `SecurityConfigApiAction`

Signed-off-by: Andrey Pleskach <[email protected]>
(cherry picked from commit e4a1b77)
Signed-off-by: Andrey Pleskach <[email protected]>
@willyborankin
Copy link
Collaborator Author

willyborankin commented Oct 4, 2023

That BWC failure looks like it was a network hiccup, but could you take a look just to confirm? It was on linux which classically hasn't had those kinds of problems.

I just re-ran it. All others tests passed. I think it was a regular hickup

@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Merging #3459 (ec3081a) into 2.x (59e0e82) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #3459      +/-   ##
============================================
+ Coverage     64.61%   64.66%   +0.04%     
- Complexity     3535     3539       +4     
============================================
  Files           261      261              
  Lines         19806    19808       +2     
  Branches       3323     3324       +1     
============================================
+ Hits          12798    12808      +10     
+ Misses         5370     5366       -4     
+ Partials       1638     1634       -4     
Files Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java 84.58% <100.00%> (+0.04%) ⬆️
...dlic/rest/api/RestApiAdminPrivilegesEvaluator.java 73.58% <100.00%> (+2.15%) ⬆️
...ecurity/dlic/rest/api/SecurityConfigApiAction.java 93.54% <100.00%> (+0.44%) ⬆️
...security/dlic/rest/api/SecurityRestApiActions.java 80.00% <ø> (ø)
...urity/dlic/rest/api/SecuritySSLCertsApiAction.java 83.82% <100.00%> (ø)

... and 1 file with indirect coverage changes

@peternied peternied merged commit fc2b875 into opensearch-project:2.x Oct 5, 2023
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2.11.0 Issues targeting the 2.11 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants