Pass allowedHosts to container runners #21676

evantahler · 2023-01-20T20:12:43Z

This PR follows #21363 and closes #21797, passing the AllowedHosts to the docker/K8s container executors. This PR stops at logging the AllowedHost information as the container is started, showing that the information is being passed though the Temporal jobs properly. Usage of this information in the K8s launcher will be done in a later story by @git-phu.

As part of this work, we needed to resolve dynamic allowed hosts from the connector's configuration. I went with StringSubstitutor for this purpose. As an example, you can see how source-postgres now has an allowed host limitation of "${host}" which is the value of "host" from the configuration of that instance of the connector. this can be verified via the logs, as "host.docker.internal" is my configuration:

airbyte-worker | 2023-01-20 23:35:35 INFO i.a.w.p.DockerProcessFactory(create):122 - Creating docker container = source-postgres-read-8-0-evhis with resources io.airbyte.config.ResourceRequirements@5f1218b0[cpuRequest=,cpuLimit=,memoryRequest=,memoryLimit=] and allowed hosts io.airbyte.config.AllowedHosts@4d32a0ff[hosts=[host.docker.internal],additionalProperties={}]

github-actions · 2023-01-20T20:14:48Z

Affected Connector Report

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

Run integration tests
Bump connector or module version
Add changelog
Publish the new version

✅ Sources (30)

Connector	Version	Changelog	Publish
`source-alloydb`	`1.0.35`	✅	✅
`source-alloydb-strict-encrypt`	`1.0.35`	🔵 (ignored)	🔵 (ignored)
`source-bigquery`	`0.2.3`	✅	✅
`source-clickhouse`	`0.1.15`	✅	✅
`source-clickhouse-strict-encrypt`	`0.1.15`	🔵 (ignored)	🔵 (ignored)
`source-cockroachdb`	`0.1.19`	✅	✅
`source-cockroachdb-strict-encrypt`	`0.1.19`	🔵 (ignored)	🔵 (ignored)
`source-db2`	`0.1.17`	✅	✅
`source-db2-strict-encrypt`	`0.1.17`	🔵 (ignored)	🔵 (ignored)
`source-dynamodb`	`0.1.0`	✅	✅
`source-e2e-test`	`2.1.3`	✅	✅
`source-e2e-test-cloud`	`2.1.1`	🔵 (ignored)	🔵 (ignored)
`source-elasticsearch`	`0.1.1`	✅	✅
`source-jdbc`	`0.3.5`	🔵 (ignored)	🔵 (ignored)
`source-kafka`	`0.2.3`	✅	✅
`source-mongodb-strict-encrypt`	`0.1.19`	🔵 (ignored)	🔵 (ignored)
`source-mongodb-v2`	`0.1.19`	✅	✅
`source-mssql`	`0.4.28`	✅	✅
`source-mssql-strict-encrypt`	`0.4.28`	🔵 (ignored)	🔵 (ignored)
`source-mysql`	`1.0.19`	✅	✅
`source-mysql-strict-encrypt`	`1.0.19`	🔵 (ignored)	🔵 (ignored)
`source-oracle`	`0.3.22`	✅	✅
`source-oracle-strict-encrypt`	`0.3.22`	🔵 (ignored)	🔵 (ignored)
`source-postgres`	`1.0.39`	✅	✅
`source-postgres-strict-encrypt`	`1.0.39`	🔵 (ignored)	🔵 (ignored)
`source-redshift`	`0.3.16`	✅	✅
`source-scaffold-java-jdbc`	`0.1.0`	🔵 (ignored)	🔵 (ignored)
`source-sftp`	`0.1.2`	✅	✅
`source-snowflake`	`0.1.29`	✅	✅
`source-tidb`	`0.2.2`	✅	✅

See "Actionable Items" below for how to resolve warnings and errors.

❌ Destinations (48)

