fix(config): stop parsing the syncerd binary as configuration, and remove the dead workflow (#50)

* fix(config): stop parsing the syncerd binary as configuration

viper matches an extensionless file when a config type is set, and the
extensionless file sitting in a working directory is very often the syncerd
binary: make build writes ./syncerd, so running ./syncerd sync beside it
made the tool read its own binary as YAML.

The error that produced, "yaml: control characters are not allowed", names
neither the file nor the cause. This repository's own scheduled workflow
has failed that way on every run since at least June.

Discovery now resolves the file itself and accepts only syncerd.yaml or
syncerd.yml, in the working directory or ./config. When there is none it
says so and lists everywhere it looked, which is the answer the operator
actually needs.

* chore(ci): remove the scheduled SyncerD workflow

It ran syncerd sync --once against a syncerd.yaml this repository has never
contained, so it failed on every run since at least June, and its push
trigger filtered on that same non existent path. The config discovery fix
alongside this turns its failure from a baffling parse error into a clear
one, which is an improvement to the message and not to the workflow.

Removed rather than repaired. Making it work would mean deciding what this
organisation wants mirrored and committing that as configuration, which is
a product decision rather than a repair, and a workflow that has only ever
failed teaches everyone to ignore a red mark on master.

---------

Co-authored-by: Anmol Nagpal <ianmolnagpal@gmail.com>

Author: clouddrove-ci

.github/workflows/syncerd.yml

@@ -21,7 +21,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - A failed state save on the fail-fast path was silent, so everything copied before the abort was copied again on the next run with no explanation
 - `git-sync` could not authenticate to Azure DevOps over git in `pat` mode, fixed in v0.2.1 and described there
 
+### Removed
+- The `SyncerD` scheduled workflow, which ran `syncerd sync --once` against a `syncerd.yaml` this repository has never contained. It had failed on every run since at least June, and its push trigger filtered on a path that does not exist. Deleted rather than repaired: the dogfooding it was meant to do is not wanted, and a workflow that has only ever failed is worse than none
+
 ### Fixed
+- SyncerD parsed its own binary as its configuration. Config discovery accepted an extensionless file named `syncerd`, which is exactly what `make build` writes into the working directory, so running `./syncerd sync` beside the binary failed with `yaml: control characters are not allowed`, an error that points at nothing. Discovery now accepts only a real `syncerd.yaml` or `syncerd.yml`, and says where it looked when it finds neither
 - A mirrored pull request with no labels failed to update, taking the whole pull request with it. Once the mirror owned labels, an empty set was sent as null rather than as an empty array, and GitHub rejects that with 422. Most pull requests carry no labels, so this affected most of them. Found by the live suite on its first real run
 
 ### Added

CHANGELOG.md

@@ -2,6 +2,8 @@ package config
 
 import (
 	"fmt"
+	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/spf13/viper"
@@ -54,16 +56,56 @@ type SlackConfig struct {
 	MessageFormat string `mapstructure:"message_format"`
 }
 
+// configSearchPaths are the directories searched for a config file, and
+// configSearchNames the file names accepted in each, in order.
+var (
+	configSearchPaths = []string{".", "./config"}
+	configSearchNames = []string{"syncerd.yaml", "syncerd.yml"}
+)
+
+// findConfigFile returns the first config file present, or an error naming
+// everywhere it looked.
+//
+// Only a real extension counts. An extensionless "syncerd" is the compiled
+// binary, not configuration.
+func findConfigFile() (string, error) {
+	var tried []string
+	for _, dir := range configSearchPaths {
+		for _, name := range configSearchNames {
+			candidate := filepath.Join(dir, name)
+			tried = append(tried, candidate)
+			info, err := os.Stat(candidate)
+			if err != nil || info.IsDir() {
+				continue
+			}
+			return candidate, nil
+		}
+	}
+	return "", fmt.Errorf("no config file found; looked for %s. Pass --config to name one explicitly",
+		strings.Join(tried, ", "))
+}
+
 func Load(configPath string) (*Config, error) {
 	viper.SetConfigType("yaml")
 	viper.SetConfigName("syncerd")
 
 	if configPath != "" {
 		viper.SetConfigFile(configPath)
 	} else {
-		// Look for config in current directory
-		viper.AddConfigPath(".")
-		viper.AddConfigPath("./config")
+		// Resolve the file here rather than letting viper search.
+		//
+		// viper matches an extensionless file when a config type is set,
+		// and the file it finds in a working directory is very often the
+		// syncerd binary itself: `make build` writes ./syncerd, and running
+		// ./syncerd sync in that directory made the tool parse its own
+		// binary as YAML. The error that produced, "control characters are
+		// not allowed", says nothing about what actually happened, and the
+		// project's own scheduled workflow failed that way for months.
+		found, err := findConfigFile()
+		if err != nil {
+			return nil, err
+		}
+		viper.SetConfigFile(found)
 	}
 
 	// Environment variables: SYNCERD_ prefix, with nested keys mapped via

internal/config/config.go

@@ -78,3 +78,56 @@ func TestValidateImageSyncAcceptsValidConfig(t *testing.T) {
 		t.Fatalf("expected valid config, got %v", err)
 	}
 }
+
+func TestABinaryNamedSyncerdIsNotTreatedAsConfig(t *testing.T) {
+	// `make build` writes ./syncerd, and viper matches an extensionless
+	// file when a config type is set, so running ./syncerd sync in that
+	// directory made the tool parse its own binary as YAML. The error was
+	// "control characters are not allowed", which points at nothing, and
+	// the project's own scheduled workflow failed that way for months.
+	dir := t.TempDir()
+	if err := os.WriteFile(filepath.Join(dir, "syncerd"), []byte("\x7fELF\x02\x01\x01\x00binary"), 0o755); err != nil {
+		t.Fatalf("write: %v", err)
+	}
+
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("getwd: %v", err)
+	}
+	if err := os.Chdir(dir); err != nil {
+		t.Fatalf("chdir: %v", err)
+	}
+	t.Cleanup(func() { _ = os.Chdir(cwd) })
+
+	_, err = findConfigFile()
+	if err == nil {
+		t.Fatal("an extensionless binary must not be accepted as a config file")
+	}
+	if !strings.Contains(err.Error(), "no config file found") {
+		t.Errorf("the error should say what is missing, got %v", err)
+	}
+}
+
+func TestFindConfigFilePrefersAnExplicitYAML(t *testing.T) {
+	dir := t.TempDir()
+	if err := os.WriteFile(filepath.Join(dir, "syncerd"), []byte("binary"), 0o755); err != nil {
+		t.Fatalf("write: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "syncerd.yaml"), []byte("source:\n  type: dockerhub\n"), 0o644); err != nil {
+		t.Fatalf("write: %v", err)
+	}
+
+	cwd, _ := os.Getwd()
+	if err := os.Chdir(dir); err != nil {
+		t.Fatalf("chdir: %v", err)
+	}
+	t.Cleanup(func() { _ = os.Chdir(cwd) })
+
+	got, err := findConfigFile()
+	if err != nil {
+		t.Fatalf("find: %v", err)
+	}
+	if filepath.Base(got) != "syncerd.yaml" {
+		t.Errorf("found %q, want syncerd.yaml", got)
+	}
+}