Use GitHub App deployment tokens and update model-access tests - #1173
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked issue
Related to #1117. This delivers the cross-repository deployment authentication slice; the broader lifecycle work remains open.
What changes
DEV schema publishing and Lambda DEV/PROD dispatch now use the organization-owned Wrangleworks Deployments GitHub App instead of
CROSS_REPO_PAT_V2. Each operation receives a short-lived token restricted to its target repository and required permission. Early checks validate configuration and installation access before tests or package publishing, and publishing/dispatch jobs generate fresh tokens immediately before use.Schema commits use the GitHub Actions bot identity. The setup guide documents the existing App and installation, required Actions configuration, DEV validation, and key rotation.
The
train.lookupmissing-model test now uses mocked HTTP responses to cover missing models (404) and inaccessible models (403) separately. It asserts the exact exception type and recipe-context message and verifies that no model update occurs. Runtime behavior is unchanged.How it was verified
.venv/Scripts/python.exe -m pytest tests/connectors/test_train.py::TestTrainLookup::test_update_model_access_errors tests/test_data.py -q.wrangleworks.github.ioand an Actions-write token restricted toLambda-Recipes. Repository/workflow metadata reads passed, and both temporary tokens were revoked.DEPLOY_APP_CLIENT_IDand savedDEPLOY_APP_PRIVATE_KEYin WranglesPY Actions.git diff --checkpassed; the branch was current withmainat publication.These live checks did not publish a schema, dispatch a workflow, or deploy DEV/PROD. New PR CI is pending; the previous full pipeline was not rerun.
Compatibility and risk
No recipe or runtime API changes. Existing test/publishing gates, downstream
mainrefs, initiating-user attribution, and AWS OIDC remain unchanged. The workflows have no PAT fallback. Successful token issuance does not prove a later schema push will satisfy repository branch rules.After merge, start a new Deploy Dev run from main, using the intended version base (currently
1.20.2). An old failed run retains its old workflow definition. Verify schema publication, RC publication, and the downstream Lambda image/update before calling DEV deployment complete.The separate prior container failure in
test_extract_custom_ai_multiple_outputremains unresolved. It expects successful extraction and received403: API Key not setup correctly; it has no stale error assertion to update. This PR preserves that integration test pending verification of its model-scoped credential configuration.Existing deployed images are unaffected. If deployment fails, keep the current image while correcting App configuration; reverting the migration also requires restoring a working previous credential route. Retire shared PAT secrets only after checking their other consumers.
Ready-for-review checklist
mainand has no merge conflictsSee the pull request workflow.