From 4b7b51cf9f8d1fc4a2e7c1c1a973f0647ed6f647 Mon Sep 17 00:00:00 2001 From: Matt Sinagra Date: Mon, 13 Jul 2026 15:34:42 +1000 Subject: [PATCH 1/2] Feature - Added route:loops for detecting loops within path specified routes.yaml files, repo routes.yaml files or config.yaml route sections to check for routing loops --- commands/list.go | 6 + commands/root.go | 1 + commands/route_loops.go | 287 ++++++++++++++++++++++++ internal/routeloops/parse.go | 202 +++++++++++++++++ internal/routeloops/routeloops.go | 181 +++++++++++++++ internal/routeloops/routeloops_test.go | 298 +++++++++++++++++++++++++ 6 files changed, 975 insertions(+) create mode 100644 commands/route_loops.go create mode 100644 internal/routeloops/parse.go create mode 100644 internal/routeloops/routeloops.go create mode 100644 internal/routeloops/routeloops_test.go diff --git a/commands/list.go b/commands/list.go index 755db330b..5475d31be 100644 --- a/commands/list.go +++ b/commands/list.go @@ -53,6 +53,12 @@ func newListCommand(cnf *config.Config) *cobra.Command { list.AddCommand(&appConfigValidateCommand) } + routeLoopsCommand := innerRouteLoopsCommand(cnf) + + if !list.DescribesNamespace() || list.Namespace == routeLoopsCommand.Name.Namespace { + list.AddCommand(&routeLoopsCommand) + } + appProjectConvertCommand := innerProjectConvertCommand(cnf) if cnf.Service.ProjectConfigFlavor == "upsun" && diff --git a/commands/root.go b/commands/root.go index 491788296..b88286536 100644 --- a/commands/root.go +++ b/commands/root.go @@ -148,6 +148,7 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob newHelpCommand(cnf), newInitCommand(cnf, assets), newListCommand(cnf), + newRouteLoopsCommand(cnf), validateCmd, versionCommand, ) diff --git a/commands/route_loops.go b/commands/route_loops.go new file mode 100644 index 000000000..ddfb5ac8f --- /dev/null +++ b/commands/route_loops.go @@ -0,0 +1,287 @@ +package commands + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + orderedmap "github.com/wk8/go-ordered-map/v2" + + "github.com/upsun/cli/internal/config" + "github.com/upsun/cli/internal/routeloops" +) + +const routeLoopsHelp = "Detects redirect loops in a project's routes. By default this reads " + + ".platform/routes.yaml or .upsun/*.yaml from the current directory. Pass --live " + + "to fetch the deployed routes via `route:list` instead (requires a deployed environment)." + +func innerRouteLoopsCommand(cnf *config.Config) Command { + noInteractionOption := NoInteractionOption(cnf) + + options := orderedmap.New[string, Option](orderedmap.WithInitialData[string, Option]( + orderedmap.Pair[string, Option]{ + Key: "file", + Value: Option{ + Name: "--file", + Shortcut: "-f", + AcceptValue: true, + IsValueRequired: true, + Description: "Path to a routes.yaml or Upsun config file (with a top-level `routes:` key).", + Default: Any{any: ""}, + }, + }, + orderedmap.Pair[string, Option]{ + Key: "live", + Value: Option{ + Name: "--live", + AcceptValue: false, + Description: "Fetch routes via `route:list` instead of reading the local repo (requires auth + a deployed environment).", + Default: Any{any: false}, + }, + }, + orderedmap.Pair[string, Option]{ + Key: "format", + Value: Option{ + Name: "--format", + AcceptValue: true, + IsValueRequired: true, + Description: "Output format: text or json.", + Default: Any{any: "text"}, + }, + }, + orderedmap.Pair[string, Option]{ + Key: "no-fail", + Value: Option{ + Name: "--no-fail", + AcceptValue: false, + Description: "Exit 0 even when redirect loops are found.", + Default: Any{any: false}, + }, + }, + orderedmap.Pair[string, Option]{ + Key: "route-project", + Value: Option{ + Name: "--project", + Shortcut: "-p", + AcceptValue: true, + IsValueRequired: true, + Description: "The project ID (only used with --live).", + Default: Any{any: ""}, + }, + }, + orderedmap.Pair[string, Option]{ + Key: "route-environment", + Value: Option{ + Name: "--environment", + Shortcut: "-e", + AcceptValue: true, + IsValueRequired: true, + Description: "The environment ID (only used with --live).", + Default: Any{any: ""}, + }, + }, + orderedmap.Pair[string, Option]{ + Key: HelpOption.GetName(), + Value: HelpOption, + }, + orderedmap.Pair[string, Option]{ + Key: VerboseOption.GetName(), + Value: VerboseOption, + }, + orderedmap.Pair[string, Option]{ + Key: VersionOption.GetName(), + Value: VersionOption, + }, + orderedmap.Pair[string, Option]{ + Key: YesOption.GetName(), + Value: YesOption, + }, + orderedmap.Pair[string, Option]{ + Key: noInteractionOption.GetName(), + Value: noInteractionOption, + }, + )) + + return Command{ + Name: CommandName{ + Namespace: "route", + Command: "loops", + }, + Usage: []string{ + cnf.Application.Executable + " route:loops", + }, + Description: "Detect redirect loops in a project's routes", + Help: CleanString(routeLoopsHelp), + Examples: []Example{ + { + Commandline: "", + Description: "Check routes.yaml in the current project for redirect loops", + }, + { + Commandline: "--live", + Description: "Check the deployed routes for the current environment", + }, + { + Commandline: "--file /path/to/routes.yaml --format=json", + Description: "Check a specific file, output JSON", + }, + }, + Definition: Definition{ + Arguments: &orderedmap.OrderedMap[string, Argument]{}, + Options: options, + }, + Hidden: false, + } +} + +func newRouteLoopsCommand(cnf *config.Config) *cobra.Command { + cmd := &cobra.Command{ + Use: "route:loops", + Short: "Detect redirect loops in a project's routes", + RunE: func(cmd *cobra.Command, _ []string) error { + return runRouteLoops(cnf, cmd) + }, + } + + cmd.Flags().StringP("file", "f", "", "Path to a routes.yaml or Upsun config file") + cmd.Flags().Bool("live", false, "Fetch routes via `route:list` instead of reading the local repo") + cmd.Flags().String("format", "text", "Output format: text or json") + cmd.Flags().Bool("no-fail", false, "Exit 0 even when loops are found") + cmd.Flags().StringP("route-project", "p", "", "The project ID (only used with --live)") + cmd.Flags().StringP("route-environment", "e", "", "The environment ID (only used with --live)") + + // Bind each flag individually so we can use distinct viper keys and avoid + // clashing with any global --project/--environment on other commands. + for _, name := range []string{"file", "live", "format", "no-fail", "route-project", "route-environment"} { + _ = viper.BindPFlag("route-loops."+name, cmd.Flags().Lookup(name)) + } + + cmd.SetHelpFunc(func(_ *cobra.Command, _ []string) { + internalCmd := innerRouteLoopsCommand(cnf) + fmt.Println(internalCmd.HelpPage(cnf)) + }) + return cmd +} + +type routeLoopsJSON struct { + Cycles [][]string `json:"cycles"` + Warnings []routeLoopsWarningOut `json:"warnings"` + Source string `json:"source"` +} + +type routeLoopsWarningOut struct { + URL string `json:"url"` + Reason string `json:"reason"` +} + +func runRouteLoops(cnf *config.Config, cmd *cobra.Command) error { + format := strings.ToLower(viper.GetString("route-loops.format")) + if format != "text" && format != "json" { + return fmt.Errorf("invalid --format %q: expected text or json", format) + } + + routes, source, err := loadRoutes(cnf, cmd) + if err != nil { + return err + } + + result := routeloops.Detect(routes) + + if format == "json" { + out := routeLoopsJSON{ + Cycles: make([][]string, 0, len(result.Cycles)), + Warnings: make([]routeLoopsWarningOut, 0, len(result.Warnings)), + Source: source, + } + for _, c := range result.Cycles { + out.Cycles = append(out.Cycles, c.URLs) + } + for _, w := range result.Warnings { + out.Warnings = append(out.Warnings, routeLoopsWarningOut{URL: w.URL, Reason: w.Reason}) + } + b, err := json.MarshalIndent(out, "", " ") + if err != nil { + return err + } + fmt.Fprintln(cmd.OutOrStdout(), string(b)) + } else { + printRouteLoopsText(cmd, result, source) + } + + if len(result.Cycles) > 0 && !viper.GetBool("route-loops.no-fail") { + os.Exit(1) + } + return nil +} + +func loadRoutes(cnf *config.Config, cmd *cobra.Command) ([]routeloops.Route, string, error) { + if viper.GetBool("route-loops.live") { + return loadLiveRoutes(cnf, cmd) + } + if path := viper.GetString("route-loops.file"); path != "" { + routes, err := routeloops.ParseFile(path) + if err != nil { + return nil, "", err + } + return routes, path, nil + } + cwd, err := os.Getwd() + if err != nil { + return nil, "", fmt.Errorf("could not get current working directory: %w", err) + } + return routeloops.DiscoverProjectRoutes(cwd) +} + +func loadLiveRoutes(cnf *config.Config, cmd *cobra.Command) ([]routeloops.Route, string, error) { + args := []string{"route:list", "--format=csv", "--columns=route,type,to", "--no-header"} + if p := viper.GetString("route-loops.route-project"); p != "" { + args = append(args, "--project="+p) + } + if e := viper.GetString("route-loops.route-environment"); e != "" { + args = append(args, "--environment="+e) + } + + var buf bytes.Buffer + wrapper := makeLegacyCLIWrapper(cnf, &buf, cmd.ErrOrStderr(), cmd.InOrStdin()) + if err := wrapper.Exec(cmd.Context(), args...); err != nil { + return nil, "", fmt.Errorf("fetch routes via legacy CLI: %w", err) + } + routes, err := routeloops.ParseLiveCSV(buf.Bytes()) + if err != nil { + return nil, "", err + } + return routes, "live (route:list)", nil +} + +func printRouteLoopsText(cmd *cobra.Command, r routeloops.Result, source string) { + arrow := " -> " + if useUnicodeArrow() { + arrow = " → " + } + out := cmd.OutOrStdout() + if source != "" { + fmt.Fprintf(out, "Source: %s\n", source) + } + if len(r.Cycles) == 0 { + fmt.Fprintln(out, "No redirect loops detected.") + } else { + fmt.Fprintf(out, "Detected %d redirect loop(s):\n", len(r.Cycles)) + for _, c := range r.Cycles { + fmt.Fprintln(out, " Loop: "+strings.Join(c.URLs, arrow)) + } + } + for _, w := range r.Warnings { + fmt.Fprintf(out, "Warning: %s: %s\n", w.URL, w.Reason) + } +} + +func useUnicodeArrow() bool { + if os.Getenv("NO_COLOR") != "" { + return false + } + return true +} diff --git a/internal/routeloops/parse.go b/internal/routeloops/parse.go new file mode 100644 index 000000000..8e9679a3a --- /dev/null +++ b/internal/routeloops/parse.go @@ -0,0 +1,202 @@ +package routeloops + +import ( + "encoding/csv" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + + "gopkg.in/yaml.v3" +) + +// routeSpec captures the fields we care about from a routes.yaml entry. +type routeSpec struct { + Type string `yaml:"type"` + To string `yaml:"to"` + Upstream string `yaml:"upstream"` +} + +// ParseRoutesYAML decodes a top-level `URL: {type, to, upstream}` map, matching +// the shape of .platform/routes.yaml. +func ParseRoutesYAML(data []byte) ([]Route, error) { + var m map[string]routeSpec + if err := yaml.Unmarshal(data, &m); err != nil { + return nil, fmt.Errorf("parse routes yaml: %w", err) + } + return mapToRoutes(m), nil +} + +// ParseUpsunConfig decodes a config file that has a top-level `routes:` map. +// Returns an empty slice (nil error) if the file has no routes section. +func ParseUpsunConfig(data []byte) ([]Route, error) { + var doc struct { + Routes map[string]routeSpec `yaml:"routes"` + } + if err := yaml.Unmarshal(data, &doc); err != nil { + return nil, fmt.Errorf("parse upsun config: %w", err) + } + return mapToRoutes(doc.Routes), nil +} + +// ParseFile reads a YAML file and picks the right shape: if it has a top-level +// `routes:` key it's treated as an Upsun-style config, otherwise as a +// routes.yaml. +func ParseFile(path string) ([]Route, error) { + data, err := os.ReadFile(path) //nolint:gosec // path comes from the CLI user + if err != nil { + return nil, err + } + return parseAuto(data) +} + +func parseAuto(data []byte) ([]Route, error) { + var probe map[string]yaml.Node + if err := yaml.Unmarshal(data, &probe); err != nil { + return nil, fmt.Errorf("parse yaml: %w", err) + } + if _, hasRoutes := probe["routes"]; hasRoutes { + return ParseUpsunConfig(data) + } + return ParseRoutesYAML(data) +} + +// ParseLiveCSV decodes CSV rows from `route:list --format=csv --columns=route,type,to --no-header`. +// The `to` column is already flattened server-side: for upstream rows it holds +// the upstream name, for redirect rows it holds the redirect target. We only +// populate Route.To when Type == "redirect" to keep semantics consistent with +// the YAML parsers. +func ParseLiveCSV(data []byte) ([]Route, error) { + r := csv.NewReader(strings.NewReader(string(data))) + r.FieldsPerRecord = -1 + r.TrimLeadingSpace = true + + var out []Route + for { + rec, err := r.Read() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return nil, fmt.Errorf("parse route:list csv: %w", err) + } + if len(rec) < 2 { + continue + } + route := Route{URL: rec[0], Type: rec[1]} + if len(rec) >= 3 && route.Type == TypeRedirect { + route.To = rec[2] + } + if route.URL == "" || route.Type == "" { + continue + } + out = append(out, route) + } + return out, nil +} + +// DiscoverProjectRoutes looks for a routes configuration under dir. It prefers +// .platform/routes.yaml; failing that it walks .upsun/**/*.{yaml,yml} and +// unions any file whose top-level has a `routes:` key. Returns the routes and +// a short human-readable description of the source path(s). +func DiscoverProjectRoutes(dir string) ([]Route, string, error) { + platformPath := filepath.Join(dir, ".platform", "routes.yaml") + if info, err := os.Stat(platformPath); err == nil && !info.IsDir() { + routes, err := ParseFile(platformPath) + if err != nil { + return nil, "", err + } + return routes, platformPath, nil + } + + upsunDir := filepath.Join(dir, ".upsun") + if info, err := os.Stat(upsunDir); err == nil && info.IsDir() { + routes, sources, err := readUpsunRoutes(upsunDir) + if err != nil { + return nil, "", err + } + if len(sources) > 0 { + return routes, strings.Join(sources, ", "), nil + } + } + + return nil, "", fmt.Errorf( + "no routes configuration found under %s (looked for .platform/routes.yaml and .upsun/*.yaml with a `routes:` key)", + dir, + ) +} + +func readUpsunRoutes(upsunDir string) ([]Route, []string, error) { + var files []string + err := filepath.WalkDir(upsunDir, func(path string, d fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if d.IsDir() { + return nil + } + ext := strings.ToLower(filepath.Ext(path)) + if ext == ".yaml" || ext == ".yml" { + files = append(files, path) + } + return nil + }) + if err != nil { + return nil, nil, err + } + sort.Strings(files) + + seen := make(map[string]struct{}) + var all []Route + var sources []string + for _, path := range files { + data, err := os.ReadFile(path) //nolint:gosec // path comes from a project dir walk + if err != nil { + return nil, nil, err + } + var probe map[string]yaml.Node + if err := yaml.Unmarshal(data, &probe); err != nil { + return nil, nil, fmt.Errorf("parse %s: %w", path, err) + } + if _, hasRoutes := probe["routes"]; !hasRoutes { + continue + } + routes, err := ParseUpsunConfig(data) + if err != nil { + return nil, nil, fmt.Errorf("parse %s: %w", path, err) + } + for _, r := range routes { + if _, dup := seen[r.URL]; dup { + continue + } + seen[r.URL] = struct{}{} + all = append(all, r) + } + sources = append(sources, path) + } + return all, sources, nil +} + +func mapToRoutes(m map[string]routeSpec) []Route { + if len(m) == 0 { + return nil + } + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + out := make([]Route, 0, len(m)) + for _, k := range keys { + spec := m[k] + if spec.Type != TypeUpstream && spec.Type != TypeRedirect { + continue + } + out = append(out, Route{URL: k, Type: spec.Type, To: spec.To}) + } + return out +} diff --git a/internal/routeloops/routeloops.go b/internal/routeloops/routeloops.go new file mode 100644 index 000000000..eb3950e0d --- /dev/null +++ b/internal/routeloops/routeloops.go @@ -0,0 +1,181 @@ +// Package routeloops detects redirect loops in a project's routes. +// +// A cycle is any chain of `type: redirect` routes whose `to:` values close a +// loop, including self-loops. Path-based `redirects.paths:` blocks are out of +// scope for this package. +package routeloops + +import ( + "sort" + "strings" + "unicode" +) + +const ( + TypeUpstream = "upstream" + TypeRedirect = "redirect" +) + +// Route is a single entry from a project's routes configuration. +type Route struct { + URL string + Type string + To string +} + +// Cycle is a sequence of route URLs that redirect back to the first entry. +// URLs[0] and URLs[len-1] are the same value. +type Cycle struct { + URLs []string +} + +// Warning flags a redirect route that could not be analyzed. +type Warning struct { + URL string + Reason string +} + +// Result is the outcome of Detect. +type Result struct { + Cycles []Cycle + Warnings []Warning +} + +// Detect finds redirect cycles among the given routes. +func Detect(routes []Route) Result { + edges := make(map[string]string, len(routes)) + known := make(map[string]struct{}, len(routes)) + var warnings []Warning + + for _, r := range routes { + url := normalize(r.URL) + if url == "" { + continue + } + known[url] = struct{}{} + if r.Type != TypeRedirect { + continue + } + to := normalize(r.To) + if to == "" { + warnings = append(warnings, Warning{ + URL: r.URL, + Reason: "redirect route has no `to:` (uses `redirects.paths` only, or is malformed)", + }) + continue + } + edges[url] = to + } + + for from, to := range edges { + if _, ok := known[to]; !ok { + warnings = append(warnings, Warning{ + URL: from, + Reason: "redirect target is not a known route: " + to, + }) + } + } + + visited := make(map[string]struct{}, len(edges)) + seenCycles := make(map[string]struct{}) + var cycles []Cycle + + for start := range edges { + if _, ok := visited[start]; ok { + continue + } + walk := []string{start} + walkIndex := map[string]int{start: 0} + current := start + for { + next, ok := edges[current] + if !ok { + break + } + if idx, seen := walkIndex[next]; seen { + cycleNodes := append([]string(nil), walk[idx:]...) + cycle := canonicalizeCycle(cycleNodes) + key := strings.Join(cycle, "\x00") + if _, dup := seenCycles[key]; !dup { + seenCycles[key] = struct{}{} + closed := append(cycle, cycle[0]) + cycles = append(cycles, Cycle{URLs: closed}) + } + break + } + walkIndex[next] = len(walk) + walk = append(walk, next) + current = next + } + for _, node := range walk { + visited[node] = struct{}{} + } + } + + sort.Slice(cycles, func(i, j int) bool { + return cycles[i].URLs[0] < cycles[j].URLs[0] + }) + sort.Slice(warnings, func(i, j int) bool { + if warnings[i].URL != warnings[j].URL { + return warnings[i].URL < warnings[j].URL + } + return warnings[i].Reason < warnings[j].Reason + }) + + return Result{Cycles: cycles, Warnings: warnings} +} + +// canonicalizeCycle rotates the cycle so it starts at its lexicographically +// smallest URL. The returned slice does NOT repeat the first element at the end. +func canonicalizeCycle(nodes []string) []string { + if len(nodes) <= 1 { + return nodes + } + minIdx := 0 + for i := 1; i < len(nodes); i++ { + if nodes[i] < nodes[minIdx] { + minIdx = i + } + } + rotated := make([]string, 0, len(nodes)) + rotated = append(rotated, nodes[minIdx:]...) + rotated = append(rotated, nodes[:minIdx]...) + return rotated +} + +// normalize prepares a URL string for edge lookup. It strips whitespace and +// smart quotes, lowercases the scheme+host, and collapses a single trailing +// slash on the host portion. Placeholders like {default} are preserved as-is. +func normalize(s string) string { + s = strings.TrimFunc(s, func(r rune) bool { + if unicode.IsSpace(r) { + return true + } + switch r { + case '“', '”', '‘', '’', '"', '\'': + return true + } + return false + }) + if s == "" { + return s + } + schemeEnd := strings.Index(s, "://") + if schemeEnd < 0 { + return strings.TrimRight(s, "/") + } + scheme := strings.ToLower(s[:schemeEnd]) + rest := s[schemeEnd+3:] + pathStart := strings.IndexAny(rest, "/?#") + if pathStart < 0 { + return scheme + "://" + strings.ToLower(rest) + } + host := strings.ToLower(rest[:pathStart]) + path := rest[pathStart:] + if path == "/" { + path = "" + } else { + path = strings.TrimRight(path, "/") + } + return scheme + "://" + host + path +} diff --git a/internal/routeloops/routeloops_test.go b/internal/routeloops/routeloops_test.go new file mode 100644 index 000000000..f76742392 --- /dev/null +++ b/internal/routeloops/routeloops_test.go @@ -0,0 +1,298 @@ +package routeloops + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDetect(t *testing.T) { + cases := []struct { + name string + routes []Route + wantCycles []Cycle + wantWarnings []Warning + }{ + { + name: "empty", + routes: nil, + }, + { + name: "single upstream, no cycles", + routes: []Route{ + {URL: "https://{default}", Type: TypeUpstream}, + }, + }, + { + name: "self-loop", + routes: []Route{ + {URL: "https://{default}", Type: TypeRedirect, To: "https://{default}"}, + }, + wantCycles: []Cycle{ + {URLs: []string{"https://{default}", "https://{default}"}}, + }, + }, + { + name: "two-node cycle", + routes: []Route{ + {URL: "https://a.example.com", Type: TypeRedirect, To: "https://b.example.com"}, + {URL: "https://b.example.com", Type: TypeRedirect, To: "https://a.example.com"}, + }, + wantCycles: []Cycle{ + {URLs: []string{"https://a.example.com", "https://b.example.com", "https://a.example.com"}}, + }, + }, + { + name: "three-node cycle deduped across starts", + routes: []Route{ + {URL: "https://a", Type: TypeRedirect, To: "https://b"}, + {URL: "https://b", Type: TypeRedirect, To: "https://c"}, + {URL: "https://c", Type: TypeRedirect, To: "https://a"}, + }, + wantCycles: []Cycle{ + {URLs: []string{"https://a", "https://b", "https://c", "https://a"}}, + }, + }, + { + name: "chain terminating in upstream: no cycle", + routes: []Route{ + {URL: "http://{default}", Type: TypeRedirect, To: "https://{default}"}, + {URL: "https://{default}", Type: TypeUpstream}, + }, + }, + { + name: "dangling redirect: no cycle, warning", + routes: []Route{ + {URL: "https://a", Type: TypeRedirect, To: "https://nowhere"}, + }, + wantWarnings: []Warning{ + {URL: "https://a", Reason: "redirect target is not a known route: https://nowhere"}, + }, + }, + { + name: "redirect with no `to`: warning, not cycle", + routes: []Route{ + {URL: "https://a", Type: TypeRedirect, To: ""}, + }, + wantWarnings: []Warning{ + {URL: "https://a", Reason: "redirect route has no `to:` (uses `redirects.paths` only, or is malformed)"}, + }, + }, + { + name: "cycle with lead-in tail: only cycle reported, tail dropped", + routes: []Route{ + {URL: "https://lead", Type: TypeRedirect, To: "https://a"}, + {URL: "https://a", Type: TypeRedirect, To: "https://b"}, + {URL: "https://b", Type: TypeRedirect, To: "https://a"}, + }, + wantCycles: []Cycle{ + {URLs: []string{"https://a", "https://b", "https://a"}}, + }, + }, + { + name: "trailing-slash and whitespace variants collapse to same cycle", + routes: []Route{ + {URL: "https://a/", Type: TypeRedirect, To: " https://b "}, + {URL: "https://b", Type: TypeRedirect, To: "https://a"}, + }, + wantCycles: []Cycle{ + {URLs: []string{"https://a", "https://b", "https://a"}}, + }, + }, + { + name: "smart-quoted values still detected", + routes: []Route{ + {URL: "“https://{default}”", Type: TypeRedirect, To: "“http://{default}”"}, + {URL: "http://{default}", Type: TypeRedirect, To: "https://{default}"}, + }, + wantCycles: []Cycle{ + {URLs: []string{"http://{default}", "https://{default}", "http://{default}"}}, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := Detect(tc.routes) + assert.Equal(t, tc.wantCycles, got.Cycles) + assert.Equal(t, tc.wantWarnings, got.Warnings) + }) + } +} + +func TestNormalize(t *testing.T) { + cases := []struct { + in, want string + }{ + {"", ""}, + {" https://example.com ", "https://example.com"}, + {"“https://{default}/”", "https://{default}"}, + {"HTTPS://Example.COM/Path", "https://example.com/Path"}, + {"https://example.com/", "https://example.com"}, + {"https://example.com/path/", "https://example.com/path"}, + {"https://{default}", "https://{default}"}, + } + for _, tc := range cases { + t.Run(tc.in, func(t *testing.T) { + assert.Equal(t, tc.want, normalize(tc.in)) + }) + } +} + +func TestParseRoutesYAML(t *testing.T) { + data := []byte(` +"https://{default}/": + type: upstream + upstream: app:http +"http://{default}": + type: redirect + to: "https://{default}/" +"https://ignored": + type: something-else +`) + routes, err := ParseRoutesYAML(data) + require.NoError(t, err) + require.Len(t, routes, 2) + + byURL := map[string]Route{} + for _, r := range routes { + byURL[r.URL] = r + } + assert.Equal(t, TypeUpstream, byURL["https://{default}/"].Type) + assert.Equal(t, TypeRedirect, byURL["http://{default}"].Type) + assert.Equal(t, "https://{default}/", byURL["http://{default}"].To) +} + +func TestParseUpsunConfig(t *testing.T) { + data := []byte(` +applications: + app: + type: php +routes: + "https://{default}/": + type: upstream + upstream: app:http + "http://{default}": + type: redirect + to: "https://{default}/" +`) + routes, err := ParseUpsunConfig(data) + require.NoError(t, err) + assert.Len(t, routes, 2) +} + +func TestParseUpsunConfig_NoRoutesSection(t *testing.T) { + data := []byte(`applications: + app: + type: php +`) + routes, err := ParseUpsunConfig(data) + require.NoError(t, err) + assert.Empty(t, routes) +} + +func TestParseFile_AutoDetectsShape(t *testing.T) { + dir := t.TempDir() + + platformFile := filepath.Join(dir, "routes.yaml") + require.NoError(t, os.WriteFile(platformFile, []byte(` +"https://{default}/": + type: upstream + upstream: app:http +`), 0o644)) + + upsunFile := filepath.Join(dir, "config.yaml") + require.NoError(t, os.WriteFile(upsunFile, []byte(` +routes: + "https://{default}/": + type: upstream + upstream: app:http +`), 0o644)) + + p, err := ParseFile(platformFile) + require.NoError(t, err) + require.Len(t, p, 1) + assert.Equal(t, "https://{default}/", p[0].URL) + + u, err := ParseFile(upsunFile) + require.NoError(t, err) + require.Len(t, u, 1) + assert.Equal(t, "https://{default}/", u[0].URL) +} + +func TestParseLiveCSV(t *testing.T) { + data := []byte(`"https://{default}/",upstream,app:http +"http://{default}",redirect,"https://{default}/" +`) + routes, err := ParseLiveCSV(data) + require.NoError(t, err) + require.Len(t, routes, 2) + + assert.Equal(t, "https://{default}/", routes[0].URL) + assert.Equal(t, TypeUpstream, routes[0].Type) + assert.Equal(t, "", routes[0].To, "upstream rows should not populate To") + + assert.Equal(t, "http://{default}", routes[1].URL) + assert.Equal(t, TypeRedirect, routes[1].Type) + assert.Equal(t, "https://{default}/", routes[1].To) +} + +func TestDiscoverProjectRoutes_PlatformFirst(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".platform"), 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, ".platform", "routes.yaml"), + []byte(`"https://{default}/": {type: upstream, upstream: "app:http"}`), + 0o644, + )) + + routes, src, err := DiscoverProjectRoutes(dir) + require.NoError(t, err) + assert.Len(t, routes, 1) + assert.Contains(t, src, ".platform/routes.yaml") +} + +func TestDiscoverProjectRoutes_UpsunMultiFile(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".upsun", "sub"), 0o755)) + + require.NoError(t, os.WriteFile( + filepath.Join(dir, ".upsun", "config.yaml"), + []byte(` +applications: + app: + type: php +routes: + "https://a": {type: redirect, to: "https://b"} +`), + 0o644, + )) + require.NoError(t, os.WriteFile( + filepath.Join(dir, ".upsun", "sub", "more-routes.yaml"), + []byte(` +routes: + "https://b": {type: redirect, to: "https://a"} +`), + 0o644, + )) + + routes, src, err := DiscoverProjectRoutes(dir) + require.NoError(t, err) + assert.Len(t, routes, 2) + assert.Contains(t, src, "config.yaml") + assert.Contains(t, src, "more-routes.yaml") + + got := Detect(routes) + require.Len(t, got.Cycles, 1, "loops across .upsun files should be caught") +} + +func TestDiscoverProjectRoutes_NoConfig(t *testing.T) { + dir := t.TempDir() + _, _, err := DiscoverProjectRoutes(dir) + require.Error(t, err) + assert.Contains(t, err.Error(), ".platform/routes.yaml") + assert.Contains(t, err.Error(), ".upsun") +} From be8a47d54809fa8ddea79cb246e69c1c02758934 Mon Sep 17 00:00:00 2001 From: Matt Sinagra Date: Tue, 14 Jul 2026 12:24:24 +1000 Subject: [PATCH 2/2] Updates from copilot conversations --- internal/routeloops/parse.go | 6 +++++- internal/routeloops/routeloops.go | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/routeloops/parse.go b/internal/routeloops/parse.go index 8e9679a3a..e9b6af710 100644 --- a/internal/routeloops/parse.go +++ b/internal/routeloops/parse.go @@ -51,7 +51,11 @@ func ParseFile(path string) ([]Route, error) { if err != nil { return nil, err } - return parseAuto(data) + routes, err := parseAuto(data) + if err != nil { + return nil, fmt.Errorf("parse %s: %w", path, err) + } + return routes, nil } func parseAuto(data []byte) ([]Route, error) { diff --git a/internal/routeloops/routeloops.go b/internal/routeloops/routeloops.go index eb3950e0d..c1c85edad 100644 --- a/internal/routeloops/routeloops.go +++ b/internal/routeloops/routeloops.go @@ -144,8 +144,8 @@ func canonicalizeCycle(nodes []string) []string { } // normalize prepares a URL string for edge lookup. It strips whitespace and -// smart quotes, lowercases the scheme+host, and collapses a single trailing -// slash on the host portion. Placeholders like {default} are preserved as-is. +// smart quotes, lowercases the scheme+host, and trims trailing slashes. +// Placeholders like {default} are preserved as-is. func normalize(s string) string { s = strings.TrimFunc(s, func(r rune) bool { if unicode.IsSpace(r) {