Skip to content

Support top level oneOf for copySecrets #20848

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jan 3, 2023
Merged

Support top level oneOf for copySecrets #20848

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a52905d
Fix copySecrets for top level oneOfs
gosusnp Dec 22, 2022
d9f2ab9
Merge branch 'master' into gosusnp/fix-copy-secrets
gosusnp Jan 3, 2023
e54efb4
Merge branch 'master' into gosusnp/fix-copy-secrets
gosusnp Jan 3, 2023
1801aa9
Merge branch 'master' into gosusnp/fix-copy-secrets
gosusnp Jan 3, 2023
6082a69
Merge branch 'master' into gosusnp/fix-copy-secrets
gosusnp Jan 3, 2023
99e4e42
Merge branch 'master' into gosusnp/fix-copy-secrets
gosusnp Jan 3, 2023
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
80 changes: 44 additions & 36 deletions ...tence/src/main/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,59 +104,67 @@ public JsonNode copySecrets(final JsonNode src, final JsonNode dst, final JsonNo
if (!isValidJsonSchema(schema)) {
return dst;
}

Preconditions.checkArgument(dst.isObject());
Preconditions.checkArgument(src.isObject());

final ObjectNode dstCopy = dst.deepCopy();
return copySecretsRecursive(src, dstCopy, schema);
}

return src;
}

// This function is modifying dstCopy in place.
private JsonNode copySecretsRecursive(final JsonNode src, JsonNode dstCopy, final JsonNode schema) {
// todo (cgardens) this is not safe. should throw.
if (!isValidJsonSchema(schema)) {
return dstCopy;
}

Preconditions.checkArgument(dstCopy.isObject());
Preconditions.checkArgument(src.isObject());

final Optional<String> combinationKey = findJsonCombinationNode(schema);
if (combinationKey.isPresent()) {
final var arrayNode = (ArrayNode) schema.get(combinationKey.get());
for (int i = 0; i < arrayNode.size(); i++) {
final JsonNode childSchema = arrayNode.get(i);
/*
* when traversing a oneOf or anyOf if multiple schema in the oneOf or anyOf have the SAME key, but
* a different type, then, without this test, we can try to apply the wrong schema to the object
* resulting in errors because of type mismatches.
*/
if (VALIDATOR.test(childSchema, dstCopy)) {
// Absorb field values if any of the combination option is declaring it as secrets
copySecretsRecursive(src, dstCopy, childSchema);
}
}
} else {
final ObjectNode properties = (ObjectNode) schema.get(PROPERTIES_FIELD);
for (final String key : Jsons.keys(properties)) {
// If the source object doesn't have this key then we have nothing to copy, so we should skip to the
// next key.
if (!src.has(key)) {
// If the source or destination doesn't have this key then we have nothing to copy, so we should
// skip to the next key.
if (!src.has(key) || !dstCopy.has(key)) {
continue;
}

final JsonNode fieldSchema = properties.get(key);
// We only copy the original secret if the destination object isn't attempting to overwrite it
// I.e. if the destination object's value is set to the mask, then we can copy the original secret
if (JsonSecretsProcessor.isSecret(fieldSchema) && dst.has(key) && AirbyteSecretConstants.SECRETS_MASK.equals(dst.get(key).asText())) {
dstCopy.set(key, src.get(key));
} else if (dstCopy.has(key)) {
// If the destination has this key, then we should consider copying it

// Check if this schema is a combination node; if it is, find a matching sub-schema and copy based
// on that sub-schema
final var combinationKey = findJsonCombinationNode(fieldSchema);
if (combinationKey.isPresent()) {
var combinationCopy = dstCopy.get(key);
final var arrayNode = (ArrayNode) fieldSchema.get(combinationKey.get());
for (int i = 0; i < arrayNode.size(); i++) {
final JsonNode childSchema = arrayNode.get(i);
/*
* when traversing a oneOf or anyOf if multiple schema in the oneOf or anyOf have the SAME key, but
* a different type, then, without this test, we can try to apply the wrong schema to the object
* resulting in errors because of type mismatches.
*/
if (VALIDATOR.test(childSchema, combinationCopy)) {
// Absorb field values if any of the combination option is declaring it as secrets
combinationCopy = copySecrets(src.get(key), combinationCopy, childSchema);
}
}
dstCopy.set(key, combinationCopy);
} else {
// Otherwise, this is just a plain old json node; recurse into it. If it's not actually an object,
// the recursive call will exit immediately.
final JsonNode copiedField = copySecrets(src.get(key), dstCopy.get(key), fieldSchema);
dstCopy.set(key, copiedField);
}
if (JsonSecretsProcessor.isSecret(fieldSchema) && AirbyteSecretConstants.SECRETS_MASK.equals(dstCopy.get(key).asText())) {
((ObjectNode) dstCopy).set(key, src.get(key));

} else {
// Otherwise, this is just a plain old json node; recurse into it. If it's not actually an object,
// the recursive call will exit immediately.
final JsonNode copiedField = copySecretsRecursive(src.get(key), dstCopy.get(key), fieldSchema);
((ObjectNode) dstCopy).set(key, copiedField);
}
}

return dstCopy;
}

return src;
return dstCopy;
}

static boolean isSecret(final JsonNode obj) {
Expand Down
62 changes: 62 additions & 0 deletions ...e/src/test/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,68 @@ void doesNotCopySecretsInNestedNonCombinationNodeWhenDestinationMissing() throws
assertEquals(expected, copied);
}

@Test
void testCopySecretsWithTopLevelOneOf() {
final JsonNode schema = Jsons.deserialize("""
{
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "E2E Test Destination Spec",
"type": "object",
"oneOf": [
{
"title": "Silent",
"required": ["type"],
"properties": {
"a_secret": {
"type": "string",
"airbyte_secret": true
}
}
},
{
"title": "Throttled",
"required": ["type", "millis_per_record"],
"properties": {
"type": {
"type": "string",
"const": "THROTTLED",
"default": "THROTTLED"
},
"millis_per_record": {
"description": "Number of milli-second to pause in between records.",
"type": "integer"
}
}
}
]
}
""");

final JsonNode source = Jsons.deserialize("""
{
"type": "THROTTLED",
"a_secret": "woot"
}
""");

final JsonNode destination = Jsons.deserialize("""
{
"type": "THROTTLED",
"a_secret": "**********"
}
""");

final JsonNode result = processor.copySecrets(source, destination, schema);
final JsonNode expected = Jsons.deserialize("""
{
"type": "THROTTLED",
"a_secret": "woot"
}
""");

assertEquals(expected, result);
}

@Nested
class NoOpTest {

Expand Down