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
28 changes: 16 additions & 12 deletions cmd/entire/cli/grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,8 +576,8 @@ func refuseUnwritableRef[Row any](t grantTarget[Row]) func(*cobra.Command, []str

// validateRole rejects a --role outside the target's set at the CLI boundary
// so the user gets a clear message instead of a server 422. The generated
// bodies use a distinct enum type per target that shares these values, so the
// targets cast the validated string to whichever type they need.
// bodies type their role field as an enum, so the targets cast the validated
// string to whichever type they need.
func validateRole(role string, allowed []string) error {
if slices.Contains(allowed, role) {
return nil
Expand Down Expand Up @@ -651,6 +651,18 @@ const leastAccessRole = "reader"
// orgRoleMember is the org's least-privileged role and the server's default.
const orgRoleMember = "member"

// grantAccessBody builds the request body the project and repo grant routes
// share. Provider and providerUserId are optional on the wire because the
// route also accepts an accountId; the CLI always addresses a grantee by
// provider handle, so it always sends the pair.
func grantAccessBody(provider, providerUserID, role string) *coreapi.GrantAccessBody {
return &coreapi.GrantAccessBody{
Provider: coreapi.NewOptString(provider),
ProviderUserId: coreapi.NewOptString(providerUserID),
Role: coreapi.GrantAccessBodyRole(role),
}
}

// orgGrantTarget is org membership: roles owner/admin/member with member as
// the server default, a target addressed by name or ULID, and no typed-id
// revoke route — members are removed by their provider identity.
Expand Down Expand Up @@ -709,11 +721,7 @@ var projectGrantTarget = grantTarget[coreapi.ProjectGrant]{
return pt.describe() + " is owned by an account, so it has no member list to choose from"
},
grant: func(ctx context.Context, c *coreapi.Client, id, provider, providerUserID, role string) (string, any, error) {
out, err := c.GrantProjectAccess(ctx, &coreapi.GrantProjectAccessInputBody{
Provider: provider,
ProviderUserId: providerUserID,
Role: coreapi.GrantProjectAccessInputBodyRole(role),
}, coreapi.GrantProjectAccessParams{ProjectId: id})
out, err := c.GrantProjectAccess(ctx, grantAccessBody(provider, providerUserID, role), coreapi.GrantProjectAccessParams{ProjectId: id})
if err != nil {
return "", nil, err
}
Expand Down Expand Up @@ -759,11 +767,7 @@ var repoGrantTarget = grantTarget[coreapi.RepoGrant]{
return fmt.Sprintf("%s is in project %s, which is owned by an account, so it has no member list to choose from", pt.describe(), project)
},
grant: func(ctx context.Context, c *coreapi.Client, id, provider, providerUserID, role string) (string, any, error) {
out, err := c.GrantRepoAccess(ctx, &coreapi.GrantRepoAccessInputBody{
Provider: provider,
ProviderUserId: providerUserID,
Role: coreapi.GrantRepoAccessInputBodyRole(role),
}, coreapi.GrantRepoAccessParams{RepoId: id})
out, err := c.GrantRepoAccess(ctx, grantAccessBody(provider, providerUserID, role), coreapi.GrantRepoAccessParams{RepoId: id})
if err != nil {
return "", nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/entire/cli/grant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ func TestGrantTargetRoles(t *testing.T) {
require.Empty(t, repoGrantTarget.defaultRole)

require.Equal(t, enumStrings(coreapi.AddOrgMemberInputBodyRole("").AllValues()), orgGrantTarget.roles)
require.Equal(t, enumStrings(coreapi.GrantProjectAccessInputBodyRole("").AllValues()), projectGrantTarget.roles)
require.Equal(t, enumStrings(coreapi.GrantRepoAccessInputBodyRole("").AllValues()), repoGrantTarget.roles)
require.Equal(t, enumStrings(coreapi.GrantAccessBodyRole("").AllValues()), projectGrantTarget.roles)
require.Equal(t, enumStrings(coreapi.GrantAccessBodyRole("").AllValues()), repoGrantTarget.roles)
}

// enumStrings converts a generated enum's AllValues() into the plain strings a
Expand Down
6 changes: 4 additions & 2 deletions cmd/entire/cli/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,11 @@ func newOrgCreateCmd() *cobra.Command {
if region != "" {
body.Region = coreapi.NewOptString(region)
}
org, err := c.CreateOrg(ctx, body)
created, err := c.CreateOrg(ctx, body)
if err != nil {
return "", nil, err
}
org := &created.Response
return fmt.Sprintf("✓ Created org %s (%s)", org.Name, org.ID), org, nil
})
},
Expand Down Expand Up @@ -114,7 +115,8 @@ func newOrgDeleteCmd() *cobra.Command {
return resolveOrgRefResolved(ctx, c, args[0])
},
func(ctx context.Context, c *coreapi.Client, id string) error {
return c.DeleteOrg(ctx, coreapi.DeleteOrgParams{OrgId: id})
_, err := c.DeleteOrg(ctx, coreapi.DeleteOrgParams{OrgId: id})
return err
})
},
}
Expand Down
6 changes: 4 additions & 2 deletions cmd/entire/cli/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,11 @@ func newProjectCreateCmd() *cobra.Command {
if region != "" {
body.Region = coreapi.NewOptString(region)
}
project, err := c.CreateProject(ctx, body)
created, err := c.CreateProject(ctx, body)
if err != nil {
return "", nil, err
}
project := &created.Response
return fmt.Sprintf("✓ Created project %s (%s)", project.Name, project.ID), project, nil
})
},
Expand Down Expand Up @@ -192,7 +193,8 @@ func newProjectDeleteCmd() *cobra.Command {
return resolveProjectRefResolved(ctx, c, args[0])
},
func(ctx context.Context, c *coreapi.Client, id string) error {
return c.DeleteProject(ctx, coreapi.DeleteProjectParams{ProjectId: id})
_, err := c.DeleteProject(ctx, coreapi.DeleteProjectParams{ProjectId: id})
return err
})
},
}
Expand Down
9 changes: 7 additions & 2 deletions cmd/entire/cli/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,11 @@ and recovery instructions go to stderr.`,
if format != "" {
body.ObjectFormat = coreapi.NewOptCreateRepoInputBodyObjectFormat(format)
}
created, err := c.CreateRepo(ctx, body)
response, err := c.CreateRepo(ctx, body)
if err != nil {
return err
}
created, err := createdRepoAsRepo(&response.Response)
if err != nil {
return err
}
Expand Down Expand Up @@ -366,7 +370,8 @@ func newRepoDeleteCmd() *cobra.Command {
return resolveRepoRefResolved(ctx, c, args[0], project)
},
func(ctx context.Context, c *coreapi.Client, id string) error {
return c.DeleteRepo(ctx, coreapi.DeleteRepoParams{RepoId: id})
_, err := c.DeleteRepo(ctx, coreapi.DeleteRepoParams{RepoId: id})
return err
})
},
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/entire/cli/repo_mirror.go
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ func chooseMirrorAddRegions(cmd *cobra.Command, ref mirrorRepoRef, nativeRepo *c
// mirrorAddOutcome bundles the create response with the clone status
// observed while waiting. polled is false for --no-wait, where status is unset.
type mirrorAddOutcome struct {
created *coreapi.CreatedMirror
created *coreapi.MirrorRequestResult
status coreapi.MirrorStatus
polled bool
}
Expand Down Expand Up @@ -1466,7 +1466,7 @@ func parseMirrorCloneURL(raw string) (clusterHost, provider, owner, repo string,
// same as the bare clone URL (matching gitremote.ParseURL). GitHub repo
// names can contain dots, so only the suffix is trimmed, not all dots.
repo = strings.ToLower(strings.TrimSuffix(parts[2], mirrorGitDirSuffix))
return u.Host, string(coreapi.CreateMirrorInputBodyProviderGithub), strings.ToLower(parts[1]), repo, nil
return u.Host, string(coreapi.CreateMirrorRequestInputBodyProviderGithub), strings.ToLower(parts[1]), repo, nil
}

func noMirrorErr(ref string) error {
Expand Down
8 changes: 2 additions & 6 deletions cmd/entire/cli/repo_mirror_probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ type mirrorRequestGetter interface {
GetMirrorRequest(ctx context.Context, params coreapi.GetMirrorRequestParams) (*coreapi.MirrorRequest, error)
}

func awaitMirrorPlacement(ctx context.Context, c mirrorRequestGetter, initial coreapi.MirrorRequest, location string, onStatus func(coreapi.MirrorRequestStatus)) (*coreapi.CreatedMirror, error) {
func awaitMirrorPlacement(ctx context.Context, c mirrorRequestGetter, initial coreapi.MirrorRequest, location string, onStatus func(coreapi.MirrorRequestStatus)) (*coreapi.MirrorRequestResult, error) {
serverURL, requestID, err := mirrorRequestPollTarget(location)
if err != nil {
return nil, err
Expand All @@ -145,11 +145,7 @@ func awaitMirrorPlacement(ctx context.Context, c mirrorRequestGetter, initial co
if !ok || result.MirrorId == "" || result.MirrorUrl == "" {
return nil, errors.New("mirror request succeeded without a mirror id and URL")
}
return &coreapi.CreatedMirror{
MirrorId: result.MirrorId,
MirrorUrl: result.MirrorUrl,
PublicUrl: result.PublicUrl,
}, nil
return &result, nil
case coreapi.MirrorRequestStatusFailed:
return nil, mirrorRequestFailureError(*request)
case coreapi.MirrorRequestStatusPending, coreapi.MirrorRequestStatusProcessing:
Expand Down
1 change: 0 additions & 1 deletion cmd/entire/cli/repo_mirror_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ func TestCreateAndAwaitMirror_AsyncSuccess(t *testing.T) {
})
require.NoError(t, err)
require.Equal(t, "mirror-1", outcome.created.MirrorId)
require.False(t, outcome.created.Created)
require.Equal(t, coreapi.MirrorStatusReady, outcome.status)
require.Equal(t, []mirrorAddPhase{mirrorAddPhaseQueued, mirrorAddPhasePlacing, mirrorAddPhaseCloning}, phases)
require.Equal(t, []string{
Expand Down
4 changes: 2 additions & 2 deletions cmd/entire/cli/repo_mirror_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1111,7 +1111,7 @@ func TestParseMirrorCloneURL(t *testing.T) {
if err != nil {
t.Fatalf("parseMirrorCloneURL(%q): %v", tt.raw, err)
}
if provider != string(coreapi.CreateMirrorInputBodyProviderGithub) {
if provider != string(coreapi.CreateMirrorRequestInputBodyProviderGithub) {
t.Errorf("provider = %q, want github", provider)
}
if cluster != tt.wantCluster || owner != tt.wantOwner || repo != tt.wantRepo {
Expand Down Expand Up @@ -1168,7 +1168,7 @@ func TestResolveMirrorRef(t *testing.T) {
}
// The (cluster, provider, owner) narrowing must be server-side; only the
// repo is matched client-side (ListMirrors has no repo filter).
if gotCluster != "aws-eu-central-1.entire.io" || gotProvider != string(coreapi.CreateMirrorInputBodyProviderGithub) || gotOwner != "entirehq" {
if gotCluster != "aws-eu-central-1.entire.io" || gotProvider != string(coreapi.CreateMirrorRequestInputBodyProviderGithub) || gotOwner != "entirehq" {
t.Errorf("filters = cluster %q provider %q owner %q, want the clone URL's coords", gotCluster, gotProvider, gotOwner)
}
})
Expand Down
4 changes: 2 additions & 2 deletions cmd/entire/cli/repo_protection.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,11 @@ func newRepoProtectionAddCmd() *cobra.Command {
// server keeps an existing rule's level and protects a new
// branch, so an add that only names a branch can never lower
// it; --server-side-merge-only=false is the explicit way down.
rule := coreapi.BranchRule{Ref: ref}
rule := coreapi.BranchRuleUpdate{Ref: ref}
if cmd.Flags().Changed(serverSideMergeOnlyFlag) {
rule.ServerSideMergeOnly = coreapi.NewOptBool(mergeOnly)
}
body := &coreapi.UpdateBranchProtectionInputBody{AddRules: []coreapi.BranchRule{rule}}
body := &coreapi.UpdateBranchProtectionInputBody{AddRules: []coreapi.BranchRuleUpdate{rule}}
out, err := c.UpdateBranchProtection(ctx, body, coreapi.UpdateBranchProtectionParams{RepoId: repoID})
if err != nil {
return nil, err
Expand Down
16 changes: 13 additions & 3 deletions cmd/entire/cli/repo_protection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (f *fakeProtectionServer) apply(body coreapi.UpdateBranchProtectionInputBod
}
}
if !present {
next = append(next, a)
next = append(next, coreapi.BranchRule{Ref: a.Ref, ServerSideMergeOnly: a.ServerSideMergeOnly})
}
}
f.rules = next
Expand All @@ -179,6 +179,16 @@ func rulesView(rs []coreapi.BranchRule) []branchRule {
return branchRulesFromWire(&coreapi.BranchProtection{Rules: rs})
}

// updatesView is rulesView for the rules a patch sends, which the spec types
// separately from the rules a read returns.
func updatesView(us []coreapi.BranchRuleUpdate) []branchRule {
rs := make([]coreapi.BranchRule, len(us))
for i, u := range us {
rs[i] = coreapi.BranchRule{Ref: u.Ref, ServerSideMergeOnly: u.ServerSideMergeOnly}
}
return rulesView(rs)
}

func execRepoProtection(t *testing.T, args ...string) (stdout string, err error) {
t.Helper()
stdout, _, err = execRepoProtectionBothStreams(t, args...)
Expand Down Expand Up @@ -278,15 +288,15 @@ func TestRepoProtection_AddAndRemove(t *testing.T) {
out, err := execRepoProtection(t, "add", testProtectionRepoULID, "release/*")
require.NoError(t, err)
require.Len(t, fake.patches, 1)
assert.Equal(t, []branchRule{{Ref: "refs/heads/release/*"}}, rulesView(fake.patches[0].AddRules))
assert.Equal(t, []branchRule{{Ref: "refs/heads/release/*"}}, updatesView(fake.patches[0].AddRules))
assert.False(t, fake.patches[0].AddRules[0].ServerSideMergeOnly.IsSet(), "no flag, no level on the wire")
assert.Empty(t, fake.patches[0].RemoveRefs)
assert.Contains(t, out, "refs/heads/release/*")

out, err = execRepoProtection(t, "add", testProtectionRepoULID, "HEAD", "--server-side-merge-only")
require.NoError(t, err)
require.Len(t, fake.patches, 2)
assert.Equal(t, []branchRule{{Ref: "HEAD", ServerSideMergeOnly: true}}, rulesView(fake.patches[1].AddRules))
assert.Equal(t, []branchRule{{Ref: "HEAD", ServerSideMergeOnly: true}}, updatesView(fake.patches[1].AddRules))
assert.True(t, fake.patches[1].AddRules[0].ServerSideMergeOnly.IsSet())
assert.Contains(t, out, protectionLevelMergeOnly)
assert.Equal(t, []branchRule{
Expand Down
18 changes: 18 additions & 0 deletions cmd/entire/cli/repo_readiness.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cli

import (
"context"
"encoding/json"
"errors"
"fmt"
"math/rand/v2"
Expand All @@ -14,6 +15,23 @@ import (
"github.com/entireio/cli/internal/coreapi"
)

// createdRepoAsRepo converts the create response into the read model that the
// readiness poll and the creation report share. The spec gives the two schemas
// the same fields apart from the deprecated commitToken, so the conversion goes
// through the wire form: it carries additional properties and any field a later
// spec adds to both, where a field-by-field copy would silently drop them.
func createdRepoAsRepo(created *coreapi.CreatedRepo) (*coreapi.Repo, error) {
raw, err := json.Marshal(created)
if err != nil {
return nil, fmt.Errorf("encode created repository: %w", err)
}
var repo coreapi.Repo
if err := json.Unmarshal(raw, &repo); err != nil {
return nil, fmt.Errorf("decode created repository: %w", err)
}
return &repo, nil
}

// These values mirror the provisioning enum in entiredb api/corev1/repos.go.
// Mirror clone readiness has its own enum: an active repo need not be cloned.
const (
Expand Down
15 changes: 13 additions & 2 deletions internal/coreapi/UPSTREAM.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,20 @@ plain strings (drop `enum`), or have ogen treat them as open enums. Enums
the *client sends* (request bodies like `SetRepoVisibilityInputBody`) should
stay strict.

`Membership.role`/`status` and `Invitation.role`/`status` are the same shape:
`entire org grant list` displays memberships; the generated
`ListOrgInvitations` client decodes invitations. A new role or lifecycle state
must not fail either listing.

**Workaround:** `spec/normalize.go` (`loosenReadModelEnums`, allowlist
`readModelEnumFields`) deletes the `enum` constraint from those response
read-model fields, so ogen emits plain strings with no `Validate()` and
unknown values pass through for display. Only response read models are
loosened; request-body enums stay strict. Locked in by
`TestListProjectRepos_UnknownEnumValuesPassThrough` in `client_test.go`.
loosened; request-body enums stay strict — `CreateOrgInvitationInputBody.role`
keeps its enum for outgoing requests. Locked in
by `TestListProjectRepos_UnknownEnumValuesPassThrough`,
`TestListOrgInvitations_UnknownEnumValuesPassThrough` and
`TestListOrgMembers_UnknownEnumValuesPassThrough` in `client_test.go`.
Retire the allowlist entries as upstream loosens the corresponding fields.

## 2b. New read-model fields ship as `required`
Expand All @@ -60,6 +68,9 @@ decoder then fails the whole response when a field is absent, so a core that
predates the field, or a mixed-version roll, breaks every list, get and
repo-routing call in a client that does not depend on any of them.

The create responses (`CreatedOrg`, `CreatedProject`, `CreatedRepo`) repeat the
shape of their read models and so repeat the problem.

**Fix upstream:** add read-model fields as optional until every deployment
sends them, then tighten.

Expand Down
Loading
Loading