Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/cli/inference.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,13 @@ func runInferenceProvisionDryRun(cmd *cobra.Command, printer *ui.Printer, org, r
parts := strings.SplitN(repo, "/", 2)
providerID := mintcore.BuildRepoProviderID(parts[0], parts[1])
printer.StepInfo(fmt.Sprintf("WIF provider: %s (repo-scoped)", providerID))
printer.StepInfo(fmt.Sprintf("Condition: assertion.repository == '%s'", strings.ToLower(repo)))
printer.StepInfo(fmt.Sprintf("Condition: assertion.repository == '%s'", repo))
} else {
printer.Header("Dry run: provision WIF for org-scoped inference")
printer.Blank()
printer.StepInfo(fmt.Sprintf("Organization: %s", org))
printer.StepInfo(fmt.Sprintf("WIF provider: %s (org-scoped)", provider))
printer.StepInfo(fmt.Sprintf("Condition: assertion.repository_owner == '%s'", strings.ToLower(org)))
printer.StepInfo(fmt.Sprintf("Condition: assertion.repository_owner == '%s'", org))
}

printer.Blank()
Expand Down
11 changes: 7 additions & 4 deletions internal/cli/mint.go
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,7 @@ func verifyEnrollment(ctx context.Context, printer *ui.Printer, provisioner enro
}

func runMintEnrollOrg(ctx context.Context, printer *ui.Printer, org, project, region string, dryRun bool) error {
originalCaseOrg := org
org = strings.ToLower(org)
if err := validateOrgName(org); err != nil {
return err
Expand Down Expand Up @@ -705,7 +706,7 @@ func runMintEnrollOrg(ctx context.Context, printer *ui.Printer, org, project, re
printer.StepInfo("Dry run — no changes will be made")
printer.Blank()
printer.StepInfo(fmt.Sprintf(" Would add %s to ALLOWED_ORGS", org))
printer.StepInfo(fmt.Sprintf(" Would add %s to WIF provider condition", org))
printer.StepInfo(fmt.Sprintf(" Would add %s to WIF provider condition", originalCaseOrg))
printer.Blank()
printer.StepInfo("To grant Agent Platform access, run 'fullsend inference provision' separately")
return nil
Expand All @@ -721,7 +722,7 @@ func runMintEnrollOrg(ctx context.Context, printer *ui.Printer, org, project, re
verifyEnrollment(ctx, printer, provisioner, org, project)

printer.StepStart("Updating WIF provider condition")
if err := provisioner.EnsureOrgInWIFCondition(ctx, org); err != nil {
if err := provisioner.EnsureOrgInWIFCondition(ctx, originalCaseOrg); err != nil {
printer.StepFail("Failed to update WIF condition")
return fmt.Errorf("updating WIF condition: %w", err)
}
Expand All @@ -739,6 +740,7 @@ func runMintEnrollOrg(ctx context.Context, printer *ui.Printer, org, project, re
}

func runMintEnrollRepo(ctx context.Context, printer *ui.Printer, repoFullName, project, region string, dryRun bool) error {
originalCaseRepo := repoFullName
repoFullName = strings.ToLower(repoFullName)
parts := strings.SplitN(repoFullName, "/", 2)
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
Expand All @@ -763,7 +765,7 @@ func runMintEnrollRepo(ctx context.Context, printer *ui.Printer, repoFullName, p
ProjectID: project,
Region: region,
GitHubOrgs: []string{owner},
Repo: repoFullName,
Repo: originalCaseRepo,
}, gcpClient)

// Step 1: Discover existing mint.
Expand Down Expand Up @@ -935,6 +937,7 @@ func confirmUnenroll(printer *ui.Printer, target string, reader *bufio.Reader, i
}

func runMintUnenrollOrg(ctx context.Context, printer *ui.Printer, org, project, region string, dryRun, yolo bool, stdin *os.File) error {
originalCaseOrg := org
org = strings.ToLower(org)
if err := validateOrgName(org); err != nil {
return err
Expand Down Expand Up @@ -1005,7 +1008,7 @@ func runMintUnenrollOrg(ctx context.Context, printer *ui.Printer, org, project,

// Step 3: Remove org from WIF provider condition.
printer.StepStart("Updating WIF provider condition")
if err := provisioner.RemoveOrgFromWIFCondition(ctx, org); err != nil {
if err := provisioner.RemoveOrgFromWIFCondition(ctx, originalCaseOrg); err != nil {
printer.StepFail("Failed to update WIF condition")
return fmt.Errorf("updating WIF condition: %w", err)
}
Expand Down
31 changes: 31 additions & 0 deletions internal/cli/mint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,15 @@ func TestRunMintEnrollOrg_DryRun(t *testing.T) {
require.NoError(t, err)
}

func TestRunMintEnrollOrg_DryRunPreservesCaseInPreview(t *testing.T) {
withMintGCFClient(t, mintDiscoveryClient())
out := &strings.Builder{}
printer := ui.New(out)
err := runMintEnrollOrg(context.Background(), printer, "AcmeCorp", "my-project", "us-central1", true)
require.NoError(t, err)
assert.Contains(t, out.String(), "Would add AcmeCorp to WIF provider condition")
}

func TestRunMintEnrollOrg_NoRoleAppIDs(t *testing.T) {
withMintGCFClient(t, gcf.NewFakeGCFClient(
gcf.WithFakeFunctionInfo(&gcf.FunctionInfo{
Expand Down Expand Up @@ -834,6 +843,18 @@ func TestRunMintEnrollOrg_Success(t *testing.T) {
require.NoError(t, err)
}

func TestRunMintEnrollOrg_PreservesCaseInWIFCondition(t *testing.T) {
client := mintDiscoveryClient()
withMintGCFClient(t, client)
printer := ui.New(&strings.Builder{})
err := runMintEnrollOrg(context.Background(), printer, "AcmeCorp", "my-project", "us-central1", false)
require.NoError(t, err)

condition := gcf.LastWIFProviderCondition(client)
assert.Contains(t, condition, "AcmeCorp")
assert.NotContains(t, condition, "acmecorp")
}

func TestRunMintEnrollOrg_PublicMode(t *testing.T) {
withMintGCFClient(t, publicMintDiscoveryClient())
out := &strings.Builder{}
Expand Down Expand Up @@ -1109,6 +1130,16 @@ func TestRunMintEnrollRepo_Success(t *testing.T) {
require.NoError(t, err)
}

func TestRunMintEnrollRepo_PreservesCaseInWIFCondition(t *testing.T) {
client := mintDiscoveryClient()
withMintGCFClient(t, client)
printer := ui.New(&strings.Builder{})
err := runMintEnrollRepo(context.Background(), printer, "Acme/Widget", "my-project", "us-central1", false)
require.NoError(t, err)

assert.Equal(t, "assertion.repository == 'Acme/Widget'", gcf.LastWIFProviderCondition(client))
}

func TestRunMintEnrollRepo_PublicMode(t *testing.T) {
withMintGCFClient(t, publicMintDiscoveryClient())
out := &strings.Builder{}
Expand Down
12 changes: 12 additions & 0 deletions internal/dispatch/gcf/fakeclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,3 +296,15 @@ func WithFakeErrors(errs map[string]error) FakeGCFOption {
func WithFakeWIFProvider(p *WIFProviderInfo) FakeGCFOption {
return func(f *fakeGCFClient) { f.wifProvider = p }
}

// LastWIFProviderCondition returns the AttributeCondition passed to the most
// recent CreateWIFProvider or UpdateWIFProvider call on a fake client, for
// cross-package test assertions. Returns "" if client isn't a fake or no
// call was made yet.
func LastWIFProviderCondition(client GCFClient) string {
f, ok := client.(*fakeGCFClient)
if !ok {
return ""
}
return f.lastWIFProviderConfig.AttributeCondition
}
76 changes: 33 additions & 43 deletions internal/dispatch/gcf/provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ func (p *Provisioner) Provision(ctx context.Context) (map[string]string, error)
return nil, fmt.Errorf("at least one GitHub org is required")
}
seen := make(map[string]bool)
for i, org := range p.cfg.GitHubOrgs {
for _, org := range p.cfg.GitHubOrgs {
if !mintcore.GitHubOrgPattern.MatchString(org) || strings.Contains(org, "--") {
return nil, fmt.Errorf("invalid GitHub org name: %q", org)
}
Expand All @@ -583,7 +583,6 @@ func (p *Provisioner) Provision(ctx context.Context) (map[string]string, error)
return nil, fmt.Errorf("duplicate GitHub org after normalization: %q", org)
}
seen[lower] = true
p.cfg.GitHubOrgs[i] = lower
}
for role := range p.cfg.AgentPEMs {
if !mintcore.RolePattern.MatchString(role) {
Expand Down Expand Up @@ -1143,10 +1142,10 @@ func parseConditionOrgs(condition string) []string {
if strings.HasSuffix(part, fullsendRepoSuffix) {
org := strings.TrimSuffix(part, fullsendRepoSuffix)
if mintcore.GitHubOrgPattern.MatchString(org) {
orgs = append(orgs, strings.ToLower(org))
orgs = append(orgs, org)
}
} else if mintcore.GitHubOrgPattern.MatchString(part) {
orgs = append(orgs, strings.ToLower(part))
orgs = append(orgs, part)
}
}
return orgs
Expand Down Expand Up @@ -1182,9 +1181,7 @@ func (p *Provisioner) ensureWIFPoolAndProvider(ctx context.Context, installingOr
attrCondition = buildPublicAttributeCondition()
} else {
allOrgs = make([]string, len(installingOrgs))
for i, org := range installingOrgs {
allOrgs[i] = strings.ToLower(org)
}
copy(allOrgs, installingOrgs)
}
existingProvider, getErr := p.gcpAPI.GetWIFProvider(ctx, projectNumber, p.cfg.WIFPoolName, p.cfg.WIFProvider)
if getErr != nil {
Expand All @@ -1197,19 +1194,21 @@ func (p *Provisioner) ensureWIFPoolAndProvider(ctx context.Context, installingOr
if !p.cfg.PublicMint {
if existingProvider != nil {
existingOrgs := parseConditionOrgs(existingProvider.AttributeCondition)
merged := make(map[string]bool)
for _, org := range allOrgs {
// Case-insensitive dedup: use lowered key, preserve canonical case.
// Installing orgs take precedence over existing ones when casing differs.
merged := make(map[string]string)
for _, org := range existingOrgs {
if org != PlaceholderOrg {
merged[org] = true
merged[strings.ToLower(org)] = org
}
}
for _, org := range existingOrgs {
if org != PlaceholderOrg && !merged[org] {
merged[org] = true
for _, org := range allOrgs {
if org != PlaceholderOrg {
merged[strings.ToLower(org)] = org
}
}
allOrgs = make([]string, 0, len(merged))
for org := range merged {
for _, org := range merged {
allOrgs = append(allOrgs, org)
}
if len(allOrgs) == 0 {
Expand All @@ -1234,8 +1233,6 @@ func (p *Provisioner) ensureWIFPoolAndProvider(ctx context.Context, installingOr
// GrantOrgVertexAIAccess grants roles/aiplatform.user to an org's .fullsend
// repo principal so that enrolled org workflows can call Agent Platform.
func (p *Provisioner) GrantOrgVertexAIAccess(ctx context.Context, org string) error {
org = strings.ToLower(org)

projectNumber, err := p.gcpAPI.GetProjectNumber(ctx, p.cfg.ProjectID)
if err != nil {
return fmt.Errorf("getting project number: %w", err)
Expand All @@ -1245,7 +1242,6 @@ func (p *Provisioner) GrantOrgVertexAIAccess(ctx context.Context, org string) er
}

func (p *Provisioner) grantOrgVertexAIAccessWithNumber(ctx context.Context, projectNumber, org string) error {
org = strings.ToLower(org)
principal := fmt.Sprintf("principalSet://iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s/attribute.repository/%s/.fullsend",
projectNumber, p.cfg.WIFPoolName, org)
if err := p.gcpAPI.SetProjectIAMBinding(ctx, p.cfg.ProjectID, principal, "roles/aiplatform.user"); err != nil {
Expand All @@ -1255,7 +1251,6 @@ func (p *Provisioner) grantOrgVertexAIAccessWithNumber(ctx context.Context, proj
}

func (p *Provisioner) grantRepoVertexAIAccessWithNumber(ctx context.Context, projectNumber, repo string) error {
repo = strings.ToLower(repo)
principal := fmt.Sprintf("principalSet://iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s/attribute.repository/%s",
projectNumber, p.cfg.WIFPoolName, repo)
if err := p.gcpAPI.SetProjectIAMBinding(ctx, p.cfg.ProjectID, principal, "roles/aiplatform.user"); err != nil {
Expand All @@ -1269,8 +1264,6 @@ func (p *Provisioner) grantRepoVertexAIAccessWithNumber(ctx context.Context, pro
// Strips the deploy-time placeholder (PlaceholderOrg) if present.
// WARNING: read-modify-write without locking — concurrent calls may race.
func (p *Provisioner) EnsureOrgInWIFCondition(ctx context.Context, org string) error {
Comment thread
waynesun09 marked this conversation as resolved.
Resolved
org = strings.ToLower(org)

projectNumber, err := p.gcpAPI.GetProjectNumber(ctx, p.cfg.ProjectID)
if err != nil {
return fmt.Errorf("getting project number: %w", err)
Expand All @@ -1285,16 +1278,16 @@ func (p *Provisioner) EnsureOrgInWIFCondition(ctx context.Context, org string) e
}
Comment thread
waynesun09 marked this conversation as resolved.
Resolved

existingOrgs := parseConditionOrgs(existing.AttributeCondition)
merged := make(map[string]bool)
merged := make(map[string]string)
for _, o := range existingOrgs {
if o != PlaceholderOrg {
merged[o] = true
merged[strings.ToLower(o)] = o
}
}
merged[org] = true
merged[strings.ToLower(org)] = org

allOrgs := make([]string, 0, len(merged))
for o := range merged {
for _, o := range merged {
allOrgs = append(allOrgs, o)
}
sort.Strings(allOrgs)
Expand All @@ -1315,8 +1308,6 @@ func (p *Provisioner) EnsureOrgInWIFCondition(ctx context.Context, org string) e
// attribute condition.
// WARNING: read-modify-write without locking — concurrent calls may race.
func (p *Provisioner) RemoveOrgFromWIFCondition(ctx context.Context, org string) error {
org = strings.ToLower(org)

projectNumber, err := p.gcpAPI.GetProjectNumber(ctx, p.cfg.ProjectID)
if err != nil {
return fmt.Errorf("getting project number: %w", err)
Expand All @@ -1333,7 +1324,7 @@ func (p *Provisioner) RemoveOrgFromWIFCondition(ctx context.Context, org string)
existingOrgs := parseConditionOrgs(existing.AttributeCondition)
var filtered []string
for _, o := range existingOrgs {
if o != org {
if !strings.EqualFold(o, org) {
filtered = append(filtered, o)
}
}
Expand Down Expand Up @@ -1429,32 +1420,31 @@ func (p *Provisioner) ProvisionWIF(ctx context.Context) (wifProvider string, err
return "", fmt.Errorf("duplicate GitHub org after normalization: %q", org)
}
seen[lower] = true
orgs[i] = lower
orgs[i] = org
}

var projectNumber string
providerID := p.cfg.WIFProvider
repo := strings.ToLower(p.cfg.Repo)
if p.cfg.Repo != "" {
// Repo-scoped: dedicated provider per repo, no org merge.
// Each repo gets a unique provider ID (via BuildRepoProviderID),
// so no risk of clobbering another repo's WIF condition.
parts := strings.SplitN(repo, "/", 2)
origParts := strings.SplitN(p.cfg.Repo, "/", 2)
parts := strings.SplitN(p.cfg.Repo, "/", 2)
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
return "", fmt.Errorf("repo must be in owner/repo format, got %q", p.cfg.Repo)
}
if !mintcore.GitHubOrgPattern.MatchString(parts[0]) || strings.Contains(parts[0], "--") {
return "", fmt.Errorf("invalid repo owner %q: must be a valid GitHub org/user name", origParts[0])
partsLower := [2]string{strings.ToLower(parts[0]), strings.ToLower(parts[1])}
if !mintcore.GitHubOrgPattern.MatchString(partsLower[0]) || strings.Contains(partsLower[0], "--") {
return "", fmt.Errorf("invalid repo owner %q: must be a valid GitHub org/user name", parts[0])
}
if !githubRepoSlugPattern.MatchString(parts[1]) {
return "", fmt.Errorf("invalid repo name %q: must contain only alphanumeric, hyphens, dots, or underscores", origParts[1])
if !githubRepoSlugPattern.MatchString(partsLower[1]) {
return "", fmt.Errorf("invalid repo name %q: must contain only alphanumeric, hyphens, dots, or underscores", parts[1])
}
if parts[1] == "." || parts[1] == ".." {
return "", fmt.Errorf("invalid repo name %q: cannot be \".\" or \"..\"", origParts[1])
if partsLower[1] == "." || partsLower[1] == ".." {
return "", fmt.Errorf("invalid repo name %q: cannot be \".\" or \"..\"", parts[1])
}
if strings.HasSuffix(parts[1], ".git") {
return "", fmt.Errorf("invalid repo name %q: cannot end with \".git\"", origParts[1])
if strings.HasSuffix(partsLower[1], ".git") {
return "", fmt.Errorf("invalid repo name %q: cannot end with \".git\"", parts[1])
}
var err error
projectNumber, err = p.gcpAPI.GetProjectNumber(ctx, p.cfg.ProjectID)
Expand All @@ -1464,8 +1454,8 @@ func (p *Provisioner) ProvisionWIF(ctx context.Context) (wifProvider string, err
if err := p.gcpAPI.CreateWIFPool(ctx, projectNumber, p.cfg.WIFPoolName, "Fullsend GitHub OIDC Pool"); err != nil {
return "", fmt.Errorf("creating WIF pool: %w", err)
}
providerID = mintcore.BuildRepoProviderID(parts[0], parts[1])
attrCondition := fmt.Sprintf("assertion.repository == '%s'", repo)
providerID = mintcore.BuildRepoProviderID(partsLower[0], partsLower[1])
attrCondition := fmt.Sprintf("assertion.repository == '%s'", p.cfg.Repo)
audiences := []string{oidcAudience, iamAudience(projectNumber, p.cfg.WIFPoolName, providerID)}
if err := p.gcpAPI.CreateWIFProvider(ctx, projectNumber, p.cfg.WIFPoolName, providerID, OIDCProviderConfig{
IssuerURI: oidcIssuer,
Expand All @@ -1484,10 +1474,10 @@ func (p *Provisioner) ProvisionWIF(ctx context.Context) (wifProvider string, err
}

if p.cfg.Repo != "" {
if err := p.grantRepoVertexAIAccessWithNumber(ctx, projectNumber, repo); err != nil {
if err := p.grantRepoVertexAIAccessWithNumber(ctx, projectNumber, p.cfg.Repo); err != nil {
return "", err
}
log.Printf("granted roles/aiplatform.user to %s (propagation may take several minutes)", repo)
log.Printf("granted roles/aiplatform.user to %s (propagation may take several minutes)", p.cfg.Repo)
} else {
for _, org := range orgs {
if err := p.grantOrgVertexAIAccessWithNumber(ctx, projectNumber, org); err != nil {
Expand Down
Loading
Loading