[#5077] feat(core): Add the check of user before creating metadata object #5096
Conversation
jerqi
changed the title
| containerSuite.startKafkaContainer(); | ||
| kafkaBootstrapServers = | ||
| String.format( | ||
| "%s:%d", | ||
| containerSuite.getKafkaContainer().getContainerIpAddress(), | ||
| KafkaContainer.DEFAULT_BROKER_PORT); |
There was a problem hiding this comment.
Why do we need to use a Kafka container to test?
If we can use Hive container to test at the same time, we can save resources.
There was a problem hiding this comment.
We need to test the creating topic case.
There was a problem hiding this comment.
Currently, This UT only have throw ForbiddenException UT.
I think we need to add normal operation UT.
There was a problem hiding this comment.
In other UTs, we have created the metadata objects. It's ok to add it.
| @@ -71,6 +72,10 @@ public Catalog createCatalog( | |||
| String comment, | |||
| Map<String, String> properties) | |||
| throws NoSuchMetalakeException, CatalogAlreadyExistsException { | |||
| // Check whether the current user exists or not | |||
| AuthorizationUtils.checkCurrentUser( | |||
There was a problem hiding this comment.
Do we only need to check createXXX operation?
There was a problem hiding this comment.
We should check every operation actually.
We don't support meta authentication now. I think this should be included to meta authentication.
But for some managers, we don't a dispatcher interface for them like ownerManager and TagManager. We will add the interfaces first. I'm not sure to do them now. So I just make a minor improvement to check createXXX operation first.
What changes were proposed in this pull request?
Add the check of user before creating metadata object.
Why are the changes needed?
Fix: #5077
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Added UT.