Connector	Version	Changelog	Publish
`destination-aws-datalake`	`0.1.1`	✅	✅
`destination-azure-blob-storage`	`0.1.6`	✅	✅
`destination-bigquery`	`1.2.12`	✅	✅
`destination-bigquery-denormalized`	`1.2.12`	✅	❌ (diff seed version)
`destination-cassandra`	`0.1.4`	✅	✅
`destination-clickhouse`	`0.2.2`	❌ (changelog missing)	✅
`destination-clickhouse-strict-encrypt`	`0.2.2`	🔵 (ignored)	🔵 (ignored)
`destination-csv`	`1.0.0`	❌ (changelog missing)	✅
`destination-databricks`	`0.3.1`	✅	✅
`destination-dev-null`	`0.2.7`	🔵 (ignored)	🔵 (ignored)
`destination-doris`	`0.1.0`	✅	✅
`destination-dynamodb`	`0.1.7`	✅	✅
`destination-e2e-test`	`0.2.4`	✅	✅
`destination-elasticsearch`	`0.1.6`	✅	✅
`destination-elasticsearch-strict-encrypt`	`0.1.6`	🔵 (ignored)	🔵 (ignored)
`destination-gcs`	`0.2.13`	✅	✅
`destination-iceberg`	`0.1.0`	✅	✅
`destination-jdbc`	`0.3.14`	🔵 (ignored)	🔵 (ignored)
`destination-kafka`	`0.1.10`	✅	✅
`destination-keen`	`0.2.4`	✅	✅
`destination-kinesis`	`0.1.5`	✅	✅
`destination-local-json`	`0.2.11`	✅	✅
`destination-mariadb-columnstore`	`0.1.7`	✅	✅
`destination-mongodb`	`0.1.9`	✅	✅
`destination-mongodb-strict-encrypt`	`0.1.9`	🔵 (ignored)	🔵 (ignored)
`destination-mqtt`	`0.1.3`	✅	✅
`destination-mssql`	`0.1.22`	✅	✅
`destination-mssql-strict-encrypt`	`0.1.22`	🔵 (ignored)	🔵 (ignored)
`destination-mysql`	`0.1.20`	✅	✅
`destination-mysql-strict-encrypt`	❌ `0.1.21` (mismatch: `0.1.20`)	🔵 (ignored)	🔵 (ignored)
`destination-oracle`	`0.1.19`	✅	✅
`destination-oracle-strict-encrypt`	`0.1.19`	🔵 (ignored)	🔵 (ignored)
`destination-postgres`	`0.3.26`	✅	✅
`destination-postgres-strict-encrypt`	`0.3.26`	🔵 (ignored)	🔵 (ignored)
`destination-pubsub`	`0.2.0`	✅	✅
`destination-pulsar`	`0.1.3`	✅	✅
`destination-r2`	`0.1.0`	✅	✅
`destination-redis`	`0.1.4`	✅	✅
`destination-redpanda`	`0.1.0`	✅	✅
`destination-redshift`	`0.3.54`	✅	✅
`destination-rockset`	`0.1.4`	✅	✅
`destination-s3`	`0.3.19`	✅	✅
`destination-s3-glue`	`0.1.1`	✅	✅
`destination-scylla`	`0.1.3`	✅	✅
`destination-snowflake`	`0.4.44`	✅	✅
`destination-teradata`	`0.1.0`	✅	✅
`destination-tidb`	`0.1.0`	✅	✅
`destination-yugabytedb`	`0.1.0`	✅	✅

See "Actionable Items" below for how to resolve warnings and errors.

✅ Other Modules (0)

Actionable Items

(click to expand)

Category	Status	Actionable Item
Version	❌ mismatch	The version of the connector is different from its normal variant. Please bump the version of the connector.
	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
Changelog	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
	❌ changelog missing	There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish	⚠ not in seed	The connector is not in the seed file (e.g. `source_definitions.yaml`), so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that it is not a bug.
	❌ diff seed version	The connector exists in the seed file, but the latest version is not listed there. This usually means that the latest version is not published. Please use the `/publish` command to publish the latest version.

github-actions · 2023-01-20T20:29:15Z

Airbyte Code Coverage

File	Coverage [43.07%]	❌
ProcessFactory.java	100%	🍏
AirbyteIntegrationLauncher.java	100%	🍏
ConfigReplacer.java	97.78%	🍏
DbConverter.java	80.16%	🍏
DockerProcessFactory.java	72.69%	🍏
DefaultNormalizationRunner.java	66.75%	🍏
ReplicationJobOrchestrator.java	11.68%	❌
ReplicationActivityImpl.java	0.68%	❌
DbtTransformationRunner.java	0%	❌
EntrypointEnvChecker.java	0%	❌
KubeProcessFactory.java	0%	❌
DiscoverCatalogActivityImpl.java	0%	❌
CheckConnectionActivityImpl.java	0%	❌
SpecActivityImpl.java	0%	❌
GenerateInputActivityImpl.java	0%	❌

