feat: add --skip-mint-check for self-provisioned mint services - #991
Conversation
When users self-provision their mint service (non-GCP, or on a GCP project they don't manage), the installer no longer requires GCP IAM access to the mint project. The new --skip-mint-check flag: - Accepts any valid HTTPS URL (not just .run.app/.cloudfunctions.net) - Skips DiscoverMint() GCP API calls - Skips GitHub App creation and PEM storage - Skips all mint provisioning (Secret Manager, EnsureOrgInMint, WIF) - Makes --mint-project optional - Still sets FULLSEND_MINT_URL repo variable for runtime workflows Resolves #983 Signed-off-by: Wayne Sun <gsun@redhat.com>
|
fullsend review is working on this — view logs |
Site previewPreview: https://c5e1527f-site.fullsend-ai.workers.dev Commit: |
Review: #991Head SHA: b1047d3 SummaryThis PR cleanly implements a FindingsInfo
FooterOutcome: approve Previous runReview: #991Head SHA: 6b9bfd0 SummaryThis PR cleanly addresses #983 by adding a FindingsInfo
FooterOutcome: approve Previous run (2)Review: #991Head SHA: 49d07d9 SummaryClean, well-scoped change that adds FindingsMedium / Low / Info
FooterOutcome: approve Previous run (3)Review: #991Head SHA: dc74f2a SummaryClean, well-scoped change that adds FindingsMedium / Low / Info
FooterOutcome: comment-only |
- Reject --skip-mint-check in per-org mode with clear error message - Reject mint URLs with embedded credentials (userinfo) to prevent leaking auth data via repo variables - Fix suppressed parse error in validateMintURL (double url.Parse) - Differentiate SkipMintCheckSkipsMintProject test from AcceptsNonCloudRunURL - Add tests for per-org rejection and userinfo rejection Signed-off-by: Wayne Sun <gsun@redhat.com>
|
fullsend review is working on this — view logs |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| if cmd.Flags().Changed("skip-mint-check") { | ||
| return fmt.Errorf("--skip-mint-check is only valid for per-repo installation (fullsend admin install <owner/repo>)") | ||
| } |
There was a problem hiding this comment.
At some point this should be allowed for org mode as well.
There was a problem hiding this comment.
Yeah, my plan was set to as unified flag for all modes, but my review squad have change that in second commit, new commit is fixing this
Add skipMintDispatcher that implements dispatch.Dispatcher to return the user-provided mint URL without GCP API calls. In per-org mode, --skip-mint-check now skips mint-project/mint-provider validation, app setup, PEM storage, and uses the no-op dispatcher in the layer stack instead of the GCF provisioner. Signed-off-by: Wayne Sun <gsun@redhat.com>
|
fullsend review is working on this — view logs |
…alidation - Pass skipMintCheck/mintURL to runDryRun so per-org dry-run uses skipMintDispatcher instead of a dummy GCF provisioner - Extract validateSkipMintCheck helper to eliminate duplicated validation logic between per-org and per-repo paths - Add skip-mint-check flag assertion to TestInstallCmd_Flags - Add skipMintDispatcher unit test verifying Dispatcher contract - Add table-driven tests for validateMintURLHTTPS edge cases - Update flag help text to document --mint-url requirement Signed-off-by: Wayne Sun <gsun@redhat.com>
|
fullsend review is working on this — view logs |
2 participants
Summary
--skip-mint-checkCLI flag tofullsend admin installthat allows users to point at a self-provisioned mint service without requiring GCP IAM access to the mint project--mint-projectbecomes optional;FULLSEND_MINT_URLrepo variable is still set for runtime workflowsResolves #983
Test plan
TestInstallCmd_SkipMintCheckRequiresMintURL— errors when--skip-mint-checkwithout--mint-urlTestInstallCmd_SkipMintCheckAcceptsNonCloudRunURL— accepts arbitrary HTTPS URL with--dry-runTestInstallCmd_SkipMintCheckSkipsMintProject— no--mint-projectrequiredTestInstallCmd_SkipMintCheckRejectsHTTP— still rejects non-HTTPS URLsgo test ./internal/cli/...)go vet ./...cleanmake lintclean