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

[fix][test] Replace calls to Auth0 with calls to wiremock #20500

Merged
merged 5 commits into from
Jun 6, 2023

Conversation

michaeljmarshall
Copy link
Member

Fixes #20473

Motivation

Our tests should be independent of external services like Auth0. We can use WireMock to create a server that responds to all OAuth2 requests. This PR removes calls to Auth0.com and replaces them with calls to a wiremock server.

Modifications

  • Add a generic class to create an OIDC/OAuth2 wiremock server that can be used to get tokens.

Verifying this change

Tests are modified and still pass.

Documentation

  • doc-not-needed

@codecov-commenter
Copy link

Codecov Report

Merging #20500 (349ab48) into master (3b862ae) will increase coverage by 36.19%.
The diff coverage is 50.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #20500       +/-   ##
=============================================
+ Coverage     36.78%   72.98%   +36.19%     
- Complexity    12059    31941    +19882     
=============================================
  Files          1690     1867      +177     
  Lines        129001   138575     +9574     
  Branches      14041    15213     +1172     
=============================================
+ Hits          47453   101133    +53680     
+ Misses        75294    29424    -45870     
- Partials       6254     8018     +1764     
Flag Coverage Δ
inttests 24.16% <0.00%> (+0.02%) ⬆️
systests 25.13% <0.00%> (+0.17%) ⬆️
unittests 72.26% <50.00%> (+40.31%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...s/runtime/kubernetes/KubernetesRuntimeFactory.java 80.21% <50.00%> (+80.21%) ⬆️

... and 1433 files with indirect coverage changes

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good work!

/**
* Mock OIDC (and therefore OAuth2) server for testing. Note that the client_id is mapped to the token's subject claim.
*/
public class MockOIDCIdentityProvider {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be useful to make this implement AutoCloseable so that it can be directly used with Java's try with resources. This would require that the stop method is renamed to "close".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we want to re-create the wiremock server for each test? Or are you thinking of using an annotation to close it when the object is no longer in scope?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we want to re-create the wiremock server for each test? Or are you thinking of using an annotation to close it when the object is no longer in scope?

No. It was a generic comment about resource classes. Supporting AutoCloseable helps since you could use it with try-with-resources if there would be a need to have instances for a short scope. It's not necessary to make this change for this PR.

@lhotari lhotari merged commit da04f24 into apache:master Jun 6, 2023
@michaeljmarshall michaeljmarshall deleted the remove-auth0-calls branch June 6, 2023 05:03
lhotari pushed a commit that referenced this pull request Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Test] Remove Auth0 service dependency from all unit tests
4 participants