Total Project Coverage	26.73%	🍏

evantahler · 2023-01-20T23:53:30Z

This work seems to have introduced a bug. @benmoriceau or @jdpgrailsdev do you happen to see what I might have done wrong? Any sync I run on this branch produces an error like this:

2023-01-20 23:52:39 ERROR i.a.s.h.JobHistoryHandler(listJobsFor):138 - Missing stats for job 1 attempt 0

airbyte-worker                    | java.lang.NullPointerException: Cannot invoke "Object.hashCode()" because "pk" is null
airbyte-worker                    | 	at java.util.ImmutableCollections$MapN.probe(ImmutableCollections.java:1321) ~[?:?]
airbyte-worker                    | 	at java.util.ImmutableCollections$MapN.get(ImmutableCollections.java:1235) ~[?:?]
airbyte-worker                    | 	at java.util.Comparator.lambda$comparingInt$7b0bb60$1(Comparator.java:494) ~[?:?]
airbyte-worker                    | 	at java.util.TimSort.countRunAndMakeAscending(TimSort.java:355) ~[?:?]
airbyte-worker                    | 	at java.util.TimSort.sort(TimSort.java:220) ~[?:?]
airbyte-worker                    | 	at java.util.Arrays.sort(Arrays.java:1307) ~[?:?]
airbyte-worker                    | 	at java.util.ArrayList.sort(ArrayList.java:1721) ~[?:?]
airbyte-worker                    | 	at io.airbyte.workers.temporal.scheduling.activities.JobCreationAndStatusUpdateActivityImpl.orderByReleaseStageAsc(JobCreationAndStatusUpdateActivityImpl.java:488) ~[io.airbyte-airbyte-workers-0.40.28.jar:?]
airbyte-worker                    | 	at io.airbyte.workers.temporal.scheduling.activities.JobCreationAndStatusUpdateActivityImpl.emitAttemptEvent(JobCreationAndStatusUpdateActivityImpl.java:517) ~[io.airbyte-airbyte-workers-0.40.28.jar:?]
airbyte-worker                    | 	at io.airbyte.workers.temporal.scheduling.activities.JobCreationAndStatusUpdateActivityImpl.emitAttemptCompletedEvent(JobCreationAndStatusUpdateActivityImpl.java:558) ~[io.airbyte-airbyte-workers-0.40.28.jar:?]
airbyte-worker                    | 	at io.airbyte.workers.temporal.scheduling.activities.JobCreationAndStatusUpdateActivityImpl.emitAttemptCompletedEventIfAttemptPresent(JobCreationAndStatusUpdateActivityImpl.java:587) ~[io.airbyte-airbyte-workers-0.40.28.jar:?]
airbyte-worker                    | 	at io.airbyte.workers.temporal.scheduling.activities.JobCreationAndStatusUpdateActivityImpl.trackCompletion(JobCreationAndStatusUpdateActivityImpl.java:576) ~[io.airbyte-airbyte-workers-0.40.28.jar:?]
airbyte-worker                    | 	at io.airbyte.workers.temporal.scheduling.activities.JobCreationAndStatusUpdateActivityImpl.jobSuccess(JobCreationAndStatusUpdateActivityImpl.java:260) ~[io.airbyte-airbyte-workers-0.40.28.jar:?]
airbyte-worker                    | 	at io.airbyte.workers.temporal.scheduling.activities.JobCreationAndStatusUpdateActivityImpl.jobSuccessWithAttemptNumber(JobCreationAndStatusUpdateActivityImpl.java:272) ~[io.airbyte-airbyte-workers-0.40.28.jar:?]
airbyte-worker                    | 	at jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) ~[?:?]
airbyte-worker                    | 	at java.lang.reflect.Method.invoke(Method.java:578) ~[?:?]
airbyte-worker                    | 	at io.temporal.internal.activity.RootActivityInboundCallsInterceptor$POJOActivityInboundCallsInterceptor.executeActivity(RootActivityInboundCallsInterceptor.java:64) ~[temporal-sdk-1.17.0.jar:?]
airbyte-worker                    | 	at io.temporal.internal.activity.RootActivityInboundCallsInterceptor.execute(RootActivityInboundCallsInterceptor.java:43) ~[temporal-sdk-1.17.0.jar:?]
airbyte-worker                    | 	at io.temporal.internal.activity.ActivityTaskExecutors$BaseActivityTaskExecutor.execute(ActivityTaskExecutors.java:95) ~[temporal-sdk-1.17.0.jar:?]
airbyte-worker                    | 	at io.temporal.internal.activity.ActivityTaskHandlerImpl.handle(ActivityTaskHandlerImpl.java:92) ~[temporal-sdk-1.17.0.jar:?]
airbyte-worker                    | 	at io.temporal.internal.worker.ActivityWorker$TaskHandlerImpl.handleActivity(ActivityWorker.java:241) ~[temporal-sdk-1.17.0.jar:?]
airbyte-worker                    | 	at io.temporal.internal.worker.ActivityWorker$TaskHandlerImpl.handle(ActivityWorker.java:206) ~[temporal-sdk-1.17.0.jar:?]
airbyte-worker                    | 	at io.temporal.internal.worker.ActivityWorker$TaskHandlerImpl.handle(ActivityWorker.java:179) ~[temporal-sdk-1.17.0.jar:?]
airbyte-worker                    | 	at io.temporal.internal.worker.PollTaskExecutor.lambda$process$0(PollTaskExecutor.java:93) ~[temporal-sdk-1.17.0.jar:?]
airbyte-worker                    | 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) ~[?:?]
airbyte-worker                    | 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) ~[?:?]
airbyte-worker                    | 	at java.lang.Thread.run(Thread.java:1589) ~[?:?]

