Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

decouple the secrets helper from connectors, to support secrets in ot… #17771

Merged
merged 1 commit into from
Oct 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,13 @@ private JsonNode statefulUpdateSecrets(final UUID workspaceId,
workspaceId,
oldConfig.get(),
fullConfig,
spec,
spec.getConnectionSpecification(),
longLivedSecretPersistence.get());
} else {
splitSecretConfig = SecretsHelpers.splitConfig(
workspaceId,
fullConfig,
spec);
spec.getConnectionSpecification());
}
splitSecretConfig.getCoordinateToPayload().forEach(longLivedSecretPersistence.get()::write);
return splitSecretConfig.getPartialConfig();
Expand All @@ -188,7 +188,7 @@ private JsonNode splitSecretConfig(final UUID workspaceId,
final ConnectorSpecification spec,
final Optional<SecretPersistence> secretPersistence) {
if (secretPersistence.isPresent()) {
final SplitSecretConfig splitSecretConfig = SecretsHelpers.splitConfig(workspaceId, fullConfig, spec);
final SplitSecretConfig splitSecretConfig = SecretsHelpers.splitConfig(workspaceId, fullConfig, spec.getConnectionSpecification());
splitSecretConfig.getCoordinateToPayload().forEach(secretPersistence.get()::write);
return splitSecretConfig.getPartialConfig();
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import io.airbyte.commons.json.JsonSchemas;
import io.airbyte.commons.json.Jsons;
import io.airbyte.commons.util.MoreIterators;
import io.airbyte.protocol.models.ConnectorSpecification;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -27,18 +26,17 @@
* Contains most of the core logic surrounding secret coordinate extraction and insertion.
*
* These are the three main helpers provided by this class:
* {@link SecretsHelpers#splitConfig(UUID, JsonNode, ConnectorSpecification)}
* {@link SecretsHelpers#splitAndUpdateConfig(UUID, JsonNode, JsonNode, ConnectorSpecification, ReadOnlySecretPersistence)}
* {@link SecretsHelpers#splitConfig(UUID, JsonNode, JsonNode)}
* {@link SecretsHelpers#splitAndUpdateConfig(UUID, JsonNode, JsonNode, JsonNode, ReadOnlySecretPersistence)}
* {@link SecretsHelpers#combineConfig(JsonNode, ReadOnlySecretPersistence)}
*
* Here's an overview on some terminology used in this class:
*
* A "full config" represents an entire connector config as specified by an end user. This should
* conform to a connector specification.
* A "full config" represents an entire config as specified by an end user.
*
* A "partial config" represents a connector config where any string that was specified as an
* airbyte_secret in the connector specification is replaced by a JSON object {"_secret": "secret
* coordinate"} that can later be used to reconstruct the "full config".
* A "partial config" represents a config where any string that was specified as an airbyte_secret
* in the specification is replaced by a JSON object {"_secret": "secret coordinate"} that can later
* be used to reconstruct the "full config".
*
* A {@link SecretPersistence} provides the ability to read and write secrets to a backing store
* such as Google Secrets Manager.
Expand All @@ -52,57 +50,55 @@ public class SecretsHelpers {
public static final String COORDINATE_FIELD = "_secret";

/**
* Used to separate secrets out of a connector configuration. This will output a partial config that
* Used to separate secrets out of some configuration. This will output a partial config that
* includes pointers to secrets instead of actual secret values and a map that can be used to update
* a {@link SecretPersistence} at coordinates with values from the full config.
*
* @param workspaceId workspace used for this connector config
* @param workspaceId workspace used for this config
* @param fullConfig config including secrets
* @param spec connector specification
* @param spec specification for the config
* @return a partial config + a map of coordinates to secret payloads
*/
public static SplitSecretConfig splitConfig(final UUID workspaceId,
final JsonNode fullConfig,
final ConnectorSpecification spec) {
final JsonNode spec) {
return internalSplitAndUpdateConfig(
UUID::randomUUID,
workspaceId,
(coordinate) -> Optional.empty(),
Jsons.emptyObject(),
fullConfig,
spec.getConnectionSpecification());
spec);
}

/**
* Used to separate secrets out of a connector configuration and output a partial config that
* includes pointers to secrets instead of actual secret values and a map that can be used to update
* a {@link SecretPersistence} at coordinates with values from the full config. If a previous config
* for this connector's configuration is provided, this method attempts to use the same base
* coordinates to refer to the same secret and increment the version of the coordinate used to
* reference a secret.
* Used to separate secrets out of a configuration and output a partial config that includes
* pointers to secrets instead of actual secret values and a map that can be used to update a
* {@link SecretPersistence} at coordinates with values from the full config. If a previous config
* for this configuration is provided, this method attempts to use the same base coordinates to
* refer to the same secret and increment the version of the coordinate used to reference a secret.
*
* @param workspaceId workspace used for this connector config
* @param oldPartialConfig previous partial config for this specific connector configuration
* @param workspaceId workspace used for this config
* @param oldPartialConfig previous partial config for this specific configuration
* @param newFullConfig new config containing secrets that will be used to update the partial config
* for this connector
* @param spec connector specification that should match both the previous partial config after
* filling in coordinates and the new full config.
* @param spec specification that should match both the previous partial config after filling in
* coordinates and the new full config.
* @param secretReader provides a way to determine if a secret is the same or updated at a specific
* location in a config
* @return a partial config + a map of coordinates to secret payloads
*/
public static SplitSecretConfig splitAndUpdateConfig(final UUID workspaceId,
final JsonNode oldPartialConfig,
final JsonNode newFullConfig,
final ConnectorSpecification spec,
final JsonNode spec,
final ReadOnlySecretPersistence secretReader) {
return internalSplitAndUpdateConfig(
UUID::randomUUID,
workspaceId,
secretReader,
oldPartialConfig,
newFullConfig,
spec.getConnectionSpecification());
spec);
}

/**
Expand Down Expand Up @@ -148,9 +144,9 @@ public static JsonNode combineConfig(final JsonNode partialConfig, final ReadOnl
public static SplitSecretConfig splitConfig(final Supplier<UUID> uuidSupplier,
final UUID workspaceId,
final JsonNode fullConfig,
final ConnectorSpecification spec) {
final JsonNode spec) {
return internalSplitAndUpdateConfig(uuidSupplier, workspaceId, (coordinate) -> Optional.empty(), Jsons.emptyObject(), fullConfig,
spec.getConnectionSpecification());
spec);
}

/**
Expand All @@ -162,9 +158,9 @@ public static SplitSecretConfig splitAndUpdateConfig(final Supplier<UUID> uuidSu
final UUID workspaceId,
final JsonNode oldPartialConfig,
final JsonNode newFullConfig,
final ConnectorSpecification spec,
final JsonNode spec,
final ReadOnlySecretPersistence secretReader) {
return internalSplitAndUpdateConfig(uuidSupplier, workspaceId, secretReader, oldPartialConfig, newFullConfig, spec.getConnectionSpecification());
return internalSplitAndUpdateConfig(uuidSupplier, workspaceId, secretReader, oldPartialConfig, newFullConfig, spec);
}

/**
Expand Down Expand Up @@ -207,7 +203,7 @@ private static SecretCoordinate getOrCreateCoordinate(final ReadOnlySecretPersis
*
* For splits that don't have a prior partial config (such as when a connector is created for a
* source or destination for the first time), the secret reader and old partial config can be set to
* empty (see {@link SecretsHelpers#splitConfig(UUID, JsonNode, ConnectorSpecification)}).
* empty (see {@link SecretsHelpers#splitConfig(UUID, JsonNode, JsonNode)}).
*
* IMPORTANT: This is used recursively. In the process, the partial config, full config, and spec
* inputs will represent internal json structures, not the entire configs/specs.
Expand All @@ -218,10 +214,9 @@ private static SecretCoordinate getOrCreateCoordinate(final ReadOnlySecretPersis
* stored for
* @param secretReader provides a way to determine if a secret is the same or updated at a specific
* location in a config
* @param persistedPartialConfig previous partial config for this specific connector configuration
* @param persistedPartialConfig previous partial config for this specific configuration
* @param newFullConfig new config containing secrets that will be used to update the partial config
* for this connector
* @param spec connector specification
* @param spec config specification
* @return a partial config + a map of coordinates to secret payloads
*/
@VisibleForTesting
Expand Down Expand Up @@ -292,7 +287,7 @@ private static SecretCoordinate getCoordinateFromTextNode(final JsonNode node) {
*
* @param newSecret new value of a secret provides a way to determine if a secret is the same or
* updated at a specific location in a config
* @param workspaceId workspace used for this connector config
* @param workspaceId workspace used for this config
* @param uuidSupplier provided to allow a test case to produce known UUIDs in order for easy
* fixture creation.
* @param oldSecretFullCoordinate a nullable full coordinate (base+version) retrieved from the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void testSplit(final SecretsTestCase testCase) {
uuidIterator::next,
WORKSPACE_ID,
inputConfig,
testCase.getSpec());
testCase.getSpec().getConnectionSpecification());

assertEquals(testCase.getPartialConfig(), splitConfig.getPartialConfig());
assertEquals(testCase.getFirstSecretMap(), splitConfig.getCoordinateToPayload());
Expand Down Expand Up @@ -131,7 +131,7 @@ void testSplitUpdate(final SecretsTestCase testCase) {
WORKSPACE_ID,
inputPartialConfig,
inputUpdateConfig,
testCase.getSpec(),
testCase.getSpec().getConnectionSpecification(),
secretPersistence::read);

assertEquals(testCase.getUpdatedPartialConfig(), updatedSplit.getPartialConfig());
Expand Down Expand Up @@ -179,7 +179,7 @@ void testUpdatingSecretsOneAtATime() {
uuidIterator::next,
WORKSPACE_ID,
testCase.getFullConfig(),
testCase.getSpec());
testCase.getSpec().getConnectionSpecification());

assertEquals(testCase.getPartialConfig(), splitConfig.getPartialConfig());
assertEquals(testCase.getFirstSecretMap(), splitConfig.getCoordinateToPayload());
Expand All @@ -193,7 +193,7 @@ void testUpdatingSecretsOneAtATime() {
WORKSPACE_ID,
testCase.getPartialConfig(),
testCase.getFullConfigUpdate1(),
testCase.getSpec(),
testCase.getSpec().getConnectionSpecification(),
secretPersistence::read);

assertEquals(testCase.getUpdatedPartialConfigAfterUpdate1(), updatedSplit1.getPartialConfig());
Expand All @@ -208,7 +208,7 @@ void testUpdatingSecretsOneAtATime() {
WORKSPACE_ID,
updatedSplit1.getPartialConfig(),
testCase.getFullConfigUpdate2(),
testCase.getSpec(),
testCase.getSpec().getConnectionSpecification(),
secretPersistence::read);

assertEquals(testCase.getUpdatedPartialConfigAfterUpdate2(), updatedSplit2.getPartialConfig());
Expand Down