ci: name what a refused submission got wrong

.github/workflows/check.yml

@@ -70,16 +70,57 @@ jobs:
             go build -tags nogui -ldflags "-X github.com/geodro/lerd/internal/version.Version=dev" -o "$RUNNER_TEMP/lerd/lerd" ./cmd/lerd
           "$RUNNER_TEMP/lerd/lerd" --version
 
-      - name: Check the changed entries
+      # Each area is a step of its own, so the step that fails names what is wrong
+      # and its log says why; the summary page carries the whole report.
+      - name: "Entry: the file, a public repository, a commit that fetches"
+        id: entry
         env:
           GITHUB_TOKEN: ${{ github.token }}
         run: |
-          changed=$(git diff --name-only --diff-filter=AM "origin/${{ github.base_ref }}...HEAD" -- 'plugins/*.yaml')
+          changed=$(git diff --name-only --diff-filter=AM "origin/${{ github.base_ref }}...HEAD" -- 'plugins/*.yaml' | tr '\n' ' ')
+          echo "changed=$changed" >> "$GITHUB_OUTPUT"
           if [ -z "$changed" ]; then
             echo "No entry added or changed." >> "$GITHUB_STEP_SUMMARY"
             exit 0
           fi
-          go run ./ci/check -lerd "$RUNNER_TEMP/lerd/lerd" -report "$GITHUB_STEP_SUMMARY" $changed
+          go build -o check-bin ./ci/check
+          ./check-bin -lerd "$RUNNER_TEMP/lerd/lerd" -only entry -report "$GITHUB_STEP_SUMMARY" $changed
+
+      - name: "Repository: a license and a README"
+        if: ${{ !cancelled() && steps.entry.outcome == 'success' && steps.entry.outputs.changed }}
+        env:
+          GITHUB_TOKEN: ${{ github.token }}
+        run: ./check-bin -lerd "$RUNNER_TEMP/lerd/lerd" -only repository -report "$GITHUB_STEP_SUMMARY" ${{ steps.entry.outputs.changed }}
+
+      - name: "Manifest: lerd bound, platforms, screenshots"
+        if: ${{ !cancelled() && steps.entry.outcome == 'success' && steps.entry.outputs.changed }}
+        env:
+          GITHUB_TOKEN: ${{ github.token }}
+        run: ./check-bin -lerd "$RUNNER_TEMP/lerd/lerd" -only manifest -report "$GITHUB_STEP_SUMMARY" ${{ steps.entry.outputs.changed }}
+
+      - name: "Services: pinned images the README names"
+        if: ${{ !cancelled() && steps.entry.outcome == 'success' && steps.entry.outputs.changed }}
+        env:
+          GITHUB_TOKEN: ${{ github.token }}
+        run: ./check-bin -lerd "$RUNNER_TEMP/lerd/lerd" -only services -report "$GITHUB_STEP_SUMMARY" ${{ steps.entry.outputs.changed }}
+
+      - name: "Images: format and size"
+        if: ${{ !cancelled() && steps.entry.outcome == 'success' && steps.entry.outputs.changed }}
+        env:
+          GITHUB_TOKEN: ${{ github.token }}
+        run: ./check-bin -lerd "$RUNNER_TEMP/lerd/lerd" -only images -report "$GITHUB_STEP_SUMMARY" ${{ steps.entry.outputs.changed }}
+
+      - name: "Code: no binaries, nothing piped to a shell"
+        if: ${{ !cancelled() && steps.entry.outcome == 'success' && steps.entry.outputs.changed }}
+        env:
+          GITHUB_TOKEN: ${{ github.token }}
+        run: ./check-bin -lerd "$RUNNER_TEMP/lerd/lerd" -only code -report "$GITHUB_STEP_SUMMARY" ${{ steps.entry.outputs.changed }}
+
+      - name: "Installs beside every listed plugin"
+        if: ${{ !cancelled() && steps.entry.outcome == 'success' && steps.entry.outputs.changed }}
+        env:
+          GITHUB_TOKEN: ${{ github.token }}
+        run: ./check-bin -lerd "$RUNNER_TEMP/lerd/lerd" -only install -report "$GITHUB_STEP_SUMMARY" ${{ steps.entry.outputs.changed }}
 
   # A submission changes its own entry and nothing else; changes to the checks
   # come from maintainers in pull requests of their own.

ci/check/check_test.go

@@ -197,3 +197,21 @@ func TestInstallableHere(t *testing.T) {
 		t.Errorf("the copy carries every file: %v", err)
 	}
 }
+
+// CI runs each area as a step of its own, so an area reports only what it
+// judges: the step that goes red then names what is wrong.
+func TestInspectArea_ReportsOnlyItsOwnFindings(t *testing.T) {
+	dir := plugin(t, map[string]string{"bin/tool": "\x7fELF\x00\x00binary"})
+	if err := os.Remove(filepath.Join(dir, "LICENSE")); err != nil {
+		t.Fatal(err)
+	}
+	var code, repo report
+	inspectArea(&code, dir, "qb", registry.Entry{}, "code")
+	inspectArea(&repo, dir, "qb", registry.Entry{}, "repository")
+	if !has(code.Fail, "bin/tool") || has(code.Fail, "LICENSE") {
+		t.Errorf("code found %v, want only the binary", code.Fail)
+	}
+	if !has(repo.Fail, "LICENSE") || has(repo.Fail, "bin/tool") {
+		t.Errorf("repository found %v, want only the license", repo.Fail)
+	}
+}

ci/check/inspect.go

@@ -24,23 +24,42 @@ type report struct {
 func (r *report) fail(format string, a ...any) { r.Fail = append(r.Fail, fmt.Sprintf(format, a...)) }
 func (r *report) flag(format string, a ...any) { r.Flag = append(r.Flag, fmt.Sprintf(format, a...)) }
 
-// inspect checks a plugin checked out at its approved commit in dir.
+// areas are the parts of a submission the check judges. CI runs each as a step
+// of its own, so the step that fails names what is wrong.
+var areas = []string{"repository", "manifest", "services", "images", "code", "install"}
+
+// inspect checks a plugin checked out at its approved commit in dir, every
+// area but installing it.
 func inspect(dir, name string, e registry.Entry) report {
 	var r report
-	if !anyFile(dir, "LICENSE", "LICENSE.md", "LICENSE.txt", "COPYING") {
-		r.fail("no LICENSE file (rule 2)")
-	}
-	readme, ok := readFirst(dir, "README.md", "README", "readme.md")
-	if !ok {
-		r.fail("no README (rule 3)")
+	for _, a := range areas {
+		inspectArea(&r, dir, name, e, a)
 	}
-	checkManifest(&r, dir, name, len(e.Images) > 0)
-	checkServices(&r, dir, readme)
-	checkImages(&r, dir, e.Images)
-	scanFiles(&r, dir)
 	return r
 }
 
+// inspectArea checks one area of the plugin; install is checkEntry's, not this.
+func inspectArea(r *report, dir, name string, e registry.Entry, area string) {
+	readme, hasReadme := readFirst(dir, "README.md", "README", "readme.md")
+	switch area {
+	case "repository":
+		if !anyFile(dir, "LICENSE", "LICENSE.md", "LICENSE.txt", "COPYING") {
+			r.fail("no LICENSE file (rule 2)")
+		}
+		if !hasReadme {
+			r.fail("no README (rule 3)")
+		}
+	case "manifest":
+		checkManifest(r, dir, name, len(e.Images) > 0)
+	case "services":
+		checkServices(r, dir, readme)
+	case "images":
+		checkImages(r, dir, e.Images)
+	case "code":
+		scanFiles(r, dir)
+	}
+}
+
 // lowerBoundRe accepts only a lower bound, so a new lerd never strands the plugin.
 var lowerBoundRe = regexp.MustCompile(`^>=\s*\d+\.\d+(\.\d+)?$`)
 

ci/check/main.go

@@ -14,16 +14,22 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"strings"
 )
 
 func main() {
 	lerd := flag.String("lerd", "", "lerd binary that validates and installs each plugin")
 	plugins := flag.String("plugins", "plugins", "folder holding every listed entry")
 	out := flag.String("report", "", "file the markdown report is appended to (stdout when empty)")
+	only := flag.String("only", "", "check one area: entry, "+strings.Join(areas, ", "))
 	flag.Parse()
 	if *lerd == "" || flag.NArg() == 0 {
-		fmt.Fprintln(os.Stderr, "usage: check -lerd <binary> plugins/<name>.yaml...")
+		fmt.Fprintln(os.Stderr, "usage: check -lerd <binary> [-only <area>] plugins/<name>.yaml...")
+		os.Exit(2)
+	}
+	if *only != "" && *only != "entry" && !slices.Contains(areas, *only) {
+		fmt.Fprintf(os.Stderr, "unknown area %q, want entry, %s\n", *only, strings.Join(areas, ", "))
 		os.Exit(2)
 	}
 
@@ -40,16 +46,20 @@ func main() {
 
 	failed := false
 	for _, path := range flag.Args() {
-		name, r := checkEntry(path, *plugins, *lerd)
-		writeReport(w, name, r)
+		name, r := checkEntry(path, *plugins, *lerd, *only)
+		writeReport(w, name, *only, r)
+		annotate(os.Stdout, name, r)
 		failed = failed || len(r.Fail) > 0
 	}
 	if failed {
 		os.Exit(1)
 	}
 }
 
-func checkEntry(path, plugins, lerd string) (string, report) {
+// checkEntry checks one entry; only names a single area, or "" for all of them.
+// The entry itself, the repository being public and the commit fetching are
+// checked whatever the area, since nothing else can be judged without them.
+func checkEntry(path, plugins, lerd, only string) (string, report) {
 	var r report
 	name, e, err := registry.ReadEntry(path)
 	if err != nil {
@@ -71,10 +81,18 @@ func checkEntry(path, plugins, lerd string) (string, report) {
 	}
 	defer os.RemoveAll(dir)
 
-	found := inspect(dir, name, e)
-	r.Fail = append(r.Fail, found.Fail...)
-	r.Flag = append(r.Flag, found.Flag...)
-	installAlongside(&r, lerd, plugins, name, dir)
+	switch only {
+	case "":
+		found := inspect(dir, name, e)
+		r.Fail = append(r.Fail, found.Fail...)
+		r.Flag = append(r.Flag, found.Flag...)
+		installAlongside(&r, lerd, plugins, name, dir)
+	case "install":
+		installAlongside(&r, lerd, plugins, name, dir)
+	case "entry":
+	default:
+		inspectArea(&r, dir, name, e, only)
+	}
 	return name, r
 }
 
@@ -180,7 +198,22 @@ func lerdInstall(lerd string, env []string, dir string) (string, error) {
 	return strings.TrimSpace(string(out)), err
 }
 
-func writeReport(w io.Writer, name string, r report) {
+// annotate writes each finding as a GitHub workflow command, so it shows in the
+// job's log and on the pull request rather than only on the summary page.
+func annotate(w io.Writer, name string, r report) {
+	esc := strings.NewReplacer("%", "%25", "\r", "%0D", "\n", "%0A")
+	for _, f := range r.Fail {
+		fmt.Fprintf(w, "::error title=%s::%s\n", name, esc.Replace(f))
+	}
+	for _, f := range r.Flag {
+		fmt.Fprintf(w, "::warning title=%s::%s\n", name, esc.Replace(f))
+	}
+}
+
+func writeReport(w io.Writer, name, only string, r report) {
+	if only != "" {
+		name += ": " + only
+	}
 	fmt.Fprintf(w, "## %s\n\n", name)
 	if len(r.Fail) > 0 {
 		fmt.Fprintln(w, "**Blocking**")
@@ -198,7 +231,7 @@ func writeReport(w io.Writer, name string, r report) {
 		}
 		fmt.Fprintln(w)
 	}
-	if len(r.Fail) == 0 {
+	if len(r.Fail) == 0 && (only == "" || only == "install") {
 		fmt.Fprintln(w, "The automated checks pass. A maintainer still reads the code for rules 9 to 15 and 19 to 23 before approving.")
 		fmt.Fprintln(w)
 	}

ci/check/main_test.go

@@ -62,13 +62,28 @@ func TestCheckEntry_InstallsAlongsideTheListedPlugins(t *testing.T) {
 	registry.RepoPattern = regexpAny
 	t.Cleanup(func() { registry.RepoPattern = origRepo })
 
-	if _, r := checkEntry(entryFor("fresh", "jobbox"), plugins, lerd); len(r.Fail) != 0 || len(r.Flag) != 0 {
+	if _, r := checkEntry(entryFor("fresh", "jobbox"), plugins, lerd, ""); len(r.Fail) != 0 || len(r.Flag) != 0 {
 		t.Errorf("a plugin with names of its own passes, got %+v", r)
 	}
-	_, r := checkEntry(entryFor("copycat", "queuebox"), plugins, lerd)
+	_, r := checkEntry(entryFor("copycat", "queuebox"), plugins, lerd, "")
 	if !has(r.Fail, "queuebox") {
 		t.Errorf("a service another listed plugin ships must block, got %+v", r)
 	}
 }
 
 var regexpAny = regexp.MustCompile(`.`)
+
+// The summary page is where the report reads best, but a failed job's log has
+// to say why too, or it ends on whatever go printed last and looks broken.
+func TestAnnotate_PutsEveryFindingInTheLog(t *testing.T) {
+	var b strings.Builder
+	annotate(&b, "site-services", report{
+		Fail: []string{"bin/x.pyc: binary file (rule 5)"},
+		Flag: []string{"bin/run:3: sudo\nnext line, 100%"},
+	})
+	want := "::error title=site-services::bin/x.pyc: binary file (rule 5)\n" +
+		"::warning title=site-services::bin/run:3: sudo%0Anext line, 100%25\n"
+	if b.String() != want {
+		t.Errorf("annotate wrote\n%q\nwant\n%q", b.String(), want)
+	}
+}