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

Read client configuration directly from Config rather than via @ConfigRoot #21530

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

TomasHofman
Copy link
Contributor

@TomasHofman TomasHofman commented Nov 18, 2021

This should make it possible to read REST client configuration from vaults, which currently doesn't work due to the fact that vault config sources do not list their contents when ConfigSource#getPropertyNames() is called. This causes the properties from vaults to not being included in @ConfigRoot instances.

Related to #21348

@TomasHofman
Copy link
Contributor Author

Hello @radcortez @michalszynkiewicz , this is my proposal on how to modify the rest client configuration.

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 18, 2021

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

Failing Jobs - Building 58b3c3b

Status Name Step Failures Logs Raw logs
JVM Tests - JDK 11 Build Failures Logs Raw logs
JVM Tests - JDK 17 Build Failures Logs Raw logs
Native Tests - Cache Build Failures Logs Raw logs
Native Tests - HTTP Build Failures Logs Raw logs
Native Tests - Main Build Failures Logs Raw logs
Native Tests - Misc3 Build Failures Logs Raw logs
Native Tests - Misc4 Build Failures Logs Raw logs
Native Tests - Security2 Build Failures Logs Raw logs
Native Tests - Security3 Build Failures Logs Raw logs

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: extensions/smallrye-reactive-messaging-kafka/deployment integration-tests/mongodb-client 
! Skipped: integration-tests/kafka-oauth-keycloak integration-tests/kafka-sasl-elytron integration-tests/kubernetes/quarkus-standard-way-kafka and 2 more

📦 extensions/smallrye-reactive-messaging-kafka/deployment

io.quarkus.smallrye.reactivemessaging.kafka.deployment.testing.KafkaDevServicesContinuousTestingWorkingAppPropsTestCase.testContinuousTestingScenario3 - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.kafka.client.deployment.DevServicesKafkaProcessor#startKafkaDevService threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed

📦 integration-tests/mongodb-client

io.quarkus.it.mongodb.BookResourceTest.health line 50 - Source on GitHub

java.lang.AssertionError: 
2 expectations failed.
JSON path checks.data doesn't match.

⚙️ JVM Tests - JDK 17 #

- Failing: integration-tests/mongodb-client 

📦 integration-tests/mongodb-client

io.quarkus.it.mongodb.BookResourceTest.health line 50 - Source on GitHub

java.lang.AssertionError: 
2 expectations failed.
JSON path checks.data doesn't match.

⚙️ Native Tests - Cache #

- Failing: integration-tests/cache 

📦 integration-tests/cache

io.quarkus.it.cache.RestClientITCase.test - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

⚙️ Native Tests - HTTP #

- Failing: integration-tests/rest-client 

📦 integration-tests/rest-client

io.quarkus.it.rest.client.MultipartResourceIT.testMultipartDataIsSent - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.rest.client.selfsigned.ExternalSelfSignedITCase.should_accept_self_signed_certs - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.rest.client.selfsigned.ExternalSelfSignedITCase.should_accept_self_signed_certs_java_url - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.rest.client.trustall.ExternalTlsTrustAllIT.restClient - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.rest.client.wronghost.ExternalWrongHostIT.restClient - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

⚙️ Native Tests - Main #

- Failing: integration-tests/main 

📦 integration-tests/main

io.quarkus.it.main.RestClientITCase.testJaxrsClientWithFilters - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.main.RestClientITCase.testMicroprofileClientHeaderPassing - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected header "Content-Type" was not "application/json", was "text/html;charset=UTF-8". Headers are:

io.quarkus.it.main.RestClientITCase.testMicroprofileClientComplexCdi - Source on GitHub

io.restassured.path.json.exception.JsonPathException: Failed to parse the JSON document
	at io.restassured.path.json.JsonPath$ExceptionCatcher.invoke(JsonPath.java:1002)
	at io.restassured.path.json.JsonPath$4.doParseWith(JsonPath.java:967)

io.quarkus.it.main.RestClientITCase.testFaultTolerance - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

io.quarkus.it.main.RestClientITCase.testMicroprofileClientComplex - Source on GitHub

io.restassured.path.json.exception.JsonPathException: Failed to parse the JSON document
	at io.restassured.path.json.JsonPath$ExceptionCatcher.invoke(JsonPath.java:1002)
	at io.restassured.path.json.JsonPath$4.doParseWith(JsonPath.java:967)

io.quarkus.it.main.RestClientITCase.testMicroprofileClientConfigKeyIntegration - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

io.quarkus.it.main.RestClientITCase.testMicroprofileClientCDIIntegration - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

io.quarkus.it.main.RestClientITCase.testMicroprofileClientDataCdi - Source on GitHub

io.restassured.path.json.exception.JsonPathException: Failed to parse the JSON document
	at io.restassured.path.json.JsonPath$ExceptionCatcher.invoke(JsonPath.java:1002)
	at io.restassured.path.json.JsonPath$4.doParseWith(JsonPath.java:967)

io.quarkus.it.main.RestClientITCase.testMicroprofileClientData - Source on GitHub

io.restassured.path.json.exception.JsonPathException: Failed to parse the JSON document
	at io.restassured.path.json.JsonPath$ExceptionCatcher.invoke(JsonPath.java:1002)
	at io.restassured.path.json.JsonPath$4.doParseWith(JsonPath.java:967)

io.quarkus.it.main.RestClientITCase.testMicroprofileAsyncRestClient - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

io.quarkus.it.main.RestClientITCase.testIssue8795 - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

io.quarkus.it.main.RestClientITCase.testMicroprofileClient - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

io.quarkus.it.main.RestClientITCase.testEmojis - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

io.quarkus.it.main.RestClientITCase.testMicroprofileClientBaseUriConfigKeyIntegration - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

io.quarkus.it.main.RestClientITCase.testMicroprofileRestClientDefaultScope - Source on GitHub

org.opentest4j.AssertionFailedError: 
expected: <javax.inject.Singleton> but was: <<!doctype html>
<html lang="en">

io.quarkus.it.main.RestClientITCase.testMicroprofileCdiClientHeaderPassing - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected header "Content-Type" was not "application/json", was "text/html;charset=UTF-8". Headers are:

⚙️ Native Tests - Misc3 #

- Failing: integration-tests/smallrye-opentracing 

📦 integration-tests/smallrye-opentracing

io.quarkus.it.opentracing.OpenTracingITCase.testAsyncClientTracing - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.opentracing.OpenTracingITCase.testClientTracing - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

⚙️ Native Tests - Misc4 #

- Failing: integration-tests/opentelemetry 

📦 integration-tests/opentelemetry

io.quarkus.it.opentelemetry.OpenTelemetryITCase.testEmptyClientPath - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.opentelemetry.OpenTelemetryITCase.testSlashClientPath - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.opentelemetry.OpenTelemetryITCase.testDeepPathNaming - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.opentelemetry.OpenTelemetryITCase.testResourceTracing - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.opentelemetry.OpenTelemetryITCase.testAsyncClientTracing - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.opentelemetry.OpenTelemetryITCase.testClientTracing - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.opentelemetry.OpenTelemetryITCase.testChainedResourceTracing - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.opentelemetry.OpenTelemetryITCase.testPathParameter - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.opentelemetry.OpenTelemetryITCase.testTracingWithParentHeaders - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

⚙️ Native Tests - Security2 #

- Failing: integration-tests/oidc-client integration-tests/oidc-client-wiremock integration-tests/oidc-token-propagation 

📦 integration-tests/oidc-client

io.quarkus.it.keycloak.OidcClientInGraalITCase.testGetUserOidcClientNameAndRefreshTokens - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.keycloak.OidcClientInGraalITCase.testGetUserNameNoOidcClient - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <401> but was <500>.

