Skip to content

Reduce Github GetTektonDir API requests from 3 requests to 1 request #2721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
aThorp96 wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Reduce Github GetTektonDir API requests from 3 requests to 1 request #2721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1472007
perf(github): reduce GetTektonDir from 3 REST calls to 1 GraphQL
aThorp96 May 11, 2026
20b9c5d
test(github): remove redundant GraphQL test cases
aThorp96 May 12, 2026
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
110 changes: 31 additions & 79 deletions pkg/provider/github/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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")
}
Comment thread
aThorp96 marked this conversation as resolved.
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
Expand Down Expand Up @@ -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 {
Expand Down
139 changes: 74 additions & 65 deletions pkg/provider/github/github_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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 ",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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{
Expand All @@ -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
},
}

Expand Down
Loading
Loading