Update azure service proxy configuration #24810

moarychan · 2021-10-15T01:50:58Z

Update proxy configuration for Azure service client builder:

HTTP-based Service builder proxy
AMQP-based service builder proxy
Cosmos Service builder proxy

...nfigure/src/main/java/com/azure/spring/cloud/autoconfigure/cosmos/AzureCosmosProperties.java

saragluna · 2021-10-15T06:25:34Z

...nfigure/src/main/java/com/azure/spring/cloud/autoconfigure/cosmos/AzureCosmosProperties.java

@@ -27,6 +30,9 @@

    public static final String PREFIX = "spring.cloud.azure.cosmos";

+    @NestedConfigurationProperty
+    protected final ProxyProperties proxy = new HttpProxyProperties();


why protected?

Will change to private.

saragluna · 2021-10-15T06:30:02Z

...om/azure/spring/cloud/autoconfigure/properties/AbstractAzureHttpConfigurationProperties.java

    @Override
    public HttpClientProperties getClient() {
        return client;
    }
+
+    @Override
+    public ProxyProperties getProxy() {


should return HttpProxyProperties here

...-core/src/main/java/com/azure/spring/core/factory/AbstractAzureAmqpClientBuilderFactory.java

saragluna · 2021-10-15T14:32:56Z

...-core/src/main/java/com/azure/spring/core/factory/AbstractAzureHttpClientBuilderFactory.java

+            if (proxyOptions != null) {
+                this.httpClientOptions.setProxyOptions(convertToProxyOptions(proxyProperties));
+            } else {
+                LOGGER.warn("Invalid http proxyProperties configuration properties.");


same here, will setting null throw exceptions?

The same with AMQP factory.
There are some properties class attribute value null check, should we not judge the null situation, such as client, proxy, retry properties

Should not use warning here

saragluna · 2021-10-15T14:33:13Z

...-core/src/main/java/com/azure/spring/core/factory/AbstractAzureHttpClientBuilderFactory.java

        }
    }

