opensearch-project / security

🔐 Secure your cluster with TLS, numerous authentication backends, data masking, audit logging as well as role-based access control on indices, documents, and fields
https://opensearch.org/docs/latest/security-plugin/index/
Apache License 2.0
180 stars 263 forks source link

[2.15] Revert "[Backport 2.x] Remove static metaFields list and get version-specific values from core" #4474

Closed DarshitChanpura closed 1 week ago

DarshitChanpura commented 1 week ago

Reverts opensearch-project/security#4412 as it is causing ISM plugin failures:

https://ci.opensearch.org/ci/dbc/integ-test/2.15.0/9978/linux/arm64/tar/test-results/8315/integ-test/index-management/with-security/stdout.txt https://ci.opensearch.org/ci/dbc/integ-test/2.15.0/9978/linux/arm64/tar/test-results/8315/integ-test/index-management/with-security/local-cluster-logs/id-0/stdout.txt)

Relevant issue: https://github.com/opensearch-project/security/issues/4475

cwperks commented 1 week ago

Let's add some info around the reason for the reversion.

This is to resolve a failure seen during release integ testing in the ISM plugin.

[2024-06-15T03:57:00,516][WARN ][o.o.i.c.IndicesClusterStateService] [node_name_9200] [openactionit_index_1][0] marking and sending shard failed due to [failed to create index]
java.lang.NullPointerException: Cannot invoke "org.opensearch.index.mapper.MapperService.getMetadataFields()" because the return value of "org.opensearch.index.IndexService.mapperService()" is null
    at org.opensearch.security.configuration.SecurityFlsDlsIndexSearcherWrapper.<init>(SecurityFlsDlsIndexSearcherWrapper.java:65) ~[?:?]
    at org.opensearch.security.OpenSearchSecurityPlugin.lambda$onIndexModule$3(OpenSearchSecurityPlugin.java:697) ~[?:?]
    at org.opensearch.index.IndexService.<init>(IndexService.java:292) ~[opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.index.IndexModule.newIndexService(IndexModule.java:634) ~[opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.indices.IndicesService.createIndexService(IndicesService.java:888) ~[opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.indices.IndicesService.createIndex(IndicesService.java:775) ~[opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.indices.IndicesService.createIndex(IndicesService.java:215) ~[opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.indices.cluster.IndicesClusterStateService.createIndices(IndicesClusterStateService.java:557) [opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.indices.cluster.IndicesClusterStateService.applyClusterState(IndicesClusterStateService.java:292) [opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.cluster.service.ClusterApplierService.callClusterStateAppliers(ClusterApplierService.java:625) [opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.cluster.service.ClusterApplierService.callClusterStateAppliers(ClusterApplierService.java:611) [opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.cluster.service.ClusterApplierService.applyChanges(ClusterApplierService.java:579) [opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.cluster.service.ClusterApplierService.runTask(ClusterApplierService.java:502) [opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.cluster.service.ClusterApplierService$UpdateTask.run(ClusterApplierService.java:204) [opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:882) [opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedOpenSearchThreadPoolExecutor.java:283) [opensearch-2.15.0.jar:2.15.0]
    at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedOpenSearchThreadPoolExecutor.java:246) [opensearch-2.15.0.jar:2.15.0]
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) [?:?]
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) [?:?]
    at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]

It was seen in CloseActionIT and OpenActionIT which have integration tests around opening and closing indices.

We reproduced the NPE by closing and opening an index, but have not fully analyzed how the NPE affects FLS rules when re-opening an index.

We plan to introduce this bug fix for FLS Includes rules in a future release and add an additional test case where a closed index is re-opened and searched upon by a user that is mapped to a role with FLS Includes rules.

cwperks commented 1 week ago

@DarshitChanpura Can you also update release notes?

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 65.35%. Comparing base (cdc792c) to head (fc3fc4c).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/opensearch-project/security/pull/4474/graphs/tree.svg?width=650&height=150&src=pr&token=rBpySfQXMt&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project)](https://app.codecov.io/gh/opensearch-project/security/pull/4474?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project) ```diff @@ Coverage Diff @@ ## 2.15 #4474 +/- ## ========================================== - Coverage 65.37% 65.35% -0.03% ========================================== Files 311 311 Lines 21868 21862 -6 Branches 3550 3550 ========================================== - Hits 14297 14288 -9 - Misses 5805 5806 +1 - Partials 1766 1768 +2 ``` | [Files](https://app.codecov.io/gh/opensearch-project/security/pull/4474?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project) | Coverage Δ | | |---|---|---| | [...figuration/SecurityFlsDlsIndexSearcherWrapper.java](https://app.codecov.io/gh/opensearch-project/security/pull/4474?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fopensearch%2Fsecurity%2Fconfiguration%2FSecurityFlsDlsIndexSearcherWrapper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#diff-c3JjL21haW4vamF2YS9vcmcvb3BlbnNlYXJjaC9zZWN1cml0eS9jb25maWd1cmF0aW9uL1NlY3VyaXR5RmxzRGxzSW5kZXhTZWFyY2hlcldyYXBwZXIuamF2YQ==) | `97.22% <100.00%> (-0.40%)` | :arrow_down: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/opensearch-project/security/pull/4474/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project)
DarshitChanpura commented 1 week ago

@cwperks done. I've created an issue to track this: https://github.com/opensearch-project/security/issues/4475