diff --git a/cmd/entire/cli/cell_target.go b/cmd/entire/cli/cell_target.go index d310c0ddfe..bcaa6b61b6 100644 --- a/cmd/entire/cli/cell_target.go +++ b/cmd/entire/cli/cell_target.go @@ -43,7 +43,7 @@ type cellCoreClient interface { } type nativeRepoCellCoreClient interface { - nativeRepoResolverClient + repoRefClient ListClusters(ctx context.Context) (*coreapi.ListClustersOutputBody, error) ListRepos(ctx context.Context, params coreapi.ListReposParams) (*coreapi.ListReposOutputBody, error) } @@ -314,9 +314,9 @@ func resolveForgeRepoCellPlacement(ctx context.Context, forge, owner, repo strin } // resolveNativeRepoCellPlacement resolves /et// through the -// native project-scoped repo lookup, then maps the repo's home cluster to its -// entire-api cell. It deliberately never consults the forge-blind repos index: -// that index can select a same-named /gh/ mirror instead. +// native path lookup, then maps the repo's home cluster to its entire-api +// cell. It deliberately never consults the forge-blind repos index: that index +// can select a same-named /gh/ mirror instead. func resolveNativeRepoCellPlacement(ctx context.Context, project, repoName string) (repoCellPlacement, error) { ctx, cancel := context.WithTimeout(ctx, requiredCellResolveTimeout) defer cancel() diff --git a/cmd/entire/cli/cell_target_test.go b/cmd/entire/cli/cell_target_test.go index ddd35817f3..f6995dfc79 100644 --- a/cmd/entire/cli/cell_target_test.go +++ b/cmd/entire/cli/cell_target_test.go @@ -79,16 +79,15 @@ func TestMatchClusterBySlug(t *testing.T) { // fakeCellCore is a stub control plane for resolveRepoCellTarget / // resolveRepoCellPlacement tests. type fakeCellCore struct { - repo *coreapi.Repo - repoErr error - projects *coreapi.ListProjectsOutputBody - projectsErr error - projectRepos *coreapi.ListProjectReposOutputBody - projectErr error - clusters []coreapi.Cluster - clustersErr error - repos *coreapi.ListReposOutputBody - reposErr error + repo *coreapi.Repo + repoErr error + // resolution answers POST /repos/resolve; nil resolves nothing. + resolution *coreapi.ResolveReposResponse + resolveErr error + clusters []coreapi.Cluster + clustersErr error + repos *coreapi.ListReposOutputBody + reposErr error // blockUntilCtxDone makes ListRepos and GetRepo hang until the caller's // deadline fires, standing in for a reachable-but-slow control plane — // both, so the owner/repo and ULID paths can each be tested. Off by @@ -118,30 +117,41 @@ func (f *fakeCellCore) GetRepo(ctx context.Context, _ coreapi.GetRepoParams) (*c return f.repo, f.repoErr } -func (f *fakeCellCore) ListProjects(ctx context.Context, _ coreapi.ListProjectsParams) (*coreapi.ListProjectsOutputBody, error) { +func (f *fakeCellCore) ResolveRepos(ctx context.Context, _ *coreapi.ResolveReposInputBody) (*coreapi.ResolveReposResponse, error) { if err := f.waitIfBlocking(ctx); err != nil { return nil, err } - if f.projectsErr != nil { - return nil, f.projectsErr + if f.resolveErr != nil { + return nil, f.resolveErr } - if f.projects != nil { - return f.projects, nil + if f.resolution != nil { + return f.resolution, nil } - return &coreapi.ListProjectsOutputBody{}, nil + return &coreapi.ResolveReposResponse{}, nil } -func (f *fakeCellCore) ListProjectRepos(ctx context.Context, _ coreapi.ListProjectReposParams) (*coreapi.ListProjectReposOutputBody, error) { - if err := f.waitIfBlocking(ctx); err != nil { - return nil, err - } - if f.projectErr != nil { - return nil, f.projectErr - } - if f.projectRepos != nil { - return f.projectRepos, nil - } - return &coreapi.ListProjectReposOutputBody{}, nil +// errProjectLookupRan fails a native-ref resolution that reaches a +// project-scoped lookup. Those need project#inspect, which a repo-only grant +// lacks. +var errProjectLookupRan = errors.New("project lookup must not run for a native ref") + +func (f *fakeCellCore) ListProjects(context.Context, coreapi.ListProjectsParams) (*coreapi.ListProjectsOutputBody, error) { + return nil, errProjectLookupRan +} + +func (f *fakeCellCore) ListProjectRepos(context.Context, coreapi.ListProjectReposParams) (*coreapi.ListProjectReposOutputBody, error) { + return nil, errProjectLookupRan +} + +// nativeResolution is the POST /repos/resolve answer for one ready native repo. +func nativeResolution(fullName, repoID string) *coreapi.ResolveReposResponse { + return &coreapi.ResolveReposResponse{Resolutions: []coreapi.RepoResolution{{ + Provider: repoProviderEntire, + RequestedFullName: fullName, + FullName: coreapi.NewOptString(fullName), + Status: coreapi.RepoResolutionStatusReady, + RepoId: coreapi.NewOptString(repoID), + }}} } func (f *fakeCellCore) ListClusters(context.Context) (*coreapi.ListClustersOutputBody, error) { @@ -186,12 +196,7 @@ func TestResolveForgeRepoCellPlacement_NativeDoesNotSelectSameNamedGitHubMirror( legacyGHID = "01LEGACYGHMIRROR000000000" ) withFakeCellCore(t, &fakeCellCore{ - projects: &coreapi.ListProjectsOutputBody{Project: coreapi.NewOptProject(coreapi.Project{ - ID: projectID, Name: "entirehq", - })}, - projectRepos: &coreapi.ListProjectReposOutputBody{Repo: coreapi.NewOptRepo(coreapi.Repo{ - ID: nativeID, Name: "marvin", OwningProjectId: projectID, - })}, + resolution: nativeResolution("entirehq/marvin", nativeID), repo: &coreapi.Repo{ ID: nativeID, Name: "marvin", OwningProjectId: projectID, ClusterHost: coreapi.NewOptString("eu.entire.io"), @@ -235,9 +240,6 @@ func TestResolveNativeRepoCellPlacement_ClassifiesDefinitiveMisses(t *testing.T) projectID = "01NATIVEPROJECT00000000000" repoID = "01NATIVEREPOSITORY00000000" ) - project := &coreapi.ListProjectsOutputBody{Project: coreapi.NewOptProject(coreapi.Project{ID: projectID, Name: "entirehq"})} - projectRepo := &coreapi.ListProjectReposOutputBody{Repo: coreapi.NewOptRepo(coreapi.Repo{ID: repoID, Name: "marvin", OwningProjectId: projectID})} - tests := []struct { name string core *fakeCellCore @@ -245,26 +247,20 @@ func TestResolveNativeRepoCellPlacement_ClassifiesDefinitiveMisses(t *testing.T) wantMessageSnippet string }{ { - name: "project does not exist", + name: "repo does not resolve", core: &fakeCellCore{}, wantNotOnboarded: true, - wantMessageSnippet: "no project named", - }, - { - name: "repo does not exist in project", - core: &fakeCellCore{projects: project}, - wantNotOnboarded: true, - wantMessageSnippet: "no repo named", + wantMessageSnippet: "not found or not shared", }, { name: "repo has no cluster host", - core: &fakeCellCore{projects: project, projectRepos: projectRepo, repo: &coreapi.Repo{ID: repoID, Name: "marvin", OwningProjectId: projectID}}, + core: &fakeCellCore{resolution: nativeResolution("entirehq/marvin", repoID), repo: &coreapi.Repo{ID: repoID, Name: "marvin", OwningProjectId: projectID}}, wantNotOnboarded: true, wantMessageSnippet: "repo has no cluster host", }, { name: "control plane failure remains retryable", - core: &fakeCellCore{projectsErr: errors.New("core unavailable")}, + core: &fakeCellCore{resolveErr: errors.New("core unavailable")}, wantNotOnboarded: false, wantMessageSnippet: "core unavailable", }, diff --git a/cmd/entire/cli/grant_wiring_test.go b/cmd/entire/cli/grant_wiring_test.go index 8cfdec79bc..458f645f49 100644 --- a/cmd/entire/cli/grant_wiring_test.go +++ b/cmd/entire/cli/grant_wiring_test.go @@ -25,14 +25,14 @@ const ( ) // grantWiringHandler serves the lookups a grant command makes before its -// DELETE — handle resolution for a provider:handle grantee, and the project and -// repo by-name lookups behind a /et// ref — and records the -// DELETE. record is called with the DELETE's method and path; deleteFn writes -// the DELETE response (e.g. 204 or a 404 problem). +// DELETE — handle resolution for a provider:handle grantee, and the path +// lookup behind a /et// ref — and records the DELETE. record is +// called with the DELETE's method and path; deleteFn writes the DELETE +// response (e.g. 204 or a 404 problem). func grantWiringHandler(t *testing.T, record func(method, path string), deleteFn func(w http.ResponseWriter)) http.HandlerFunc { t.Helper() return func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet { + if r.Method != http.MethodGet && !strings.HasSuffix(r.URL.Path, "/repos/resolve") { record(r.Method, r.URL.Path) deleteFn(w) return @@ -47,12 +47,10 @@ func grantWiringHandler(t *testing.T, record func(method, path string), deleteFn Handle: "alice", ProviderUserId: "12345", } - case strings.HasSuffix(r.URL.Path, "/repos"): - payload = &coreapi.ListProjectReposOutputBody{Repo: coreapi.NewOptRepo(coreapi.Repo{ID: wiringRepoULID, Name: "web"})} - case strings.HasSuffix(r.URL.Path, "/projects"): - payload = &coreapi.ListProjectsOutputBody{Project: coreapi.NewOptProject(coreapi.Project{ID: wiringProjULID, Name: "acme", OwnerId: wiringOrgULID, OwnerType: coreapi.ProjectOwnerTypeOrg})} + case strings.HasSuffix(r.URL.Path, "/repos/resolve"): + payload = nativeResolution("acme/web", wiringRepoULID) default: - t.Errorf("unexpected GET %s", r.URL.Path) + t.Errorf("unexpected %s %s", r.Method, r.URL.Path) return } if err := printJSON(w, payload); err != nil { diff --git a/cmd/entire/cli/repo_clone.go b/cmd/entire/cli/repo_clone.go index 538323a98c..1de1c58c62 100644 --- a/cmd/entire/cli/repo_clone.go +++ b/cmd/entire/cli/repo_clone.go @@ -187,15 +187,12 @@ func parseNativeCloneRef(ref string) (project, repo string, err error) { return project, repo, nil } -type nativeRepoResolverClient interface { - repoRefClient - GetRepo(ctx context.Context, params coreapi.GetRepoParams) (*coreapi.Repo, error) -} - // resolveNativeRepo performs the canonical /et// identity -// lookup shared by clone and repo-scoped data commands. -func resolveNativeRepo(ctx context.Context, c nativeRepoResolverClient, project, repoName string) (*coreapi.Repo, error) { - repoID, err := resolveRepoRef(ctx, c, repoName, project) +// lookup shared by clone and repo-scoped data commands. It bypasses +// resolveRepoRef: both segments are names, and a ULID-shaped project name +// must not be read as an id. +func resolveNativeRepo(ctx context.Context, c repoRefClient, project, repoName string) (*coreapi.Repo, error) { + repoID, err := resolveNativeRepoByPath(ctx, c, project, repoName) if err != nil { return nil, err } @@ -207,7 +204,7 @@ func resolveNativeRepo(ctx context.Context, c nativeRepoResolverClient, project, } // resolveNativeCloneURL resolves an Entire-native repo (by project and repo -// name) to its entire:// clone URL: name → ULID via the project-scoped lookup, +// name) to its entire:// clone URL: name → ULID via the pull-gated path lookup, // then GetRepo — the one call that returns both clusterHost and path. The URL // is the server's own coordinates, never synthesized from the user's ref: the // path is the repo's, and the host is one of its readable placements — the diff --git a/cmd/entire/cli/repo_clone_test.go b/cmd/entire/cli/repo_clone_test.go index f310e0b35c..7307591867 100644 --- a/cmd/entire/cli/repo_clone_test.go +++ b/cmd/entire/cli/repo_clone_test.go @@ -349,10 +349,11 @@ type nativeRepoFixture struct { clustersStatus int } -// serveNativeRepo fakes the three-call native resolution chain: project by -// name, repo by name within the project, then the single-repo GET (the one -// response that carries clusterHost + path). The mirror listing answers empty, -// so resolution sees exactly one placement: the home cluster. +// serveNativeRepo fakes the two-call native resolution chain: POST +// /repos/resolve, then the single-repo GET (the one response that carries +// clusterHost + path). No /projects route is served: a native ref must resolve +// with repo#pull alone. The mirror listing answers empty, so resolution sees +// exactly one placement: the home cluster. func serveNativeRepo(t *testing.T, repo coreapi.Repo) *coreapi.Client { t.Helper() return serveNativeRepoFixture(t, nativeRepoFixture{repo: repo}) @@ -364,14 +365,8 @@ func serveNativeRepoFixture(t *testing.T, fx nativeRepoFixture) *coreapi.Client w.Header().Set("Content-Type", "application/json") var body any switch r.URL.Path { - case "/api/v1/projects": - body = &coreapi.ListProjectsOutputBody{Project: coreapi.NewOptProject(coreapi.Project{ - ID: testProjectULID, Name: "paul", OwnerId: testProjectULID, OwnerType: coreapi.ProjectOwnerTypeOrg, - })} - case "/api/v1/projects/" + testProjectULID + "/repos": - body = &coreapi.ListProjectReposOutputBody{Repo: coreapi.NewOptRepo(coreapi.Repo{ - ID: testNativeRepoULID, Name: fx.repo.Name, OwningProjectId: testProjectULID, - })} + case "/api/v1/repos/resolve": + body = nativeResolution("paul/"+fx.repo.Name, testNativeRepoULID) case "/api/v1/repos/" + testNativeRepoULID: body = &fx.repo case "/api/v1/repos/" + testNativeRepoULID + "/native-mirrors": diff --git a/cmd/entire/cli/resolveref.go b/cmd/entire/cli/resolveref.go index 4cb121dbba..ee5568b95f 100644 --- a/cmd/entire/cli/resolveref.go +++ b/cmd/entire/cli/resolveref.go @@ -56,6 +56,8 @@ type projectRefClient interface { type repoRefClient interface { projectRefClient ListProjectRepos(ctx context.Context, params coreapi.ListProjectReposParams) (*coreapi.ListProjectReposOutputBody, error) + ResolveRepos(ctx context.Context, request *coreapi.ResolveReposInputBody) (*coreapi.ResolveReposResponse, error) + GetRepo(ctx context.Context, params coreapi.GetRepoParams) (*coreapi.Repo, error) } // looksLikeULID reports whether s has the shape of a ULID: 26 characters drawn @@ -242,11 +244,40 @@ func resolveRepoRef(ctx context.Context, c repoRefClient, ref, projectRef string if projectRef == "" { return "", fmt.Errorf("repo %q is a name; pass --project to resolve it, use its /%s// path, or a repo ULID", ref, nativeCloneForge) } - projID, err := resolveProjectRef(ctx, c, projectRef) + // The path lookup takes names only. A project ULID has no name route, so + // it stays on the project-scoped listing. + if !looksLikeULID(projectRef) { + return resolveNativeRepoByPath(ctx, c, projectRef, ref) + } + return resolveRepoInProject(ctx, c, ref, projectRef) +} + +// resolveNativeRepoByPath resolves / through POST /repos/resolve, +// which needs repo#pull only. The project-scoped lookups need project#inspect, +// which a direct repo grant does not confer. +// +// The server answers unknown and unauthorized alike, so the miss is one error. +// It wraps errNamedRefNotFound so routing callers classify it as definitive. +func resolveNativeRepoByPath(ctx context.Context, c repoRefClient, project, repoName string) (string, error) { + fullName := project + "/" + repoName + out, err := c.ResolveRepos(ctx, &coreapi.ResolveReposInputBody{ + Repositories: []coreapi.RepoReference{{Provider: repoProviderEntire, FullName: fullName}}, + }) if err != nil { - return "", err + return "", fmt.Errorf("resolve repo: %w", err) + } + for _, r := range out.Resolutions { + if r.Provider != repoProviderEntire || !strings.EqualFold(r.RequestedFullName, fullName) { + continue + } + if r.Status == coreapi.RepoResolutionStatusUnavailable { + break + } + if id := strings.TrimSpace(r.RepoId.Or("")); id != "" { + return id, nil + } } - return resolveRepoInProject(ctx, c, ref, projID) + return "", noRepoAtPathErr(project, repoName) } // resolveRepoPathRef resolves a slash-bearing repo ref: the native @@ -254,8 +285,8 @@ func resolveRepoRef(ctx context.Context, c repoRefClient, ref, projectRef string // parseNativeCloneRef owns that grammar). The ref names its own project, so a // --project given alongside it is checked for agreement rather than trusted or // ignored: a name compares case-insensitively (the server matches lower(name) -// and project names are globally unique), a ULID against the resolved project -// id — neither costs an extra round trip. +// and project names are globally unique) before any request; a ULID compares +// against the resolved repo's owning project, which costs one GetRepo. func resolveRepoPathRef(ctx context.Context, c repoRefClient, ref, projectRef string) (string, error) { // The refusal is about the ref SHAPE, not about the command. Several // commands sharing this resolver DO address mirror repos by ULID — `repo @@ -299,14 +330,20 @@ func resolveRepoPathRef(ctx context.Context, c repoRefClient, ref, projectRef st if projectRef != "" && !looksLikeULID(projectRef) && !strings.EqualFold(projectRef, project) { return "", projectMismatchErr(projectRef, project, ref) } - projID, err := resolveProjectRef(ctx, c, project) + repoID, err := resolveNativeRepoByPath(ctx, c, project, repoName) if err != nil { return "", err } - if projectRef != "" && looksLikeULID(projectRef) && !strings.EqualFold(projectRef, projID) { - return "", projectMismatchErr(projectRef, project, ref) + if projectRef != "" && looksLikeULID(projectRef) { + repo, err := c.GetRepo(ctx, coreapi.GetRepoParams{RepoId: repoID}) + if err != nil { + return "", fmt.Errorf("get repo: %w", err) + } + if !strings.EqualFold(projectRef, repo.OwningProjectId) { + return "", projectMismatchErr(projectRef, project, ref) + } } - return resolveRepoInProject(ctx, c, repoName, projID) + return repoID, nil } // resolveRepoPath resolves the one repo spelling `repo grant` accepts, the @@ -314,7 +351,7 @@ func resolveRepoPathRef(ctx context.Context, c repoRefClient, ref, projectRef st // names the repo the way the API and `repo clone` do, and access management // should not need a lookup to know which project it is touching. The grammar // is parseNativeCloneRef, shared with clone. Both parsed segments are names by -// construction, so they take the by-name lookups directly rather than +// construction, so they take the path lookup directly rather than // resolveRepoRef, whose ULID passthrough would read a ULID-shaped NAME as an id. // // A ref that never named the et/ token is answered with the accepted shape and @@ -331,11 +368,7 @@ func resolveRepoPath(ctx context.Context, c repoRefClient, ref string) (string, if err != nil { return "", fmt.Errorf("invalid repo ref %q: %w", ref, err) } - projID, err := resolveProjectByName(ctx, c, project) - if err != nil { - return "", err - } - return resolveRepoInProject(ctx, c, repoName, projID) + return resolveNativeRepoByPath(ctx, c, project, repoName) } func projectMismatchErr(projectRef, project, ref string) error { @@ -382,6 +415,10 @@ func noRepoNamedErr(name string) error { return &namedRefNotFoundError{message: fmt.Sprintf("no repo named %q in that project (run `entire repo list --project ` to see names, or pass a ULID)", name)} } +func noRepoAtPathErr(project, repoName string) error { + return &namedRefNotFoundError{message: fmt.Sprintf("repo /%s/%s/%s not found or not shared with you", nativeCloneForge, project, repoName)} +} + // resolvedRefLabel formats a reference for a success message so it always // names the resolved ULID. When the user passed a ULID (ref == id) it returns // the id alone; when they passed a name it returns "name (id)" so the message diff --git a/cmd/entire/cli/resolveref_test.go b/cmd/entire/cli/resolveref_test.go index a5ac053dfe..91b2f4e8ae 100644 --- a/cmd/entire/cli/resolveref_test.go +++ b/cmd/entire/cli/resolveref_test.go @@ -2,6 +2,7 @@ package cli import ( "context" + "encoding/json" "fmt" "net/http" "net/http/httptest" @@ -246,6 +247,42 @@ func TestResolveRepoRef(t *testing.T) { t.Errorf("resolveRepoRef unknown name: err = %v, want a \"no repo named\" error", err) } }) + + t.Run("name under --project resolves like a path, no project lookup", func(t *testing.T) { + t.Parallel() + // `repo view web --project widgets`: a repo-only grantee holds repo#pull + // but not project#inspect, so the name pair must go through + // repos/resolve. nativePathHandler refuses any /projects call. + var gotFullName string + c, calls := resolveTestClient(t, nativePathHandler(t, &gotFullName)) + got, err := resolveRepoRef(context.Background(), c, "web", "widgets") + if err != nil { + t.Fatalf("resolveRepoRef: %v", err) + } + if got != ulidRepoWeb { + t.Errorf("resolveRepoRef = %q, want web id", got) + } + if gotFullName != "widgets/web" { + t.Errorf("server received fullName=%q, want %q", gotFullName, "widgets/web") + } + if n := calls.Load(); n != 1 { + t.Errorf("name under project name made %d HTTP calls, want 1 (repos/resolve)", n) + } + }) + + t.Run("unknown name under --project is one friendly miss", func(t *testing.T) { + t.Parallel() + c, _ := resolveTestClient(t, func(w http.ResponseWriter, _ *http.Request) { + if err := printJSON(w, &coreapi.ResolveReposResponse{Resolutions: []coreapi.RepoResolution{{ + Provider: repoProviderEntire, RequestedFullName: "widgets/nope", Status: coreapi.RepoResolutionStatusUnavailable, + }}}); err != nil { + t.Errorf("encode resolution: %v", err) + } + }) + _, err := resolveRepoRef(context.Background(), c, "nope", "widgets") + require.ErrorIs(t, err, errNamedRefNotFound) + require.EqualError(t, err, "repo /et/widgets/nope not found or not shared with you") + }) } // TestResolveRepoRef_NativePath covers the /et// path grammar @@ -253,35 +290,52 @@ func TestResolveRepoRef(t *testing.T) { // every repo-ref command, --project alongside it is checked for agreement, and // the #2252 rule holds at the resolver — no ref is read as a forge it did not // name, and no slash-bearing ref reaches the by-name lookup. -// nativePathHandler serves the two lookups a /et/widgets/web ref makes: -// GET /projects?name=widgets and GET /projects/{id}/repos?name=web. It also -// records the repo name the server was asked for, so tests can pin server-side -// filtering and the .git trim. -func nativePathHandler(t *testing.T, gotRepoName *string) http.HandlerFunc { +// nativePathHandler serves the one lookup a /et/widgets/web ref makes, POST +// /repos/resolve, plus GET /repos/{id} for the --project ULID agreement check. +// It records the full name the server was asked to resolve, so tests can pin +// server-side matching and the .git trim. Any /projects call is refused: a +// native ref must resolve with repo#pull alone. +func nativePathHandler(t *testing.T, gotFullName *string) http.HandlerFunc { t.Helper() return func(w http.ResponseWriter, r *http.Request) { - if strings.HasSuffix(r.URL.Path, "/repos") { - *gotRepoName = r.URL.Query().Get("name") - if err := printJSON(w, &coreapi.ListProjectReposOutputBody{Repo: coreapi.NewOptRepo(coreapi.Repo{ID: ulidRepoWeb, Name: "web"})}); err != nil { + switch { + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/repos/resolve"): + var in coreapi.ResolveReposInputBody + if err := json.NewDecoder(r.Body).Decode(&in); err != nil { + t.Errorf("decode resolve body: %v", err) + w.WriteHeader(http.StatusBadRequest) + return + } + if len(in.Repositories) != 1 || in.Repositories[0].Provider != repoProviderEntire { + t.Errorf("resolve body = %+v, want one entire reference", in.Repositories) + } + if len(in.Repositories) > 0 { + *gotFullName = in.Repositories[0].FullName + } + // The CLI matches on the echoed requested name. + if err := printJSON(w, nativeResolution(*gotFullName, ulidRepoWeb)); err != nil { + t.Errorf("encode resolution: %v", err) + } + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/repos/"+ulidRepoWeb): + if err := printJSON(w, &coreapi.Repo{ID: ulidRepoWeb, Name: "web", OwningProjectId: ulidProjectWidgets}); err != nil { t.Errorf("encode repo: %v", err) } - return - } - if err := printJSON(w, &coreapi.ListProjectsOutputBody{Project: coreapi.NewOptProject(coreapi.Project{ID: ulidProjectWidgets, Name: "widgets", OwnerId: ulidOrgAcme, OwnerType: coreapi.ProjectOwnerTypeOrg})}); err != nil { - t.Errorf("encode project: %v", err) + default: + t.Errorf("unexpected %s %s: a native ref must not need a project lookup", r.Method, r.URL.Path) + w.WriteHeader(http.StatusForbidden) } } } func TestResolveRepoRef_NativePath(t *testing.T) { t.Parallel() - t.Run("native /et/ path resolves via its embedded project", func(t *testing.T) { + t.Run("native /et/ path resolves in one pull-gated call", func(t *testing.T) { t.Parallel() for _, ref := range []string{"/et/widgets/web", "et/widgets/web", "/et/widgets/web.git"} { t.Run(ref, func(t *testing.T) { t.Parallel() - var gotRepoName string - c, calls := resolveTestClient(t, nativePathHandler(t, &gotRepoName)) + var gotFullName string + c, calls := resolveTestClient(t, nativePathHandler(t, &gotFullName)) got, err := resolveRepoRef(context.Background(), c, ref, "") if err != nil { t.Fatalf("resolveRepoRef(%q): %v", ref, err) @@ -289,25 +343,39 @@ func TestResolveRepoRef_NativePath(t *testing.T) { if got != ulidRepoWeb { t.Errorf("resolveRepoRef = %q, want web id", got) } - if gotRepoName != "web" { - t.Errorf("server received repo name=%q, want %q", gotRepoName, "web") + if gotFullName != "widgets/web" { + t.Errorf("server received fullName=%q, want %q", gotFullName, "widgets/web") } - if n := calls.Load(); n != 2 { - t.Errorf("path ref made %d HTTP calls, want 2 (project + repo lookup)", n) + if n := calls.Load(); n != 1 { + t.Errorf("path ref made %d HTTP calls, want 1 (repos/resolve)", n) } }) } }) + t.Run("a repo the server does not resolve is one friendly miss", func(t *testing.T) { + t.Parallel() + // Unknown and unshared repos share the server's "unavailable" answer. + c, _ := resolveTestClient(t, func(w http.ResponseWriter, _ *http.Request) { + if err := printJSON(w, &coreapi.ResolveReposResponse{Resolutions: []coreapi.RepoResolution{{ + Provider: repoProviderEntire, RequestedFullName: "widgets/web", Status: coreapi.RepoResolutionStatusUnavailable, + }}}); err != nil { + t.Errorf("encode resolution: %v", err) + } + }) + _, err := resolveRepoRef(context.Background(), c, "/et/widgets/web", "") + require.ErrorIs(t, err, errNamedRefNotFound) + require.EqualError(t, err, "repo /et/widgets/web not found or not shared with you") + }) + t.Run("--project agreeing with the path is allowed", func(t *testing.T) { t.Parallel() - // A name compares case-insensitively (the server matches lower(name)); - // a ULID compares against the resolved project id. - for _, project := range []string{"widgets", "WIDGETS", ulidProjectWidgets} { + // A name compares locally; a ULID costs one GetRepo. + for project, wantCalls := range map[string]int64{"widgets": 1, "WIDGETS": 1, ulidProjectWidgets: 2} { t.Run(project, func(t *testing.T) { t.Parallel() - var gotRepoName string - c, _ := resolveTestClient(t, nativePathHandler(t, &gotRepoName)) + var gotFullName string + c, calls := resolveTestClient(t, nativePathHandler(t, &gotFullName)) got, err := resolveRepoRef(context.Background(), c, "/et/widgets/web", project) if err != nil { t.Fatalf("resolveRepoRef with --project %q: %v", project, err) @@ -315,6 +383,9 @@ func TestResolveRepoRef_NativePath(t *testing.T) { if got != ulidRepoWeb { t.Errorf("resolveRepoRef = %q, want web id", got) } + if n := calls.Load(); n != wantCalls { + t.Errorf("--project %q made %d HTTP calls, want %d", project, n, wantCalls) + } }) } }) @@ -336,15 +407,12 @@ func TestResolveRepoRef_NativePath(t *testing.T) { t.Run("--project ULID disagreeing with the path is rejected", func(t *testing.T) { t.Parallel() - var gotRepoName string - c, _ := resolveTestClient(t, nativePathHandler(t, &gotRepoName)) + var gotFullName string + c, _ := resolveTestClient(t, nativePathHandler(t, &gotFullName)) _, err := resolveRepoRef(context.Background(), c, "/et/widgets/web", ulidOrgGlobex) if err == nil || !strings.Contains(err.Error(), "does not match") { t.Errorf("mismatched --project ULID: err = %v, want a \"does not match\" error", err) } - if gotRepoName != "" { - t.Error("repo lookup must not run when --project disagrees with the path") - } }) // The remaining subtests pin the #2252 rule at the resolver: no ref is ever @@ -437,25 +505,25 @@ func TestResolveRepoRef_NativePath(t *testing.T) { } // TestResolveRepoPath covers the one repo spelling `repo grant` accepts, the -// native /et// path: it resolves through the project and repo -// by-name lookups exactly as the path does elsewhere, and everything else — a -// ULID, a bare name, a bare pair, a /gh/ mirror ref — is refused locally with -// the shape named, before any request is made. +// native /et// path: it resolves through the same path lookup +// as the path does elsewhere, and everything else — a ULID, a bare name, a +// bare pair, a /gh/ mirror ref — is refused locally with the shape named, +// before any request is made. func TestResolveRepoPath(t *testing.T) { t.Parallel() - t.Run("a native path resolves via its embedded project", func(t *testing.T) { + t.Run("a native path resolves in one call", func(t *testing.T) { t.Parallel() for _, ref := range []string{"/et/widgets/web", "et/widgets/web", "/et/widgets/web.git"} { t.Run(ref, func(t *testing.T) { t.Parallel() - var gotRepoName string - c, calls := resolveTestClient(t, nativePathHandler(t, &gotRepoName)) + var gotFullName string + c, calls := resolveTestClient(t, nativePathHandler(t, &gotFullName)) got, err := resolveRepoPath(context.Background(), c, ref) require.NoError(t, err) require.Equal(t, ulidRepoWeb, got) - require.Equal(t, "web", gotRepoName) - require.EqualValues(t, 2, calls.Load(), "project + repo lookup") + require.Equal(t, "widgets/web", gotFullName) + require.EqualValues(t, 1, calls.Load(), "repos/resolve") }) } }) @@ -464,17 +532,18 @@ func TestResolveRepoPath(t *testing.T) { t.Parallel() // The server's name rules admit 26 base32 characters, so a project or // repo can be NAMED like a ULID. Inside a path both segments are names - // by construction and must go through the by-name lookups, never the - // ULID passthrough that a bare ref gets. + // by construction and must go to the path lookup, never the ULID + // passthrough that a bare ref gets. for _, ref := range []string{"/et/widgets/" + ulidAccount, "/et/" + ulidAccount + "/web"} { t.Run(ref, func(t *testing.T) { t.Parallel() - var gotRepoName string - c, calls := resolveTestClient(t, nativePathHandler(t, &gotRepoName)) + var gotFullName string + c, calls := resolveTestClient(t, nativePathHandler(t, &gotFullName)) got, err := resolveRepoPath(context.Background(), c, ref) require.NoError(t, err) require.Equal(t, ulidRepoWeb, got) - require.EqualValues(t, 2, calls.Load(), "project + repo lookup") + require.Equal(t, strings.TrimPrefix(ref, "/et/"), gotFullName) + require.EqualValues(t, 1, calls.Load(), "repos/resolve") }) } }) diff --git a/docs/development/cli-conventions.md b/docs/development/cli-conventions.md index f5fb7e2ba4..4a80c343aa 100644 --- a/docs/development/cli-conventions.md +++ b/docs/development/cli-conventions.md @@ -231,16 +231,19 @@ the commands are always runnable in every build. `--project`, no bare name, no ULID — through `resolveRepoPath`, which parses with `parseNativeCloneRef` and resolves both segments by name only (a project or repo can be *named* like a ULID, so path segments never touch the - `looksLikeULID` passthrough; `resolveRepoPathRef` and `resolveNativeRepo` - still do, pending the removal of repo-ULID addressing). The other two - clone shapes are not: a `/gh/` mirror ref is refused there (the by-name - lookup resolves a project and then a repo inside it, and a mirror is in no - project — so a mirror is addressed by ULID), and an `entire://` URL is not - parsed at all. `--project` serves the **bare-name** spelling alone, because - the control plane has no by-name repo route that is not project-scoped; the - path form is checked against it for agreement, and a ULID warns that it is - ignored rather than validating, which would cost a `GetRepo` on every command - but `repo view`. + `looksLikeULID` passthrough). The other two clone shapes are not: a `/gh/` + mirror ref is refused there (a mirror is in no project, so it is addressed by + ULID), and an `entire://` URL is not parsed at all. + Every `/` name pair resolves through **one** call, + `POST /repos/resolve` (`resolveNativeRepoByPath`), because that route needs + `repo#pull` alone. The project-scoped routes (`GET /projects?name=`, + `GET /projects/{id}/repos?name=`) need `project#inspect`, which a direct + repo grant does not confer, so a repo shared with one person must never + resolve through them. Only a `--project` **ULID** with a bare name takes the + project-scoped listing, since there is no ULID→name route. Alongside the + path form, `--project` is checked for agreement: a name compares + case-insensitively before any request, a ULID against the resolved repo's + owning project at the cost of one `GetRepo`. Native names are validated client-side against the server's own rules (`nativeProjectRe`/`nativeRepoRe`, mirroring `normalizeName` in entiredb `core/resource/project_name.go`); those bounds are server parity only and buy @@ -252,7 +255,7 @@ the commands are always runnable in every build. that declared a forge token keeps its own parser's reason, a bare pair is offered the forge-qualified readings that would actually parse (`bareRefSuggestions`), and anything left lists the accepted shapes. - A native ref resolves project → repo ULID → `GetRepo`, whose response is the + A native ref resolves name → repo ULID → `GetRepo`, whose response is the only one carrying both `clusterHost` and `path`, then picks among the repo's readable placements — the home cluster plus ready native mirrors (`nativePlacements`, joining mirror slugs against the cluster catalog) —