Repository navigation
feat(source-ashby): add metadata-driven smoke-test scenario and 1Password CI creds - #77129
Aaron ("AJ") Steers (aaronsteers) wants to merge 3 commits into
Conversation
…word CI creds
Adds the first concrete consumer of the metadata.yaml smoke-test dialect:
* source-ashby/metadata.yaml grows a smokeTests suite with one default scenario whose configSettings reference the literal 1Password section name (no prefix-stripping):
api_key: ${secret-ref:ashby/test-credentials-api_key/api_key}
start_date: ${relative-date:-P30D}
* connector-ci-checks.yml exposes AIRBYTE_TEST_CREDS_PROVIDER, AIRBYTE_TEST_CREDS_1PASS_VAULT_NAME, and OP_SERVICE_ACCOUNT_TOKEN to both jvm-connectors-test and non-jvm-connectors-test jobs. The vault name is overridable via repository vars and falls back to the existing 'connector-integration-test-creds' default. The provider/vault env vars are required only when a ${secret-ref:...} sigil is actually present, so legacy connectors that fetch from GSM are unaffected.
Functional resolution of the new sigils requires the airbyte-python-cdk #999 scenario loader and the airbyte-ops-mcp per-section .env.<section> writer to land first; this PR sets up the CI and metadata shape so those follow-ups can ship without further monorepo changes.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
|
…orks Per review feedback: main repo doesn't ship a default smoke-test provider; forks opt in by setting `AIRBYTE_TEST_CREDS_PROVIDER` and `OP_SERVICE_ACCOUNT_TOKEN` themselves. Vault name fallback stays so forks can override via repository vars without changing the workflow.
| # are present; legacy GSM-fetched connectors are unaffected. | ||
| AIRBYTE_TEST_CREDS_PROVIDER: 1pass | ||
| AIRBYTE_TEST_CREDS_1PASS_VAULT_NAME: ${{ vars.AIRBYTE_TEST_CREDS_1PASS_VAULT_NAME || 'connector-integration-test-creds' }} | ||
| OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} |
There was a problem hiding this comment.
Here and above, let's rename OP_SERVICE_ACCOUNT_TOKEN to AIRBYTE_TEST_CREDS_1PASS_SERVICE_TOKEN
There was a problem hiding this comment.
Done in 6e46be5 — renamed at both call sites and updated the comment to reflect the new name.
Note for downstream wiring: the underlying op CLI reads OP_SERVICE_ACCOUNT_TOKEN natively, so the OPS CLI consumer (which is what actually shells out to op) will need to read AIRBYTE_TEST_CREDS_1PASS_SERVICE_TOKEN and re-export it as OP_SERVICE_ACCOUNT_TOKEN for the subprocess. I'll handle that translation in the OPS CLI follow-up PR so this naming convention stays clean at the workflow boundary.
Per review feedback: rename to `AIRBYTE_TEST_CREDS_1PASS_SERVICE_TOKEN` to keep all smoke-test creds env vars in the same `AIRBYTE_TEST_CREDS_1PASS_*` namespace. The downstream consumer (OPS CLI) is responsible for translating this to `OP_SERVICE_ACCOUNT_TOKEN` for the underlying `op` invocation.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
↪️ Triggering Reason: PR is ready for review with no prior AI review on file. |
Reviewing PR for connector safety and quality.
|
AI PR Review ReportReview Action: REQUEST CHANGES
🔧 Remediation RequiredRequired ActionsCI Checks (FAIL) Both This failure occurs at the Note: This appears to be a pre-existing CI infrastructure issue, not caused by this PR's changes. The
📋 PR DetailsConnector & PR InfoConnector(s): Risk LevelLevel: 2 — Low Risk Level is reported for downstream consumers (e.g. auto-merge policy, reviewer routing). It does not change the review action — APPROVE here means "no blocking objection," not "safe to merge unattended." Review Action DetailsREQUEST CHANGES — Submitted PR review requesting changes due to the failing
🔍 Gate Evaluation DetailsGate-by-Gate Analysis
PR Hygiene — Evidence:
CI Checks — Evidence:
Notable: CI workflow changes 📚 Evidence ConsultedEvidence
❓ How to RespondProviding Context or JustificationYou can add explanations that the bot will see on the next review: Option 1: PR Description (recommended) ## AI PR Review Justification
### CI Checks
[Your explanation here]Option 2: PR Comment After adding your response, re-run Note: The CI Checks failure appears to be an infrastructure issue ( |
What
First concrete consumer of the new
metadata.yamlsmoke-test dialect (proposed in airbytehq/airbyte-connector-models#42 and consumed in airbytehq/airbyte-python-cdk#999) plus the CI plumbing that exposes 1Password-backed credentials to the connector test jobs.Two changes, intentionally combined per request:
airbyte-integrations/connectors/source-ashby/metadata.yaml— adds asmokeTestssuite with onedefaultscenario:The
secret-refsigil uses the literal 1Password section name (test-credentials-api_key) — no prefix-stripping. Therelative-datesigil renders to an RFC 3339 timestamp anchored at today's UTC midnight (e.g.2026-03-22T00:00:00Z)..github/workflows/connector-ci-checks.yml— adds three env vars to bothjvm-connectors-testandnon-jvm-connectors-testjobs:The vault name is overridable via repo
varsand falls back to the existing default. These env vars are required only when a${secret-ref:...}sigil actually appears inconfigSettings; legacy GSM-fetched connectors are unaffected.How
connectorTestSuitesOptionsalready listsunitTests, this just appendssmokeTests. No spec, dockerImageTag, or other connector-runtime fields are touched.GCP_GSM_CREDENTIALS/GCP_PROJECT_IDblock, with an inline comment explaining thesmokeTestsopt-in semantics. Missing org-level secrets resolve to empty strings, which is fine — the CDK only validates the provider env vars when a sigil is actually present.Review guide
airbyte-integrations/connectors/source-ashby/metadata.yaml— confirm 3-segment sigil shape and literal section name..github/workflows/connector-ci-checks.yml— two near-identical env blocks (jvm + non-jvm jobs).User Impact
None today. This PR is structurally complete but functionally inert until two follow-ups land:
.env.<literal-section>writer that the CDK reads.Once those land,
airbyte-ci connectors test --name=source-ashbywill auto-discover the smoke-test scenario, fetch creds from 1Password (vaultconnector-integration-test-creds, itemashby, sectiontest-credentials-api_key), and run a check/spec/read against a freshstart_date = today - 30 daysconfiguration.Legacy GSM-based credential fetching for any other connector is not affected by this change.
Can this PR be safely reverted and rolled back?
The metadata.yaml addition is additive (no existing scenarios consume it yet). The workflow env vars only matter for connectors that opt in via a
smokeTestsblock, so reverting just removes plumbing that nothing in production reads today.Link to Devin session: https://app.devin.ai/sessions/5c2ccf537b70486b9e6fbc974d26063e
Requested by: Aaron ("AJ") Steers (@aaronsteers)