HIVE-28957: Enhance TestHiveMetaStoreAuthorizer #5819
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the TestHiveMetaStoreAuthorizer tests to verify that proper privilege objects are generated and filter contexts are applied as expected. Key changes include switching from the DummyHiveAuthorizerFactory to a dedicated MockHiveAuthorizerFactory, adding additional authorization failure checks with explicit fail messages, and introducing a new mock filter hook (MockMetaStoreFilterHook) to validate ownership-based filtering in metadata operations.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java | Updated tests to use a mock authorizer, added privilege capture and ownership filtering tests, and modified configuration to use the new mock classes. |
| ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/MockMetaStoreFilterHook.java | Introduced a mock filter hook implementation to verify that filtering logic based on ownership behaves as expected. |
...ache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java
Outdated
Show resolved
Hide resolved
asf-ci-hive
added
tests pending
tests failed
and removed
tests unstable
tests pending
labels
asf-ci-hive
added
tests pending
tests failed
tests passed
and removed
tests failed
tests pending
labels
| private static final Logger LOG = LoggerFactory.getLogger(MockMetaStoreFilterHook.class); | ||
| private static final String AUTHORIZED_OWNER = TestHiveMetaStoreAuthorizer.authorizedUser; | ||
|
|
||
| public MockMetaStoreFilterHook(Configuration conf) { |
There was a problem hiding this comment.
conf variable is unused
There was a problem hiding this comment.
Adding @SuppressWarnings("unused") annotation here to document that this is intentional; I keep the conf parameters to follow the established MetaStoreFilterHook interface contract. What do you think? Should we delete it or keep it? Thanks a lot
...g/apache/hadoop/hive/ql/security/authorization/plugin/metastore/MockMetaStoreFilterHook.java
Show resolved
Hide resolved
...ache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java
Outdated
Show resolved
Hide resolved
asf-ci-hive
added
tests pending
tests failed
and removed
tests passed
tests pending
labels
|
Pending green automated tests |
|
What changes were proposed in this pull request?
Enhance TestHiveMetaStoreAuthorizer to verify that the correct privilege objects are created for each operation and ensure the appropriate filter contexts are constructed and applied
Why are the changes needed?
Existing TestHiveMetaStoreAuthorizer uses DummyHiveAuthorizerFactory, which bypasses validation of the real HiveMetaStoreAuthorizer's filtering behavior.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Regression tests, unit tests