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.

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

Jump to bottom

Introduce secrets management into workspace persistence #17851

Merged
merged 3 commits into from
Oct 12, 2022
Merged

Introduce secrets management into workspace persistence #17851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
50232a8
no-op refactor: rename getStandardWorkspace to prepare to introduce s…
mfsiega-airbyte Oct 11, 2022
634a5fc
no-op refactor: rename writeStandardWorkspace to prepare to introduce…
mfsiega-airbyte Oct 11, 2022
c5e6657
introduce secrets management into workspace persistence
mfsiega-airbyte Oct 11, 2022
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
2 changes: 1 addition & 1 deletion airbyte-analytics/src/main/java/io/airbyte/analytics/TrackingClientSingleton.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private static void initialize() {
@VisibleForTesting
static TrackingIdentity getTrackingIdentity(final ConfigRepository configRepository, final AirbyteVersion airbyteVersion, final UUID workspaceId) {
try {
final StandardWorkspace workspace = configRepository.getStandardWorkspace(workspaceId, true);
final StandardWorkspace workspace = configRepository.getStandardWorkspaceNoSecrets(workspaceId, true);
String email = null;
if (workspace.getEmail() != null && workspace.getAnonymousDataCollection() != null && !workspace.getAnonymousDataCollection()) {
email = workspace.getEmail();
Expand Down
10 changes: 5 additions & 5 deletions airbyte-analytics/src/test/java/io/airbyte/analytics/TrackingClientSingletonTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ void testGetTrackingIdentityRespectsWorkspaceId() throws JsonValidationException
final StandardWorkspace workspace1 = new StandardWorkspace().withWorkspaceId(WORKSPACE_ID).withCustomerId(UUID.randomUUID());
final StandardWorkspace workspace2 = new StandardWorkspace().withWorkspaceId(UUID.randomUUID()).withCustomerId(UUID.randomUUID());

when(configRepository.getStandardWorkspace(workspace1.getWorkspaceId(), true)).thenReturn(workspace1);
when(configRepository.getStandardWorkspace(workspace2.getWorkspaceId(), true)).thenReturn(workspace2);
when(configRepository.getStandardWorkspaceNoSecrets(workspace1.getWorkspaceId(), true)).thenReturn(workspace1);
when(configRepository.getStandardWorkspaceNoSecrets(workspace2.getWorkspaceId(), true)).thenReturn(workspace2);

final TrackingIdentity workspace1Actual =
TrackingClientSingleton.getTrackingIdentity(configRepository, AIRBYTE_VERSION, workspace1.getWorkspaceId());
Expand All @@ -96,7 +96,7 @@ void testGetTrackingIdentityRespectsWorkspaceId() throws JsonValidationException
void testGetTrackingIdentityInitialSetupNotComplete() throws JsonValidationException, IOException, ConfigNotFoundException {
final StandardWorkspace workspace = new StandardWorkspace().withWorkspaceId(WORKSPACE_ID).withCustomerId(UUID.randomUUID());

when(configRepository.getStandardWorkspace(WORKSPACE_ID, true)).thenReturn(workspace);
when(configRepository.getStandardWorkspaceNoSecrets(WORKSPACE_ID, true)).thenReturn(workspace);

final TrackingIdentity actual = TrackingClientSingleton.getTrackingIdentity(configRepository, AIRBYTE_VERSION, WORKSPACE_ID);
final TrackingIdentity expected = new TrackingIdentity(AIRBYTE_VERSION, workspace.getCustomerId(), null, null, null, null);
Expand All @@ -115,7 +115,7 @@ void testGetTrackingIdentityNonAnonymous() throws JsonValidationException, IOExc
.withSecurityUpdates(true)
.withDefaultGeography(Geography.AUTO);

when(configRepository.getStandardWorkspace(WORKSPACE_ID, true)).thenReturn(workspace);
when(configRepository.getStandardWorkspaceNoSecrets(WORKSPACE_ID, true)).thenReturn(workspace);

final TrackingIdentity actual = TrackingClientSingleton.getTrackingIdentity(configRepository, AIRBYTE_VERSION, WORKSPACE_ID);
final TrackingIdentity expected = new TrackingIdentity(AIRBYTE_VERSION, workspace.getCustomerId(), workspace.getEmail(), false, true, true);
Expand All @@ -134,7 +134,7 @@ void testGetTrackingIdentityAnonymous() throws JsonValidationException, IOExcept
.withSecurityUpdates(true)
.withDefaultGeography(Geography.AUTO);

when(configRepository.getStandardWorkspace(WORKSPACE_ID, true)).thenReturn(workspace);
when(configRepository.getStandardWorkspaceNoSecrets(WORKSPACE_ID, true)).thenReturn(workspace);

final TrackingIdentity actual = TrackingClientSingleton.getTrackingIdentity(configRepository, AIRBYTE_VERSION, WORKSPACE_ID);
final TrackingIdentity expected = new TrackingIdentity(AIRBYTE_VERSION, workspace.getCustomerId(), null, true, true, true);
Expand Down
4 changes: 3 additions & 1 deletion airbyte-bootloader/src/main/java/io/airbyte/bootloader/BootloaderApp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,9 @@ private static void createWorkspaceIfNoneExists(final ConfigRepository configRep
.withDisplaySetupWizard(true)
.withTombstone(false)
.withDefaultGeography(Geography.AUTO);
configRepository.writeStandardWorkspace(workspace);
// NOTE: it's safe to use the NoSecrets version since we know that the user hasn't supplied any
// secrets yet.
configRepository.writeStandardWorkspaceNoSecrets(workspace);
}

private static void assertNonBreakingMigration(final JobPersistence jobPersistence, final AirbyteVersion airbyteVersion)
Expand Down
2 changes: 1 addition & 1 deletion airbyte-bootloader/src/test/java/io/airbyte/bootloader/BootloaderAppTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ void testBootloaderAppRunSecretMigration() throws Exception {
final ObjectMapper mapper = new ObjectMapper();

final UUID workspaceId = UUID.randomUUID();
configRepository.writeStandardWorkspace(new StandardWorkspace()
configRepository.writeStandardWorkspaceNoSecrets(new StandardWorkspace()
.withWorkspaceId(workspaceId)
.withName("wName")
.withSlug("wSlug")
Expand Down
17 changes: 13 additions & 4 deletions ...nfig/config-persistence/src/main/java/io/airbyte/config/persistence/ConfigRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public boolean healthCheck() {
return true;
}

public StandardWorkspace getStandardWorkspace(final UUID workspaceId, final boolean includeTombstone)
public StandardWorkspace getStandardWorkspaceNoSecrets(final UUID workspaceId, final boolean includeTombstone)
throws JsonValidationException, IOException, ConfigNotFoundException {
final StandardWorkspace workspace = persistence.getConfig(ConfigSchema.STANDARD_WORKSPACE, workspaceId.toString(), StandardWorkspace.class);

Expand Down Expand Up @@ -166,12 +166,21 @@ public List<StandardWorkspace> listStandardWorkspaces(final boolean includeTombs
return workspaces;
}

public void writeStandardWorkspace(final StandardWorkspace workspace) throws JsonValidationException, IOException {
/**
* MUST NOT ACCEPT SECRETS - Should only be called from { @link SecretsRepositoryWriter }
*
* Write a StandardWorkspace to the database.
*
* @param workspace - The configuration of the workspace
* @throws JsonValidationException - throws is the workspace is invalid
* @throws IOException - you never know when you IO
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😆

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@terencecho I can't claim credit for this one, I stole it from elsewhere in this file :)

*/
public void writeStandardWorkspaceNoSecrets(final StandardWorkspace workspace) throws JsonValidationException, IOException {
persistence.writeConfig(ConfigSchema.STANDARD_WORKSPACE, workspace.getWorkspaceId().toString(), workspace);
}

public void setFeedback(final UUID workflowId) throws JsonValidationException, ConfigNotFoundException, IOException {
final StandardWorkspace workspace = getStandardWorkspace(workflowId, false);
final StandardWorkspace workspace = getStandardWorkspaceNoSecrets(workflowId, false);

workspace.setFeedbackDone(true);

Expand Down Expand Up @@ -209,7 +218,7 @@ public StandardWorkspace getStandardWorkspaceFromConnection(final UUID connectio
try {
final StandardSync sync = getStandardSync(connectionId);
final SourceConnection source = getSourceConnection(sync.getSourceId());
return getStandardWorkspace(source.getWorkspaceId(), isTombstone);
return getStandardWorkspaceNoSecrets(source.getWorkspaceId(), isTombstone);
} catch (final Exception e) {
throw new RuntimeException(e);
}
Expand Down
8 changes: 8 additions & 0 deletions ...nfig-persistence/src/main/java/io/airbyte/config/persistence/SecretsRepositoryReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import io.airbyte.config.ConfigSchema;
import io.airbyte.config.DestinationConnection;
import io.airbyte.config.SourceConnection;
import io.airbyte.config.StandardWorkspace;
import io.airbyte.config.WorkspaceServiceAccount;
import io.airbyte.config.persistence.split_secrets.SecretsHydrator;
import io.airbyte.validation.json.JsonValidationException;
Expand Down Expand Up @@ -107,4 +108,11 @@ public WorkspaceServiceAccount getWorkspaceServiceAccountWithSecrets(final UUID
return Jsons.clone(workspaceServiceAccount).withJsonCredential(jsonCredential).withHmacKey(hmacKey);
}

public StandardWorkspace getWorkspaceWithSecrets(final UUID workspaceId, final boolean includeTombstone)
throws JsonValidationException, ConfigNotFoundException, IOException {
final StandardWorkspace workspace = configRepository.getStandardWorkspaceNoSecrets(workspaceId, includeTombstone);
// TODO: hydrate any secrets once they're introduced.
return workspace;
}

}
7 changes: 7 additions & 0 deletions ...nfig-persistence/src/main/java/io/airbyte/config/persistence/SecretsRepositoryWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import io.airbyte.config.SourceConnection;
import io.airbyte.config.StandardDestinationDefinition;
import io.airbyte.config.StandardSourceDefinition;
import io.airbyte.config.StandardWorkspace;
import io.airbyte.config.WorkspaceServiceAccount;
import io.airbyte.config.persistence.split_secrets.SecretCoordinateToPayload;
import io.airbyte.config.persistence.split_secrets.SecretPersistence;
Expand Down Expand Up @@ -321,4 +322,10 @@ public Optional<WorkspaceServiceAccount> getOptionalWorkspaceServiceAccount(fina
}
}

public void writeWorkspace(final StandardWorkspace workspace)
throws JsonValidationException, IOException {
// TODO(msiega): split secrets once they're introduced.
configRepository.writeStandardWorkspaceNoSecrets(workspace);
}

}
2 changes: 1 addition & 1 deletion ...istence/src/test/java/io/airbyte/config/persistence/ConfigRepositoryE2EReadWriteTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ void setup() throws IOException, JsonValidationException, SQLException, Database
final DevDatabaseMigrator devDatabaseMigrator = new DevDatabaseMigrator(configsDatabaseMigrator);
MigrationDevHelper.runLastMigration(devDatabaseMigrator);
for (final StandardWorkspace workspace : MockData.standardWorkspaces()) {
configRepository.writeStandardWorkspace(workspace);
configRepository.writeStandardWorkspaceNoSecrets(workspace);
}
for (final StandardSourceDefinition sourceDefinition : MockData.standardSourceDefinitions()) {
configRepository.writeStandardSourceDefinition(sourceDefinition);
Expand Down
8 changes: 4 additions & 4 deletions .../config-persistence/src/test/java/io/airbyte/config/persistence/ConfigRepositoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void testWorkspaceWithTrueTombstone() throws ConfigNotFoundException, IOExceptio
void assertReturnsWorkspace(final StandardWorkspace workspace) throws ConfigNotFoundException, IOException, JsonValidationException {
when(configPersistence.getConfig(ConfigSchema.STANDARD_WORKSPACE, WORKSPACE_ID.toString(), StandardWorkspace.class)).thenReturn(workspace);

assertEquals(workspace, configRepository.getStandardWorkspace(WORKSPACE_ID, true));
assertEquals(workspace, configRepository.getStandardWorkspaceNoSecrets(WORKSPACE_ID, true));
}

@ParameterizedTest
Expand All @@ -111,11 +111,11 @@ void testWorkspaceByConnectionId(final boolean isTombstone) throws ConfigNotFoun
.getSourceConnection(sourceId);
doReturn(mWorkflow)
.when(configRepository)
.getStandardWorkspace(WORKSPACE_ID, isTombstone);
.getStandardWorkspaceNoSecrets(WORKSPACE_ID, isTombstone);

configRepository.getStandardWorkspaceFromConnection(connectionId, isTombstone);

verify(configRepository).getStandardWorkspace(WORKSPACE_ID, isTombstone);
verify(configRepository).getStandardWorkspaceNoSecrets(WORKSPACE_ID, isTombstone);
}

@Test
Expand Down Expand Up @@ -452,7 +452,7 @@ void testUpdateFeedback() throws JsonValidationException, ConfigNotFoundExceptio
final StandardWorkspace workspace = new StandardWorkspace().withWorkspaceId(WORKSPACE_ID).withTombstone(false);
doReturn(workspace)
.when(configRepository)
.getStandardWorkspace(WORKSPACE_ID, false);
.getStandardWorkspaceNoSecrets(WORKSPACE_ID, false);

configRepository.setFeedback(WORKSPACE_ID);

Expand Down
2 changes: 1 addition & 1 deletion .../config-persistence/src/test/java/io/airbyte/config/persistence/StatePersistenceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ private void setupTestData() throws JsonValidationException, IOException {
final DestinationConnection destinationConnection = MockData.destinationConnections().get(0);
final StandardSync sync = MockData.standardSyncs().get(0);

configRepository.writeStandardWorkspace(workspace);
configRepository.writeStandardWorkspaceNoSecrets(workspace);
configRepository.writeStandardSourceDefinition(sourceDefinition);
configRepository.writeSourceConnectionNoSecrets(sourceConnection);
configRepository.writeStandardDestinationDefinition(destinationDefinition);
Expand Down
4 changes: 2 additions & 2 deletions ...yte-persistence/job-persistence/src/main/java/io/airbyte/persistence/job/JobNotifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public JobNotifier(final WebUrlHelper webUrlHelper,
private void notifyJob(final String reason, final String action, final Job job) {
try {
final UUID workspaceId = workspaceHelper.getWorkspaceForJobIdIgnoreExceptions(job.getId());
final StandardWorkspace workspace = configRepository.getStandardWorkspace(workspaceId, true);
final StandardWorkspace workspace = configRepository.getStandardWorkspaceNoSecrets(workspaceId, true);
notifyJob(reason, action, job, workspaceId, workspace, workspace.getNotifications());
} catch (final Exception e) {
LOGGER.error("Unable to read configuration:", e);
Expand Down Expand Up @@ -146,7 +146,7 @@ public void notifyJobByEmail(final String reason, final String action, final Job
emailNotification.setNotificationType(NotificationType.CUSTOMERIO);
try {
final UUID workspaceId = workspaceHelper.getWorkspaceForJobIdIgnoreExceptions(job.getId());
final StandardWorkspace workspace = configRepository.getStandardWorkspace(workspaceId, true);
final StandardWorkspace workspace = configRepository.getStandardWorkspaceNoSecrets(workspaceId, true);
notifyJob(reason, action, job, workspaceId, workspace, Collections.singletonList(emailNotification));
} catch (final Exception e) {
LOGGER.error("Unable to read configuration:", e);
Expand Down
6 changes: 3 additions & 3 deletions ...-persistence/src/main/java/io/airbyte/persistence/job/errorreporter/JobErrorReporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public void reportSourceCheckJobFailure(final UUID sourceDefinitionId,
final FailureReason failureReason,
final ConnectorJobReportingContext jobContext)
throws JsonValidationException, ConfigNotFoundException, IOException {
final StandardWorkspace workspace = workspaceId != null ? configRepository.getStandardWorkspace(workspaceId, true) : null;
final StandardWorkspace workspace = workspaceId != null ? configRepository.getStandardWorkspaceNoSecrets(workspaceId, true) : null;
final StandardSourceDefinition sourceDefinition = configRepository.getStandardSourceDefinition(sourceDefinitionId);
final Map<String, String> metadata = MoreMaps.merge(
getSourceMetadata(sourceDefinition),
Expand All @@ -158,7 +158,7 @@ public void reportDestinationCheckJobFailure(final UUID destinationDefinitionId,
final FailureReason failureReason,
final ConnectorJobReportingContext jobContext)
throws JsonValidationException, ConfigNotFoundException, IOException {
final StandardWorkspace workspace = workspaceId != null ? configRepository.getStandardWorkspace(workspaceId, true) : null;
final StandardWorkspace workspace = workspaceId != null ? configRepository.getStandardWorkspaceNoSecrets(workspaceId, true) : null;
final StandardDestinationDefinition destinationDefinition = configRepository.getStandardDestinationDefinition(destinationDefinitionId);
final Map<String, String> metadata = MoreMaps.merge(
getDestinationMetadata(destinationDefinition),
Expand All @@ -178,7 +178,7 @@ public void reportDiscoverJobFailure(final UUID sourceDefinitionId,
final FailureReason failureReason,
final ConnectorJobReportingContext jobContext)
throws JsonValidationException, ConfigNotFoundException, IOException {
final StandardWorkspace workspace = workspaceId != null ? configRepository.getStandardWorkspace(workspaceId, true) : null;
final StandardWorkspace workspace = workspaceId != null ? configRepository.getStandardWorkspaceNoSecrets(workspaceId, true) : null;
final StandardSourceDefinition sourceDefinition = configRepository.getStandardSourceDefinition(sourceDefinitionId);
final Map<String, String> metadata = MoreMaps.merge(
getSourceMetadata(sourceDefinition),
Expand Down
2 changes: 1 addition & 1 deletion ...sistence/job-persistence/src/main/java/io/airbyte/persistence/job/tracker/JobTracker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ private void track(final UUID workspaceId, final Map<String, Object> metadata)
// unfortunate but in the case of jobs that cannot be linked to a workspace there not a sensible way
// track it.
if (workspaceId != null) {
final StandardWorkspace standardWorkspace = configRepository.getStandardWorkspace(workspaceId, true);
final StandardWorkspace standardWorkspace = configRepository.getStandardWorkspaceNoSecrets(workspaceId, true);
if (standardWorkspace != null && standardWorkspace.getName() != null) {
final Map<String, Object> standardTrackingMetadata = Map.of(
"workspace_id", workspaceId,
Expand Down
2 changes: 1 addition & 1 deletion ...persistence/job-persistence/src/test/java/io/airbyte/persistence/job/JobNotifierTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ void testFailJob() throws IOException, InterruptedException, JsonValidationExcep
when(configRepository.getDestinationDefinitionFromConnection(any())).thenReturn(destinationDefinition);
when(configRepository.getStandardSourceDefinition(any())).thenReturn(sourceDefinition);
when(configRepository.getStandardDestinationDefinition(any())).thenReturn(destinationDefinition);
when(configRepository.getStandardWorkspace(WORKSPACE_ID, true)).thenReturn(getWorkspace());
when(configRepository.getStandardWorkspaceNoSecrets(WORKSPACE_ID, true)).thenReturn(getWorkspace());
when(workspaceHelper.getWorkspaceForJobIdIgnoreExceptions(job.getId())).thenReturn(WORKSPACE_ID);
when(notificationClient.notifyJobFailure(anyString(), anyString(), anyString(), anyString(), anyLong())).thenReturn(true);

Expand Down
6 changes: 3 additions & 3 deletions ...sistence/src/test/java/io/airbyte/persistence/job/errorreporter/JobErrorReporterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ void testReportSourceCheckJobFailure() throws JsonValidationException, ConfigNot

final StandardWorkspace mWorkspace = Mockito.mock(StandardWorkspace.class);
Mockito.when(mWorkspace.getWorkspaceId()).thenReturn(WORKSPACE_ID);
Mockito.when(configRepository.getStandardWorkspace(WORKSPACE_ID, true)).thenReturn(mWorkspace);
Mockito.when(configRepository.getStandardWorkspaceNoSecrets(WORKSPACE_ID, true)).thenReturn(mWorkspace);

jobErrorReporter.reportSourceCheckJobFailure(SOURCE_DEFINITION_ID, WORKSPACE_ID, failureReason, jobContext);

Expand Down Expand Up @@ -337,7 +337,7 @@ void testReportDestinationCheckJobFailure() throws JsonValidationException, Conf

final StandardWorkspace mWorkspace = Mockito.mock(StandardWorkspace.class);
Mockito.when(mWorkspace.getWorkspaceId()).thenReturn(WORKSPACE_ID);
Mockito.when(configRepository.getStandardWorkspace(WORKSPACE_ID, true)).thenReturn(mWorkspace);
Mockito.when(configRepository.getStandardWorkspaceNoSecrets(WORKSPACE_ID, true)).thenReturn(mWorkspace);

jobErrorReporter.reportDestinationCheckJobFailure(DESTINATION_DEFINITION_ID, WORKSPACE_ID, failureReason, jobContext);

Expand Down Expand Up @@ -415,7 +415,7 @@ void testReportDiscoverJobFailure() throws JsonValidationException, ConfigNotFou

final StandardWorkspace mWorkspace = Mockito.mock(StandardWorkspace.class);
Mockito.when(mWorkspace.getWorkspaceId()).thenReturn(WORKSPACE_ID);
Mockito.when(configRepository.getStandardWorkspace(WORKSPACE_ID, true)).thenReturn(mWorkspace);
Mockito.when(configRepository.getStandardWorkspaceNoSecrets(WORKSPACE_ID, true)).thenReturn(mWorkspace);

jobErrorReporter.reportDiscoverJobFailure(SOURCE_DEFINITION_ID, WORKSPACE_ID, failureReason, jobContext);

Expand Down
Loading