From e4be0df9a1c3b6cfd6cf60ccacb2b95fb8b8a52a Mon Sep 17 00:00:00 2001 From: Edward Gao Date: Wed, 9 Mar 2022 17:01:05 -0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20=20Correctly=20populate=20secret?= =?UTF-8?q?s=20inside=20nested=20non-combination=20objects=20(#10926)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../split_secrets/JsonSecretsProcessor.java | 30 ++++-- .../JsonSecretsProcessorTest.java | 101 ++++++++++++++++++ 2 files changed, 121 insertions(+), 10 deletions(-) diff --git a/airbyte-config/persistence/src/main/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessor.java b/airbyte-config/persistence/src/main/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessor.java index 41073286972..39fda6d2fb5 100644 --- a/airbyte-config/persistence/src/main/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessor.java +++ b/airbyte-config/persistence/src/main/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessor.java @@ -163,19 +163,25 @@ public class JsonSecretsProcessor { 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)) { + 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 value of the secret isn't set to the mask - if (isSecret(fieldSchema) && src.has(key)) { - if (dst.has(key) && dst.get(key).asText().equals(SECRETS_MASK)) { - dstCopy.set(key, src.get(key)); - } - } + if (isSecret(fieldSchema) && dst.has(key) && dst.get(key).asText().equals(SECRETS_MASK)) { + dstCopy.set(key, src.get(key)); + } else if (dstCopy.has(key)) { + // If the destination has this key, then we should consider copying it - final var combinationKey = findJsonCombinationNode(fieldSchema); - if (combinationKey.isPresent() && dstCopy.has(key)) { - var combinationCopy = dstCopy.get(key); - if (src.has(key)) { + // 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); @@ -189,8 +195,12 @@ public class JsonSecretsProcessor { combinationCopy = copySecrets(src.get(key), combinationCopy, childSchema); } } + dstCopy.set(key, combinationCopy); + } else { + // Otherwise, this is just a plain old json object; recurse into it. + final JsonNode copiedField = copySecrets(src.get(key), dstCopy.get(key), fieldSchema); + dstCopy.set(key, copiedField); } - dstCopy.set(key, combinationCopy); } } diff --git a/airbyte-config/persistence/src/test/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessorTest.java b/airbyte-config/persistence/src/test/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessorTest.java index 7a5c513973c..d9a4dac28c6 100644 --- a/airbyte-config/persistence/src/test/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessorTest.java +++ b/airbyte-config/persistence/src/test/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessorTest.java @@ -6,6 +6,7 @@ package io.airbyte.config.persistence.split_secrets; import static org.junit.jupiter.api.Assertions.assertEquals; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; @@ -324,4 +325,104 @@ public class JsonSecretsProcessorTest { assertEquals(expected, actual); } + @Test + public void copiesSecrets_inNestedNonCombinationNode() throws JsonProcessingException { + final ObjectMapper objectMapper = new ObjectMapper(); + + final JsonNode source = objectMapper.readTree( + """ + { + "top_level": { + "a_secret": "hunter2" + } + } + """); + final JsonNode dest = objectMapper.readTree( + """ + { + "top_level": { + "a_secret": "**********" + } + } + """); + final JsonNode schema = objectMapper.readTree( + """ + { + "type": "object", + "properties": { + "top_level": { + "type": "object", + "properties": { + "a_secret": { + "type": "string", + "airbyte_secret": true + } + } + } + } + } + """); + + final JsonNode copied = processor.copySecrets(source, dest, schema); + + final JsonNode expected = objectMapper.readTree( + """ + { + "top_level": { + "a_secret": "hunter2" + } + } + """); + assertEquals(expected, copied); + } + + @Test + public void doesNotCopySecrets_inNestedNonCombinationNodeWhenDestinationMissing() throws JsonProcessingException { + final ObjectMapper objectMapper = new ObjectMapper(); + + final JsonNode source = objectMapper.readTree( + """ + { + "top_level": { + "a_secret": "hunter2" + } + } + """); + final JsonNode dest = objectMapper.readTree( + """ + { + "top_level": { + } + } + """); + final JsonNode schema = objectMapper.readTree( + """ + { + "type": "object", + "properties": { + "top_level": { + "type": "object", + "properties": { + "a_secret": { + "type": "string", + "airbyte_secret": true + } + } + } + } + } + """); + + final JsonNode copied = processor.copySecrets(source, dest, schema); + + final JsonNode expected = objectMapper.readTree( + """ + { + "top_level": { + } + } + """); + assertEquals(expected, copied); + } + }