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.

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

Add Airbyte rock #18

Merged
merged 32 commits into from
Oct 17, 2024
Merged

Add Airbyte rock #18

merged 32 commits into from
Oct 17, 2024

Conversation

kelkawi-a
Copy link
Collaborator

@kelkawi-a kelkawi-a commented Oct 7, 2024

This PR builds the Airbyte app from source using Rockcraft, and builds the Airbyte charm with it. A few things to note:

  • Upgraded to v0.63.8 from v0.60.0 due to a version mismatch with the destination connector introduced in the integration test. Versions past v0.63.8 introduce breaking changes to the charm and the overall structure of Airbyte. A future update will look into updating to the latest v1.x version.
  • Part of the Airbyte gradle build spins up a docker image for generating database migration queries. This was initially not possible to do through the integration tests, so I landed this PR on the operator workflows to enable a setting on the lxd container running the tests to allow this.
  • In initial development, I had everything done in one part. However, the gradle build was hanging after a successful build. This seems to not be an issue when the gradle build is separated into its own part.
  • Including kubectl as part of stage-snaps did not seem to include it in the final build, hence why I manually install it using override-build.

@kelkawi-a kelkawi-a force-pushed the airbyte-rock branch 29 times, most recently from 96c0b50 to b5f852d Compare October 8, 2024 12:52
airbyte_rock/rockcraft.yaml Show resolved Hide resolved
airbyte_rock/rockcraft.yaml Outdated Show resolved Hide resolved
airbyte_rock/rockcraft.yaml Outdated Show resolved Hide resolved
airbyte_rock/rockcraft.yaml Outdated Show resolved Hide resolved
airbyte_rock/rockcraft.yaml Outdated Show resolved Hide resolved
airbyte_rock/rockcraft.yaml Show resolved Hide resolved
Copy link

@linostar linostar left a comment

Choose a reason for hiding this comment

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

LGTM!

@kelkawi-a kelkawi-a force-pushed the airbyte-rock branch 4 times, most recently from ad2fec8 to b0daddc Compare October 14, 2024 21:35
@kelkawi-a kelkawi-a marked this pull request as ready for review October 15, 2024 08:26
Copy link

@AmberCharitos AmberCharitos left a comment

Choose a reason for hiding this comment

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

Great work! Some small comments but nothing blocking. Nice work on a complex rock!

mkdir -p ${CRAFT_PART_INSTALL}/usr/local/lib/python3.10/dist-packages
cp -r $(which kubectl) ${CRAFT_PART_INSTALL}/usr/local/bin/kubectl

pip install --upgrade setuptools pip airbyte-cdk==5.12.0 \

Choose a reason for hiding this comment

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

If you end up having more of these in future you can add a requirements.txt file. Not necessary now.

- airbyte-connector-builder-server

local-files:
after: [organize-tars]

Choose a reason for hiding this comment

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

Does this really need to go after organize-tars?

curl -fsSL https://pkgs.k8s.io/core:/stable:/v1.31/deb/Release.key | gpg --dearmor -o /etc/apt/keyrings/kubernetes-apt-keyring.gpg
chmod 644 /etc/apt/keyrings/kubernetes-apt-keyring.gpg # allow unprivileged APT programs to read this keyring
apt-get update
apt-get install -y kubectl

Choose a reason for hiding this comment

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

Can this not be a build-package?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It actually needs to be included in the final build as the application itself uses kubectl. Including it as a stage-snap did not work for some reason, so I opted to install it manually.

# stdlib CVEs
CVE-2024-24790
CVE-2023-24538
CVE-2023-24540
Copy link

Choose a reason for hiding this comment

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

If they are not present anymore, we can remove it

@kelkawi-a kelkawi-a merged commit 9e14108 into main Oct 17, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants