Skip to content

ci(sdk): stage placeholder credentials when secrets are unavailable - #2129

Open
Utkal059 wants to merge 1 commit into
juspay:mainfrom
Utkal059:ci/fork-pr-placeholder-creds
Open

ci(sdk): stage placeholder credentials when secrets are unavailable#2129
Utkal059 wants to merge 1 commit into
juspay:mainfrom
Utkal059:ci/fork-pr-placeholder-creds

Conversation

@Utkal059

Copy link
Copy Markdown

Fork pull requests cannot pass SDK Tests, and the reason is one line in this workflow rather than anything in sdk/.

A PR from a fork receives no secrets.CONNECTOR_SPECIFIC_AUTH, so "Create connector credentials" writes nothing and logs "running in dry-run mode". Nothing downstream implements that mode. Every SDK harness resolves creds.json, and the four disagree on what a missing file means:

══ gRPC Results Summary ══
  ✗ Rust (exit 2)      Configuration error: Credentials file 'creds.json' not found
  ✓ JavaScript         loadCreds() returns {} → placeholder api_key
  ✓ Python             same
  ✗ Kotlin (exit 2)    IllegalArgumentException: Credentials file not found: creds.json

Run SDK gRPC Tests is the first test step, so the job dies there — and the two steps behind it need the file just as much:

  • Run SDK FFI Tests: all four harnesses abort. run_tests panics with Credentials file not found (sdk/rust/smoke-test/src/main.rs), loadCredentials throws (sdk/java/smoke-test/src/main/kotlin/SmokeTest.kt), and the JavaScript and Python FFI harnesses raise as well — only their gRPC harnesses degrade.
  • Run SDK Mock Tests: scripts/run_smoke_tests_parallel.py:499 copies repo_root/creds.json unconditionally.

So three of the job's steps are unreachable without secrets, and the message a contributor sees points at their own diff.

Change

Copy creds_dummy.json — the file the mock tests already use — when the secret is absent, so all four harnesses take one path.

cp creds_dummy.json creds.json

The values stay <REPLACE_WITH_YOUR_VALUE> rather than being scrubbed to "placeholder", and that detail is the whole fix. has_valid_credentials() reads a recognised placeholder as nothing configured, so every harness would skip every connector, and all four then agree on this:

if passed == 0 && skipped > 0 { return 1; }   // "All tests skipped (no valid credentials found)"

A scrubbed file turns four crashes into four green-looking skips that still exit 1. The unscrubbed dummy makes the flows actually run: the connector rejects the key, each harness classifies that as skipped (connector error) with the connector itself passing, exactly as the JavaScript and Python gRPC harnesses do on fork PRs today. Packaging, native-library loading, transport and request building stay covered.

Runs that hold the secret are untouched — the live branch is unchanged, and the placeholder branch is reachable only when CONNECTOR_SPECIFIC_AUTH is empty.

Verified

Loading the real predicates out of sdk/python/smoke-test/test_smoke.py (AST-extracted, so it is the shipped code, not a re-implementation):

load_credentials(missing)                 : FileNotFoundError — today's fork failure
load_credentials(after cp)                : OK, 72 connectors, stripe present
has_valid_credentials(stripe dummy entry) : True    → flows run, no all-skipped exit 1
is_placeholder('<REPLACE_WITH_YOUR_VALUE>'): False
is_placeholder('placeholder')             : True    → why scrubbing would break it

The Rust, Kotlin and JavaScript harnesses carry the same two predicates and the same passed == 0 && skipped > 0 rule; the Rust and Kotlin gRPC harnesses classify a non-transport error as skipped (connector error) and leave the connector passed, so they land where JavaScript and Python already do.

bash -n on the changed step, and the workflow parses.

This PR is its own test: it changes .github/**, which sets changes.ci, which is one of the triggers for sdk-test — and being a fork PR, it runs the placeholder branch it adds.

Not included

The harness divergence is real on its own — make -C sdk test-grpc without a creds.json gives a contributor ✓✓✗✗ locally too. Converging that is four code paths across two languages I cannot build here, and it is a separate concern from unblocking CI; happy to follow up if it is wanted.

One cosmetic leftover: Run SDK FFI Tests still branches on [[ -f "creds.json" ]] to choose its log line, which is now always true, so a fork run prints "with connector credentials". Say the word and I will make it read the mode instead.

Refs #2029

@Utkal059
Utkal059 requested a review from a team as a code owner August 13, 2026 23:44
Copilot AI lite review requested due to automatic review settings August 13, 2026 23:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts the CI workflow so SDK tests can run on fork pull requests where secrets.CONNECTOR_SPECIFIC_AUTH is unavailable, by ensuring a creds.json file is always staged for the SDK harnesses.

Changes:

  • In .github/workflows/ci.yml, when CONNECTOR_SPECIFIC_AUTH is missing, copy creds_dummy.json to creds.json instead of logging “dry-run mode”.
  • Add workflow comments explaining why creds.json must exist and why the dummy values must remain unsanitized.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

A pull request from a fork receives no CONNECTOR_SPECIFIC_AUTH, so the
"Create connector credentials" step wrote no creds.json and logged
"running in dry-run mode" — but nothing downstream implements that mode.
Every SDK harness resolves creds.json, and the four disagree on what a
missing file means: the Rust gRPC harness exits with "Configuration
error", Kotlin throws IllegalArgumentException, JavaScript and Python
fall back to placeholders. The job died in its first test step, and the
FFI and mock steps behind it never ran — both need the file too.

Copy creds_dummy.json, the file the mock tests already use, so all four
harnesses take one path. The values stay <REPLACE_WITH_YOUR_VALUE>
rather than being scrubbed to "placeholder": has_valid_credentials()
reads a recognised placeholder as "nothing configured", every harness
then skips every connector, and passed == 0 && skipped > 0 exits 1. With
the dummy values the flows run, the connector rejects the key, and each
harness records that as "skipped (connector error)" with the connector
still passing — so packaging, native-library loading, transport and
request building stay covered without secrets.

Runs that hold the secret are unaffected; that branch is untouched.
@Utkal059

Copy link
Copy Markdown
Author

Worth recording why the two obvious alternatives don't hold, since staging a credentials file isn't the first thing that comes to mind for this failure.

make ... --dry-run || true. --dry-run there is GNU make's -n: it prints the recipes and runs none of them, so the step would pass without executing a single test. || true then makes the step incapable of failing at all. SDK Tests is a required check — this would leave it green permanently.

Skipping the step when credentials are absent. That drops SDK gRPC coverage for every fork PR, including the run #2029 leans on to show its resolver change works end to end (stage native → pack → install → load). Fork PRs are where most SDK contributions come from; they'd be the ones losing the check.

"The harnesses already support dry-run and mock." Only the FFI ones. SmokeTest.kt:503-504 is the Args data class of the FFI harness; the step that fails is the gRPC one, and neither gRPC harness accepts those flags:

GrpcSmokeTest.kt:150-153    --creds-file  --connectors  --all        --help
grpc_smoke_test.rs:198-210  --creds-file  --endpoint    --connector  --help

Even in the FFI harness the modes don't help: runTests installs the mock intercept and then calls loadCredentials, so --mock throws on the missing file just the same. Which is really the point — mock mode has always needed a credentials file, and make test-package-mock supplies one by copying creds_dummy.json (sdk/java/Makefile:176). This PR does that one level up so all four harnesses get it, instead of each deciding for itself what a missing file means.

The distinction that matters: this keeps the tests running and failing honestly. The alternatives get the check green by making it stop looking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants