🎉 Integration Testing for SSH using a docker container | Postgres Source and Destination update integration tests using ssh bastion in docker container #6312

VitaliiMaltsev · 2021-09-20T11:42:48Z

What

Improve the process of acceptance testing by allowing doing acceptance / integration testing using a bastion running in a docker container. Right now we have a dedicated host in AWS that we use.

Why do we care?

We'd rather not maintain the bastion infra if we don't have to
It means that the tests require secrets as an input which adds a bit more friction. If we use docker that would not be necessary.
It requires having a static instance of the database we trying to test running! Because the bastion is static it also needs to be forwarding to some database in our VPC so we need a statically running db as well. This limits the tests we can right and means we're paying for and maintaining a database for no good reason. Ideally we could spin up dbs and bastion all in docker!

How

A new SshBastion class has been implemented, which allows you to initialize and run a bastion ssh server created in a docker test container. The test database and bastion run in containers on the same network. Connection configuration for integration tests is now taken directly from container settings and does not require a real database connection. Establishing an ssh tunnel implies 2 authentication options - using a password or using a private key

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
Credentials added to Github CI. Instructions.
/test connector=connectors/<name> command is passing.
New Connector version released on Dockerhub by running the /publish command described here

Updating a connector

Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
Credentials added to Github CI. Instructions.
/test connector=connectors/<name> command is passing.
New Connector version released on Dockerhub by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed.

…gres testcontainer

…ion tests

…hBastion

…g-in-docker-containers

…ource and destination to 0.3.12

alexandr-shegeda

LGTM

DoNotPanicUA · 2021-09-21T15:49:59Z

I would only recommend not to increase the version at files airbyte-config/init/src/main/resources/seed/destination_definitions.yaml like that before you get external approval. If you increase the version before pulling the image, builds, and tests fail.

sherifnada

Looks great! a few comments:

Is there a reason we can't treat the bastion as a normal object the same way other test containers are? Right now it's static/global which could introduce isolation issues
could you remove the places where we inject secrets in ci_credentials.sh for the current postgres ssh test case and once you merge this pr remove them from environment secrets as well in github?
you actually don't need to bump the postgres container versions since this is just changing their tests not the connector itself

sherifnada · 2021-09-23T18:13:20Z

docs/integrations/sources/postgres.md

@@ -263,14 +263,21 @@ Using this feature requires additional configuration, when creating the source.
 6. If you are using `Password Authentication`, then `SSH Login Username` should be set to the password of the User from the previous step. If you are using `SSH Key Authentication` leave this blank. Again, this is not the Postgres password, but the password for the OS-user that Airbyte is using to perform commands on the bastion.
 7. If you are using `SSH Key Authentication`, then `SSH Private Key` should be set to the RSA Private Key that you are using to create the SSH connection. This should be the full contents of the key file starting with `-----BEGIN RSA PRIVATE KEY-----` and ending with `-----END RSA PRIVATE KEY-----`.

+
+## Integration tests to Postgres via an SSH Tunnel with bastion in docker container


this should go in the README for the connector rather than this doc. This doc is meant for consumption by Airbyte users, and this is an implementation detail of how we test the connector

this should go in the README for the connector rather than this doc. This doc is meant for consumption by Airbyte users, and this is an implementation detail of how we test the connector

removed description from this doc

sherifnada · 2021-09-23T18:15:18Z

...-integrations/bases/base-java/src/main/java/io/airbyte/integrations/base/ssh/SshBastion.java

+  private static final String SSH_USER = "sshuser";
+  private static final String SSH_PASSWORD = "secret";
+  private static Network network;
+  private static GenericContainer bastion;


why is this all static instead of instance variables?

why is this all static instead of instance variables?

replaced with instance variables

sherifnada · 2021-09-23T18:21:01Z

...-integrations/bases/base-java/src/main/java/io/airbyte/integrations/base/ssh/SshBastion.java