+    protected ProxyOptions convertToProxyOptions(ProxyProperties proxyProperties) {


why do we need this

Is to allow a subclass to override this method so that a mock HttpClientOptions can be verified. I am looking for a better way.

…re-sdk-for-java into feature-update/azure-service-proxy-configuration

check-enforcer · 2021-10-20T00:56:01Z

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

What if I am onboarding a new service?

Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment:
/azp run prepare-pipelines
This will run a pipeline that analyzes the source tree and creates the pipelines necessary to build and validate your pull request. Once the pipeline has been created you can trigger the pipeline using the following comment:
/azp run java - [service] - ci

saragluna · 2021-10-22T03:16:25Z

...azure-core/src/main/java/com/azure/spring/core/converter/AzureHttpProxyOptionsConverter.java

@@ -34,8 +35,12 @@ public ProxyOptions convert(ProxyProperties proxyProperties) {
        if (StringUtils.hasText(proxyProperties.getUsername()) && StringUtils.hasText(proxyProperties.getPassword())) {
            proxyOptions.setCredentials(proxyProperties.getUsername(), proxyProperties.getPassword());
        }
-        // TODO (xiada) non proxy hosts
+        if (proxyProperties instanceof HttpProxyProperties) {


If this class is called AzureHttpProxyOptionsConverter.java, should we change the method to pass in a HttpProxyProperties?

saragluna · 2021-10-22T03:16:59Z

...-core/src/main/java/com/azure/spring/core/factory/AbstractAzureAmqpClientBuilderFactory.java

+        if (proxyOptions != null) {
+            consumeProxyOptions().accept(builder, proxyOptions);
+        } else {
+            LOGGER.warn("Invalid AMQP proxy configuration properties.");


We shouldn't add waning here, it's okay not to set proxy.

...-core/src/main/java/com/azure/spring/core/factory/AbstractAzureAmqpClientBuilderFactory.java

saragluna · 2021-10-22T03:18:07Z

...-core/src/main/java/com/azure/spring/core/factory/AbstractAzureHttpClientBuilderFactory.java

+        if (proxyProperties != null) {
+            ProxyOptions proxyOptions = proxyOptionsConverter.convert(proxyProperties);
+            if (proxyOptions != null) {
+                this.httpClientOptions.setProxyOptions(proxyOptionsConverter.convert(proxyProperties));


why call proxyOptionsConverter.convert(proxyProperties) twice?

saragluna · 2021-10-22T03:18:23Z

...-core/src/main/java/com/azure/spring/core/factory/AbstractAzureHttpClientBuilderFactory.java

+            if (proxyOptions != null) {
+                this.httpClientOptions.setProxyOptions(convertToProxyOptions(proxyProperties));
+            } else {
+                LOGGER.warn("Invalid http proxyProperties configuration properties.");


Should not use warning here

...oud-azure-core/src/main/java/com/azure/spring/core/properties/proxy/HttpProxyProperties.java

saragluna · 2021-10-22T03:19:24Z

...-azure-service/src/main/java/com/azure/spring/service/cosmos/CosmosClientBuilderFactory.java

-            builder.gatewayMode(this.cosmosProperties.getGatewayConnection());
-        } else if (ConnectionMode.DIRECT.equals(this.cosmosProperties.getConnectionMode())) {
-            // TODO (xiada): public CosmosClientBuilder directMode(DirectConnectionConfig directConnectionConfig, GatewayConnectionConfig gatewayConnectionConfig) {
+        builder.gatewayMode(this.cosmosProperties.getGatewayConnection());


why not add

if (ConnectionMode.GATEWAY.equals(this.cosmosProperties.getConnectionMode())) {
}

here

The direct mode will also use the proxy configuration which is from the gateconnection configuration, then the gateconnection will have the high priority to apply.

…re-sdk-for-java into feature-update/azure-service-proxy-configuration

saragluna · 2021-10-28T07:13:55Z

...-core/src/main/java/com/azure/spring/core/factory/AbstractAzureHttpClientBuilderFactory.java

-        if (proxy != null) {
-            this.httpClientOptions.setProxyOptions(proxyOptionsConverter.convert(proxy));
+        final ProxyProperties proxyProperties = getAzureProperties().getProxy();
+        if (proxyProperties != null && proxyProperties instanceof HttpProxyProperties) {


Should we warn here if the proxy properties is not of type HttpProxyProperties?

…thod to apply direct mode connection

saragluna

LGTM.

...nfigure/src/main/java/com/azure/spring/cloud/autoconfigure/cosmos/AzureCosmosProperties.java

saragluna · 2021-10-28T08:31:48Z

/azp run java - spring - tests

azure-pipelines · 2021-10-28T08:32:02Z

Azure Pipelines successfully started running 1 pipeline(s).

ghost added the azure-spring All azure-spring related issues label Oct 15, 2021

saragluna force-pushed the feature/azure-spring-cloud-4.0 branch from cc863ca to a7cc5ac Compare October 15, 2021 10:03

saragluna requested changes Oct 15, 2021

View reviewed changes

moarychan force-pushed the feature-update/azure-service-proxy-configuration branch from 3255e2f to 73ea443 Compare October 18, 2021 13:22

update azure proxy configuration

ef99985

moarychan force-pushed the feature-update/azure-service-proxy-configuration branch from 73ea443 to ef99985 Compare October 19, 2021 00:50

moarychan linked an issue Oct 19, 2021 that may be closed by this pull request

Support configure proxy #21552

Closed

saragluna self-assigned this Oct 19, 2021

Moary Chen added 2 commits October 20, 2021 08:45

fix comments and update testcases

17aa6f5

Merge branch 'feature/azure-spring-cloud-4.0' of github.com:Azure/azu…

8cb387b

…re-sdk-for-java into feature-update/azure-service-proxy-configuration

Moary Chen added 2 commits October 21, 2021 14:41

Merge 4.0 branch

97b5532

update testcases

e2a0b25

saragluna requested changes Oct 22, 2021

View reviewed changes

Moary Chen added 2 commits October 26, 2021 14:13

Merge branch 'feature/azure-spring-cloud-4.0' of github.com:Azure/azu…

3e994c1

…re-sdk-for-java into feature-update/azure-service-proxy-configuration

fix comments

2af0429

saragluna marked this pull request as ready for review October 28, 2021 07:05

saragluna requested review from backwind1233, chenrujun, stliu, yiliuTo and zhichengliu12581 as code owners October 28, 2021 07:05

saragluna requested changes Oct 28, 2021

View reviewed changes

add warning log if found non-http proxy configuration; use another me…

1a3a0d2

…thod to apply direct mode connection

saragluna approved these changes Oct 28, 2021

View reviewed changes

...nfigure/src/main/java/com/azure/spring/cloud/autoconfigure/cosmos/AzureCosmosProperties.java Show resolved Hide resolved

moarychan merged commit 876e9af into Azure:feature/azure-spring-cloud-4.0 Oct 28, 2021

moarychan deleted the feature-update/azure-service-proxy-configuration branch November 17, 2021 01:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update azure service proxy configuration #24810

Update azure service proxy configuration #24810

moarychan commented Oct 15, 2021

saragluna Oct 15, 2021

moarychan Oct 18, 2021

saragluna Oct 15, 2021

moarychan Oct 18, 2021

saragluna Oct 15, 2021

moarychan Oct 18, 2021 •

edited

Loading

saragluna Oct 22, 2021

saragluna Oct 15, 2021

moarychan Oct 18, 2021

check-enforcer bot commented Oct 20, 2021

saragluna Oct 22, 2021

moarychan Oct 26, 2021

saragluna Oct 22, 2021

saragluna Oct 22, 2021

moarychan Oct 26, 2021

saragluna Oct 22, 2021

saragluna Oct 22, 2021

moarychan Oct 26, 2021

saragluna Oct 28, 2021

saragluna left a comment

saragluna commented Oct 28, 2021

azure-pipelines bot commented Oct 28, 2021

Update azure service proxy configuration #24810

Update azure service proxy configuration #24810

Conversation

moarychan commented Oct 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moarychan Oct 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

check-enforcer bot commented Oct 20, 2021

What is Check Enforcer?

Why am I getting this message?

What should I do now?

What if I am onboarding a new service?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saragluna left a comment

Choose a reason for hiding this comment

saragluna commented Oct 28, 2021

azure-pipelines bot commented Oct 28, 2021

moarychan Oct 18, 2021 •

edited

Loading