feat: validate mint infrastructure during per-repo install#904
Conversation
|
fullsend review is working on this — view logs |
Site previewPreview: https://53418a8f-site.fullsend-ai.workers.dev Commit: |
Review: #904Head SHA: ce0ca7e SummaryThis PR adds mint infrastructure validation during per-repo install, ensuring the Cloud Function exists, URLs match, and the org is registered in FindingsMedium
Low
Info
FooterOutcome: approve Previous runReview: #904Head SHA: 11f0d9a SummaryThis PR adds mint infrastructure validation during per-repo install, including org registration in ALLOWED_ORGS/ROLE_APP_IDS, shared PEM copying, per-repo WIF provider resolution, and an env-var-only Cloud Function update API. The changes are well-structured, well-tested (17+ new tests), and follow existing patterns. No critical or high findings. A few medium items worth noting around code duplication risk and a default value that could override an operator's intent. FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (2)Review: #904Head SHA: 046e3b5 SummaryThis PR adds mint infrastructure validation during per-repo install, including org registration in FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (3)Review: #904Head SHA: f32697e SummaryThis PR adds mint infrastructure validation to per-repo installs, including a new FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (4)Review: #904Head SHA: ac741a7 SummaryThis PR adds mint infrastructure validation during per-repo install — verifying the Cloud Function exists, registering the org in FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (5)Review: #904Head SHA: 7e5a7b6 SummaryThis PR adds mint validation to per-repo install — verifying the Cloud Function exists, its URL matches, and the org is registered in FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (6)Review: #904Head SHA: a07e933 SummaryThe change is well-structured and adds a coherent mint validation step to per-repo install, plus per-repo WIF provider resolution in the mint handler. The security design is sound — using the repository claim to select a WIF provider before STS validation is analogous to using a JWT FindingsMedium
Info
FooterOutcome: comment-only Previous run (7)Review: #904Head SHA: 21e4b31 SummaryThe PR adds mint infrastructure validation to per-repo install, including a new FindingsMedium
Low
Info
FooterOutcome: comment-only |
|
fullsend review is working on this — view logs |
|
fullsend review is working on this — view logs |
Add mint validation step to runPerRepoInstall that verifies the mint function exists at the expected URL, checks org coverage in ALLOWED_ORGS and ROLE_APP_IDS, and updates env vars if the org is missing. - Add UpdateFunctionEnvVars to GCFClient for env-var-only PATCH - Add EnsureOrgInMint provisioner method with URL and org validation - Add resolveSharedRoleAppIDs to discover app IDs from existing mint - Wire mint validation into per-repo install before WIF provisioning - Copy shared PEM secrets for new org with deterministic source selection - Make --mint-region available in per-repo mode Signed-off-by: Wayne Sun <gsun@redhat.com>
The mint was hardcoded to validate all OIDC tokens against a single WIF
provider (github-oidc), which only accepts tokens from .fullsend repos.
Per-repo installs create repo-scoped providers (gh-{owner}-{repo}) that
the mint didn't know about, causing STS validation failures.
Now the mint extracts the repository claim from the OIDC token and
dynamically resolves the correct WIF provider: .fullsend repos use the
default provider, per-repo repos use the repo-scoped provider ID
constructed with the same naming convention as the CLI.
Signed-off-by: Wayne Sun <gsun@redhat.com>
- Sort existingIDs keys in resolveSharedRoleAppIDs for deterministic app selection when multiple orgs share the same role - Use strings.EqualFold in EnsureOrgInMint to match the mint handler's case-insensitive org check and prevent duplicate ALLOWED_ORGS entries - Warn when no shared PEM source is found during per-repo install instead of silently continuing - Check json.Marshal error in EnsureOrgInMint instead of discarding it Signed-off-by: Wayne Sun <gsun@redhat.com>
7e5a7b6 to
ac741a7
Compare
|
fullsend review is working on this — view logs |
- Remove mint-project from perOrgOnlyFlags so per-repo install can specify a separate GCP project for the mint Cloud Function - Default perRepoMintProject to inferenceProject when not explicitly set - Add PerRepoDefaultRoles() excluding the fullsend dispatch role, which is not needed in per-repo mode (target repo's shim handles dispatch) - Update runPerRepoInstall to accept mintProject separately from inferenceProject Signed-off-by: Wayne Sun <gsun@redhat.com>
|
fullsend review is working on this — view logs |
…RKFLOW_FILES CopyAgentPEM now ensures the mint service account has secretAccessor on existing secrets, fixing 403 when secrets were created by an older install that granted a different SA. Mint deployment now sets ALLOWED_WORKFLOW_FILES (default '*') and preserves the existing value across redeployments, preventing workflow file validation from failing after a per-org install redeploys the mint. Signed-off-by: Wayne Sun <gsun@redhat.com>
|
fullsend review is working on this — view logs |
- CopyAgentPEM now distinguishes ErrSecretNotFound from other GetSecret errors instead of falling through to the copy path on any error - EnsureOrgInMint normalizes org to lowercase before adding to ALLOWED_ORGS, matching the per-org install path behavior - EnsureOrgInMint defaults ALLOWED_WORKFLOW_FILES to "*" when missing, preventing fail-closed denial on older mint upgrades - Update TestCopyAgentPEM to verify ensureSecretIAM is called when destination secret exists - Add cross-validation test cases for buildRepoProviderID matching both mint and provisioner implementations Signed-off-by: Wayne Sun <gsun@redhat.com>
|
fullsend review is working on this — view logs |
rh-hemartin
left a comment
There was a problem hiding this comment.
I need this because of the WORKFLOW_FILES thing, now when I install from main it breaks because it empties the WORKFLOW_FILES variable on the mint.
|
fullsend review is working on this — view logs |
Clarify that the mint URL is stable across redeploys within the same project and region, and only changes when --mint-region is changed. Signed-off-by: Wayne Sun <gsun@redhat.com>
93775b3 to
ce0ca7e
Compare
|
fullsend review is working on this — view logs |
Summary
runPerRepoInstallthat verifies the mint Cloud Function exists at the expected URL, checks if the org is already registered inALLOWED_ORGSandROLE_APP_IDS, and updates the function's env vars if the org is missingUpdateFunctionEnvVarsto the GCF client for env-var-only PATCH (avoids corruptingbuildConfig.source)EnsureOrgInMintprovisioner method with URL verification, org coverage check, and additive env var updateresolveSharedRoleAppIDshelper to discover app IDs for the new org by matching installed apps against existing mint entries--mint-regionavailable in per-repo mode (removed fromperOrgOnlyFlags)Test plan
go test ./internal/dispatch/gcf/...— 11 new tests coveringUpdateFunctionEnvVars,EnsureOrgInMint(org covered, adds new org, function not found, nil return, URL mismatch, partial coverage, update fails, wait fails, empty/malformed ROLE_APP_IDS, value mismatch trigger)go test ./internal/cli/...— 6 new tests covering--mint-regionacceptance in per-repo mode andresolveSharedRoleAppIDs(matches installed apps, error when not installed, error when no existing IDs, skips same org, uses own entry)go vet ./internal/...— clean