diff --git a/pkg/provider/github/github.go b/pkg/provider/github/github.go index 09af833e6..72ada5cdb 100644 --- a/pkg/provider/github/github.go +++ b/pkg/provider/github/github.go @@ -375,11 +375,10 @@ func (v *Provider) GetCommitStatuses(_ context.Context, _ *info.Event) ([]provid // GetTektonDir retrieves all YAML files from the .tekton directory and returns them as a single concatenated multi-document YAML file. func (v *Provider) GetTektonDir(ctx context.Context, runevent *info.Event, path, provenance string) (string, error) { - tektonDirSha := "" - v.provenance = provenance - // default set provenance from the SHA revision := runevent.SHA + + // For default_branch, get the branch SHA first (keep REST API call for simplicity) if provenance == "default_branch" { v.Logger.Infof("Using PipelineRun definition from default_branch: %s", runevent.DefaultBranch) branch, _, err := wrapAPI(v, "get_default_branch", func() (*github.Branch, *github.Response, error) { @@ -400,38 +399,43 @@ func (v *Provider) GetTektonDir(ctx context.Context, runevent *info.Event, path, v.Logger.Infof("Using PipelineRun definition from source %s %s on commit SHA %s", runevent.TriggerTarget.String(), prInfo, runevent.SHA) } - rootobjects, _, err := wrapAPI(v, "get_root_tree", func() (*github.Tree, *github.Response, error) { - return v.Client().Git.GetTree(ctx, runevent.Organization, runevent.Repository, revision, false) - }) + // Try GraphQL - fetches tekton tree + blob contents in single request + if v.graphQLClient == nil { + var err error + v.graphQLClient, err = newGraphQLClient(v) + if err != nil { + return "", fmt.Errorf("failed to create GraphQL client: %w", err) + } + } + + result, err := v.graphQLClient.fetchTektonDirGraphQL(ctx, runevent.Organization, runevent.Repository, revision, path) if err != nil { return "", err } - for _, object := range rootobjects.Entries { - if object.GetPath() == path { - if object.GetType() != "tree" { - return "", fmt.Errorf("%s has been found but is not a directory", path) - } - tektonDirSha = object.GetSHA() - } - } - // If we didn't find a .tekton directory then just silently ignore the error. - if tektonDirSha == "" { + // If no yaml files found, return empty (directory doesn't exist or is empty) + if len(result.FileContents) == 0 { return "", nil } - // Get all files in the .tekton directory recursively - // there is a limit on this recursive calls to 500 entries, as documented here: - // https://docs.github.com/en/rest/reference/git#get-a-tree - // so we may need to address it in the future. - tektonDirObjects, _, err := wrapAPI(v, "get_tekton_tree", func() (*github.Tree, *github.Response, error) { - return v.Client().Git.GetTree(ctx, runevent.Organization, runevent.Repository, tektonDirSha, - true) - }) - if err != nil { - return "", err + // Concatenate YAML files (result already filtered to .yaml/.yml blobs) + var buf strings.Builder + for filePath, content := range result.FileContents { + // Validate YAML + if err := provider.ValidateYaml(content, filePath); err != nil { + return "", err + } + + // Concatenate with separator + if buf.Len() > 0 && !strings.HasPrefix(string(content), "---") { + buf.WriteString("---") + } + buf.WriteString("\n") + buf.Write(content) + buf.WriteString("\n") } - return v.concatAllYamlFiles(ctx, tektonDirObjects.Entries, runevent, path, revision) + + return buf.String(), nil } // GetCommitInfo get info (url and title) on a commit in runevent, this needs to @@ -539,58 +543,6 @@ func (v *Provider) GetFileInsideRepo(ctx context.Context, runevent *info.Event, return string(getobj), nil } -// concatAllYamlFiles concat all yaml files from a directory as one big multi document yaml string. -func (v *Provider) concatAllYamlFiles(ctx context.Context, objects []*github.TreeEntry, runevent *info.Event, tektonDirPath, ref string) (string, error) { - var yamlFiles []string - for _, value := range objects { - if strings.HasSuffix(value.GetPath(), ".yaml") || - strings.HasSuffix(value.GetPath(), ".yml") { - fullPath := tektonDirPath + "/" + value.GetPath() - yamlFiles = append(yamlFiles, fullPath) - } - } - - if len(yamlFiles) == 0 { - return "", nil - } - - if v.graphQLClient == nil { - var err error - v.graphQLClient, err = newGraphQLClient(v) - if err != nil { - return "", fmt.Errorf("failed to create GraphQL client: %w", err) - } - } - - graphQLResults, err := v.graphQLClient.fetchFiles(ctx, runevent.Organization, runevent.Repository, ref, yamlFiles) - if err != nil { - return "", fmt.Errorf("failed to fetch .tekton files via GraphQL: %w", err) - } - - var buf strings.Builder - for _, path := range yamlFiles { - content, ok := graphQLResults[path] - if !ok { - return "", fmt.Errorf("file %s not found in GraphQL response", path) - } - - // it used to be like that (stripped prefix) before we moved to GraphQL so - // let's keep it that way. - relativePath := strings.TrimPrefix(path, tektonDirPath+"/") - if err := provider.ValidateYaml(content, relativePath); err != nil { - return "", err - } - if buf.Len() > 0 && !strings.HasPrefix(string(content), "---") { - buf.WriteString("---") - } - buf.WriteString("\n") - buf.Write(content) - buf.WriteString("\n") - } - - return buf.String(), nil -} - // getPullRequest get a pull request details, caching the result for the lifetime of the event. func (v *Provider) getPullRequest(ctx context.Context, runevent *info.Event) (*info.Event, error) { if v.cachedPullRequest != nil { diff --git a/pkg/provider/github/github_test.go b/pkg/provider/github/github_test.go index 42682a916..177831dad 100644 --- a/pkg/provider/github/github_test.go +++ b/pkg/provider/github/github_test.go @@ -252,10 +252,8 @@ func TestGetTektonDir(t *testing.T) { expectedString: "PipelineRun", treepath: "testdata/tree/simple", filterMessageSnippet: "Using PipelineRun definition from source pull_request tekton/cat#0", - // 1. Get Repo root objects - // 2. Get Tekton Dir objects - // 3. GraphQL batch fetch for 2 files (replaces 2 REST calls) - expectedGHApiCalls: 3, + // 1. Single GraphQL call fetches tekton tree + inline blobs + expectedGHApiCalls: 1, }, { name: "test no subtree on push", @@ -268,10 +266,8 @@ func TestGetTektonDir(t *testing.T) { expectedString: "PipelineRun", treepath: "testdata/tree/simple", filterMessageSnippet: "Using PipelineRun definition from source push", - // 1. Get Repo root objects - // 2. Get Tekton Dir objects - // 3. GraphQL batch fetch for 2 files (replaces 2 REST calls) - expectedGHApiCalls: 3, + // 1. Single GraphQL call fetches tekton tree + inline blobs + expectedGHApiCalls: 1, }, { name: "test provenance default_branch ", @@ -285,10 +281,8 @@ func TestGetTektonDir(t *testing.T) { provenance: "default_branch", filterMessageSnippet: "Using PipelineRun definition from default_branch: main", // 1. Resolve default branch to a commit SHA - // 2. Get Repo root objects - // 3. Get Tekton Dir objects - // 4. GraphQL batch fetch for 2 files - expectedGHApiCalls: 4, + // 2. Single GraphQL call fetches tekton tree + inline blobs + expectedGHApiCalls: 2, }, { name: "test with subtree", @@ -299,10 +293,8 @@ func TestGetTektonDir(t *testing.T) { }, expectedString: "FROMSUBTREE", treepath: "testdata/tree/subdir", - // 1. Get Repo root objects - // 2. Get Tekton Dir objects - // 3-5. Get object content for each object (foo/bar/pipeline.yaml) - expectedGHApiCalls: 3, + // 1. Single GraphQL call fetches tekton tree + inline blobs (including subdirs) + expectedGHApiCalls: 1, }, { name: "test with badly formatted yaml", @@ -314,10 +306,9 @@ func TestGetTektonDir(t *testing.T) { expectedString: "FROMSUBTREE", treepath: "testdata/tree/badyaml", wantErr: "error unmarshalling yaml file badyaml.yaml: yaml: line 2: did not find expected key", - // 1. Get Repo root objects - // 2. Get Tekton Dir objects - // 3. Get object content for object (badyaml.yaml) - expectedGHApiCalls: 3, + // 1. Single GraphQL call fetches tekton tree + inline blobs + // Error occurs during YAML validation after fetch + expectedGHApiCalls: 1, }, { name: "test no tekton directory", @@ -432,15 +423,48 @@ func TestGetTektonDirGraphQL(t *testing.T) { skipSetupGitTree bool }{ { - name: "graphql batch fetch reduces api calls", + name: "graphql fetch tekton dir with inline blobs", event: &info.Event{ Organization: "tekton", Repository: "cat", TriggerTarget: triggertype.PullRequest, }, - treepath: "testdata/tree/simple", - wantLogSnippet: "GraphQL batch fetch", - expectedAPICount: 3, + skipSetupGitTree: true, + setup: func(t *testing.T, mux *http.ServeMux, event *info.Event) { + t.Helper() + shaDir := fmt.Sprintf("%x", sha256.Sum256([]byte("testdata/tree/simple"))) + event.SHA = shaDir + + // Setup GraphQL endpoint with inline blob contents + mux.HandleFunc("/api/graphql", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "data": map[string]any{ + "repository": map[string]any{ + "tektonTree": map[string]any{ + "entries": []map[string]any{ + { + "name": "pipeline.yaml", + "type": "blob", + "path": "pipeline.yaml", + "oid": "pipeline-sha", + "object": map[string]any{"text": "kind: Pipeline\nmetadata:\n name: test\n"}, + }, + { + "name": "task.yaml", + "type": "blob", + "path": "task.yaml", + "oid": "task-sha", + "object": map[string]any{"text": "kind: Task\nmetadata:\n name: test\n"}, + }, + }, + }, + }, + }, + }) + }) + }, + wantLogSnippet: "GitHub API call completed", + expectedAPICount: 1, // Single GraphQL call fetches tree + blobs }, { name: "graphql error handling", @@ -500,7 +524,7 @@ func TestGetTektonDirGraphQL(t *testing.T) { http.Error(w, "GraphQL endpoint not available", http.StatusNotFound) }) }, - wantErr: "failed to fetch .tekton files via GraphQL", + wantErr: "GraphQL request failed with status 404", }, { name: "default branch uses resolved sha for graphql", @@ -514,7 +538,6 @@ func TestGetTektonDirGraphQL(t *testing.T) { setup: func(t *testing.T, mux *http.ServeMux, _ *info.Event) { t.Helper() resolvedSHA := "resolved-default-branch-sha" - tektonDirSHA := "tektondirsha" mux.HandleFunc("/repos/tekton/cat/branches/main", func(rw http.ResponseWriter, _ *http.Request) { branch := &github.Branch{ @@ -526,58 +549,44 @@ func TestGetTektonDirGraphQL(t *testing.T) { b, _ := json.Marshal(branch) fmt.Fprint(rw, string(b)) }) - mux.HandleFunc("/repos/tekton/cat/git/trees/"+resolvedSHA, func(rw http.ResponseWriter, _ *http.Request) { - tree := &github.Tree{ - SHA: github.Ptr(resolvedSHA), - Entries: []*github.TreeEntry{ - { - Path: github.Ptr(".tekton"), - Type: github.Ptr("tree"), - SHA: github.Ptr(tektonDirSHA), - }, - }, - } - b, _ := json.Marshal(tree) - fmt.Fprint(rw, string(b)) - }) - mux.HandleFunc("/repos/tekton/cat/git/trees/"+tektonDirSHA, func(rw http.ResponseWriter, _ *http.Request) { - tree := &github.Tree{ - SHA: github.Ptr(tektonDirSHA), - Entries: []*github.TreeEntry{ - { - Path: github.Ptr("pipeline.yaml"), - Type: github.Ptr("blob"), - SHA: github.Ptr("pipeline-sha"), - }, - { - Path: github.Ptr("pipelinerun.yaml"), - Type: github.Ptr("blob"), - SHA: github.Ptr("pipelinerun-sha"), - }, - }, - } - b, _ := json.Marshal(tree) - fmt.Fprint(rw, string(b)) - }) mux.HandleFunc("/api/graphql", func(w http.ResponseWriter, r *http.Request) { var graphQLReq struct { - Query string `json:"query"` + Query string `json:"query"` + Variables map[string]any `json:"variables"` } assert.NilError(t, json.NewDecoder(r.Body).Decode(&graphQLReq)) - assert.Assert(t, strings.Contains(graphQLReq.Query, resolvedSHA+":.tekton/pipeline.yaml"), graphQLReq.Query) - assert.Assert(t, !strings.Contains(graphQLReq.Query, `main:.tekton/pipeline.yaml`), graphQLReq.Query) + // New implementation uses tektonExpr variable: "resolvedSHA:.tekton" + assert.Assert(t, strings.Contains(graphQLReq.Query, "tektonTree:"), graphQLReq.Query) + assert.Assert(t, graphQLReq.Variables["tektonExpr"] == resolvedSHA+":.tekton", "expected tektonExpr=%s:.tekton, got %v", resolvedSHA, graphQLReq.Variables["tektonExpr"]) + // Return tree structure with inline blob contents _ = json.NewEncoder(w).Encode(map[string]any{ "data": map[string]any{ "repository": map[string]any{ - "file0": map[string]any{"text": "kind: Pipeline\nmetadata:\n name: pipeline\n"}, - "file1": map[string]any{"text": "kind: PipelineRun\nmetadata:\n name: run\n"}, + "tektonTree": map[string]any{ + "entries": []map[string]any{ + { + "name": "pipeline.yaml", + "type": "blob", + "path": "pipeline.yaml", + "oid": "pipeline-sha", + "object": map[string]any{"text": "kind: Pipeline\nmetadata:\n name: pipeline\n"}, + }, + { + "name": "pipelinerun.yaml", + "type": "blob", + "path": "pipelinerun.yaml", + "oid": "pipelinerun-sha", + "object": map[string]any{"text": "kind: PipelineRun\nmetadata:\n name: run\n"}, + }, + }, + }, }, }, }) }) }, - expectedAPICount: 4, + expectedAPICount: 2, // 1 for get_default_branch + 1 for GraphQL }, } diff --git a/pkg/provider/github/graphql.go b/pkg/provider/github/graphql.go index 4541afc27..7e4d52830 100644 --- a/pkg/provider/github/graphql.go +++ b/pkg/provider/github/graphql.go @@ -2,18 +2,13 @@ package github import ( "context" - "encoding/json" "fmt" - "io" - "maps" "net/http" "net/url" "strings" - "time" "github.com/google/go-github/v85/github" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" - providerMetrics "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/providermetrics" "go.uber.org/zap" ) @@ -87,38 +82,62 @@ func buildGraphQLEndpoint(p *Provider) (string, error) { return parsedURL.String(), nil } -// buildGraphQLQuery constructs a GraphQL query string with aliases for batch fetching multiple files. -func buildGraphQLQuery(ref string, paths []string) string { - // Escape ref for GraphQL string (escape quotes and backslashes) - escapedRef := strings.ReplaceAll(ref, `\`, `\\`) - escapedRef = strings.ReplaceAll(escapedRef, `"`, `\"`) +// TektonDirResult holds the result of fetching tekton directory via GraphQL. +type TektonDirResult struct { + SHA string // Resolved SHA + FileContents map[string][]byte // YAML file contents by path (only .yaml/.yml blobs) +} - aliases := make([]string, 0, len(paths)) - for i, path := range paths { - // Escape path for GraphQL string (escape quotes and backslashes) - escapedPath := strings.ReplaceAll(path, `\`, `\\`) - escapedPath = strings.ReplaceAll(escapedPath, `"`, `\"`) - aliases = append(aliases, fmt.Sprintf(` file%d: object(expression: "%s:%s") { - ... on Blob { - text - } - }`, i, escapedRef, escapedPath)) - } +// buildTektonDirQuery constructs a GraphQL query for fetching tekton directory tree + blob contents. +// Uses the provided SHA to fetch the .tekton tree with inline blob contents. +func buildTektonDirQuery(sha, path string) (query string, variables map[string]any) { + tektonExpr := sha + ":" + path - query := fmt.Sprintf(`query($owner: String!, $name: String!) { + query = `query($owner: String!, $name: String!, $tektonExpr: String!) { repository(owner: $owner, name: $name) { -%s + tektonTree: object(expression: $tektonExpr) { + ... on Tree { + entries { + name + type + path + oid + object { + ... on Blob { + text + } + } + } + } + } } -}`, strings.Join(aliases, "\n")) +}` + + variables = map[string]any{ + "tektonExpr": tektonExpr, + } - return query + return query, variables } -// graphQLResponse represents the structure of a GraphQL API response. -type graphQLResponse struct { +// tektonDirResponse represents the GraphQL response structure for tekton directory fetch. +type tektonDirResponse struct { Data struct { - Repository map[string]struct { - Text *string `json:"text"` + Repository struct { + TektonTree *struct { + // Tree response has entries field + Entries []struct { + Name string `json:"name"` + Type string `json:"type"` + Path string `json:"path"` + Oid string `json:"oid"` + Object *struct { + Text *string `json:"text"` + } `json:"object,omitempty"` + } `json:"entries,omitempty"` + // Blob response has text field (indicates .tekton is a file, not directory) + Text *string `json:"text,omitempty"` + } `json:"tektonTree"` } `json:"repository"` } `json:"data"` Errors []struct { @@ -126,55 +145,12 @@ type graphQLResponse struct { } `json:"errors,omitempty"` } -type rateLimitHeaders struct { - limit string - remaining string - reset string -} - -func getRateLimitHeaders(header http.Header) rateLimitHeaders { - return rateLimitHeaders{ - limit: header.Get("X-RateLimit-Limit"), - remaining: header.Get("X-RateLimit-Remaining"), - reset: header.Get("X-RateLimit-Reset"), - } -} - -// fetchFiles fetches multiple file contents using GraphQL batch queries. -// Returns a map of path -> content. -func (c *graphQLClient) fetchFiles(ctx context.Context, owner, repo, ref string, paths []string) (map[string][]byte, error) { - if len(paths) == 0 { - return make(map[string][]byte), nil - } - - // Limit batch size to avoid query complexity issues - const maxBatchSize = 50 - result := make(map[string][]byte, len(paths)) - for start := 0; start < len(paths); start += maxBatchSize { - end := min(start+maxBatchSize, len(paths)) - batch := paths[start:end] - batchResult, err := c.fetchFilesBatch(ctx, owner, repo, ref, batch) - if err != nil { - return nil, err - } - maps.Copy(result, batchResult) - } - - return result, nil -} - -// fetchFilesBatch fetches multiple file contents in a single GraphQL query. -// Returns a map of path -> content. -func (c *graphQLClient) fetchFilesBatch(ctx context.Context, owner, repo, ref string, paths []string) (map[string][]byte, error) { - if len(paths) == 0 { - return make(map[string][]byte), nil - } - - query := buildGraphQLQuery(ref, paths) - variables := map[string]any{ - "owner": owner, - "name": repo, - } +// fetchTektonDirGraphQL fetches the tekton directory tree + blob contents using GraphQL. +// Fetches the .tekton tree at the given SHA with inline blob contents in a single request. +func (c *graphQLClient) fetchTektonDirGraphQL(ctx context.Context, owner, repo, sha, path string) (*TektonDirResult, error) { + query, variables := buildTektonDirQuery(sha, path) + variables["owner"] = owner + variables["name"] = repo requestBody := map[string]any{ "query": query, @@ -185,51 +161,18 @@ func (c *graphQLClient) fetchFilesBatch(ctx context.Context, owner, repo, ref st if err != nil { return nil, fmt.Errorf("failed to create GraphQL request: %w", err) } - req = req.WithContext(ctx) - - // Record metrics for GraphQL API call - if c.logger != nil { - providerMetrics.RecordAPIUsage(c.logger, c.provider.providerName, c.triggerEvent, c.repo) - } - - start := time.Now() - resp, err := c.httpClient.Do(req) - duration := time.Since(start) - - if err != nil { - if c.logger != nil { - c.logger.Debugw("GraphQL request failed", - "error", err.Error(), - "duration_ms", duration.Milliseconds(), - ) - } - return nil, fmt.Errorf("GraphQL request failed: %w", err) - } - defer resp.Body.Close() - - rateLimit := getRateLimitHeaders(resp.Header) - body, err := io.ReadAll(resp.Body) + graphQLResp, resp, err := wrapAPI(c.provider, "graphql_get_tekton_dir", func() (tektonDirResponse, *github.Response, error) { + var graphQLResp tektonDirResponse + resp, err := c.ghClient.Do(ctx, req.WithContext(ctx), &graphQLResp) + return graphQLResp, resp, err + }) if err != nil { - return nil, fmt.Errorf("failed to read GraphQL response: %w", err) + return nil, fmt.Errorf("error fetching tekton directory using GraphQL: %w", err) } if resp.StatusCode != http.StatusOK { - if c.logger != nil { - c.logger.Debugw("GraphQL request returned non-200 status", - "status_code", resp.StatusCode, - "response", string(body), - "rate_limit", rateLimit.limit, - "rate_limit_remaining", rateLimit.remaining, - "rate_limit_reset", rateLimit.reset, - ) - } - return nil, fmt.Errorf("GraphQL request failed with status %d: %s", resp.StatusCode, string(body)) - } - - var graphQLResp graphQLResponse - if err := json.Unmarshal(body, &graphQLResp); err != nil { - return nil, fmt.Errorf("failed to unmarshal GraphQL response: %w", err) + return nil, fmt.Errorf("GraphQL request failed with status %d: %w", resp.StatusCode, err) } if len(graphQLResp.Errors) > 0 { @@ -238,36 +181,40 @@ func (c *graphQLClient) fetchFilesBatch(ctx context.Context, owner, repo, ref st errorMsgs[i] = e.Message } if c.logger != nil { - c.logger.Debugw("GraphQL returned errors", + c.logger.Debugw("GraphQL tekton dir returned errors", "errors", strings.Join(errorMsgs, "; "), ) } return nil, fmt.Errorf("GraphQL errors: %s", strings.Join(errorMsgs, "; ")) } - result := make(map[string][]byte, len(paths)) - for i, path := range paths { - alias := fmt.Sprintf("file%d", i) - blobData, ok := graphQLResp.Data.Repository[alias] - if !ok { - return nil, fmt.Errorf("file %s (alias %s) not found in GraphQL response", path, alias) - } + result := &TektonDirResult{ + SHA: sha, + FileContents: make(map[string][]byte), + } - if blobData.Text == nil { - return nil, fmt.Errorf("file %s returned null content (may be binary)", path) + // Check if tektonTree exists and what type it is + if graphQLResp.Data.Repository.TektonTree != nil { + // If TektonTree has Text field, it's a Blob (file), not a Tree (directory) + if graphQLResp.Data.Repository.TektonTree.Text != nil { + return nil, fmt.Errorf("%s has been found but is not a directory", path) } - result[path] = []byte(*blobData.Text) - } - - if c.logger != nil { - c.logger.Debugw("GraphQL batch fetch completed", - "files_requested", len(paths), - "duration_ms", duration.Milliseconds(), - "rate_limit", rateLimit.limit, - "rate_limit_remaining", rateLimit.remaining, - "rate_limit_reset", rateLimit.reset, - ) + // Extract YAML blob contents from Tree entries (filter for .yaml/.yml files during parsing) + for _, entry := range graphQLResp.Data.Repository.TektonTree.Entries { + // Only process yaml/yml blobs + if entry.Type != "blob" { + continue + } + if !strings.HasSuffix(entry.Path, ".yaml") && !strings.HasSuffix(entry.Path, ".yml") { + continue + } + + // Extract blob content + if entry.Object != nil && entry.Object.Text != nil { + result.FileContents[entry.Path] = []byte(*entry.Object.Text) + } + } } return result, nil diff --git a/pkg/provider/github/graphql_test.go b/pkg/provider/github/graphql_test.go index 1b0981aad..449e41adc 100644 --- a/pkg/provider/github/graphql_test.go +++ b/pkg/provider/github/graphql_test.go @@ -3,11 +3,9 @@ package github import ( "context" "encoding/json" - "fmt" "net/http" "net/http/httptest" "net/url" - "strings" "testing" "github.com/google/go-github/v85/github" @@ -56,36 +54,6 @@ func withServer(t *testing.T, h http.Handler) *httptest.Server { return s } -func graphqlOK(repo map[string]any) http.HandlerFunc { - return func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("X-RateLimit-Limit", "5000") - w.Header().Set("X-RateLimit-Remaining", "4999") - w.Header().Set("X-RateLimit-Reset", "1735689600") - _ = json.NewEncoder(w).Encode(map[string]any{ - "data": map[string]any{"repository": repo}, - }) - } -} - -func graphqlStatus(code int) http.HandlerFunc { - return func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("X-RateLimit-Limit", "5000") - w.Header().Set("X-RateLimit-Remaining", "4998") - w.Header().Set("X-RateLimit-Reset", "1735689601") - w.WriteHeader(code) - } -} - -func mockRepo(n int) map[string]any { - repo := make(map[string]any) - for i := range n { - repo[fmt.Sprintf("file%d", i)] = map[string]any{ - "text": fmt.Sprintf("content-%d", i), - } - } - return repo -} - func TestBuildGraphQLEndpoint(t *testing.T) { cases := []struct { name string @@ -112,74 +80,121 @@ func TestBuildGraphQLEndpoint(t *testing.T) { } } -func TestBuildGraphQLQuery(t *testing.T) { +func TestBuildTektonDirQuery(t *testing.T) { cases := []struct { - name string - ref string - paths []string - want []string + name string + sha string + path string + wantExpr string }{ { - name: "two files", - ref: "main", - paths: []string{"a", "b"}, - want: []string{"query(", "repository(", "file0:", "file1:"}, - }, - { - name: "no files", - ref: "main", - paths: nil, - want: []string{"query(", "repository("}, + name: "simple path", + sha: "abc123", + path: ".tekton", + wantExpr: "abc123:.tekton", }, { - name: "filename with double quotes", - ref: "main", - paths: []string{`file"name.yaml`}, - want: []string{"query(", "repository(", `file\"name.yaml`}, + name: "nested path", + sha: "def456", + path: ".tekton/pipelines", + wantExpr: "def456:.tekton/pipelines", }, { - name: "filename with backslash", - ref: "main", - paths: []string{`file\name.yaml`}, - want: []string{"query(", "repository(", `file\\name.yaml`}, - }, - { - name: "filename with both quote and backslash", - ref: "main", - paths: []string{`file\"name.yaml`}, - want: []string{"query(", "repository(", `file\\\"name.yaml`}, + name: "branch name as sha", + sha: "main", + path: ".tekton", + wantExpr: "main:.tekton", }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - q := buildGraphQLQuery(tc.ref, tc.paths) - for _, s := range tc.want { - assert.Check(t, strings.Contains(q, s)) - } + query, vars := buildTektonDirQuery(tc.sha, tc.path) + + // Verify query contains key GraphQL structure + assert.Check(t, cmp.Contains(query, "query(")) + assert.Check(t, cmp.Contains(query, "repository(")) + assert.Check(t, cmp.Contains(query, "tektonTree:")) + assert.Check(t, cmp.Contains(query, "object(expression:")) + assert.Check(t, cmp.Contains(query, "... on Tree")) + assert.Check(t, cmp.Contains(query, "entries")) + assert.Check(t, cmp.Contains(query, "... on Blob")) + assert.Check(t, cmp.Contains(query, "text")) + + // Verify variables + assert.Check(t, cmp.Equal(tc.wantExpr, vars["tektonExpr"])) }) } } -func TestFetchFilesBatch(t *testing.T) { +func TestFetchTektonDirGraphQL(t *testing.T) { cases := []struct { - name string - paths []string - handler http.HandlerFunc - want int - wantErr bool + name string + handler http.HandlerFunc + wantFiles int + wantErr bool + errContains string }{ { - name: "single batch", - paths: []string{"a", "b"}, - handler: graphqlOK(mockRepo(2)), - want: 2, + name: "http error", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-RateLimit-Limit", "5000") + w.Header().Set("X-RateLimit-Remaining", "4997") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte("GraphQL endpoint not available")) + }, + wantErr: true, + errContains: "GraphQL request failed with status 404", + }, + { + name: "graphql errors", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-RateLimit-Limit", "5000") + w.Header().Set("X-RateLimit-Remaining", "4996") + _ = json.NewEncoder(w).Encode(map[string]any{ + "errors": []map[string]any{ + {"message": "Field 'tektonTree' doesn't exist"}, + }, + }) + }, + wantErr: true, + errContains: "GraphQL errors", }, { - name: "http error", - paths: []string{"a"}, - handler: graphqlStatus(http.StatusNotFound), - wantErr: true, + name: "null blob content", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-RateLimit-Limit", "5000") + w.Header().Set("X-RateLimit-Remaining", "4992") + _ = json.NewEncoder(w).Encode(map[string]any{ + "data": map[string]any{ + "repository": map[string]any{ + "tektonTree": map[string]any{ + "entries": []map[string]any{ + { + "name": "empty.yaml", + "type": "blob", + "path": "empty.yaml", + "oid": "abc123", + "object": map[string]any{ + "text": nil, // Null content + }, + }, + { + "name": "valid.yaml", + "type": "blob", + "path": "valid.yaml", + "oid": "def456", + "object": map[string]any{ + "text": "kind: Pipeline", + }, + }, + }, + }, + }, + }, + }) + }, + wantFiles: 1, // Only valid.yaml counted }, } @@ -191,34 +206,32 @@ func TestFetchFilesBatch(t *testing.T) { srv := withServer(t, mux) c, observedLogs := newTestGraphQLClient(t, srv.URL+"/api/v3/") - res, err := c.fetchFilesBatch(context.Background(), "o", "r", "main", tc.paths) + result, err := c.fetchTektonDirGraphQL(context.Background(), "owner", "repo", "abc123", ".tekton") + if tc.wantErr { assert.Assert(t, err != nil) - entries := observedLogs.FilterMessage("GraphQL request returned non-200 status").All() - assert.Check(t, cmp.Len(entries, 1)) - assert.Check(t, cmp.Equal(entries[0].ContextMap()["rate_limit"], "5000")) - assert.Check(t, cmp.Equal(entries[0].ContextMap()["rate_limit_remaining"], "4998")) - assert.Check(t, cmp.Equal(entries[0].ContextMap()["rate_limit_reset"], "1735689601")) + if tc.errContains != "" { + assert.Check(t, cmp.Contains(err.Error(), tc.errContains)) + } return } assert.NilError(t, err) - assert.Check(t, cmp.Len(res, tc.want)) - entries := observedLogs.FilterMessage("GraphQL batch fetch completed").All() + assert.Check(t, cmp.Equal(tc.wantFiles, len(result.FileContents))) + assert.Check(t, cmp.Equal("abc123", result.SHA)) + + // Verify API call logging from wrapAPI + entries := observedLogs.FilterMessage("GitHub API call completed").All() assert.Check(t, cmp.Len(entries, 1)) - assert.Check(t, cmp.Equal(entries[0].ContextMap()["files_requested"], int64(len(tc.paths)))) - assert.Check(t, cmp.Equal(entries[0].ContextMap()["rate_limit"], "5000")) - assert.Check(t, cmp.Equal(entries[0].ContextMap()["rate_limit_remaining"], "4999")) - assert.Check(t, cmp.Equal(entries[0].ContextMap()["rate_limit_reset"], "1735689600")) - _, exists := entries[0].ContextMap()["files_fetched"] - assert.Check(t, !exists) + assert.Check(t, cmp.Equal(entries[0].ContextMap()["operation"], "graphql_get_tekton_dir")) }) } } -func TestFetchFilesBatchPreservesGitHubHeaders(t *testing.T) { +func TestFetchTektonDirGraphQLPreservesGitHubHeaders(t *testing.T) { mux := http.NewServeMux() mux.HandleFunc("/api/graphql", func(w http.ResponseWriter, r *http.Request) { + // Verify GitHub client headers are preserved assert.Assert(t, r.Header.Get("User-Agent") != "") assert.Assert(t, r.Header.Get("X-GitHub-Api-Version") != "") assert.Check(t, cmp.Equal("application/json", r.Header.Get("Content-Type"))) @@ -226,7 +239,17 @@ func TestFetchFilesBatchPreservesGitHubHeaders(t *testing.T) { _ = json.NewEncoder(w).Encode(map[string]any{ "data": map[string]any{ "repository": map[string]any{ - "file0": map[string]any{"text": "content-0"}, + "tektonTree": map[string]any{ + "entries": []map[string]any{ + { + "name": "test.yaml", + "type": "blob", + "path": "test.yaml", + "oid": "abc", + "object": map[string]any{"text": "test"}, + }, + }, + }, }, }, }) @@ -235,7 +258,6 @@ func TestFetchFilesBatchPreservesGitHubHeaders(t *testing.T) { srv := withServer(t, mux) c, _ := newTestGraphQLClient(t, srv.URL+"/api/v3/") - res, err := c.fetchFilesBatch(context.Background(), "o", "r", "main", []string{"a"}) + _, err := c.fetchTektonDirGraphQL(context.Background(), "o", "r", "sha", ".tekton") assert.NilError(t, err) - assert.Check(t, cmp.Len(res, 1)) } diff --git a/pkg/test/github/github.go b/pkg/test/github/github.go index 51bc2fb7b..ba49b5fcf 100644 --- a/pkg/test/github/github.go +++ b/pkg/test/github/github.go @@ -68,6 +68,107 @@ type graphQLFileMapType map[string]struct { isdir bool } +// graphQLRequest represents a GraphQL request structure. +type graphQLRequest struct { + Query string `json:"query"` + Variables map[string]any `json:"variables"` +} + +// handleTektonTreeQuery handles the new GraphQL query format for fetching tekton tree with inline blobs. +func handleTektonTreeQuery(w http.ResponseWriter, graphQLReq graphQLRequest, allFiles graphQLFileMapType) { + // Extract tektonExpr from variables (e.g., "sha123:.tekton") + tektonExpr, ok := graphQLReq.Variables["tektonExpr"].(string) + if !ok { + http.Error(w, "Missing tektonExpr variable", http.StatusBadRequest) + return + } + + // Parse tektonExpr: "ref:path" -> extract path + parts := strings.SplitN(tektonExpr, ":", 2) + if len(parts) != 2 { + http.Error(w, "Invalid tektonExpr format", http.StatusBadRequest) + return + } + tektonPath := parts[1] + + // Check if .tekton itself is a file in allFiles + if fileInfo, exists := allFiles[tektonPath]; exists { + // .tekton is a file, not a directory - return Blob response (no entries field) + content, err := os.ReadFile(fileInfo.name) + if err != nil { + http.Error(w, fmt.Sprintf("Failed to read file: %v", err), http.StatusInternalServerError) + return + } + + // Return Blob object response (this will cause the implementation to see no Tree) + responseData := map[string]any{ + "data": map[string]any{ + "repository": map[string]any{ + "tektonTree": map[string]any{ + "text": string(content), // Blob has text field, not entries + }, + }, + }, + } + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(responseData) + return + } + + // Build tree entries for all files under the tekton path + entries := []map[string]any{} + for relPath, fileInfo := range allFiles { + // Check if file is under the tekton path + // For .tekton directory, files should be like "pipeline.yaml", "task.yaml", "subdir/file.yaml" + if !strings.HasPrefix(relPath, tektonPath+"/") && relPath != tektonPath { + continue + } + + // Extract path relative to tekton directory + pathInTekton := strings.TrimPrefix(relPath, tektonPath+"/") + if pathInTekton == "" { + continue + } + + // Read file content + content, err := os.ReadFile(fileInfo.name) + if err != nil { + continue + } + + // Only include .yaml and .yml files (matching implementation behavior) + if !strings.HasSuffix(relPath, ".yaml") && !strings.HasSuffix(relPath, ".yml") { + continue + } + + entry := map[string]any{ + "name": filepath.Base(pathInTekton), + "type": "blob", + "path": pathInTekton, + "oid": fileInfo.sha, + "object": map[string]any{ + "text": string(content), + }, + } + entries = append(entries, entry) + } + + // Build response + responseData := map[string]any{ + "data": map[string]any{ + "repository": map[string]any{ + "tektonTree": map[string]any{ + "entries": entries, + }, + }, + }, + } + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(responseData) +} + // SetupGitTree Take a dir and fake a full GitTree GitHub api calls reply recursively over a muxer. func SetupGitTree(t *testing.T, mux *http.ServeMux, dir string, event *info.Event, recursive bool) { type file struct { @@ -155,7 +256,7 @@ func SetupGitTree(t *testing.T, mux *http.ServeMux, dir string, event *info.Even fmt.Fprint(rw, string(b)) }) - // Setup GraphQL endpoint handler for batch file fetching (only once per mux) + // Setup GraphQL endpoint handler for tekton directory fetching (only once per mux) // Only register GraphQL handler once (at the root level, when recursive=false) if !recursive { // Walk the entire directory tree to collect all files for the GraphQL handler @@ -184,90 +285,13 @@ func SetupGitTree(t *testing.T, mux *http.ServeMux, dir string, event *info.Even return } - var graphQLReq struct { - Query string `json:"query"` - Variables map[string]any `json:"variables"` - } + var graphQLReq graphQLRequest if err := json.NewDecoder(r.Body).Decode(&graphQLReq); err != nil { http.Error(w, fmt.Sprintf("Invalid GraphQL request: %v", err), http.StatusBadRequest) return } - // Build response with file contents - repositoryData := make(map[string]any) - - // Parse query to extract aliases and paths - queryLines := strings.SplitSeq(graphQLReq.Query, "\n") - for line := range queryLines { - line = strings.TrimSpace(line) - if strings.Contains(line, ": object(expression:") && strings.Contains(line, "file") { - // Extract alias (e.g., "file0") - aliasEnd := strings.Index(line, ":") - if aliasEnd <= 0 { - continue - } - alias := strings.TrimSpace(line[:aliasEnd]) - - // Extract expression value between quotes: "ref:path" - exprStart := strings.Index(line, `expression: "`) - if exprStart < 0 { - continue - } - exprStart += len(`expression: "`) - exprEnd := strings.Index(line[exprStart:], `"`) - if exprEnd < 0 { - continue - } - expr := line[exprStart : exprStart+exprEnd] - // Unescape the expression (handle \" and \\) - expr = strings.ReplaceAll(expr, `\"`, `"`) - expr = strings.ReplaceAll(expr, `\\`, `\`) - // Split "ref:path" and take path - parts := strings.SplitN(expr, ":", 2) - if len(parts) != 2 { - continue - } - path := parts[1] - - // Look up file by path in the file map - var foundFile struct { - sha, name string - isdir bool - } - var found bool - if f, ok := allFiles[path]; ok { - foundFile = f - found = true - } else { - // Try to find by matching the end of the path or other variations - for k, f := range allFiles { - if strings.HasSuffix(k, "/"+path) || k == path { - foundFile = f - found = true - break - } - } - } - - if found { - content, err := os.ReadFile(foundFile.name) - if err == nil { - repositoryData[alias] = map[string]any{ - "text": string(content), - } - } - } - } - } - - responseData := map[string]any{ - "data": map[string]any{ - "repository": repositoryData, - }, - } - - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(responseData) + handleTektonTreeQuery(w, graphQLReq, allFiles) }) } }