Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .github/simplycubed.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
# decides whether a change is good.
labelPrefix: sc

# The App this repository installed. Comment commands address it:
# "@simplycubed-code go" on an issue starts work. The caller workflow triggers
# on this same handle, and preflight fails if the two ever disagree.
appName: simplycubed-code

gate: make check

prDescription: rich
2 changes: 1 addition & 1 deletion .github/workflows/simplycubed-caller.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ jobs:
if: >-
github.event_name == 'issue_comment' &&
github.event.comment.user.type != 'Bot' &&
startsWith(github.event.comment.body, '/simplycubed') &&
startsWith(github.event.comment.body, '@simplycubed-code') &&
contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)
uses: simplycubed/code/.github/workflows/simplycubed.yml@main
with:
Expand Down
30 changes: 25 additions & 5 deletions cmd/simplycubed/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,18 +459,38 @@ func reply(argv []string, body string, stdout io.Writer) error {
// way — comments simply stop working, with no error anywhere — so it is checked
// on every run rather than left to be discovered.
func checkMentionAgreement(repoDir, appName string) error {
path := filepath.Join(repoDir, ".github", "workflows", "simplycubed.yml")
b, err := os.ReadFile(path)
// Find the caller by what it does, not by what it is called. init writes
// .github/workflows/simplycubed.yml, but an adopter can rename it, and in
// this repository that name belongs to the reusable workflow itself. A check
// keyed on the filename would pass on the wrong file, or fail on a valid
// install, which is worse than not checking.
dir := filepath.Join(repoDir, ".github", "workflows")
entries, err := os.ReadDir(dir)
if err != nil {
// No caller workflow is a valid local setup, not a misconfiguration.
// No workflows at all is a valid local setup, not a misconfiguration.
return nil
}
want := "'" + command.MentionFor(appName) + "'"
if strings.Contains(string(b), want) {
var callers []string
for _, e := range entries {
if e.IsDir() {
continue
}
path := filepath.Join(dir, e.Name())
b, err := os.ReadFile(path)
if err != nil || !strings.Contains(string(b), "simplycubed/code/.github/workflows/simplycubed.yml@") {
continue
}
callers = append(callers, path)
if strings.Contains(string(b), want) {
return nil
}
}
if len(callers) == 0 {
return nil
}
return fmt.Errorf("%w: .github/simplycubed.yml sets appName %q, but %s does not trigger on %s. Comment commands will never fire. Re-run \"simplycubed init --workflow\" to rewrite the trigger from the config",
ErrConfigMissing, appName, path, want)
ErrConfigMissing, appName, strings.Join(callers, ", "), want)
}

func preflightCmd(argv []string, stdout io.Writer) error {
Expand Down
103 changes: 102 additions & 1 deletion cmd/simplycubed/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1290,7 +1290,11 @@ func TestPreflightCatchesHandleDrift(t *testing.T) {
if err := os.MkdirAll(filepath.Dir(wf), 0o755); err != nil {
t.Fatal(err)
}
body := "on: [issue_comment]\njobs:\n comment:\n if: startsWith(github.event.comment.body, '" + trigger + "')\n"
// Must look like a caller: the check finds it by the reusable-workflow
// call, not by filename.
body := "on: [issue_comment]\njobs:\n comment:\n" +
" uses: simplycubed/code/.github/workflows/simplycubed.yml@v0.3.0\n" +
" if: startsWith(github.event.comment.body, '" + trigger + "')\n"
if err := os.WriteFile(wf, []byte(body), 0o644); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -1417,3 +1421,100 @@ func TestCommandCmdSurfacesConfigProblems(t *testing.T) {
t.Fatal("--body with no value must be an error")
}
}

// init writes the caller as simplycubed.yml, but an adopter can rename it, and
// in this repository that name belongs to the reusable workflow itself. The
// drift check has to find the caller by what it calls, or it passes on the
// wrong file and fails on a valid install.
func TestDriftCheckFindsTheCallerByContentNotFilename(t *testing.T) {
t.Setenv("SIMPLYCUBED_AZURE_OPENAI_ENDPOINT", "https://r.openai.azure.com")
t.Setenv("SIMPLYCUBED_AZURE_OPENAI_API_KEY", "k")

dir := repoWithConfigBody(t, "gate: make check\nappName: acme-code\n")
wfDir := filepath.Join(dir, ".github", "workflows")
if err := os.MkdirAll(wfDir, 0o755); err != nil {
t.Fatal(err)
}
// A workflow with the canonical name that is not a caller must be ignored.
if err := os.WriteFile(filepath.Join(wfDir, "simplycubed.yml"), []byte("on: workflow_call\njobs:\n run:\n runs-on: ubuntu-latest\n"), 0o644); err != nil {
t.Fatal(err)
}
// The real caller, under a different name.
caller := "on: [issue_comment]\njobs:\n comment:\n" +
" uses: simplycubed/code/.github/workflows/simplycubed.yml@v0.3.0\n" +
" if: startsWith(github.event.comment.body, '@acme-code')\n"
if err := os.WriteFile(filepath.Join(wfDir, "simplycubed-caller.yml"), []byte(caller), 0o644); err != nil {
t.Fatal(err)
}

if err := preflightCmd([]string{"--repo-dir", dir}, io.Discard); err != nil {
t.Fatalf("a renamed caller that agrees must pass: %v", err)
}
}

// The drift check walks .github/workflows/ looking for whatever calls the
// reusable workflow. Everything it can meet in there has to be handled, because
// a check that errors on an ordinary repository layout is a check people turn
// off.
func TestDriftCheckSkipsWhatIsNotACaller(t *testing.T) {
setAzure := func(t *testing.T) {
t.Helper()
t.Setenv("SIMPLYCUBED_AZURE_OPENAI_ENDPOINT", "https://r.openai.azure.com")
t.Setenv("SIMPLYCUBED_AZURE_OPENAI_API_KEY", "k")
}
repoWithWorkflows := func(t *testing.T, files map[string]string, dirs ...string) string {
t.Helper()
dir := repoWithConfigBody(t, "gate: make check\nappName: acme-code\n")
wfDir := filepath.Join(dir, ".github", "workflows")
if err := os.MkdirAll(wfDir, 0o755); err != nil {
t.Fatal(err)
}
for _, d := range dirs {
if err := os.MkdirAll(filepath.Join(wfDir, d), 0o755); err != nil {
t.Fatal(err)
}
}
for name, body := range files {
if err := os.WriteFile(filepath.Join(wfDir, name), []byte(body), 0o644); err != nil {
t.Fatal(err)
}
}
return dir
}
caller := func(trigger string) string {
return "on: [issue_comment]\njobs:\n comment:\n" +
" uses: simplycubed/code/.github/workflows/simplycubed.yml@v0.3.0\n" +
" if: startsWith(github.event.comment.body, '" + trigger + "')\n"
}
plain := "on: push\njobs:\n build:\n runs-on: ubuntu-latest\n"

t.Run("a directory in there is not a workflow", func(t *testing.T) {
setAzure(t)
dir := repoWithWorkflows(t, map[string]string{"zz-caller.yml": caller("@acme-code")}, "archive")
if err := preflightCmd([]string{"--repo-dir", dir}, io.Discard); err != nil {
t.Fatalf("a subdirectory must be skipped, not read: %v", err)
}
})

t.Run("unrelated workflows are skipped before the caller is found", func(t *testing.T) {
setAzure(t)
// "aa" sorts first, so the loop must skip it rather than stop there.
dir := repoWithWorkflows(t, map[string]string{
"aa-ci.yml": plain,
"zz-caller.yml": caller("@acme-code"),
})
if err := preflightCmd([]string{"--repo-dir", dir}, io.Discard); err != nil {
t.Fatalf("an unrelated workflow must not decide the answer: %v", err)
}
})

t.Run("no caller at all is a valid local setup", func(t *testing.T) {
setAzure(t)
// Running the CLI yourself needs no caller, so workflows that call
// nothing must not be reported as drift.
dir := repoWithWorkflows(t, map[string]string{"ci.yml": plain})
if err := preflightCmd([]string{"--repo-dir", dir}, io.Discard); err != nil {
t.Fatalf("no caller is not a misconfiguration: %v", err)
}
})
}