benmoriceau · 2023-01-21T00:25:54Z

@evantahler I don't think it comes from you, it has the place where the NPE happens has been added in: #21286, Will try to come up with a fix.

benmoriceau · 2023-01-21T01:44:20Z

#21685 this should the sort NPE issue. It did on my local.

benmoriceau · 2023-01-23T16:51:21Z

@evantahler Just merged the fix, merging master should fix the issue.

evantahler · 2023-01-24T16:42:41Z

Ping @benmoriceau @pedroslopez @git-phu - I'd love a review please!

benmoriceau

LGTM

evantahler · 2023-01-25T00:04:39Z

airbyte-config/init/src/main/resources/seed/source_definitions.yaml

+  allowedHosts:
+    hosts:
+      - "${host}"


@git-phu I'd love your 👍 on how I ended up enabling interpolation. By the time we get to the KubeProcessFactory (or DockerProcessFactory), these strings are all resolved, but another pair of 👀 can't hurt.

pedroslopez

Looks good! Added a note on the new dependency of actor configuration for the IntegrationLauncherConfigs.

I'm assuming we decided we won't be needing to use any source other than actor configuration for the variables since the substitution isn't "namespaced" (e.g. just host rrather than something like config.host)

pedroslopez · 2023-01-25T00:46:46Z

...c/main/java/io/airbyte/workers/temporal/scheduling/activities/GenerateInputActivityImpl.java

@@ -109,7 +116,8 @@ public GeneratedJobInput getSyncWorkflowInput(final SyncInput input) {
          .withAttemptId((long) attempt)
          .withDockerImage(config.getSourceDockerImage())
          .withProtocolVersion(config.getSourceProtocolVersion())
-          .withIsCustomConnector(config.getIsSourceCustomConnector());
+          .withIsCustomConnector(config.getIsSourceCustomConnector())
+          .withAllowedHosts(configReplacer.getAllowedHosts(sourceDefinition.getAllowedHosts(), config.getSourceConfiguration()));


Note to self: this affects the intended plan for part 2 of the configuration update project. (it was proposed to build IntegrationLauncherConfigs separately at the beginning to re-use for the check and sync, but because this now depends on the source/destination configuration we should keep building the IntegrationLauncherConfigs whenever we generate the check and sync inputs)

evantahler added 3 commits January 20, 2023 11:39

Pass allowed hosts to Process Factories and log

4d34335

fix loading method

de7cb40

github allowed hosts

3785178

octavia-squidington-iv added area/platform issues related to the platform area/worker Related to worker labels Jan 20, 2023

evantahler temporarily deployed to more-secrets January 20, 2023 20:14 — with GitHub Actions Inactive

StringSubstitutor

b40404c

evantahler temporarily deployed to more-secrets January 20, 2023 23:37 — with GitHub Actions Inactive

lint

59a7d14

evantahler temporarily deployed to more-secrets January 20, 2023 23:45 — with GitHub Actions Inactive

evantahler requested a review from benmoriceau January 21, 2023 00:21

evantahler assigned davinchia and unassigned davinchia Jan 21, 2023

evantahler requested review from davinchia, pedroslopez and git-phu January 21, 2023 00:21

evantahler mentioned this pull request Jan 21, 2023

Github Action to enforce allowed hosts are set for GA and Beta connectors #21684

Merged

evantahler added 2 commits January 23, 2023 10:27

ConfigReplacerTest class + tests

8d2dc35

Merge branch 'master' into evan/log-allowed-hosts

97cba6f

evantahler temporarily deployed to more-secrets January 23, 2023 18:30 — with GitHub Actions Inactive

tests are private

1c11f9c

evantahler temporarily deployed to more-secrets January 23, 2023 18:36 — with GitHub Actions Inactive

evantahler temporarily deployed to more-secrets January 23, 2023 18:37 — with GitHub Actions Inactive

Merge branch 'master' into evan/log-allowed-hosts

cdb2220

evantahler marked this pull request as ready for review January 23, 2023 20:20

evantahler requested a review from a team as a code owner January 23, 2023 20:20

evantahler temporarily deployed to more-secrets January 23, 2023 20:22 — with GitHub Actions Inactive

evantahler requested a review from alafanechere January 23, 2023 21:15

benmoriceau approved these changes Jan 24, 2023

View reviewed changes

evantahler commented Jan 25, 2023

View reviewed changes

pedroslopez approved these changes Jan 25, 2023

View reviewed changes

evantahler merged commit e39ee94 into master Jan 25, 2023

evantahler deleted the evan/log-allowed-hosts branch January 25, 2023 00:55

github-actions bot mentioned this pull request Jan 25, 2023

Bump helm chart version reference to 0.43.22 #21833

Merged

This was referenced Jan 27, 2023

Bump Airbyte version from 0.40.29 to 0.40.30 #21954

Closed

Bump Airbyte version from 0.40.29 to 0.40.30 #21991

Closed

Bump Airbyte version from 0.40.29 to 0.40.30 #22045

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass allowedHosts to container runners #21676

Pass allowedHosts to container runners #21676

evantahler commented Jan 20, 2023 •

edited

Loading

github-actions bot commented Jan 20, 2023 •

edited

Loading

✅ Sources (30)

❌ Destinations (48)

✅ Other Modules (0)

Actionable Items

github-actions bot commented Jan 20, 2023 •

edited

Loading

evantahler commented Jan 20, 2023 •

edited

Loading

benmoriceau commented Jan 21, 2023 •

edited

Loading

benmoriceau commented Jan 21, 2023 •

edited

Loading

benmoriceau commented Jan 23, 2023

evantahler commented Jan 24, 2023

benmoriceau left a comment

evantahler Jan 25, 2023

pedroslopez left a comment

pedroslopez Jan 25, 2023

Pass allowedHosts to container runners #21676

Pass allowedHosts to container runners #21676

Conversation

evantahler commented Jan 20, 2023 • edited Loading

github-actions bot commented Jan 20, 2023 • edited Loading

Affected Connector Report

✅ Sources (30)

❌ Destinations (48)

✅ Other Modules (0)

Actionable Items

github-actions bot commented Jan 20, 2023 • edited Loading

Airbyte Code Coverage

evantahler commented Jan 20, 2023 • edited Loading

benmoriceau commented Jan 21, 2023 • edited Loading

benmoriceau commented Jan 21, 2023 • edited Loading

benmoriceau commented Jan 23, 2023

evantahler commented Jan 24, 2023

benmoriceau left a comment

Choose a reason for hiding this comment

evantahler Jan 25, 2023

Choose a reason for hiding this comment

pedroslopez left a comment

Choose a reason for hiding this comment

pedroslopez Jan 25, 2023

Choose a reason for hiding this comment

evantahler commented Jan 20, 2023 •

edited

Loading

github-actions bot commented Jan 20, 2023 •

edited

Loading

github-actions bot commented Jan 20, 2023 •

edited

Loading

evantahler commented Jan 20, 2023 •

edited

Loading

benmoriceau commented Jan 21, 2023 •

edited

Loading

benmoriceau commented Jan 21, 2023 •

edited

Loading