From 16e267c7daa8b6b278d3d5eb6802c75e53256c02 Mon Sep 17 00:00:00 2001 From: dawn Date: Sun, 05 Jul 2026 19:20:24 +0000 Subject: [PATCH] spindle,knotserver: honor skip-ci and restore verbose-ci push options this adds back skip-ci and verbose-ci, they were regressed in earlier spindle rewrite work Signed-off-by: dawn --- docs/DOCS.md | 8 ++++++++ knotserver/git/post_receive.go | 9 +++++++++ knotserver/internal.go | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ knotserver/internal_pushoptions_test.go | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ spindle/server.go | 5 +++++ spindle/server_test.go | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 6 file(s) changed, 249 insertion(s)(+), 6 deletion(s)(-) diff --git a/docs/DOCS.md b/docs/DOCS.md --- a/docs/DOCS.md +++ b/docs/DOCS.md @@ -843,6 +843,14 @@ branch: ["main", "release-*"] tag: ["v*", "stable"] ``` +To skip CI for a push, pass a Git push option: + +```sh +git push -o skip-ci +``` + +`ci-skip` is also accepted. + ### Engine Next is the engine on which the workflow should run, defined diff --git a/knotserver/git/post_receive.go b/knotserver/git/post_receive.go --- a/knotserver/git/post_receive.go +++ b/knotserver/git/post_receive.go @@ -171,3 +171,12 @@ Inputs: langs, }, } } +func HasSkipCIPushOption(pushOptions []string) bool { + for _, opt := range pushOptions { + switch opt { + case "skip-ci", "ci-skip": + return true + } + } + return false +} diff --git a/knotserver/internal.go b/knotserver/internal.go --- a/knotserver/internal.go +++ b/knotserver/internal.go @@ -28,6 +28,7 @@ "tangled.org/core/log" "tangled.org/core/notifier" "tangled.org/core/rbac" "tangled.org/core/tid" + "tangled.org/core/workflow" ) type InternalHandle struct { @@ -242,12 +243,19 @@ if len(pushOptions) > 50 { pushOptions = pushOptions[:50] } + repoPath, _, _, resolveErr := h.db.ResolveRepoDIDOnDisk(h.c.Repo.ScanPath, repoDid) + if resolveErr != nil { + l.Error("failed to resolve repo on disk", "repoDid", repoDid, "err", resolveErr) + w.WriteHeader(http.StatusInternalServerError) + return + } + resp := hook.HookResponse{ Messages: make([]string, 0), } for _, line := range lines { - err := h.insertRefUpdate(line, gitUserDid, ownerDid, repoDid, pushOptions) + err := h.insertRefUpdate(line, gitUserDid, ownerDid, repoDid, repoPath, pushOptions) if err != nil { l.Error("failed to insert op", "err", err, "line", line, "did", gitUserDid, "repo", gitRelativeDir) } @@ -257,6 +265,13 @@ if err != nil { l.Error("failed to reply with pull request link", "err", err, "line", line, "did", gitUserDid, "repo", gitRelativeDir) } + if !git.HasSkipCIPushOption(pushOptions) { + verbose := hasVerboseCIPushOption(pushOptions) + if err := h.emitCiDiagnostics(&resp.Messages, line, ownerDid, repoName, repoDid, repoPath, verbose); err != nil { + l.Error("failed to emit ci diagnostics", "err", err, "line", line, "did", gitUserDid, "repo", gitRelativeDir) + } + } + // emit pipeline logs link if h.c.LogsAddr != "" { host, port, err := net.SplitHostPort(h.c.LogsAddr) @@ -270,7 +285,7 @@ writeJSON(w, resp) } -func (h *InternalHandle) insertRefUpdate(line git.PostReceiveLine, gitUserDid, ownerDid, repoDid string, pushOptions []string) error { +func (h *InternalHandle) insertRefUpdate(line git.PostReceiveLine, gitUserDid, ownerDid, repoDid string, repoPath string, pushOptions []string) error { refUpdate := tangled.GitRefUpdate{ OldSha: line.OldSha.String(), NewSha: line.NewSha.String(), @@ -283,10 +298,6 @@ PushOptions: pushOptions, } if !line.NewSha.IsZero() { - repoPath, _, _, resolveErr := h.db.ResolveRepoDIDOnDisk(h.c.Repo.ScanPath, repoDid) - if resolveErr != nil { - return fmt.Errorf("failed to resolve repo on disk: %w", resolveErr) - } gr, err := git.Open(repoPath, line.Ref) if err != nil { @@ -320,6 +331,104 @@ EventJson: eventJson, } return h.db.InsertEvent(event, h.n) +} + +func hasVerboseCIPushOption(pushOptions []string) bool { + for _, opt := range pushOptions { + switch opt { + case "verbose-ci", "ci-verbose": + return true + } + } + return false +} + +func (h *InternalHandle) emitCiDiagnostics( + clientMsgs *[]string, + line git.PostReceiveLine, + ownerDid string, + repoName string, + repoDid string, + repoPath string, + verbose bool, +) error { + if line.NewSha.IsZero() { + return nil + } + + gr, err := git.Open(repoPath, line.Ref) + if err != nil { + return fmt.Errorf("failed to open git repo at ref %s: %w", line.Ref, err) + } + + workflowDir, err := gr.FileTree(context.Background(), workflow.WorkflowDir) + if err != nil { + return nil + } + + var pipeline workflow.RawPipeline + for _, e := range workflowDir { + if !e.IsFile() { + continue + } + + fpath := filepath.Join(workflow.WorkflowDir, e.Name) + contents, err := gr.RawContent(fpath) + if err != nil { + continue + } + + pipeline = append(pipeline, workflow.RawWorkflow{ + Name: e.Name, + Contents: contents, + }) + } + + defaultBranch, _ := gr.FindMainBranch() + + trigger := tangled.Pipeline_PushTriggerData{ + Ref: line.Ref, + OldSha: line.OldSha.String(), + NewSha: line.NewSha.String(), + } + + triggerRepo := &tangled.Pipeline_TriggerRepo{ + Did: ownerDid, + Knot: h.c.Server.Hostname, + Repo: &repoName, + RepoDid: &repoDid, + DefaultBranch: defaultBranch, + } + + changedFiles, err := gr.ChangedFilesBetween(line.OldSha.String(), line.NewSha.String()) + if err != nil { + return fmt.Errorf("getting changed files: %w", err) + } + + compiler := workflow.Compiler{ + Trigger: tangled.Pipeline_TriggerMetadata{ + Kind: string(workflow.TriggerKindPush), + Push: &trigger, + Repo: triggerRepo, + }, + ChangedFiles: changedFiles, + } + + compiler.Compile(compiler.Parse(pipeline)) + + for _, e := range compiler.Diagnostics.Errors { + *clientMsgs = append(*clientMsgs, e.String()) + } + if verbose { + if compiler.Diagnostics.IsEmpty() { + *clientMsgs = append(*clientMsgs, "success: pipeline compiled with no diagnostics") + } + for _, w := range compiler.Diagnostics.Warnings { + *clientMsgs = append(*clientMsgs, w.String()) + } + } + + return nil } func (h *InternalHandle) emitPullRequestLink( diff --git a/knotserver/internal_pushoptions_test.go b/knotserver/internal_pushoptions_test.go new file mode 100644 --- /dev/null +++ b/knotserver/internal_pushoptions_test.go @@ -0,0 +1,57 @@ +package knotserver + +import ( + "testing" + + "tangled.org/core/knotserver/git" +) + +func TestHasVerboseCIPushOption(t *testing.T) { + cases := []struct { + name string + pushOptions []string + want bool + }{ + {"verbose-ci token", []string{"verbose-ci"}, true}, + {"ci-verbose token", []string{"ci-verbose"}, true}, + {"verbose token among others", []string{"foo", "ci-verbose", "bar"}, true}, + {"skip-ci is not verbose", []string{"skip-ci"}, false}, + {"ci-skip is not verbose", []string{"ci-skip"}, false}, + {"unrelated token", []string{"whatever"}, false}, + {"empty slice", []string{}, false}, + {"nil slice", nil, false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := hasVerboseCIPushOption(tc.pushOptions); got != tc.want { + t.Errorf("hasVerboseCIPushOption(%v) = %v, want %v", tc.pushOptions, got, tc.want) + } + }) + } +} + +func TestHasSkipCIPushOption(t *testing.T) { + cases := []struct { + name string + pushOptions []string + want bool + }{ + {"skip-ci token", []string{"skip-ci"}, true}, + {"ci-skip token", []string{"ci-skip"}, true}, + {"skip token among others", []string{"foo", "skip-ci", "bar"}, true}, + {"verbose-ci is not skip", []string{"verbose-ci"}, false}, + {"ci-verbose is not skip", []string{"ci-verbose"}, false}, + {"unrelated token", []string{"whatever"}, false}, + {"empty slice", []string{}, false}, + {"nil slice", nil, false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := git.HasSkipCIPushOption(tc.pushOptions); got != tc.want { + t.Errorf("hasSkipCIPushOption(%v) = %v, want %v", tc.pushOptions, got, tc.want) + } + }) + } +} diff --git a/spindle/server.go b/spindle/server.go --- a/spindle/server.go +++ b/spindle/server.go @@ -427,6 +427,11 @@ if src.Host != repo.Knot { return fmt.Errorf("repo knot does not match event source: %s != %s", src.Host, repo.Knot) } + if kgit.HasSkipCIPushOption(event.PushOptions) { + l.Info("push event requested ci skip, skipping the event") + return nil + } + // NOTE: we are blindly trusting the knot that it will return only repos it own repoCloneUri := s.newRepoCloneUrl(src.Host, repoDid) repoPath := s.newRepoPath(repoDid) diff --git a/spindle/server_test.go b/spindle/server_test.go new file mode 100644 --- /dev/null +++ b/spindle/server_test.go @@ -0,0 +1,55 @@ +package spindle + +import ( + "testing" + + kgit "tangled.org/core/knotserver/git" +) + +func TestHasSkipCIPushOption(t *testing.T) { + tests := []struct { + name string + pushOptions []string + want bool + }{ + { + name: "skip-ci requests skip", + pushOptions: []string{"skip-ci"}, + want: true, + }, + { + name: "ci-skip requests skip", + pushOptions: []string{"ci-skip"}, + want: true, + }, + { + name: "unrelated ci options do not skip", + pushOptions: []string{"verbose-ci", "ci-verbose"}, + want: false, + }, + { + name: "empty options do not skip", + pushOptions: []string{}, + want: false, + }, + { + name: "nil options do not skip", + pushOptions: nil, + want: false, + }, + { + name: "mixed options skip when any skip option appears", + pushOptions: []string{"verbose-ci", "skip-ci", "ci-verbose"}, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := kgit.HasSkipCIPushOption(tt.pushOptions) + if got != tt.want { + t.Fatalf("hasSkipCIPushOption(%v) = %v, want %v", tt.pushOptions, got, tt.want) + } + }) + } +} -- tangled.sh