+import org.testcontainers.containers.Network;
+import org.testcontainers.images.builder.ImageFromDockerfile;
+
+public class SshBastion {


I suggest adding the word Container to the. name e.g: SshBastionContainer to make it clear it's containerized

I suggest adding the word Container to the. name e.g: SshBastionContainer to make it clear it's containerized

good point! Renamed class to SshBastionContainer

sherifnada · 2021-09-23T18:22:38Z

...tegrations/io/airbyte/integration_tests/sources/AbstractSshPostgresSourceAcceptanceTest.java

@@ -24,45 +24,79 @@

 package io.airbyte.integrations.io.airbyte.integration_tests.sources;

+import static io.airbyte.integrations.base.ssh.SshBastion.*;


style: prefer using explicit import instead of star imports

style: prefer using explicit import instead of star imports

replaced star import with explicit

…stionContainer, removed ci credentials for ssh Postgres Source and Destination

VitaliiMaltsev · 2021-09-23T19:43:42Z

Looks great! a few comments:

Is there a reason we can't treat the bastion as a normal object the same way other test containers are? Right now it's static/global which could introduce isolation issues

could you remove the places where we inject secrets in ci_credentials.sh for the current postgres ssh test case and once you merge this pr remove them from environment secrets as well in github?

you actually don't need to bump the postgres container versions since this is just changing their tests not the connector itself

Yes, you are right. Replaced static initialization with initialization of the new instance of class
Done
Revert changes with version bumping

…g-in-docker-containers # Conflicts: # tools/bin/ci_credentials.sh

sherifnada

Great work!

cgardens · 2021-09-27T15:33:01Z

@VitaliiMaltsev this is awesome! it will make developing new ssh connectors much easier. great work.

ChristopheDuong · 2022-02-28T10:36:11Z

airbyte-integrations/bases/base-java/src/main/java/io/airbyte/integrations/base/ssh/readme.md

+### Acceptance Testing via ssh tunnel using SshBastion and JdbcDatabaseContainer in Docker 
+1. The `SshBastion` class provides 3 helper functions:
+   `initAndStartBastion()`to initialize and start SSH Bastion server in Docker test container and creates new `Network` for bastion and tested jdbc container
+   `getTunnelConfig()`which return JsoneNode with all necessary configuration to establish ssh tunnel. Connection configuration for integration tests is now taken directly from container settings and does not require a real database connection
+   `stopAndCloseContainers` to stop and close SshBastion and JdbcDatabaseContainer at the end of the test
+


Does this mean we can update the Future Work section of this readme.md file and remove the bullet point?

Improve the process of acceptance testing by allowing doing acceptance testing using a bastion running in a docker container instead of having to use dedicated infrastructure and a static database.

How could we enable these ssh integration tests to other destinations too?

Would these integration tests catch errors in ssh tunnels handling with normalization too? see https://github.com/airbytehq/oncall/issues/145

vmaltsev added 9 commits September 14, 2021 16:41

ssh-test

72f474f

add authentification via ssh tunnel with bastion docker host and post…

bb7dcbb

…gres testcontainer

created SshBastion class in base-java module

79a1002

implement Postgres source basic ssh tunneling connection for integrat…

08a1d31

…ion tests

implement Postgres source ssh tunneling connection and refactoring Ss…

67cfc01

…hBastion

generate keys inside a bastion container

43aa77a

remove throwing Exception from startTestContainers method

82e9598

fix checkstyle

4b42a9d

Merge branch 'master' into vmaltsev/5811ssh-tunnel-integration-testin…

bd40929

…g-in-docker-containers

github-actions bot added the area/connectors Connector related issues label Sep 20, 2021

VitaliiMaltsev temporarily deployed to more-secrets September 20, 2021 11:43 Inactive

VitaliiMaltsev changed the title ~~Vmaltsev/5811ssh tunnel integration testing in docker containers~~ 🎉 Integration Testing for SSH using a docker container Sep 20, 2021

add documentation and changelog for Posthres source and destination

035241c

github-actions bot added the area/documentation Improvements or additions to documentation label Sep 20, 2021

VitaliiMaltsev temporarily deployed to more-secrets September 20, 2021 13:17 Inactive

update documentation for ssh readme.md | update version fo Postgres s…

c07ea7a

…ource and destination to 0.3.12

VitaliiMaltsev temporarily deployed to more-secrets September 20, 2021 13:47 Inactive

VitaliiMaltsev changed the title ~~🎉 Integration Testing for SSH using a docker container~~ 🎉 Integration Testing for SSH using a docker container | Postgres Source and Destination update integration tests using ssh bastion in docker container Sep 20, 2021

update version of Postgres source and destination to 0.3.12

bb35359

VitaliiMaltsev temporarily deployed to more-secrets September 20, 2021 14:23 Inactive

VitaliiMaltsev requested review from DoNotPanicUA and alexandr-shegeda September 20, 2021 14:24

VitaliiMaltsev linked an issue Sep 20, 2021 that may be closed by this pull request

Integration Testing for SSH using a docker container #5811

Closed

alexandr-shegeda approved these changes Sep 21, 2021

View reviewed changes

DoNotPanicUA approved these changes Sep 21, 2021

View reviewed changes

VitaliiMaltsev marked this pull request as ready for review September 22, 2021 09:19

VitaliiMaltsev requested a review from sherifnada September 22, 2021 09:19

sherifnada requested a review from cgardens September 23, 2021 18:09

sherifnada reviewed Sep 23, 2021

View reviewed changes

removed static variables, removed version bump, rename class to SshBa…

b90272e

…stionContainer, removed ci credentials for ssh Postgres Source and Destination

github-actions bot removed the area/documentation Improvements or additions to documentation label Sep 23, 2021

VitaliiMaltsev temporarily deployed to more-secrets September 23, 2021 19:36 Inactive

Merge branch 'master' into vmaltsev/5811ssh-tunnel-integration-testin…

27a40f2

…g-in-docker-containers # Conflicts: # tools/bin/ci_credentials.sh

VitaliiMaltsev temporarily deployed to more-secrets September 23, 2021 19:55 Inactive

VitaliiMaltsev requested a review from sherifnada September 23, 2021 19:55

sherifnada approved these changes Sep 23, 2021

View reviewed changes

VitaliiMaltsev temporarily deployed to more-secrets September 24, 2021 06:53 Inactive

VitaliiMaltsev mentioned this pull request Sep 24, 2021

🎉 Destination Oracle - Added support for connection via ssh tunnel #6370

Merged

38 tasks

VitaliiMaltsev merged commit ec3951b into master Sep 24, 2021

VitaliiMaltsev deleted the vmaltsev/5811ssh-tunnel-integration-testing-in-docker-containers branch September 24, 2021 17:00

jrhizor mentioned this pull request Sep 25, 2021

Bump Airbyte version from 0.29.21-alpha to 0.29.22-alpha #6450

Merged

alexandr-shegeda mentioned this pull request Sep 28, 2021

🎉 Source MSSQL: Implement SSH bastion for acceptance/integration tests #6502

Closed

ChristopheDuong reviewed Feb 28, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 Integration Testing for SSH using a docker container | Postgres Source and Destination update integration tests using ssh bastion in docker container #6312

🎉 Integration Testing for SSH using a docker container | Postgres Source and Destination update integration tests using ssh bastion in docker container #6312

VitaliiMaltsev commented Sep 20, 2021 •

edited

Loading

alexandr-shegeda left a comment

DoNotPanicUA commented Sep 21, 2021

sherifnada left a comment •

edited

Loading

sherifnada Sep 23, 2021

VitaliiMaltsev Sep 23, 2021

sherifnada Sep 23, 2021

VitaliiMaltsev Sep 23, 2021

sherifnada Sep 23, 2021

VitaliiMaltsev Sep 23, 2021

sherifnada Sep 23, 2021

VitaliiMaltsev Sep 23, 2021

VitaliiMaltsev commented Sep 23, 2021

sherifnada left a comment

cgardens commented Sep 27, 2021 •

edited

Loading

ChristopheDuong Feb 28, 2022 •

edited

Loading

		@@ -24,45 +24,79 @@

		package io.airbyte.integrations.io.airbyte.integration_tests.sources;

		import static io.airbyte.integrations.base.ssh.SshBastion.*;

🎉 Integration Testing for SSH using a docker container | Postgres Source and Destination update integration tests using ssh bastion in docker container #6312

🎉 Integration Testing for SSH using a docker container | Postgres Source and Destination update integration tests using ssh bastion in docker container #6312

Conversation

VitaliiMaltsev commented Sep 20, 2021 • edited Loading

What

How

Recommended reading order

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

alexandr-shegeda left a comment

Choose a reason for hiding this comment

DoNotPanicUA commented Sep 21, 2021

sherifnada left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VitaliiMaltsev commented Sep 23, 2021

sherifnada left a comment

Choose a reason for hiding this comment

cgardens commented Sep 27, 2021 • edited Loading

ChristopheDuong Feb 28, 2022 • edited Loading

Choose a reason for hiding this comment

VitaliiMaltsev commented Sep 20, 2021 •

edited

Loading

sherifnada left a comment •

edited

Loading

cgardens commented Sep 27, 2021 •

edited

Loading

ChristopheDuong Feb 28, 2022 •

edited

Loading