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.

Sign up for GitHub

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

Jump to bottom

Support non-proxy hosts in REST Client Reactive #22649

Merged
merged 1 commit into from
Jan 5, 2022
Merged

Support non-proxy hosts in REST Client Reactive #22649

merged 1 commit into from
Jan 5, 2022

Conversation

michalszynkiewicz
Copy link
Member

No description provided.

gsmet
gsmet reviewed Jan 5, 2022
View reviewed changes
.../client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/ClientBuilderImpl.java
if (nonProxyHosts != null) {
for (String host : nonProxyHosts.split(PIPE)) {
if (!host.isBlank()) {
options.addNonProxyHost(host);
Copy link
Member

Choose a reason for hiding this comment

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

Does it support the format for the JVM nonProxyHosts (e.g. wildcards)?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

Copy link
Member

Choose a reason for hiding this comment

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

Cool, thanks for confirming.

BTW, I insisted on it because it's very common in enterprise setups to have a proxy for external websites and to have to avoid it for internal sites. And that could happen when using the REST client.

Copy link
Member Author

Choose a reason for hiding this comment

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

here's an example from vertx pr that introduced it: eclipse-vertx/vert.x@1e95256#diff-4d9e05c07eed6e23cc3e1e3e32861b7689d8334e963d6bedb7531c00c3853c6aR1031

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool, thanks for confirming.

BTW, I insisted on it because it's very common in enterprise setups to have a proxy for external websites and to have to avoid it for internal sites. And that could happen when using the REST client.

sure. I missed it in the original PR, it wasn't omitted intentionally

@quarkus-bot
Copy link

quarkus-bot bot commented Jan 5, 2022 •
edited
Loading

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

Failing Jobs - Building d66bfd0

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

Failures

⚙️ Initial JDK 11 Build #

- Failing: extensions/resteasy-reactive/rest-client-reactive/runtime 
! Skipped: devtools/bom-descriptor-json docs extensions/oidc-client-reactive-filter/deployment and 16 more

📦 extensions/resteasy-reactive/rest-client-reactive/runtime

✖ Failed to execute goal net.revelc.code.formatter:formatter-maven-plugin:2.17.1:validate (default) on project quarkus-rest-client-reactive: File '/home/runner/work/quarkus/quarkus/extensions/resteasy-reactive/rest-client-reactive/runtime/src/main/java/io/quarkus/rest/client/reactive/runtime/RestClientBuilderImpl.java' has not been previously formatted. Please format file and commit before running validation!

@michalszynkiewicz michalszynkiewicz force-pushed the rest-client-reactive-no-proxy-hosts branch from d66bfd0 to a0d6291 Compare January 5, 2022 13:05
geoand
geoand approved these changes Jan 5, 2022
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

LGTM

@quarkus-bot
Copy link

quarkus-bot bot commented Jan 5, 2022 •
edited
Loading

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

Failing Jobs - Building a0d6291

Status Name Step Failures Logs Raw logs
✖ JVM Tests - JDK 11 Build Failures Logs Raw logs
✖ JVM Tests - JDK 11 Windows Build Failures Logs Raw logs
✖ JVM Tests - JDK 17 Build Failures Logs Raw logs
⌛ Native Tests - Security2 Build ⚠️ Check → Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: extensions/resteasy-reactive/rest-client-reactive/runtime 
! Skipped: extensions/oidc-client-reactive-filter/deployment extensions/oidc-client-reactive-filter/runtime extensions/resteasy-reactive/rest-client-reactive-jackson/deployment and 12 more

📦 extensions/resteasy-reactive/rest-client-reactive/runtime

✖ io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilderTest.testQuarkusConfig line 79 - More details - Source on GitHub

java.lang.NullPointerException
	at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.configureProxy(RestClientCDIDelegateBuilder.java:129)
	at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.build(RestClientCDIDelegateBuilder.java:74)

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/resteasy-reactive/rest-client-reactive/runtime 
! Skipped: extensions/oidc-client-reactive-filter/deployment extensions/oidc-client-reactive-filter/runtime extensions/resteasy-reactive/rest-client-reactive-jackson/deployment and 12 more

📦 extensions/resteasy-reactive/rest-client-reactive/runtime

✖ io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilderTest.testQuarkusConfig line 79 - More details - Source on GitHub

java.lang.NullPointerException
	at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.configureProxy(RestClientCDIDelegateBuilder.java:129)
	at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.build(RestClientCDIDelegateBuilder.java:74)

⚙️ JVM Tests - JDK 17 #

- Failing: extensions/resteasy-reactive/rest-client-reactive/runtime 
! Skipped: extensions/oidc-client-reactive-filter/deployment extensions/oidc-client-reactive-filter/runtime extensions/resteasy-reactive/rest-client-reactive-jackson/deployment and 12 more

📦 extensions/resteasy-reactive/rest-client-reactive/runtime

✖ io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilderTest.testQuarkusConfig line 79 - More details - Source on GitHub

java.lang.NullPointerException: Cannot invoke "java.util.Optional.ifPresent(java.util.function.Consumer)" because the return value of "io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.oneOf(java.util.Optional, java.util.Optional)" is null
	at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.configureProxy(RestClientCDIDelegateBuilder.java:129)
	at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.build(RestClientCDIDelegateBuilder.java:74)

@michalszynkiewicz michalszynkiewicz force-pushed the rest-client-reactive-no-proxy-hosts branch from a0d6291 to 6de74ac Compare January 5, 2022 15:40
@michalszynkiewicz michalszynkiewicz merged commit 99049e5 into quarkusio:main Jan 5, 2022
@quarkus-bot quarkus-bot bot added this to the 2.7 - main milestone Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsmet gsmet gsmet left review comments

@geoand geoand geoand approved these changes

Assignees
No one assigned
Labels
area/documentation area/rest
Projects
None yet
Milestone
2.7.0.CR1
Development

Successfully merging this pull request may close these issues.
3 participants
@michalszynkiewicz @gsmet @geoand