Add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES and fix validation of getAntiAffinityNamespaces #13723

equanz · 2022-01-12T08:38:50Z

Motivation

Since #6428, we can define authz policy to each namespace operation.
However, I think the operation policy validateNamespacePolicyOperation(namespaceName, PolicyName.ANTI_AFFINITY, PolicyOperation.READ); is not suitable for getAntiAffinityNamespaces method because this method is a tenant-wise operation. (is not specific namespace-wise operation)

Currently, the validation method throws NPE when using default authz provider org.apache.pulsar.broker.authorization.PulsarAuthorizationProvider.
#13723 (comment)

Modifications

Add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES
Modify operate validation to LIST_ANTI_AFFINITY_NAMESPACES

If this approach is not accepted, I think we can use an existing operation like LIST_NAMESPACES instead of adding a new tenant operation.

Verifying this change

Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

Dependencies (does it add or upgrade a dependency): (no)
The public API: (no)
The schema: (no)
The default values of configurations: (no)
The wire protocol: (no)
The rest endpoints: (yes)
- modified endpoint's validation
The admin cli options: (no)
Anything that affects deployment: (no)

Documentation

no-need-doc

codelipenghui · 2022-01-12T14:13:53Z

@equanz After the change, how do the users using the old version migrate to the new version? We will introduce a breaking change in this PR, we should discuss in the dev email thread first.

equanz · 2022-01-25T09:58:03Z

@codelipenghui Sorry for the late reply.

If user defines custom authz provider and follow namespaceName=null, policy=PolicyName.ANTI_AFFINITY, operation=PolicyOperation.READ as correct state, they should change definition when updating the version. Therefore, we should consider migration plan just like you said.

By the way, I think the validation method may throw NPE when using default authz provider org.apache.pulsar.broker.authorization.PulsarAuthorizationProvider.

pulsar/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java

Line 598 in 8c8738f

return validateTenantAdminAccess(namespaceName.getTenant(), role, authData);

Therefore, it is one of issue fix for default authz provider.
Moreover, this endpoint doesn't return correct value in the latest version. (Please see #13644)

...
2022-01-25T18:43:37,980+0900 [pulsar-web-30-6] ERROR org.apache.pulsar.broker.web.AuthenticationFilter - [127.0.0.1] Error performing authentication for HTTP
javax.servlet.ServletException: java.lang.NullPointerException
	at org.glassfish.jersey.servlet.WebComponent.serviceImpl(WebComponent.java:410) ~[jersey-container-servlet-core-2.34.jar:?]
	at org.glassfish.jersey.servlet.WebComponent.service(WebComponent.java:346) ~[jersey-container-servlet-core-2.34.jar:?]
	at org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:366) ~[jersey-container-servlet-core-2.34.jar:?]
	at org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:319) ~[jersey-container-servlet-core-2.34.jar:?]
	at org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:205) ~[jersey-container-servlet-core-2.34.jar:?]
	at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:799) ~[jetty-servlet-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1631) ~[jetty-servlet-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.apache.pulsar.broker.web.ResponseHandlerFilter.doFilter(ResponseHandlerFilter.java:67) ~[classes/:?]
	at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193) ~[jetty-servlet-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1601) ~[jetty-servlet-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.apache.pulsar.broker.web.AuthenticationFilter.doFilter(AuthenticationFilter.java:81) ~[pulsar-broker-common-2.10.0-SNAPSHOT.jar:?]
	at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193) ~[jetty-servlet-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1601) ~[jetty-servlet-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:548) ~[jetty-servlet-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1624) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1434) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:501) ~[jetty-servlet-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1594) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1349) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:234) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:146) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.handler.StatisticsHandler.handle(StatisticsHandler.java:179) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.Server.handle(Server.java:516) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:400) ~[jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:645) [jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:392) [jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277) [jetty-server-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311) [jetty-io-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105) [jetty-io-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.io.ssl.SslConnection$DecryptedEndPoint.onFillable(SslConnection.java:555) [jetty-io-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.io.ssl.SslConnection.onFillable(SslConnection.java:410) [jetty-io-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.io.ssl.SslConnection$2.succeeded(SslConnection.java:164) [jetty-io-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105) [jetty-io-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104) [jetty-io-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:338) [jetty-util-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:315) [jetty-util-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:173) [jetty-util-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:131) [jetty-util-9.4.44.v20210927.jar:9.4.44.v20210927]
	at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:409) [jetty-util-9.4.44.v20210927.jar:9.4.44.v20210927]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:1.8.0_312]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:1.8.0_312]
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [netty-common-4.1.72.Final.jar:4.1.72.Final]
	at java.lang.Thread.run(Thread.java:748) [?:1.8.0_312]
Caused by: java.lang.NullPointerException
	at org.apache.pulsar.broker.authorization.PulsarAuthorizationProvider.allowNamespacePolicyOperationAsync(PulsarAuthorizationProvider.java:598) ~[pulsar-broker-common-2.10.0-SNAPSHOT.jar:2.10.0-SNAPSHOT]
	at org.apache.pulsar.broker.authorization.AuthorizationService.allowNamespacePolicyOperationAsync(AuthorizationService.java:445) ~[pulsar-broker-common-2.10.0-SNAPSHOT.jar:2.10.0-SNAPSHOT]
	at org.apache.pulsar.broker.authorization.AuthorizationService.allowNamespacePolicyOperationAsync(AuthorizationService.java:463) ~[pulsar-broker-common-2.10.0-SNAPSHOT.jar:2.10.0-SNAPSHOT]
	at org.apache.pulsar.broker.authorization.AuthorizationService.allowNamespacePolicyOperation(AuthorizationService.java:474) ~[pulsar-broker-common-2.10.0-SNAPSHOT.jar:2.10.0-SNAPSHOT]
	at org.apache.pulsar.broker.web.PulsarWebResource.validateNamespacePolicyOperation(PulsarWebResource.java:898) ~[classes/:?]
	at org.apache.pulsar.broker.admin.impl.NamespacesBase.internalGetAntiAffinityNamespaces(NamespacesBase.java:1873) ~[classes/:?]
	at org.apache.pulsar.broker.admin.v1.Namespaces.getAntiAffinityNamespaces(Namespaces.java:453) ~[classes/:?]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_312]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_312]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_312]
	at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_312]
	at org.glassfish.jersey.server.model.internal.ResourceMethodInvocationHandlerFactory.lambda$static$0(ResourceMethodInvocationHandlerFactory.java:52) ~[jersey-server-2.34.jar:?]
	at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher$1.run(AbstractJavaResourceMethodDispatcher.java:124) ~[jersey-server-2.34.jar:?]
	at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.invoke(AbstractJavaResourceMethodDispatcher.java:167) ~[jersey-server-2.34.jar:?]
	at org.glassfish.jersey.server.model.internal.JavaResourceMethodDispatcherProvider$TypeOutInvoker.doDispatch(JavaResourceMethodDispatcherProvider.java:219) ~[jersey-server-2.34.jar:?]
	at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.dispatch(AbstractJavaResourceMethodDispatcher.java:79) ~[jersey-server-2.34.jar:?]
	at org.glassfish.jersey.server.model.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:475) ~[jersey-server-2.34.jar:?]
	at org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:397) ~[jersey-server-2.34.jar:?]
	at org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:81) ~[jersey-server-2.34.jar:?]
	at org.glassfish.jersey.server.ServerRuntime$1.run(ServerRuntime.java:255) ~[jersey-server-2.34.jar:?]
	at org.glassfish.jersey.internal.Errors$1.call(Errors.java:248) ~[jersey-common-2.34.jar:?]
	at org.glassfish.jersey.internal.Errors$1.call(Errors.java:244) ~[jersey-common-2.34.jar:?]
	at org.glassfish.jersey.internal.Errors.process(Errors.java:292) ~[jersey-common-2.34.jar:?]
	at org.glassfish.jersey.internal.Errors.process(Errors.java:274) ~[jersey-common-2.34.jar:?]
	at org.glassfish.jersey.internal.Errors.process(Errors.java:244) ~[jersey-common-2.34.jar:?]
	at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:265) ~[jersey-common-2.34.jar:?]
	at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:234) ~[jersey-server-2.34.jar:?]
	at org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:680) ~[jersey-server-2.34.jar:?]
	at org.glassfish.jersey.servlet.WebComponent.serviceImpl(WebComponent.java:394) ~[jersey-container-servlet-core-2.34.jar:?]
	... 48 more
2022-01-25T18:43:38,000+0900 [pulsar-web-30-6] INFO  org.eclipse.jetty.server.RequestLog - 127.0.0.1 - - [25/1/2022:18:43:37 +0900] "GET /admin/namespaces/test/antiAffinity/group?property=my-property HTTP/1.1" 500 174 "-" "Pulsar-Java-v2.10.0-SNAPSHOT" 35
...

equanz · 2022-01-27T03:33:21Z

One of the alternative approaches is using an existing definition of operation. For example, validate TenantOperation.LIST_NAMESPACES for a tenant and PolicyName.ANTI_AFFINITY, PolicyOperation.READ for each namespace.

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java

Line 1886 in 8c8738f

List<String> namespaces = tenantResources().getListOfNamespaces(tenant);

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java

Lines 1888 to 1898 in 8c8738f

    
           return namespaces.stream().filter(ns -> { 
        
               Optional<LocalPolicies> policies; 
        
               try { 
        
                   policies = getLocalPolicies().getLocalPolicies(NamespaceName.get(ns)); 
        
               } catch (Exception e) { 
        
                   throw new RuntimeException(e); 
        
               } 
        
               String storedAntiAffinityGroup = policies.orElse(new LocalPolicies()).namespaceAntiAffinityGroup; 
        
               return antiAffinityGroup.equalsIgnoreCase(storedAntiAffinityGroup); 
        
           }).collect(Collectors.toList());

This approach doesn't add a new definition of operation. However, it requires TenantOperation.LIST_NAMESPACES to run the operation. Therefore, I think it is one of the breaking changes. In addition, validate tenant admin many times on the default authz provider.

Another approach is using only TenantOperation.LIST_NAMESPACES. It validates tenant admin single times on the default authz provider. However, it also doesn't follow existing behavior and doesn't support granularity.

equanz · 2022-02-07T07:15:40Z

@codelipenghui Could you check these comments please?

github-actions · 2022-03-10T02:00:30Z

The pr had no activity for 30 days, mark with Stale label.

github-actions · 2022-06-10T02:14:45Z

The pr had no activity for 30 days, mark with Stale label.

github-actions bot assigned equanz Jan 12, 2022

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 12, 2022

github-actions bot added the lifecycle/stale label Mar 10, 2022

equanz force-pushed the fix_get_anti_affinity_validation branch from e8175c7 to db1d6f3 Compare April 1, 2022 03:11

github-actions bot removed the lifecycle/stale label Apr 2, 2022

equanz force-pushed the fix_get_anti_affinity_validation branch from db1d6f3 to 9adf620 Compare April 7, 2022 02:23

equanz changed the title ~~Add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES and fix validation of getAntiAffinityNamespaces~~ Add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES and fix validation of getAntiAffinityNamespaces to avoid NPE Apr 7, 2022

equanz changed the title ~~Add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES and fix validation of getAntiAffinityNamespaces to avoid NPE~~ Add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES and fix validation of getAntiAffinityNamespaces Apr 7, 2022

equanz force-pushed the fix_get_anti_affinity_validation branch from 9adf620 to 97a49b1 Compare April 19, 2022 09:16

equanz force-pushed the fix_get_anti_affinity_validation branch from 97a49b1 to 0dc11ae Compare April 27, 2022 00:29

fix: add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES

84300ce

equanz force-pushed the fix_get_anti_affinity_validation branch from 0dc11ae to 84300ce Compare May 10, 2022 03:49

github-actions bot added the Stale label Jun 10, 2022

Technoboy- added this to the 3.2.0 milestone Jul 31, 2023

Technoboy- modified the milestones: 3.2.0, 3.3.0 Dec 22, 2023

coderzc modified the milestones: 3.3.0, 3.4.0 May 8, 2024

lhotari modified the milestones: 4.0.0, 4.1.0 Oct 14, 2024

coderzc modified the milestones: 4.1.0, 4.2.0 Sep 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES and fix validation of getAntiAffinityNamespaces #13723

Add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES and fix validation of getAntiAffinityNamespaces #13723

Uh oh!

equanz commented Jan 12, 2022 •

edited

Loading

Uh oh!

codelipenghui commented Jan 12, 2022

Uh oh!

equanz commented Jan 25, 2022 •

edited

Loading

Uh oh!

equanz commented Jan 27, 2022 •

edited

Loading

Uh oh!

equanz commented Feb 7, 2022

Uh oh!

github-actions bot commented Mar 10, 2022

Uh oh!

github-actions bot commented Jun 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES and fix validation of getAntiAffinityNamespaces #13723

Are you sure you want to change the base?

Add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES and fix validation of getAntiAffinityNamespaces #13723

Uh oh!

Conversation

equanz commented Jan 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

codelipenghui commented Jan 12, 2022

Uh oh!

equanz commented Jan 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

equanz commented Jan 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

equanz commented Feb 7, 2022

Uh oh!

github-actions bot commented Mar 10, 2022

Uh oh!

github-actions bot commented Jun 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

equanz commented Jan 12, 2022 •

edited

Loading

equanz commented Jan 25, 2022 •

edited

Loading

equanz commented Jan 27, 2022 •

edited

Loading