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

Improve the way OIDC init failures are handled #16773

Merged
merged 1 commit into from
Apr 28, 2021

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Apr 23, 2021

Fixes #16725

This PR:

  • restores the OIDC server failing to start if auth-server-url is invalid and logs the message
  • keeps attempting recovering the connection if the URL is valid when the requests arrive and logs the message
  • if auth-server-url is empty and it is a DEV mode - lets server to start - this is typical scenario for starting with dev mode after for ex generating a test project - but fails to start if it is not a DEV mode
  • test showing the recovery in action is added - the mock server is started after the Quarkus endpoint has been initialized
  • updated the retry code to report ConnectException as opposed to IllegalStateException which is reported right now when the retries are exhausted

Going forward we can also do:

  • always fail as it used to be a case - but only if the system property is set
  • let users handle the OIDC startup events - and fail only of the user handler confirms it is needed, etc

@sberyozkin sberyozkin marked this pull request as ready for review April 26, 2021 11:54
@sberyozkin sberyozkin force-pushed the oidc_invalid_auth_server_url branch from 2fcafe7 to 4ae75ad Compare April 26, 2021 11:55
@quarkus-bot
Copy link

quarkus-bot bot commented Apr 26, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 2fcafe7

⚠️ Artifacts of the workflow run were not available thus the report misses some details.

Status Name Step Test failures Logs Raw logs
Initial JDK 11 Build ⚠️ Check → Logs Raw logs
Attach pull request number ⚠️ Check → Logs Raw logs
CI Sanity Check ⚠️ Check → Logs Raw logs
🚫 Devtools Tests - JDK ${{ matrix.java.name }}
🚫 Devtools Tests - JDK 11 Windows
🚫 Gradle Tests - JDK 11 ${{ matrix.os.family }}
🚫 JVM Tests - JDK ${{ matrix.java.name }}
🚫 JVM Tests - JDK 11 Windows
🚫 Maven Tests - JDK ${{ matrix.java.name }}
🚫 Maven Tests - JDK 11 Windows
🚫 MicroProfile TCKs Tests
🚫 Native Tests - ${{ matrix.category }}
Native Tests - Read JSON matrix ⚠️ Check → Logs Raw logs
🚫 Native Tests - Windows - ${{ matrix.category }}

@quarkus-bot
Copy link

quarkus-bot bot commented Apr 26, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 4ae75ad

Status Name Step Test failures Logs Raw logs
JVM Tests - JDK 11 Build Test failures Logs Raw logs
✔️ JVM Tests - JDK 15

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/oidc/deployment

io.quarkus.oidc.test.CodeFlowDevModeDefaultTenantTestCase.testAccessAndRefreshTokenInjectionDevMode - More details - Source on GitHub

io.quarkus.oidc.test.CodeFlowDevModeTestCase.testAccessAndRefreshTokenInjectionDevMode - More details - Source on GitHub

io.quarkus.oidc.test.SecurityDisabledTestCase. - More details - Source on GitHub

@sberyozkin
Copy link
Member Author

There was a Keycloak image pull problem which cause a test failure - but I had to restart

@quarkus-bot
Copy link

quarkus-bot bot commented Apr 26, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 4ae75ad

Status Name Step Test failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 15 Build Test failures Logs Raw logs
Native Tests - Security3 Build ⚠️ Check → Logs Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 15 #

📦 extensions/oidc/deployment

io.quarkus.oidc.test.CodeFlowDevModeDefaultTenantTestCase.testAccessAndRefreshTokenInjectionDevMode - More details - Source on GitHub

io.quarkus.oidc.test.CodeFlowDevModeTestCase.testAccessAndRefreshTokenInjectionDevMode - More details - Source on GitHub

io.quarkus.oidc.test.SecurityDisabledTestCase. - More details - Source on GitHub

@sberyozkin
Copy link
Member Author

Seeing

Failed to execute goal io.fabric8:docker-maven-plugin:0.31.0:start (docker-start) on project quarkus-integration-test-keycloak-authorization: I/O Error: Unable to pull 'quay.io/keycloak/keycloak:12.0.4' from registry 'quay.io' : filesystem layer verification failed for digest sha256:76cc9a281497e32fd06bee0e3bbcb1d9610db913f8811425beb8f491af8af0ac -> [Help 1]

will retry tomorrow

@sberyozkin sberyozkin force-pushed the oidc_invalid_auth_server_url branch from 4ae75ad to 7199cd3 Compare April 27, 2021 09:22
@quarkus-bot
Copy link

quarkus-bot bot commented Apr 27, 2021

This workflow status is outdated as a new workflow run has been triggered.

🚫 This workflow run has been cancelled.

Failing Jobs - Building 7199cd3

Status Name Step Test failures Logs Raw logs
JVM Tests - JDK 11 Build Test failures Logs Raw logs
✔️ JVM Tests - JDK 15
Native Tests - Misc4 ⚠️ Check → Logs Raw logs
Native Tests - Security2 Build Test failures Logs Raw logs
Native Tests - Security3 Build ⚠️ Check → Logs Raw logs
Native Tests - Spring ⚠️ Check → Logs Raw logs
Native Tests - gRPC ⚠️ Check → Logs Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/oidc/deployment

io.quarkus.oidc.test.CodeFlowDevModeDefaultTenantTestCase.testAccessAndRefreshTokenInjectionDevMode - More details - Source on GitHub

io.quarkus.oidc.test.CodeFlowDevModeTestCase.testAccessAndRefreshTokenInjectionDevMode - More details - Source on GitHub

io.quarkus.oidc.test.SecurityDisabledTestCase. - More details - Source on GitHub


⚙️ Native Tests - Security2 #

📦 integration-tests/oidc

io.quarkus.it.keycloak.BearerTokenAuthorizationInGraalITCase.testAccessAdminResourceCustomHeaderNoBearerScheme - More details - Source on GitHub

@sberyozkin sberyozkin force-pushed the oidc_invalid_auth_server_url branch from 7199cd3 to cee4dde Compare April 27, 2021 14:39
@sberyozkin sberyozkin force-pushed the oidc_invalid_auth_server_url branch from cee4dde to eb8dc43 Compare April 28, 2021 14:19
@sberyozkin
Copy link
Member Author

Hi @pedroigor oh thanks :-), was just about to update you I've added a config property :-)
cheers

@sberyozkin
Copy link
Member Author

Adding a backport label given that before 1.13.1 it was failing immediately in case of the invalid/badly structured URL which this PR restores - but it is probably not critical

@sberyozkin sberyozkin merged commit e1c5136 into quarkusio:main Apr 28, 2021
@quarkus-bot quarkus-bot bot added this to the 2.0 - main milestone Apr 28, 2021
@sberyozkin sberyozkin deleted the oidc_invalid_auth_server_url branch April 28, 2021 17:37
@gsmet gsmet modified the milestones: 2.0.0.Alpha2, 1.13.4.Final May 10, 2021
@gsmet
Copy link
Member

gsmet commented May 10, 2021

This one cannot be backported because you remove extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/KeycloakDevModeRealmResourceManager.java which is used by some tests in 1.13.

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.

Oidc doesn't recover when auth-server was not avaliable at startup
3 participants