-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
airbyte-ci: enable the use of local connector secrets #31766
airbyte-ci: enable the use of local connector secrets #31766
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
3dd85a6
to
8f9131f
Compare
8f9131f
to
b0698e4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome improvement! LGTM.
Usually community connectors have only one secret file and it's easy to share a single credential with the contributor. This feature will help a lot! |
"DOCKER_HUB_USERNAME", | ||
"DOCKER_HUB_PASSWORD", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the case if the community member or us want to publish the connector from local?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, it's a check to make sure that these env var are set in the CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't test it myself, but code looks good! A few nits, but nothing blocking
airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/commands.py
Outdated
Show resolved
Hide resolved
airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/commands.py
Outdated
Show resolved
Hide resolved
airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/commands.py
Show resolved
Hide resolved
""" | ||
if context.use_remote_secrets: | ||
connector_secrets = await download(context) | ||
else: | ||
raise NotImplementedError("Local secrets are not implemented yet. See https://github.com/airbytehq/airbyte/issues/25621") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥳
airbyte-ci/connectors/pipelines/tests/test_commands/test_groups/test_connectors.py
Outdated
Show resolved
Hide resolved
a538baa
to
b1c0ac7
Compare
Motivation
Closes #25621
As
airbyte-ci
is now the recommended way to build and test connectors we should enable community users to use it.So far one blocker for community use was the fact that the
connectors
command group did not support the use of local connector secrets and relied on our GSM secrets store.We want the
airbyte-ci connectors
command to be runnable without theGCP_GSM_CREDENTIALS
env var and have a sensible default wether to use remote or local secrets if the--use-local-secret/--use-remote-secrets
flags are not passed.CLI behavior
If
--use-local-secret/--use-remote-secrets
flags are not passed:GCP_GSM_CREDENTIALS
env var is setGCP_GSM_CREDENTIALS
is not setIf
--use-local-secrets
is passed:We'll mount secrets from the connector
secrets
directory.We log a warning if this directory is empty and does not exist.
Unit/Integration/CAT tests will likely fail if these file are missing, we let them handle the error handling when the secret file is missing. (We have some connectors, like source-pokeapi, that do not require secret files).
If
--use-remote-secrets
:We check that
GCP_GSM_CREDENTIALS
is set or raise aclick.UsageError
exception otherwise.🚨 User Impact 🚨
Community users should be able to run the
test
command without access to our secret store.test
will use their local connector secrets.