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

[Test] Remove Auth0 service dependency from all unit tests #20473

Closed
lhotari opened this issue Jun 2, 2023 · 1 comment · Fixed by #20500
Closed

[Test] Remove Auth0 service dependency from all unit tests #20473

lhotari opened this issue Jun 2, 2023 · 1 comment · Fixed by #20500

Comments

@lhotari
Copy link
Member

lhotari commented Jun 2, 2023

Description

Auth0 service dependency in TokenOauth2AuthenticatedProducerConsumerTest was removed in #20465.

It turns out that there is an Auth0 service dependency also in these 2 locations:

proxyConfig.setBrokerClientAuthenticationParameters("{\"grant_type\":\"client_credentials\"," +
" \"issuerUrl\":\"https://dev-kt-aa9ne.us.auth0.com\"," +
" \"audience\": \"https://dev-kt-aa9ne.us.auth0.com/api/v2/\"," +
" \"privateKey\":\"file://" + tempFile.getAbsolutePath() + "\"}");

params.put("issuerUrl", new URL("https://dev-kt-aa9ne.us.auth0.com"));
params.put("privateKey", path.toUri().toURL());
params.put("audience", "https://dev-kt-aa9ne.us.auth0.com/api/v2/");

These should also be covered.

An additional detail is that it would be better to change the OAuth2 audience claim https://dev-kt-aa9ne.us.auth0.com/api/v2/ in TokenOauth2AuthenticatedProducerConsumerTest during this change since that could cause confusion for future maintainers.

Implementation hints

It's possible to share test classes via pulsar-broker test-jar dependency:

        <dependency>
          <groupId>${project.groupId}</groupId>
          <artifactId>pulsar-broker</artifactId>
          <version>${project.version}</version>
          <scope>test</scope>
          <type>test-jar</type>
        </dependency>

This is already specified in pulsar-proxy/pom.xml and pulsar-testclient/pom.xml

It would be possible to extract some test classes from TokenOauth2AuthenticatedProducerConsumerTest for Wiremocking OAuth and put the classes under some package in pulsar-broker/src/test/java/org/apache/pulsar . This would be a way to reduce code duplication across tests where similar logic is needed.

@michaeljmarshall
Copy link
Member

Great points @lhotari. I can follow up on this tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants