diff --git a/plugin-infra/go-plugin-access/src/main/java/com/thoughtworks/go/plugin/access/exceptions/SecretResolutionFailureException.java b/plugin-infra/go-plugin-access/src/main/java/com/thoughtworks/go/plugin/access/exceptions/SecretResolutionFailureException.java index a90133023a7..dcfb344a4f1 100644 --- a/plugin-infra/go-plugin-access/src/main/java/com/thoughtworks/go/plugin/access/exceptions/SecretResolutionFailureException.java +++ b/plugin-infra/go-plugin-access/src/main/java/com/thoughtworks/go/plugin/access/exceptions/SecretResolutionFailureException.java @@ -16,8 +16,25 @@ package com.thoughtworks.go.plugin.access.exceptions; +import java.util.Set; + +import static java.lang.String.format; + public class SecretResolutionFailureException extends RuntimeException { public SecretResolutionFailureException(String message) { super(message); } + + public static SecretResolutionFailureException withMissingSecretParams(Set secretsToResolve, Set missingSecrets) { + return new SecretResolutionFailureException(format("Expected plugin to resolve secret(s) `%s` but plugin failed resolve secret param(s) `%s`. Please make sure that secret with same name exist in your secret management tool.", csv(secretsToResolve), csv(missingSecrets))); + } + + + public static SecretResolutionFailureException withUnwantedSecretParams(Set secretsToResolve, Set unwantedSecrets) { + return new SecretResolutionFailureException(format("Expected plugin to resolve secret(s) `%s` but plugin sent addition secret param(s) `%s`.", csv(secretsToResolve), csv(unwantedSecrets))); + } + + private static String csv(Set secretsToResolve) { + return String.join(", ", secretsToResolve); + } } diff --git a/plugin-infra/go-plugin-access/src/main/java/com/thoughtworks/go/plugin/access/secrets/SecretsExtension.java b/plugin-infra/go-plugin-access/src/main/java/com/thoughtworks/go/plugin/access/secrets/SecretsExtension.java index 2d03e0d775a..cc7e61c6ed4 100644 --- a/plugin-infra/go-plugin-access/src/main/java/com/thoughtworks/go/plugin/access/secrets/SecretsExtension.java +++ b/plugin-infra/go-plugin-access/src/main/java/com/thoughtworks/go/plugin/access/secrets/SecretsExtension.java @@ -20,16 +20,19 @@ import com.thoughtworks.go.plugin.access.ExtensionsRegistry; import com.thoughtworks.go.plugin.access.PluginRequestHelper; import com.thoughtworks.go.plugin.access.common.AbstractExtension; +import com.thoughtworks.go.plugin.access.exceptions.SecretResolutionFailureException; import com.thoughtworks.go.plugin.access.secrets.v1.SecretsExtensionV1; import com.thoughtworks.go.plugin.api.response.validation.ValidationResult; import com.thoughtworks.go.plugin.domain.common.Image; import com.thoughtworks.go.plugin.domain.common.PluginConfiguration; import com.thoughtworks.go.plugin.domain.secrets.Secret; import com.thoughtworks.go.plugin.infra.PluginManager; +import org.apache.commons.collections4.SetUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import java.util.*; +import java.util.stream.Collectors; import static com.thoughtworks.go.plugin.domain.common.PluginConstants.SECRETS_EXTENSION; @@ -73,7 +76,20 @@ public ValidationResult validateSecretsConfig(final String pluginId, final Map lookupSecrets(String pluginId, SecretConfig secretConfig, Set keys) { - return getVersionedSecretsExtension(pluginId).lookupSecrets(pluginId, secretConfig, keys); + final List secrets = getVersionedSecretsExtension(pluginId).lookupSecrets(pluginId, secretConfig, keys); + final Set resolvedSecrets = secrets.stream().map(Secret::getKey).collect(Collectors.toSet()); + + final Set additionalSecretsInResponse = SetUtils.difference(resolvedSecrets, keys).toSet(); + if (!additionalSecretsInResponse.isEmpty()) { + throw SecretResolutionFailureException.withUnwantedSecretParams(keys, additionalSecretsInResponse); + } + + if (resolvedSecrets.containsAll(keys)) { + return secrets; + } + + final Set missingSecrets = SetUtils.disjunction(resolvedSecrets, keys).toSet(); + throw SecretResolutionFailureException.withMissingSecretParams(keys, missingSecrets); } protected VersionedSecretsExtension getVersionedSecretsExtension(String pluginId) { diff --git a/plugin-infra/go-plugin-access/src/test/java/com/thoughtworks/go/plugin/access/secrets/SecretsExtensionTest.java b/plugin-infra/go-plugin-access/src/test/java/com/thoughtworks/go/plugin/access/secrets/SecretsExtensionTest.java index 453756ed39a..8f835a7f76f 100644 --- a/plugin-infra/go-plugin-access/src/test/java/com/thoughtworks/go/plugin/access/secrets/SecretsExtensionTest.java +++ b/plugin-infra/go-plugin-access/src/test/java/com/thoughtworks/go/plugin/access/secrets/SecretsExtensionTest.java @@ -19,21 +19,25 @@ import com.thoughtworks.go.config.SecretConfig; import com.thoughtworks.go.plugin.access.ExtensionsRegistry; import com.thoughtworks.go.plugin.access.common.AbstractExtension; +import com.thoughtworks.go.plugin.access.exceptions.SecretResolutionFailureException; import com.thoughtworks.go.plugin.access.secrets.v1.SecretsExtensionV1; +import com.thoughtworks.go.plugin.domain.secrets.Secret; import com.thoughtworks.go.plugin.infra.PluginManager; import com.thoughtworks.go.plugin.infra.plugininfo.GoPluginDescriptor; import com.thoughtworks.go.util.ReflectionUtil; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; import java.util.*; import static com.thoughtworks.go.plugin.access.secrets.SecretsExtension.SUPPORTED_VERSIONS; import static com.thoughtworks.go.plugin.domain.common.PluginConstants.SECRETS_EXTENSION; import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.mockito.Mockito.*; public class SecretsExtensionTest { @@ -43,8 +47,8 @@ public class SecretsExtensionTest { protected GoPluginDescriptor descriptor; protected SecretsExtension extension; - @Before - public void setUp() throws Exception { + @BeforeEach + void setUp() { pluginManager = mock(PluginManager.class); extensionsRegistry = mock(ExtensionsRegistry.class); descriptor = mock(GoPluginDescriptor.class); @@ -56,7 +60,7 @@ public void setUp() throws Exception { } @Test - public void shouldHaveVersionedSecretsExtensionForAllSupportedVersions() { + void shouldHaveVersionedSecretsExtensionForAllSupportedVersions() { for (String supportedVersion : SUPPORTED_VERSIONS) { final String message = String.format("Must define versioned extension class for %s extension with version %s", SECRETS_EXTENSION, supportedVersion); @@ -64,13 +68,13 @@ public void shouldHaveVersionedSecretsExtensionForAllSupportedVersions() { final VersionedSecretsExtension extension = this.extension.getVersionedSecretsExtension(PLUGIN_ID); - assertNotNull(message, extension); - assertThat(ReflectionUtil.getField(extension, "VERSION"), is(supportedVersion)); + assertThat(extension).as(message).isNotNull(); + assertThat(ReflectionUtil.getField(extension, "VERSION")).isEqualTo(supportedVersion); } } @Test - public void getIcon_shouldDelegateToVersionedExtension() { + void getIcon_shouldDelegateToVersionedExtension() { SecretsExtensionV1 secretsExtensionV1 = mock(SecretsExtensionV1.class); Map secretsExtensionMap = singletonMap("1.0", secretsExtensionV1); extension = new SecretsExtension(pluginManager, extensionsRegistry, secretsExtensionMap); @@ -83,7 +87,7 @@ public void getIcon_shouldDelegateToVersionedExtension() { } @Test - public void getSecretsConfigMetadata_shouldDelegateToVersionedExtension() { + void getSecretsConfigMetadata_shouldDelegateToVersionedExtension() { SecretsExtensionV1 secretsExtensionV1 = mock(SecretsExtensionV1.class); Map secretsExtensionMap = singletonMap("1.0", secretsExtensionV1); extension = new SecretsExtension(pluginManager, extensionsRegistry, secretsExtensionMap); @@ -96,7 +100,7 @@ public void getSecretsConfigMetadata_shouldDelegateToVersionedExtension() { } @Test - public void getSecretsConfigView_shouldDelegateToVersionedExtension() { + void getSecretsConfigView_shouldDelegateToVersionedExtension() { SecretsExtensionV1 secretsExtensionV1 = mock(SecretsExtensionV1.class); Map secretsExtensionMap = singletonMap("1.0", secretsExtensionV1); extension = new SecretsExtension(pluginManager, extensionsRegistry, secretsExtensionMap); @@ -109,7 +113,7 @@ public void getSecretsConfigView_shouldDelegateToVersionedExtension() { } @Test - public void validateSecretsConfig_shouldDelegateToVersionedExtension() { + void validateSecretsConfig_shouldDelegateToVersionedExtension() { SecretsExtensionV1 secretsExtensionV1 = mock(SecretsExtensionV1.class); Map secretsExtensionMap = singletonMap("1.0", secretsExtensionV1); extension = new SecretsExtension(pluginManager, extensionsRegistry, secretsExtensionMap); @@ -122,22 +126,65 @@ public void validateSecretsConfig_shouldDelegateToVersionedExtension() { verify(secretsExtensionV1).validateSecretsConfig(PLUGIN_ID, configuration); } - @Test - public void lookupSecrets_shouldDelegateToVersionedExtension() { - SecretsExtensionV1 secretsExtensionV1 = mock(SecretsExtensionV1.class); - Map secretsExtensionMap = singletonMap("1.0", secretsExtensionV1); - extension = new SecretsExtension(pluginManager, extensionsRegistry, secretsExtensionMap); - Set keys = new HashSet<>(asList("key1", "key2")); + @Nested + class lookupSecrets { + @Test + void shouldDelegateToVersionedExtension() { + SecretsExtensionV1 secretsExtensionV1 = mock(SecretsExtensionV1.class); + Map secretsExtensionMap = singletonMap("1.0", secretsExtensionV1); + extension = new SecretsExtension(pluginManager, extensionsRegistry, secretsExtensionMap); + Set keys = new HashSet<>(asList("key1", "key2")); + + when(pluginManager.resolveExtensionVersion(PLUGIN_ID, SECRETS_EXTENSION, SUPPORTED_VERSIONS)).thenReturn(SecretsExtensionV1.VERSION); + when(secretsExtensionV1.lookupSecrets(PLUGIN_ID, new SecretConfig(), keys)).thenReturn(Arrays.asList( + new Secret("key1", "value-1"), + new Secret("key2", "value-2") + )); + + final List secrets = extension.lookupSecrets(PLUGIN_ID, new SecretConfig(), keys); + + assertThat(secrets).hasSize(2) + .contains(new Secret("key1", "value-1"), new Secret("key2", "value-2")); + verify(secretsExtensionV1).lookupSecrets(PLUGIN_ID, new SecretConfig(), keys); + } - when(pluginManager.resolveExtensionVersion(PLUGIN_ID, SECRETS_EXTENSION, SUPPORTED_VERSIONS)).thenReturn(SecretsExtensionV1.VERSION); + @Test + void shouldBombIfResolvedSecretContainsAdditionalSecrets() { + SecretsExtensionV1 secretsExtensionV1 = mock(SecretsExtensionV1.class); + Map secretsExtensionMap = singletonMap("1.0", secretsExtensionV1); + extension = new SecretsExtension(pluginManager, extensionsRegistry, secretsExtensionMap); + final Set secretsToLookup = new HashSet<>(asList("key1", "key2")); + + when(pluginManager.resolveExtensionVersion(PLUGIN_ID, SECRETS_EXTENSION, SUPPORTED_VERSIONS)).thenReturn(SecretsExtensionV1.VERSION); + when(secretsExtensionV1.lookupSecrets(PLUGIN_ID, new SecretConfig(), secretsToLookup)).thenReturn(Arrays.asList( + new Secret("key1", "value-1"), + new Secret("key2", "value-2"), + new Secret("key3", "value-3") + )); + + assertThatCode(() -> extension.lookupSecrets(PLUGIN_ID, new SecretConfig(), secretsToLookup)) + .isInstanceOf(SecretResolutionFailureException.class) + .hasMessage("Expected plugin to resolve secret(s) `key1, key2` but plugin sent addition secret param(s) `key3`."); + } - this.extension.lookupSecrets(PLUGIN_ID, new SecretConfig(), keys); + @Test + void shouldBombWhenPluginReturnsPartiallyResolvedSecretParams() { + SecretsExtensionV1 secretsExtensionV1 = mock(SecretsExtensionV1.class); + Map secretsExtensionMap = singletonMap("1.0", secretsExtensionV1); + extension = new SecretsExtension(pluginManager, extensionsRegistry, secretsExtensionMap); + final Set secretsToLookup = new HashSet<>(asList("key1", "key2", "key3")); - verify(secretsExtensionV1).lookupSecrets(PLUGIN_ID, new SecretConfig(), keys); + when(pluginManager.resolveExtensionVersion(PLUGIN_ID, SECRETS_EXTENSION, SUPPORTED_VERSIONS)).thenReturn(SecretsExtensionV1.VERSION); + when(secretsExtensionV1.lookupSecrets(PLUGIN_ID, new SecretConfig(), secretsToLookup)).thenReturn(singletonList(new Secret("key1", "value-1"))); + + assertThatCode(() -> extension.lookupSecrets(PLUGIN_ID, new SecretConfig(), secretsToLookup)) + .isInstanceOf(SecretResolutionFailureException.class) + .hasMessage("Expected plugin to resolve secret(s) `key1, key2, key3` but plugin failed resolve secret param(s) `key2, key3`. Please make sure that secret with same name exist in your secret management tool."); + } } @Test - public void shouldExtendAbstractExtension() { - assertTrue(new SecretsExtension(pluginManager, extensionsRegistry) instanceof AbstractExtension); + void shouldExtendAbstractExtension() { + assertThat(new SecretsExtension(pluginManager, extensionsRegistry) instanceof AbstractExtension).isTrue(); } } diff --git a/server/src/main/java/com/thoughtworks/go/server/service/SecretParamResolver.java b/server/src/main/java/com/thoughtworks/go/server/service/SecretParamResolver.java index 3e8e0ef57ed..04f1e282ba2 100644 --- a/server/src/main/java/com/thoughtworks/go/server/service/SecretParamResolver.java +++ b/server/src/main/java/com/thoughtworks/go/server/service/SecretParamResolver.java @@ -64,7 +64,6 @@ private BiConsumer lookupAndUpdateSecretParamsValue() { List resolvedSecrets = secretsExtension.lookupSecrets(secretConfig.getPluginId(), secretConfig, secretParamMap.keySet()); LOGGER.debug("Resolved secret size '{}'", resolvedSecrets.size()); - LOGGER.debug("Updating secret params '{}' with values.", secretParamMap.keySet()); resolvedSecrets.forEach(assignValue(secretParamMap)); LOGGER.debug("Secret params '{}' updated with values.", secretParamMap.keySet());