Fix S3/Control dualstack config resolution (#5579)

Fix an issue in S3 and S3 Control client's dualstack config resolution
where the default value in S3Configuration and S3ControlConfiguration
prevents the client from correctly falling back to the environment
variable/system property value when the dualstack configuration is not
set on the client or on configuration object.

Author: dagnir

codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/BaseClientBuilderClass.java

@@ -406,7 +406,11 @@ private MethodSpec finalizeServiceConfigurationMethod() {
         builder.addStatement("builder.option($1T.EXECUTION_INTERCEPTORS, interceptors)", SdkClientOption.class);
 
         if (model.getCustomizationConfig().getServiceConfig().hasDualstackProperty()) {
-            builder.addStatement("builder.option($T.DUALSTACK_ENDPOINT_ENABLED, finalServiceConfig.dualstackEnabled())",
+            // NOTE: usage of serviceConfigBuilder and not finalServiceConfig is intentional. We need the nullable boolean here
+            // to ensure fallback resolution of the dualstack configuration if dualstack was not explicitly configured on
+            // serviceConfigBuilder; the service configuration classes (e.g. S3Configuration) return primitive booleans that
+            // have a default when not present.
+            builder.addStatement("builder.option($T.DUALSTACK_ENDPOINT_ENABLED, serviceConfigBuilder.dualstackEnabled())",
                             AwsClientOption.class);
         }
 

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/builder/sra/test-client-builder-class.java

@@ -64,14 +64,14 @@ protected final String serviceName() {
     @Override
     protected final SdkClientConfiguration mergeServiceDefaults(SdkClientConfiguration config) {
         return config.merge(c -> c
-                .option(SdkClientOption.ENDPOINT_PROVIDER, defaultEndpointProvider())
-                .option(SdkClientOption.AUTH_SCHEME_PROVIDER, defaultAuthSchemeProvider())
-                .option(SdkClientOption.AUTH_SCHEMES, authSchemes())
-                .option(SdkClientOption.CRC32_FROM_COMPRESSED_DATA_ENABLED, false)
-                .option(SdkClientOption.SERVICE_CONFIGURATION, ServiceConfiguration.builder().build())
-                .lazyOption(AwsClientOption.TOKEN_PROVIDER,
+            .option(SdkClientOption.ENDPOINT_PROVIDER, defaultEndpointProvider())
+            .option(SdkClientOption.AUTH_SCHEME_PROVIDER, defaultAuthSchemeProvider())
+            .option(SdkClientOption.AUTH_SCHEMES, authSchemes())
+            .option(SdkClientOption.CRC32_FROM_COMPRESSED_DATA_ENABLED, false)
+            .option(SdkClientOption.SERVICE_CONFIGURATION, ServiceConfiguration.builder().build())
+            .lazyOption(AwsClientOption.TOKEN_PROVIDER,
                         p -> TokenUtils.toSdkTokenProvider(p.get(AwsClientOption.TOKEN_IDENTITY_PROVIDER)))
-                .option(AwsClientOption.TOKEN_IDENTITY_PROVIDER, defaultTokenProvider()));
+            .option(AwsClientOption.TOKEN_IDENTITY_PROVIDER, defaultTokenProvider()));
     }
 
     @Override
@@ -82,64 +82,64 @@ protected final SdkClientConfiguration finalizeServiceConfiguration(SdkClientCon
         endpointInterceptors.add(new JsonRequestSetEndpointInterceptor());
         ClasspathInterceptorChainFactory interceptorFactory = new ClasspathInterceptorChainFactory();
         List<ExecutionInterceptor> interceptors = interceptorFactory
-                .getInterceptors("software/amazon/awssdk/services/json/execution.interceptors");
+            .getInterceptors("software/amazon/awssdk/services/json/execution.interceptors");
         List<ExecutionInterceptor> additionalInterceptors = new ArrayList<>();
         interceptors = CollectionUtils.mergeLists(endpointInterceptors, interceptors);
         interceptors = CollectionUtils.mergeLists(interceptors, additionalInterceptors);
         interceptors = CollectionUtils.mergeLists(interceptors, config.option(SdkClientOption.EXECUTION_INTERCEPTORS));
         ServiceConfiguration.Builder serviceConfigBuilder = ((ServiceConfiguration) config
-                .option(SdkClientOption.SERVICE_CONFIGURATION)).toBuilder();
+            .option(SdkClientOption.SERVICE_CONFIGURATION)).toBuilder();
         serviceConfigBuilder.profileFile(serviceConfigBuilder.profileFileSupplier() != null ? serviceConfigBuilder
-                .profileFileSupplier() : config.option(SdkClientOption.PROFILE_FILE_SUPPLIER));
+            .profileFileSupplier() : config.option(SdkClientOption.PROFILE_FILE_SUPPLIER));
         serviceConfigBuilder.profileName(serviceConfigBuilder.profileName() != null ? serviceConfigBuilder.profileName() : config
-                .option(SdkClientOption.PROFILE_NAME));
+            .option(SdkClientOption.PROFILE_NAME));
         if (serviceConfigBuilder.dualstackEnabled() != null) {
             Validate.validState(
-                    config.option(AwsClientOption.DUALSTACK_ENDPOINT_ENABLED) == null,
-                    "Dualstack has been configured on both ServiceConfiguration and the client/global level. Please limit dualstack configuration to one location.");
+                config.option(AwsClientOption.DUALSTACK_ENDPOINT_ENABLED) == null,
+                "Dualstack has been configured on both ServiceConfiguration and the client/global level. Please limit dualstack configuration to one location.");
         } else {
             serviceConfigBuilder.dualstackEnabled(config.option(AwsClientOption.DUALSTACK_ENDPOINT_ENABLED));
         }
         if (serviceConfigBuilder.fipsModeEnabled() != null) {
             Validate.validState(
-                    config.option(AwsClientOption.FIPS_ENDPOINT_ENABLED) == null,
-                    "Fips has been configured on both ServiceConfiguration and the client/global level. Please limit fips configuration to one location.");
+                config.option(AwsClientOption.FIPS_ENDPOINT_ENABLED) == null,
+                "Fips has been configured on both ServiceConfiguration and the client/global level. Please limit fips configuration to one location.");
         } else {
             serviceConfigBuilder.fipsModeEnabled(config.option(AwsClientOption.FIPS_ENDPOINT_ENABLED));
         }
         if (serviceConfigBuilder.useArnRegionEnabled() != null) {
             Validate.validState(
-                    clientContextParams.get(JsonClientContextParams.USE_ARN_REGION) == null,
-                    "UseArnRegion has been configured on both ServiceConfiguration and the client/global level. Please limit UseArnRegion configuration to one location.");
+                clientContextParams.get(JsonClientContextParams.USE_ARN_REGION) == null,
+                "UseArnRegion has been configured on both ServiceConfiguration and the client/global level. Please limit UseArnRegion configuration to one location.");
         } else {
             serviceConfigBuilder.useArnRegionEnabled(clientContextParams.get(JsonClientContextParams.USE_ARN_REGION));
         }
         if (serviceConfigBuilder.multiRegionEnabled() != null) {
             Validate.validState(
-                    clientContextParams.get(JsonClientContextParams.DISABLE_MULTI_REGION_ACCESS_POINTS) == null,
-                    "DisableMultiRegionAccessPoints has been configured on both ServiceConfiguration and the client/global level. Please limit DisableMultiRegionAccessPoints configuration to one location.");
+                clientContextParams.get(JsonClientContextParams.DISABLE_MULTI_REGION_ACCESS_POINTS) == null,
+                "DisableMultiRegionAccessPoints has been configured on both ServiceConfiguration and the client/global level. Please limit DisableMultiRegionAccessPoints configuration to one location.");
         } else if (clientContextParams.get(JsonClientContextParams.DISABLE_MULTI_REGION_ACCESS_POINTS) != null) {
             serviceConfigBuilder.multiRegionEnabled(!clientContextParams
-                    .get(JsonClientContextParams.DISABLE_MULTI_REGION_ACCESS_POINTS));
+                .get(JsonClientContextParams.DISABLE_MULTI_REGION_ACCESS_POINTS));
         }
         if (serviceConfigBuilder.pathStyleAccessEnabled() != null) {
             Validate.validState(
-                    clientContextParams.get(JsonClientContextParams.FORCE_PATH_STYLE) == null,
-                    "ForcePathStyle has been configured on both ServiceConfiguration and the client/global level. Please limit ForcePathStyle configuration to one location.");
+                clientContextParams.get(JsonClientContextParams.FORCE_PATH_STYLE) == null,
+                "ForcePathStyle has been configured on both ServiceConfiguration and the client/global level. Please limit ForcePathStyle configuration to one location.");
         } else {
             serviceConfigBuilder.pathStyleAccessEnabled(clientContextParams.get(JsonClientContextParams.FORCE_PATH_STYLE));
         }
         if (serviceConfigBuilder.accelerateModeEnabled() != null) {
             Validate.validState(
-                    clientContextParams.get(JsonClientContextParams.ACCELERATE) == null,
-                    "Accelerate has been configured on both ServiceConfiguration and the client/global level. Please limit Accelerate configuration to one location.");
+                clientContextParams.get(JsonClientContextParams.ACCELERATE) == null,
+                "Accelerate has been configured on both ServiceConfiguration and the client/global level. Please limit Accelerate configuration to one location.");
         } else {
             serviceConfigBuilder.accelerateModeEnabled(clientContextParams.get(JsonClientContextParams.ACCELERATE));
         }
         ServiceConfiguration finalServiceConfig = serviceConfigBuilder.build();
         clientContextParams.put(JsonClientContextParams.USE_ARN_REGION, finalServiceConfig.useArnRegionEnabled());
         clientContextParams.put(JsonClientContextParams.DISABLE_MULTI_REGION_ACCESS_POINTS,
-                !finalServiceConfig.multiRegionEnabled());
+                                !finalServiceConfig.multiRegionEnabled());
         clientContextParams.put(JsonClientContextParams.FORCE_PATH_STYLE, finalServiceConfig.pathStyleAccessEnabled());
         clientContextParams.put(JsonClientContextParams.ACCELERATE, finalServiceConfig.accelerateModeEnabled());
         SdkClientConfiguration.Builder builder = config.toBuilder();
