Skip to content

Commit

Permalink
Fix copySecrets for top level oneOfs (#20848)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gosusnp
gosusnp authored Jan 3, 2023
1 parent 4eca4a4 commit 9dc4189
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 36 deletions.
80 changes: 44 additions & 36 deletions ...tence/src/main/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessor.java
View file
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
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

0 comments on commit 9dc4189

Please sign in to comment.