fix(sdk-clients): close AWS SDK clients when creating new copies (#991)

ignore some config change events to prevent us from creating new clients when not needed
aws-greengrass · Jun 4, 2021 · 77596ad · 77596ad
1 parent a6d92d4
commit 77596ad
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 21 deletions.
diff --git a/src/main/java/com/aws/greengrass/componentmanager/plugins/docker/EcrAccessor.java b/src/main/java/com/aws/greengrass/componentmanager/plugins/docker/EcrAccessor.java
@@ -10,7 +10,6 @@
 import com.aws.greengrass.tes.LazyCredentialProvider;
 import com.aws.greengrass.util.Coerce;
 import com.aws.greengrass.util.ProxyUtils;
-import lombok.AllArgsConstructor;
 import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.regions.Region;
 import software.amazon.awssdk.services.ecr.EcrClient;
@@ -26,28 +25,43 @@
 
 /**
  * AWS ECR SDK client wrapper.
- *
  */
-@AllArgsConstructor
 public class EcrAccessor {
-    private EcrClient ecrClient;
+    private final EcrClient injectedClient;
+    private final DeviceConfiguration deviceConfiguration;
+    private final LazyCredentialProvider lazyCredentialProvider;
 
     /**
      * Constructor.
      *
-     * @param deviceConfiguration Device config
+     * @param deviceConfiguration    Device config
      * @param lazyCredentialProvider AWS credentials provider
      */
     @Inject
+    @SuppressWarnings("PMD.NullAssignment")
     public EcrAccessor(DeviceConfiguration deviceConfiguration, LazyCredentialProvider lazyCredentialProvider) {
-        configureClient(deviceConfiguration, lazyCredentialProvider);
-        deviceConfiguration.getAWSRegion()
-                .subscribe((what, node) -> configureClient(deviceConfiguration, lazyCredentialProvider));
+        this.deviceConfiguration = deviceConfiguration;
+        this.lazyCredentialProvider = lazyCredentialProvider;
+        this.injectedClient = null;
     }
 
-    private void configureClient(DeviceConfiguration deviceConfiguration,
-                                 LazyCredentialProvider lazyCredentialProvider) {
-        this.ecrClient = EcrClient.builder().httpClient(ProxyUtils.getSdkHttpClient())
+    /**
+     * Constructor for testing with a mocked client.
+     *
+     * @param client EcrClient
+     */
+    @SuppressWarnings("PMD.NullAssignment")
+    public EcrAccessor(EcrClient client) {
+        this.injectedClient = client;
+        this.deviceConfiguration = null;
+        this.lazyCredentialProvider = null;
+    }
+
+    private EcrClient getClient() {
+        if (injectedClient != null) {
+            return injectedClient;
+        }
+        return EcrClient.builder().httpClient(ProxyUtils.getSdkHttpClient())
                 .region(Region.of(Coerce.toString(deviceConfiguration.getAWSRegion())))
                 .credentialsProvider(lazyCredentialProvider).build();
     }
@@ -61,22 +75,20 @@ private void configureClient(DeviceConfiguration deviceConfiguration,
      */
     @SuppressWarnings("PMD.AvoidRethrowingException")
     public Registry.Credentials getCredentials(String registryId) throws RegistryAuthException {
-        try {
-            AuthorizationData authorizationData = ecrClient.getAuthorizationToken(
+        try (EcrClient client = getClient()) {
+            AuthorizationData authorizationData = client.getAuthorizationToken(
                     GetAuthorizationTokenRequest.builder().registryIds(Collections.singletonList(registryId)).build())
                     .authorizationData().get(0);
             // Decoded auth token is of the format <username>:<password>
-            String[] authTokenParts =
-                    new String(Base64.getDecoder().decode(authorizationData.authorizationToken()),
-                            StandardCharsets.UTF_8).split(":");
-            return new Registry.Credentials(authTokenParts[0], authTokenParts[1],
-                    authorizationData.expiresAt());
+            String[] authTokenParts = new String(Base64.getDecoder().decode(authorizationData.authorizationToken()),
+                    StandardCharsets.UTF_8).split(":");
+            return new Registry.Credentials(authTokenParts[0], authTokenParts[1], authorizationData.expiresAt());
         } catch (ServerException | SdkClientException e) {
             // Errors we can retry on
             throw e;
         } catch (EcrException e) {
-            throw new RegistryAuthException(String.format("Failed to get credentials for ECR registry - %s",
-                    registryId), e);
+            throw new RegistryAuthException(
+                    String.format("Failed to get credentials for ECR registry - %s", registryId), e);
         }
     }
 }
diff --git a/src/main/java/com/aws/greengrass/util/GreengrassServiceClientFactory.java b/src/main/java/com/aws/greengrass/util/GreengrassServiceClientFactory.java
@@ -7,6 +7,7 @@
 
 import com.aws.greengrass.componentmanager.ClientConfigurationUtils;
 import com.aws.greengrass.config.Node;
+import com.aws.greengrass.config.WhatHappened;
 import com.aws.greengrass.deployment.DeviceConfiguration;
 import com.aws.greengrass.deployment.exceptions.DeviceConfigurationException;
 import com.aws.greengrass.logging.api.Logger;
@@ -54,6 +55,9 @@ public GreengrassServiceClientFactory(DeviceConfiguration deviceConfiguration) {
         }
         configureClient(deviceConfiguration);
         deviceConfiguration.onAnyChange((what, node) -> {
+            if (WhatHappened.interiorAdded.equals(what) || WhatHappened.timestampUpdated.equals(what)) {
+                return;
+            }
             if (validString(node, DEVICE_PARAM_AWS_REGION) || validPath(node, DEVICE_PARAM_ROOT_CA_PATH) || validPath(
                     node, DEVICE_PARAM_CERTIFICATE_FILE_PATH) || validPath(node, DEVICE_PARAM_PRIVATE_KEY_PATH)
                     || validString(node, DEVICE_PARAM_GG_DATA_PLANE_PORT)) {
@@ -70,6 +74,10 @@ private boolean validPath(Node node, String key) {
         return validString(node, key) && Files.exists(Paths.get(key));
     }
 
+    public synchronized GreengrassV2DataClient getGreengrassV2DataClient() {
+        return greengrassV2DataClient;
+    }
+
     private void configureClient(DeviceConfiguration deviceConfiguration) {
 
         ApacheHttpClient.Builder httpClient = ClientConfigurationUtils.getConfiguredClientBuilder(deviceConfiguration);
@@ -98,6 +106,11 @@ private void configureClient(DeviceConfiguration deviceConfiguration) {
                 clientBuilder.region(Region.of(region));
             }
         }
-        this.greengrassV2DataClient = clientBuilder.build();
+        synchronized (this) {
+            if (this.greengrassV2DataClient != null) {
+                this.greengrassV2DataClient.close();
+            }
+            this.greengrassV2DataClient = clientBuilder.build();
+        }
     }
 }