fix(ci): seed placeholder credentials for SDK tests on fork pull requests - #2104
fix(ci): seed placeholder credentials for SDK tests on fork pull requests#2104thegoodengineer wants to merge 5 commits into
Conversation
The `Run SDK gRPC Tests` step fails whenever `creds.json` is not present, which is always the case for a pull request opened from a fork, because `secrets.CONNECTOR_SPECIFIC_AUTH` does not resolve there. The workflow already anticipates this: the `Create connector credentials` step falls through to "No CONNECTOR_SPECIFIC_AUTH secret found, running in dry-run mode", and `Run SDK FFI Tests` guards its own output on `[ -f creds.json ]`. The four gRPC targets do not agree on this. `sdk/javascript` and `sdk/python` degrade gracefully, so both pass with no credentials. `sdk/java` copies `creds.json` only when it exists and has no fallback, so `loadCredentials` throws `Credentials file not found: creds.json`. `sdk/rust` invokes the smoke test binary with no credentials argument at all, so it exits with `Configuration error: Credentials file 'creds.json' not found`. Both targets now fall back to the `creds_dummy.json` already committed at the repository root, which is the same fallback `test-package-mock` uses in both of these Makefiles. Placeholder credentials produce connector errors rather than transport errors, and both harnesses already treat those as non-fatal: `GrpcSmokeTest.kt` only sets `anyFailed` for transport failures, and `grpc_smoke_test.rs` records `connector_error` scenarios as `skipped` and passes when every scenario is `passed` or `skipped`. Behaviour is unchanged when `creds.json` is present.
The same gap exists in the FFI path. `Run SDK FFI Tests` announces "Running SDK FFI tests without connector credentials (dry-run mode)", but `test-package` in sdk/rust/Makefile invokes the smoke test binary with no `--creds-file`, so it defaults to `creds.json` and panics at sdk/rust/smoke-test/src/main.rs:338 with "Credentials file not found". The JavaScript, Python and Java `test-package` targets all guard on `[ -f creds.json ]` already; Rust was the only one without a guard. Resolve the credentials path once and reuse it in both the cached binary and the cargo run branch. The harness has placeholder handling (`is_placeholder`, `PLACEHOLDER_VALUES`) and records affected scenarios as skipped, so dummy credentials do not fail the run. The webhook smoke test needs no credentials and is left untouched.
…kefile Replaces the per SDK Makefile fallbacks from the previous two commits with a single fix at the source. The Makefile approach was correct but incomplete: the gRPC targets went green, which then exposed the same failure in the FFI targets for Rust, and behind that for Python and Java, since every harness requires creds.json to exist. All of those share one cause. The `Create connector credentials` step takes its else branch when `secrets.CONNECTOR_SPECIFIC_AUTH` does not resolve, and that branch only prints "running in dry-run mode" without producing a file. Seeding creds.json from the committed creds_dummy.json there fixes every SDK and both the gRPC and FFI paths at once, and leaves the Makefiles untouched. Net change against main is now six lines in one workflow step.
Seeding creds.json from creds_dummy.json got the gRPC suite, the Rust FFI
suite and the webhook suite green, but the Python FFI suite still failed:
FAILED: Invalid API Key provided: <REPLACE*************LUE>
The harnesses all detect unusable credentials with the same rule: an exact
match against {"", "placeholder", "test", "dummy", "sk_test_placeholder"} or a
value containing "placeholder" (is_placeholder in test_smoke.py and main.rs,
isPlaceholder in SmokeTest.kt). The tokens in creds_dummy.json are
"<REPLACE_WITH_YOUR_VALUE>" and "<REPLACE_WITH_PEM>", which match neither, so
the harnesses treated them as real keys and sent them to the connector.
Rewrite the tokens to "placeholder" while seeding, so the existing detection
fires and those connectors are skipped rather than called. creds_dummy.json is
left untouched, so the mock targets that consume it are unaffected.
One value, cybersource api_secret, carries the token without angle brackets, so
the expression matches both forms. Verified against the fixture: 131 credential
values across 73 connectors, all detected as placeholders, no tokens left.
|
Update: I have taken this as far as I think an outside contributor should, and the last step is a decision for you rather than a patch from me. Seeding The job still exits non-zero, and this is where it stops being a bug. The Rust FFI harness deliberately refuses to report success when nothing ran: That guard looks intentional, and I do not think it should be removed to get a green tick. Without real credentials there is genuinely nothing for the connector suites to assert, so the honest options are:
I am happy to implement whichever you prefer, or to close this if you would rather handle it internally. I have deliberately not touched the harness guard or the required check configuration, since both are your call. For context on why this matters beyond my own change: #2103 and #2080 are the two open pull requests from forks and both are blocked by this, and |
…sts-without-credentials
Description
SDK Testsis a required check that cannot pass on a pull request opened from a fork.secrets.CONNECTOR_SPECIFIC_AUTHdoes not resolve for forks, soCreate connector credentialstakes its else branch, which prints a message but never produces a file:Every smoke test harness then fails on the missing file rather than degrading:
This seeds
creds.jsonfrom thecreds_dummy.jsonalready committed at the repository root, which is what thetest-package-mocktargets in these same Makefiles already do.Motivation and Context
The workflow already intends to run without credentials.
Run SDK FFI TestsannouncesRunning SDK FFI tests without connector credentials (dry-run mode), and each SDK Makefile guards with[ -f creds.json ]. Those guards only print a note, though, so nothing supplies the file the harnesses require, and the dry-run mode never actually works.Fixing it in the workflow step covers all four SDKs and both the gRPC and FFI paths in one place. The alternative, adding a fallback to each Makefile, needs eight separate edits and leaves the same trap for anything added later.
Placeholder credentials produce connector errors rather than transport errors, and every harness already treats those as non-fatal:
GrpcSmokeTest.ktsetsanyFailedonly for transport failures (unavailable,deadlineexceeded,connection refused,transport error); a connector status >= 400 prints~ connector error.grpc_smoke_test.rsrecordsconnector_errorscenarios asskippedand passes when all scenarios arepassedorskipped.main.rsandSmokeTest.ktboth carryis_placeholder/PLACEHOLDER_VALUESand mark affected connectorsskippedwith reasonplaceholder_credentials.So the suite still exercises transport, wiring and serialisation on fork pull requests, and only the parts that genuinely need live credentials are skipped.
Runs that do have the secret are completely unaffected: the if branch is unchanged.
Additional Changes
Changes one step in
.github/workflows/ci.yml. No application configuration.How did you test it?
This pull request is its own test case: it is opened from a fork, so
CONNECTOR_SPECIFIC_AUTHdoes not resolve, which is exactly the condition that breaks today.It also has evidence from its own history. The branch first carried per SDK Makefile fallbacks, and that run proved the approach works, taking
Run SDK gRPC Testsfrom two failures to a clean sweep:Before, on #2103:
After:
That run then surfaced the same failure in the FFI path for Rust, and fixing that surfaced it again for Python. Rather than continue patching each target, this pull request now fixes the single shared cause and reverts the Makefile edits. The net change against
mainis six lines in one workflow step.Supporting detail checked by reading the harnesses, since the full suite needs a
release-fastbuild plus Gradle, Node and Python:creds_dummy.jsonholds 73 connectors includingstripe, the default forCONNECTORSandGRPC_CONNECTOR. Every value is<REPLACE_WITH_YOUR_VALUE>.Related: this is the failure blocking #2103 and #2080, the two open pull requests from forks.