io.quarkus.it.keycloak.OidcClientInGraalITCase.testGetUserNameOidcClient - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.keycloak.OidcClientInGraalITCase.testGetUserNameRegisterProvider - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

📦 integration-tests/oidc-client-wiremock

io.quarkus.it.keycloak.OidcClientInGraalITCase.testEchoTokensNonStandardResponse - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.keycloak.OidcClientInGraalITCase.testEchoTokensRefreshTokenOnly - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.keycloak.OidcClientInGraalITCase.testEchoTokensNonStandardResponseWithoutHeader - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <401> but was <500>.

io.quarkus.it.keycloak.OidcClientInGraalITCase.testEchoAndRefreshTokens - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

📦 integration-tests/oidc-token-propagation

io.quarkus.it.keycloak.OidcTokenPropagationITCase.testGetUserNameFromServiceAccount - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.keycloak.OidcTokenPropagationITCase.testGetUserNameWithJwtTokenPropagation - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.keycloak.OidcTokenPropagationITCase.testGetUserNameWithAccessTokenPropagation - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

⚙️ Native Tests - Security3 #

- Failing: integration-tests/smallrye-jwt-token-propagation 

📦 integration-tests/smallrye-jwt-token-propagation

io.quarkus.it.keycloak.OidcTokenPropagationInGraalITCase.testGetUserNameWithJwtTokenPropagation - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

io.quarkus.it.keycloak.OidcTokenPropagationInGraalITCase.testGetUserNameWithAccessTokenPropagation - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

@radcortez
Copy link
Member

I guess the CI is not happy :(

@TomasHofman
Copy link
Contributor Author

TomasHofman commented Nov 18, 2021

I guess the CI is not happy :(

Yes, I will be looking into that :).

@TomasHofman
Copy link
Contributor Author

CI looks OK now.

@radcortez radcortez linked an issue Nov 22, 2021 that may be closed by this pull request
@radcortez
Copy link
Member

I guess the only downside is that the config documentation is not going to be auto-generated anymore. Maybe we can keep the @ConfigGroup and @ConfigItems for documentation purposes?

@TomasHofman
Copy link
Contributor Author

I didn't thought of that. I will put the annotations back in (hopefully that's enough for the docs generation to keep working).

@radcortez
Copy link
Member

Since everything is Optional, it should be ok.

@geoand geoand changed the title REST Client config - read client configuration directly from Config Read client configuration directly from Config rather than via @ConfigRoot Nov 22, 2021
@geoand
Copy link
Contributor

geoand commented Nov 22, 2021

@radcortez I'll leave it to you to merge this

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 22, 2021

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

Failing Jobs - Building e7642de

Status Name Step Failures Logs Raw logs
Initial JDK 11 Build Build Failures Logs Raw logs

Failures

⚙️ Initial JDK 11 Build #

- Failing: extensions/resteasy-classic/rest-client/config 
! Skipped: devtools/bom-descriptor-json docs extensions/grpc/deployment and 109 more

📦 extensions/resteasy-classic/rest-client/config

Failed to execute goal net.revelc.code:impsort-maven-plugin:1.6.2:check (check-imports) on project quarkus-rest-client-config: Imports are not sorted in /home/runner/work/quarkus/quarkus/extensions/resteasy-classic/rest-client/config/src/main/java/io/quarkus/restclient/config/RestClientConfig.java

...rather than via `@ConfigRoot`

This should help in situations when REST Client config is obtained from
a vault ConfigSource, which doesn't list it's contents when
`ConfigSource#getPropertyNames()` is called. This causes the
configuration to not be included in the `@ConfigRoot` instance.
@gwenneg
Copy link
Member

gwenneg commented Nov 25, 2021

@gsmet Can this be backported to the next 2.5 release? All our projects are stuck with 2.3.1.Final because of the rest-client issues fixed by this PR.

@TomasHofman
Copy link
Contributor Author

If this is backported, following PR should be backported too: #21709

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment