From 49d71c1ef1ac8b22d5dbc504246daa598c1ffb04 Mon Sep 17 00:00:00 2001 From: Saranya Date: Mon, 8 Jul 2024 17:34:22 -0700 Subject: [PATCH] fix: make sdk changes compatible + reload cache (#89) --- .../secretmanager/SecretManager.java | 41 ++++++++++++++----- .../exception/FileSecretStoreException.java | 7 ++++ .../secretmanager/store/FileSecretStore.java | 34 ++------------- .../secretmanager/store/SecretStore.java | 1 - .../secretmanager/SecretManagerTest.java | 11 +---- 5 files changed, 44 insertions(+), 50 deletions(-) create mode 100644 src/main/java/com/aws/greengrass/secretmanager/exception/FileSecretStoreException.java diff --git a/src/main/java/com/aws/greengrass/secretmanager/SecretManager.java b/src/main/java/com/aws/greengrass/secretmanager/SecretManager.java index 2efa296..68905a9 100644 --- a/src/main/java/com/aws/greengrass/secretmanager/SecretManager.java +++ b/src/main/java/com/aws/greengrass/secretmanager/SecretManager.java @@ -8,6 +8,7 @@ import com.aws.greengrass.config.Topic; import com.aws.greengrass.logging.api.Logger; import com.aws.greengrass.logging.impl.LogManager; +import com.aws.greengrass.secretmanager.exception.FileSecretStoreException; import com.aws.greengrass.secretmanager.exception.SecretCryptoException; import com.aws.greengrass.secretmanager.exception.SecretManagerException; import com.aws.greengrass.secretmanager.exception.v1.GetSecretException; @@ -30,13 +31,13 @@ import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.locks.Lock; import java.util.regex.Pattern; import javax.inject.Inject; @@ -57,6 +58,7 @@ public class SecretManager { "arn:([^:]+):secretsmanager:[a-z0-9\\-]+:[0-9]{12}:secret:([a-zA-Z0-9\\\\]+/)*" + "[a-zA-Z0-9/_+=,.@\\-]+(-[a-zA-Z0-9]+)?"; private static final String secretNotFoundErr = "Secret not found "; + private static final String IPC_REQUEST_REFRESH_FIELD = "refresh"; private final Logger logger = LogManager.getLogger(SecretManager.class); // Cache which holds aws secrets result private final Map cache = new HashMap<>(); @@ -120,12 +122,26 @@ private List getSecretConfiguration() { * When the component is installed, it firsts cleans up/syncs the existing local secrets as per the component * configuration. It then tries to download latest secret from cloud for each configured secret-label. It then * updates the local store with that secret and refreshes the cache. - * + * @throws RuntimeException secret manager exceptions */ public void syncFromCloud() { try (LockScope ls = LockScope.lock(syncFromCloudLock)) { List secretConfiguration = getSecretConfiguration(); localStoreMap.syncWithConfig(secretConfiguration); + try { + reloadCache(); + } catch (SecretManagerException e) { + if (e.getCause() instanceof FileSecretStoreException) { + // Happens when the local store is corrupted. Local secret cache is cleared by the time this + // exception is thrown as it is no longer valid. New secrets will be downloaded as needed. So, just + // log and proceed. + logger.atError().log("Exception occurred while updating the local secret cache."); + } else { + // Should never happen. Throw any unexpected exceptions. + throw new RuntimeException(e); + } + } + for (SecretConfiguration configuredSecret : secretConfiguration) { String arn = configuredSecret.getArn(); if (!Pattern.matches(VALID_SECRET_ARN_PATTERN, arn)) { @@ -141,20 +157,20 @@ public void syncFromCloud() { } /** - * load the secrets from a local store. This is used across restarts to load secrets from store. This method can be - * called from several threads 1. Service thread during start up 2. IPC thread when refresh is set to true. 3. - * Secret manager config subscription thread - syncFromCloud 4. scheduler thread that periodically refreshes secrets - * - syncFromCloud + * Load secrets into cache from the local store. This method can be called from several threads 1. Service thread + * during start up 2. IPC thread when refresh is set to true. 3. Secret manager config subscription thread - + * syncFromCloud 4. scheduler thread that periodically refreshes secrets - syncFromCloud * * @throws SecretManagerException when there are issues reading from disk */ public void reloadCache() throws SecretManagerException { try (LockScope ls = LockScope.lock(cacheLock)) { + logger.atDebug("clear-local-secret-cache").log(); + nameToArnMap.clear(); + cache.clear(); logger.atDebug("load-secret-local-store").log(); // read the db List secrets = secretStore.getAll().getSecrets(); - nameToArnMap.clear(); - cache.clear(); if (!Utils.isEmpty(secrets)) { for (AWSSecretResponse secretResult : secrets) { nameToArnMap.put(secretResult.getName(), secretResult.getArn()); @@ -301,9 +317,14 @@ private GetSecretValueResponse getSecret(String secretId, String versionId, Stri public software.amazon.awssdk.aws.greengrass.model.GetSecretValueResponse getSecret(software.amazon.awssdk.aws.greengrass.model.GetSecretValueRequest request) throws GetSecretException { + boolean refreshField = false; + boolean isRefreshFieldSupported = Arrays.stream(request.getClass().getDeclaredFields()) + .anyMatch(((x) -> x.getName().equals(IPC_REQUEST_REFRESH_FIELD))); + if (isRefreshFieldSupported) { + refreshField = Coerce.toBoolean(request.isRefresh()); + } GetSecretValueResponse secretResponse = - getSecret(request.getSecretId(), request.getVersionId(), request.getVersionStage(), - Coerce.toBoolean(request.isRefresh())); + getSecret(request.getSecretId(), request.getVersionId(), request.getVersionStage(), refreshField); return translateModeltoIpc(secretResponse); } diff --git a/src/main/java/com/aws/greengrass/secretmanager/exception/FileSecretStoreException.java b/src/main/java/com/aws/greengrass/secretmanager/exception/FileSecretStoreException.java new file mode 100644 index 0000000..ecd0d0d --- /dev/null +++ b/src/main/java/com/aws/greengrass/secretmanager/exception/FileSecretStoreException.java @@ -0,0 +1,7 @@ +package com.aws.greengrass.secretmanager.exception; + +public class FileSecretStoreException extends SecretManagerException { + public FileSecretStoreException(String err, Exception e) { + super(err, e); + } +} diff --git a/src/main/java/com/aws/greengrass/secretmanager/store/FileSecretStore.java b/src/main/java/com/aws/greengrass/secretmanager/store/FileSecretStore.java index e08d95f..23d52eb 100644 --- a/src/main/java/com/aws/greengrass/secretmanager/store/FileSecretStore.java +++ b/src/main/java/com/aws/greengrass/secretmanager/store/FileSecretStore.java @@ -7,6 +7,7 @@ import com.aws.greengrass.config.Topic; import com.aws.greengrass.secretmanager.SecretManagerService; +import com.aws.greengrass.secretmanager.exception.FileSecretStoreException; import com.aws.greengrass.secretmanager.exception.NoSecretFoundException; import com.aws.greengrass.secretmanager.exception.SecretManagerException; import com.aws.greengrass.secretmanager.kernel.KernelClient; @@ -41,10 +42,9 @@ public class FileSecretStore implements SecretStore responseList = doc.getSecrets(); - // If the existing secrets in the store contain the version stages(labels) of the newly added secret, we have - // to remove those labels as labels are unique across different versions of a secret. - Iterator secretsItr = - responseList.stream().filter(secret -> secret.getArn().equals(encryptedResult.getArn())).iterator(); - encryptedResult.getVersionStages().forEach((label -> { - while (secretsItr.hasNext()) { - AWSSecretResponse response = secretsItr.next(); - response.getVersionStages().remove(label); - } - })); - responseList.add(encryptedResult); - SecretDocument updatedDoc = SecretDocument.builder().secrets(responseList).build(); - try { - secretTopic.withValue(OBJECT_MAPPER.writeValueAsString(updatedDoc)); - } catch (JsonProcessingException e) { - throw new SecretManagerException("Cannot write secret response to store", e); - } - } - /** * Save a secret document to underlying file store. * @@ -130,7 +104,7 @@ public void saveAll(SecretDocument doc) throws SecretManagerException { try { secretTopic.withValue(OBJECT_MAPPER.writeValueAsString(doc)); } catch (IOException e) { - throw new SecretManagerException("Cannot write secret response to store", e); + throw new FileSecretStoreException("Cannot write secret response to store", e); } } } diff --git a/src/main/java/com/aws/greengrass/secretmanager/store/SecretStore.java b/src/main/java/com/aws/greengrass/secretmanager/store/SecretStore.java index ea37c22..688529d 100644 --- a/src/main/java/com/aws/greengrass/secretmanager/store/SecretStore.java +++ b/src/main/java/com/aws/greengrass/secretmanager/store/SecretStore.java @@ -15,5 +15,4 @@ public interface SecretStore { T get(String secretArn, String label) throws SecretManagerException; - void save(T encryptedResult) throws SecretManagerException, JsonProcessingException; } diff --git a/src/test/java/com/aws/greengrass/secretmanager/SecretManagerTest.java b/src/test/java/com/aws/greengrass/secretmanager/SecretManagerTest.java index 534ab9e..80f44e2 100644 --- a/src/test/java/com/aws/greengrass/secretmanager/SecretManagerTest.java +++ b/src/test/java/com/aws/greengrass/secretmanager/SecretManagerTest.java @@ -39,8 +39,6 @@ import software.amazon.awssdk.services.secretsmanager.model.GetSecretValueResponse; import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.nio.file.Paths; @@ -53,13 +51,10 @@ import java.util.Collections; import java.util.List; import java.util.UUID; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import static com.aws.greengrass.componentmanager.KernelConfigResolver.CONFIGURATION_CONFIG_KEY; -import static com.aws.greengrass.secretmanager.SecretManagerService.CLOUD_REQUEST_QUEUE_SIZE_TOPIC; -import static com.aws.greengrass.secretmanager.SecretManagerService.PERFORMANCE_TOPIC; import static com.aws.greengrass.secretmanager.SecretManagerService.SECRETS_TOPIC; import static com.aws.greengrass.testcommons.testutilities.ExceptionLogProtector.ignoreExceptionOfType; import static org.hamcrest.MatcherAssert.assertThat; @@ -68,7 +63,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doReturn; @@ -646,7 +640,7 @@ void GIVEN_secret_manager_WHEN_cloud_errors_THEN_secrets_are_not_loaded(Extensio @Test void GIVEN_secret_manager_WHEN_network_error_and_new_secret_THEN_throws(ExtensionContext context) throws Exception { ignoreExceptionOfType(context, SecretManagerException.class); - + when(mockDao.getAll()).thenReturn(SecretDocument.builder().secrets(Collections.singletonList(loadMockDaoSecretA())).build()); SecretManager sm = new SecretManager(mockAWSSecretClient, mockDao, mockKernelClient, localStoreMap); SecretConfiguration secret1 = SecretConfiguration.builder().arn(ARN_1).build(); Configuration config = new Configuration(new Context()); @@ -736,12 +730,11 @@ void GIVEN_secret_manager_WHEN_load_from_disk_fails_THEN_throws(ExtensionContext ignoreExceptionOfType(context, SecretManagerException.class); loadMockSecrets(); - when(mockAWSSecretClient.getSecret(any())).thenReturn(getMockSecretA()); doThrow(SecretManagerException.class).when(mockDao).getAll(); SecretManager sm = new SecretManager(mockAWSSecretClient, mockDao, mockKernelClient, localStoreMap); - sm.syncFromCloud(); assertThrows(SecretManagerException.class, sm::reloadCache); + assertThrows(RuntimeException.class, sm::syncFromCloud); } @Test