@@ -156,7 +156,7 @@ protected final SdkClientConfiguration finalizeServiceConfiguration(SdkClientCon
             return result.build();
         });
         builder.option(SdkClientOption.EXECUTION_INTERCEPTORS, interceptors);
-        builder.option(AwsClientOption.DUALSTACK_ENDPOINT_ENABLED, finalServiceConfig.dualstackEnabled());
+        builder.option(AwsClientOption.DUALSTACK_ENDPOINT_ENABLED, serviceConfigBuilder.dualstackEnabled());
         builder.option(AwsClientOption.FIPS_ENDPOINT_ENABLED, finalServiceConfig.fipsModeEnabled());
         builder.option(SdkClientOption.RETRY_STRATEGY, MyServiceRetryStrategy.resolveRetryStrategy(config));
         if (builder.option(SdkClientOption.RETRY_STRATEGY) == null) {
@@ -270,6 +270,6 @@ private List<SdkPlugin> internalPlugins(SdkClientConfiguration config) {
 
     protected static void validateClientOptions(SdkClientConfiguration c) {
         Validate.notNull(c.option(AwsClientOption.TOKEN_IDENTITY_PROVIDER),
-                "The 'tokenProvider' must be configured in the client builder.");
+                         "The 'tokenProvider' must be configured in the client builder.");
     }
 }