diff --git a/CLAUDE.md b/CLAUDE.md index 9b5b2d58a..f29fbf478 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -78,7 +78,7 @@ The CLI operates as a wrapper around a legacy PHP CLI: **Commands**: `commands/` - `root.go`: Root command that sets up the Cobra CLI and delegates to legacy CLI when needed - Native Go commands: init, list, version, config:install, project:convert, completion, lint, and with Go auth (below): auth:browser-login (login), auth:api-token-login, auth:logout (logout), auth:token -- `lint.go`: Native config linter, `lint` (also `validate`; registered under the namespaced name `app:config-validate`). Validates Flex (`.upsun`) and Fixed (`.platform`) config in `internal/lint`, reporting all errors at once. The Flex JSON schema and the image registry come from meta.upsun.com (`make update-schema`, `make update-registry`): schema changes are made in the upsun/meta repository first. Fixed config is checked with the same schema, patched by `internal/lint/schema/fixed-overlay.json` (a JSON Merge Patch adding Fixed-only keys such as `disk`) +- `lint.go`: Native config linter, `lint` (also `validate`; registered under the namespaced name `app:config-validate`). Validates Flex (`.upsun`) and Fixed (`.platform`) config in `pkg/lint`, reporting all errors at once. It looks for those names only, so it is not registered in CLIs configured with other names. `pkg/lint` and `pkg/registry` are public because other Upsun projects import them; their API is not covered by semantic versioning. The Flex JSON schema and the image registry come from meta.upsun.com (`make update-schema`, `make update-registry`): schema changes are made in the upsun/meta repository first. Fixed config is checked with the same schema, patched by `internal/schema/fixed-overlay.json` (a JSON Merge Patch adding Fixed-only keys such as `disk`) - Unrecognized commands are passed to the legacy PHP CLI **Configuration**: `internal/config/` diff --git a/Makefile b/Makefile index 37176a6dd..6a6417154 100644 --- a/Makefile +++ b/Makefile @@ -236,23 +236,23 @@ lint-php: composer-dev ## Run PHP linters (php-cs-fixer, PHPStan) # The image registry, used by lint, is transformed from https://meta.upsun.com/images by gen.go. .PHONY: update-registry update-registry: ## Refresh the embedded image registry from meta.upsun.com - cd internal/lint/registry && go run gen.go + cd pkg/registry && go run gen.go # Fail if the embedded image registry is stale. .PHONY: check-registry check-registry: update-registry - git diff --exit-code -- internal/lint/registry/registry.json + git diff --exit-code -- pkg/registry/registry.json # The Flex config schema, used by lint, is from https://meta.upsun.com/schema/upsun (the upsun/meta repository). -# Fixed config is checked with the same schema and an overlay, internal/lint/schema/fixed-overlay.json. +# Fixed config is checked with the same schema and an overlay, internal/schema/fixed-overlay.json. .PHONY: update-schema update-schema: ## Refresh the embedded Flex config schema from meta.upsun.com - cd internal/lint/schema && go run gen.go + cd internal/schema && go run gen.go # Fail if the embedded Flex config schema is stale. .PHONY: check-schema check-schema: update-schema - git diff --exit-code -- internal/lint/schema/upsun-config-schema.json + git diff --exit-code -- internal/schema/upsun-config-schema.json # Installs dev dependencies, which a phar build removes. PHPStan's binary marks that they are installed. .PHONY: composer-dev diff --git a/commands/lint.go b/commands/lint.go index a4c9313be..41ef92822 100644 --- a/commands/lint.go +++ b/commands/lint.go @@ -17,7 +17,7 @@ import ( "github.com/spf13/cobra" "github.com/upsun/cli/internal/config" - "github.com/upsun/cli/internal/lint" + "github.com/upsun/cli/pkg/lint" ) // errLintFailed exits with code 1 when the configuration has errors, without another message. @@ -41,26 +41,28 @@ func newLintCommand(cnf *config.Config) *cobra.Command { Args: cobra.MaximumNArgs(1), SilenceErrors: true, SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - return runLint(cmd, args, vendorFromConfig(cnf)) - }, + RunE: runLint, } cmd.Flags().Bool("stdin", false, "Read merged Flex configuration from standard input") cmd.Flags().String("format", "text", `The output format: "text" or "json"`) return cmd } -// vendorFromConfig builds the linter's vendor conventions from the CLI config. -func vendorFromConfig(cnf *config.Config) lint.Vendor { - return lint.Vendor{ - Flavor: cnf.Service.ProjectConfigFlavor, - ConfigDir: cnf.Service.ProjectConfigDir, - AppFile: cnf.Service.AppConfigFile, +// lintSupported reports whether the CLI uses the config names that lint looks +// for (.upsun, or .platform with .platform.app.yaml). +func lintSupported(cnf *config.Config) bool { + switch cnf.Service.ProjectConfigDir { + case ".upsun": + return true + case ".platform": + return cnf.Service.AppConfigFile == "" || cnf.Service.AppConfigFile == ".platform.app.yaml" + default: + return false } } -func runLint(cmd *cobra.Command, args []string, vendor lint.Vendor) error { - result, format, err := lintInput(cmd, args, vendor) +func runLint(cmd *cobra.Command, args []string) error { + result, format, err := lintInput(cmd, args) if err != nil { // Report operational errors as lint errors in JSON, so there is always a document. if format == "json" { @@ -75,7 +77,7 @@ func runLint(cmd *cobra.Command, args []string, vendor lint.Vendor) error { return printLintResult(cmd, result, format) } -func lintInput(cmd *cobra.Command, args []string, vendor lint.Vendor) (*lint.Result, string, error) { +func lintInput(cmd *cobra.Command, args []string) (*lint.Result, string, error) { explicitStdin, _ := cmd.Flags().GetBool("stdin") format, _ := cmd.Flags().GetString("format") if format != "text" && format != "json" { @@ -104,15 +106,37 @@ func lintInput(cmd *cobra.Command, args []string, vendor lint.Vendor) (*lint.Res } root = abs } else { - root = lint.FindProjectRoot(".") + root = findProjectRoot(".") } if format == "text" { fmt.Fprintln(cmd.ErrOrStderr(), "Validating configuration in directory: "+color.CyanString(root)) } - result, _, err := lint.CheckDir(root, vendor) + result, err := lint.CheckDir(root) return result, format, err } +// findProjectRoot resolves the project root for a path: the nearest ancestor +// directory that contains a .git entry, or the path itself if none is found. +// This lets the command be run from anywhere inside a repository. The nearest +// (not topmost) .git is used so a stray repository higher up the tree (e.g. a +// dotfiles repo in the home directory) cannot hijack the result. +func findProjectRoot(path string) string { + abs, err := filepath.Abs(path) + if err != nil { + return path + } + for dir := abs; ; { + if _, err := os.Lstat(filepath.Join(dir, ".git")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + return abs + } + dir = parent + } +} + // capitalizeFirst upper-cases the first rune of s for user-facing display. func capitalizeFirst(s string) string { if s == "" { diff --git a/commands/lint_test.go b/commands/lint_test.go index 919b3b547..ed8569411 100644 --- a/commands/lint_test.go +++ b/commands/lint_test.go @@ -10,12 +10,13 @@ import ( "testing" "github.com/fatih/color" + "github.com/platformsh/platformify/vendorization" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/upsun/cli/internal/config" - "github.com/upsun/cli/internal/lint" + "github.com/upsun/cli/pkg/lint" ) // The JSON output documents "errors" and "warnings" as arrays, so a valid @@ -46,9 +47,10 @@ func TestPrintLintResult_JSONWithIssues(t *testing.T) { cmd.SetOut(&out) cmd.SetErr(io.Discard) - result := &lint.Result{} - result.AddError("applications.app1.type", "invalid type") - result.AddWarning("applications.app1.web.commands.start", "a start command is needed") + result := &lint.Result{ + Errors: []lint.Issue{{Path: "applications.app1.type", Message: "invalid type"}}, + Warnings: []lint.Issue{{Path: "applications.app1.web.commands.start", Message: "a start command is needed"}}, + } require.ErrorIs(t, printLintResult(cmd, result, "json"), errLintFailed) @@ -157,11 +159,58 @@ func TestPrintLintResult_ValidWithWarnings(t *testing.T) { var out bytes.Buffer cmd.SetOut(&out) - result := &lint.Result{} - result.AddWarning("", "both .upsun and .platform configuration found") + result := &lint.Result{Warnings: []lint.Issue{{Message: "both .upsun and .platform configuration found"}}} require.NoError(t, printLintResult(cmd, result, "text")) assert.Equal(t, `Warnings: both .upsun and .platform configuration found ✓ The configuration is valid, with 1 warning. `, out.String()) } + +func TestFindProjectRoot(t *testing.T) { + gitInit := func(t *testing.T, dir string) { + t.Helper() + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".git"), 0o755)) + } + + t.Run("ascends to the enclosing git root", func(t *testing.T) { + root := t.TempDir() + gitInit(t, root) + sub := filepath.Join(root, "services", "api") + require.NoError(t, os.MkdirAll(sub, 0o755)) + assert.Equal(t, root, findProjectRoot(sub)) + }) + + t.Run("nearest git root wins over an outer one", func(t *testing.T) { + outer := t.TempDir() + gitInit(t, outer) + inner := filepath.Join(outer, "vendor", "pkg") + gitInit(t, inner) + assert.Equal(t, inner, findProjectRoot(inner)) + }) +} + +func TestLintCommand_Registration(t *testing.T) { + cases := []struct { + name string + configDir string + appFile string + want bool + }{ + {"upsun", ".upsun", "", true}, + {"platform", ".platform", ".platform.app.yaml", true}, + {"custom config dir", ".acme", "", false}, + {"custom app file", ".platform", ".acme.app.yaml", false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + cnf := testConfig() + cnf.Service.ProjectConfigDir = c.configDir + cnf.Service.AppConfigFile = c.appFile + root := newRootCommand(cnf, &vendorization.VendorAssets{}) + cmd, _, err := root.Find([]string{"lint"}) + require.NoError(t, err) + assert.Equal(t, c.want, cmd != root) + }) + } +} diff --git a/commands/root.go b/commands/root.go index 7359cded1..944ab40bb 100644 --- a/commands/root.go +++ b/commands/root.go @@ -183,10 +183,12 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob newCompletionCommand(cnf), newHelpCommand(cnf), useLegacyStyleHelp(cnf, newInitCommand(cnf, assets)), - useLegacyStyleHelp(cnf, newLintCommand(cnf)), newListCommand(cnf), versionCommand, ) + if lintSupported(cnf) { + cmd.AddCommand(useLegacyStyleHelp(cnf, newLintCommand(cnf))) + } if cnf.Service.ProjectConfigFlavor == "upsun" { cmd.AddCommand(useLegacyStyleHelp(cnf, newProjectConvertCommand(cnf))) } diff --git a/internal/lint/linter.go b/internal/lint/linter.go deleted file mode 100644 index 27e09cd05..000000000 --- a/internal/lint/linter.go +++ /dev/null @@ -1,54 +0,0 @@ -package lint - -import ( - "errors" - "fmt" - - "github.com/upsun/cli/internal/lint/registry" - "github.com/upsun/cli/internal/lint/schema" -) - -var ErrEmptyContent = errors.New("empty content") - -// CheckContent checks merged Flex-style configuration content and returns a Result. -func CheckContent(content string) (*Result, error) { - if len(content) == 0 { - return nil, ErrEmptyContent - } - - yamlSchema, err := schema.Load() - if err != nil { - return nil, fmt.Errorf("failed to load schema: %w", err) - } - - // Check YAML validity and schema compliance. - if result := CheckYAMLSchema(content, yamlSchema); result.HasErrors() { - return result, nil - } - - cfg, err := DecodeConfig(content) - if err != nil { - return nil, err - } - - return runChecks(cfg) -} - -// runChecks runs the semantic checks over a decoded config. -func runChecks(cfg *Config) (*Result, error) { - reg, err := registry.Parsed() - if err != nil { - return nil, fmt.Errorf("failed to load registry: %w", err) - } - - return Combine( - CheckRelationships(cfg), - CheckNames(cfg), - CheckTypes(cfg, reg), - CheckScripts(cfg), - CheckWebConfig(cfg), - CheckDependencies(cfg), - CheckRoutes(cfg), - CheckTasks(cfg), - ), nil -} diff --git a/internal/lint/normalize.go b/internal/lint/normalize.go deleted file mode 100644 index 0fc4cbf35..000000000 --- a/internal/lint/normalize.go +++ /dev/null @@ -1,297 +0,0 @@ -package lint - -import ( - "errors" - "fmt" - "io/fs" - "os" - "path/filepath" - "strings" -) - -// Style identifies which configuration style a project uses. -type Style int - -const ( - StyleUnknown Style = iota - // StyleFlex is the Upsun unified configuration (.upsun/*.yaml). - StyleFlex - // StyleFixed is the legacy Platform.sh configuration (.platform.app.yaml, .platform/*.yaml). - StyleFixed -) - -// First-party config conventions. The native format and directory of any CLI -// come from its config (see Vendor); these constants are the cross-brand -// counterpart that a first-party build also recognizes. -const ( - flavorUpsun = "upsun" // project_config_flavor for the Flex format - firstPartyFlexDir = ".upsun" // Upsun (Flex) project config directory - firstPartyFixedDir = ".platform" - firstPartyFixedApp = ".platform.app.yaml" -) - -// Vendor describes the running CLI's configuration conventions, taken from the -// service section of its config (project_config_flavor, project_config_dir, -// app_config_file). It determines the native format and directory names, and -// whether a first-party cross-brand counterpart is also recognized. -type Vendor struct { - Flavor string // "upsun" (Flex) or "platform" (Fixed) - ConfigDir string // e.g. ".upsun", ".platform", ".acme" - AppFile string // Fixed per-app config file, e.g. ".platform.app.yaml" -} - -// fixedNames is the pair of names that identify a Fixed-style layout. -type fixedNames struct { - dir string // config directory, e.g. ".platform" - app string // per-app config file, e.g. ".platform.app.yaml" -} - -// candidates returns the Flex directories and Fixed name-sets to look for. The -// native format comes from the vendor config; a first-party build (config dir -// is .upsun or .platform) also recognizes the other first-party format. -func (v Vendor) candidates() (flexDirs []string, fixed []fixedNames) { - cfgDir := v.ConfigDir - if cfgDir == "" { - cfgDir = firstPartyFlexDir - } - if v.Flavor == flavorUpsun { - flexDirs = []string{cfgDir} - if cfgDir == firstPartyFlexDir { // First-party Upsun: Platform.sh counterpart. - fixed = []fixedNames{{firstPartyFixedDir, firstPartyFixedApp}} - } - return flexDirs, fixed - } - app := v.AppFile - if app == "" { - app = firstPartyFixedApp - } - fixed = []fixedNames{{cfgDir, app}} - if cfgDir == firstPartyFixedDir { // First-party Platform.sh: Upsun counterpart. - flexDirs = []string{firstPartyFlexDir} - } - return flexDirs, fixed -} - -// CheckDir detects the configuration style in dir and lints it, returning the -// detected style. Detection uses the vendor's name conventions and the files -// present: Flex wins when present, else Fixed, else the native format. -func CheckDir(dir string, vendor Vendor) (*Result, Style, error) { - flexDirs, fixedSet := vendor.candidates() - flexDir, flexOK := detectFlex(dir, flexDirs) - fixedCfg, fixedOK := detectFixed(dir, fixedSet) - - switch { - case flexOK: - root, err := os.OpenRoot(dir) - if err != nil { - return nil, StyleFlex, err - } - defer root.Close() - // The root keeps !include tags from following symbolic links out of the project. - content, sources, err := getMergedConfigFiles(root.FS(), ".", flexDir) - var result *Result - var srcErr *sourceError - switch { - case errors.As(err, &srcErr): - result = &Result{Errors: []Issue{srcErr.issue()}} - case err != nil: - return nil, StyleFlex, err - default: - if result, err = CheckContent(content); err != nil { - return nil, StyleFlex, err - } - sources.locate(result) - } - addStrayWarnings(result, dir, flexDirs, fixedSet) - if fixedOK { - result.AddWarning("", fmt.Sprintf( - "both %s and %s configuration found; linting the %s (Flex) configuration", - flexDir, fixedCfg.dir, flexDir)) - } - return result, StyleFlex, nil - case fixedOK: - result, err := lintFixed(dir, fixedCfg) - if err != nil { - return nil, StyleFixed, err - } - addStrayWarnings(result, dir, flexDirs, fixedSet) - return result, StyleFixed, nil - default: - return nil, StyleUnknown, fmt.Errorf( - "no configuration found in %q (looked for %s)", dir, lookedFor(flexDirs, fixedSet)) - } -} - -// detectFlex returns the first candidate Flex directory that contains YAML files. -func detectFlex(root string, dirs []string) (string, bool) { - for _, d := range dirs { - if files, err := findFlexConfigFiles(os.DirFS(root), ".", d); err == nil && len(files) > 0 { - return d, true - } - } - return "", false -} - -// detectFixed returns the first candidate Fixed layout anchored at the root: its -// config directory exists, or a per-app config file sits at the top level. The -// anchor is required so a stray nested config file (e.g. a test fixture) does not -// turn an unrelated repository into a project; nested per-app files are still -// collected once a project is confirmed (see loadFixedApplications). -func detectFixed(root string, set []fixedNames) (fixedNames, bool) { - for _, f := range set { - if fi, err := os.Stat(filepath.Join(root, f.dir)); err == nil && fi.IsDir() { - return f, true - } - if _, ok := firstExistingYAML(root, "", f.app); ok { - return f, true - } - } - return fixedNames{}, false -} - -// lookedFor builds the hint listing the locations CheckDir searched. -func lookedFor(flexDirs []string, fixed []fixedNames) string { - parts := make([]string, 0, len(flexDirs)+2*len(fixed)) - for _, d := range flexDirs { - parts = append(parts, d+"/*.yaml") - } - for _, f := range fixed { - parts = append(parts, f.app, f.dir+"/") - } - return strings.Join(parts, ", ") -} - -// FindProjectRoot resolves the project root for a path: the nearest ancestor -// directory that contains a .git entry, or the path itself if none is found. -// This lets the command be run from anywhere inside a repository. The nearest -// (not topmost) .git is used so a stray repository higher up the tree (e.g. a -// dotfiles repo in the home directory) cannot hijack the result. -func FindProjectRoot(path string) string { - abs, err := filepath.Abs(path) - if err != nil { - return path - } - for dir := abs; ; { - if _, err := os.Lstat(filepath.Join(dir, ".git")); err == nil { - return dir - } - parent := filepath.Dir(dir) - if parent == dir { - return abs - } - dir = parent - } -} - -// addStrayWarnings warns about nested copies of any config directory the vendor -// knows about. The platform only reads a config directory at the project root, -// so any below it are ignored. -func addStrayWarnings(result *Result, root string, flexDirs []string, fixed []fixedNames) { - configDirs := map[string]bool{} - for _, d := range flexDirs { - configDirs[d] = true - } - for _, f := range fixed { - configDirs[f.dir] = true - } - walkProject(root, func(path string, d fs.DirEntry) { - // A known config directory below the root (not a root-level copy, which - // detection already handles) is ignored by the platform. - if d.IsDir() && configDirs[d.Name()] && filepath.Dir(path) != root { - result.Warnings = append(result.Warnings, Issue{ - File: filepath.ToSlash(relTo(root, path)), - Message: fmt.Sprintf("this %s directory is not at the project root and will be ignored", d.Name()), - }) - } - }) -} - -// maxFixedAppDepth bounds how deep the walk descends looking for per-app config -// files. It mirrors the legacy CLI's safeguard against slow, over-broad searches. -const maxFixedAppDepth = 5 - -// walkProject walks root, pruning VCS/dependency directories, separate Git -// checkouts, and limiting depth, invoking visit for each remaining entry (files -// and directories). -func walkProject(root string, visit func(path string, d fs.DirEntry)) { - _ = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { - if err != nil { - return nil - } - if d.IsDir() && path != root { - switch d.Name() { - case ".git", ".idea", "node_modules", "vendor", "builds": - return fs.SkipDir - } - if rel, err := filepath.Rel(root, path); err == nil && - len(strings.Split(filepath.ToSlash(rel), "/")) >= maxFixedAppDepth { - return fs.SkipDir - } - if isSeparateCheckout(path) { - return fs.SkipDir - } - } - visit(path, d) - return nil - }) -} - -// findFixedAppFiles returns the paths of per-app config files (appFile, or its -// .yml variant) under dir, descending into subdirectories up to maxFixedAppDepth. -func findFixedAppFiles(dir, appFile string) []string { - names := yamlVariants(appFile) - var found []string - walkProject(dir, func(path string, d fs.DirEntry) { - if d.IsDir() { - return - } - for _, name := range names { - if d.Name() == name { - found = append(found, path) - break - } - } - }) - return found -} - -// firstExistingYAML returns the first existing path among the .yaml/.yml variants -// of base within dir under root, and whether one was found. -func firstExistingYAML(root, dir, base string) (string, bool) { - for _, name := range yamlVariants(base) { - p := filepath.Join(root, dir, name) - if _, err := os.Stat(p); err == nil { - return p, true - } - } - return "", false -} - -// yamlVariants returns base with both .yaml and .yml extensions, stripping any -// existing .yaml/.yml suffix first. -func yamlVariants(base string) []string { - base = strings.TrimSuffix(strings.TrimSuffix(base, ".yaml"), ".yml") - return []string{base + ".yaml", base + ".yml"} -} - -// isSeparateCheckout reports whether dir is a Git checkout other than a -// submodule: a separate clone (a .git directory) or a linked worktree (a .git -// file pointing into another repository's "worktrees" directory). These are not -// part of the project. Submodules also have a .git file, pointing into -// "modules", and are kept since they are deployed with the project. -func isSeparateCheckout(dir string) bool { - gitPath := filepath.Join(dir, ".git") - fi, err := os.Lstat(gitPath) - if err != nil { - return false - } - if fi.IsDir() { - return true - } - b, err := os.ReadFile(gitPath) - if err != nil { - return false - } - gitDir, ok := strings.CutPrefix(strings.TrimSpace(string(b)), "gitdir:") - return ok && strings.Contains(filepath.ToSlash(strings.TrimSpace(gitDir)), "/worktrees/") -} diff --git a/internal/lint/registry/model.go b/internal/lint/registry/model.go deleted file mode 100644 index 8d985970e..000000000 --- a/internal/lint/registry/model.go +++ /dev/null @@ -1,109 +0,0 @@ -package registry - -import ( - "encoding/json" - "slices" -) - -type Registry map[string]Image - -func (r Registry) AllTypes(runtime bool) []string { - types := make([]string, 0, len(r)) - for k := range r { - if r[k].IsRuntime == runtime { - types = append(types, k) - } - } - slices.Sort(types) - return types -} - -// Image describes a single service/runtime container image. -type Image struct { - Name string `json:"name" yaml:"name,omitempty"` - Type string `json:"type" yaml:"type"` - IsRuntime bool `json:"runtime" yaml:"is_runtime"` - Versions VersionInfo `json:"versions,omitzero" yaml:"versions,omitempty"` - - Description string `json:"description,omitempty" yaml:"description,omitempty"` - Configuration string `json:"configuration,omitempty" yaml:"configuration,omitempty"` - Docs Docs `json:"docs,omitzero" yaml:"docs,omitempty"` -} - -// Docs contains documentation for the image. -type Docs struct { - RelationshipName *string `json:"relationship_name,omitempty" yaml:"relationship_name,omitempty"` - ServiceName *string `json:"service_name,omitempty" yaml:"service_name,omitempty"` - URL string `json:"url,omitempty" yaml:"url,omitempty"` - Web Web `json:"web,omitzero" yaml:"web,omitempty"` - Hooks Hooks `json:"hooks,omitzero" yaml:"hooks,omitempty"` - Build BuildConfig `json:"build,omitzero" yaml:"build,omitempty"` - Dependencies map[string]Dependency `json:"dependencies,omitempty" yaml:"dependencies,omitempty"` -} - -// Web describes web server configuration for runtime images. -type Web struct { - Commands struct { - Start string `json:"start,omitempty" yaml:"start,omitempty"` - } `json:"commands,omitzero" yaml:"commands,omitempty"` - Locations map[string]Location `json:"locations,omitempty" yaml:"locations,omitempty"` - Upstream Upstream `json:"upstream,omitzero" yaml:"upstream,omitempty"` -} - -// Upstream config for protocols/sockets. -type Upstream struct { - SocketFamily string `json:"socket_family,omitempty" yaml:"socket_family,omitempty"` - Protocol string `json:"protocol,omitempty" yaml:"protocol,omitempty"` -} - -// Location defines routing rules inside the Web configuration. -type Location struct { - Root string `json:"root,omitempty" yaml:"root,omitempty"` - Allow bool `json:"allow,omitempty" yaml:"allow,omitempty"` - Passthru any `json:"passthru,omitempty" yaml:"passthru,omitempty"` // String or bool - Expires any `json:"expires,omitempty" yaml:"expires,omitempty"` // String or int -} - -// Hooks define build and deploy lifecycle commands. -type Hooks struct { - Build any `json:"build,omitempty" yaml:"build,omitempty"` // String or list? - Deploy any `json:"deploy,omitempty" yaml:"deploy,omitempty"` -} - -// BuildConfig holds custom build settings. -type BuildConfig struct { - Flavor string `json:"flavor,omitempty" yaml:"flavor,omitempty"` -} - -// Dependency version map under Docs. -type Dependency map[string]string - -// VersionInfo lists deprecated, supported, and optional legacy and retired versions. -type VersionInfo struct { - Supported []string `json:"supported"` - - Deprecated []string `json:"deprecated,omitempty" yaml:"deprecated,omitempty"` - Legacy []string `json:"legacy,omitempty" yaml:"legacy,omitempty"` - Retired []string `json:"retired,omitempty" yaml:"retired,omitempty"` -} - -// UnmarshalJSON handles both string and object formats for versions field. -// Some registry entries have versions as a string (e.g. "25.05") instead of an object. -func (v *VersionInfo) UnmarshalJSON(data []byte) error { - // Try unmarshaling as a string first. - var str string - if err := json.Unmarshal(data, &str); err == nil { - // It's a string, treat it as a single supported version. - v.Supported = []string{str} - return nil - } - - // Try unmarshaling as the normal object structure. - type versionInfoAlias VersionInfo - var obj versionInfoAlias - if err := json.Unmarshal(data, &obj); err != nil { - return err - } - *v = VersionInfo(obj) - return nil -} diff --git a/internal/schema/doc.go b/internal/schema/doc.go new file mode 100644 index 000000000..e15db4320 --- /dev/null +++ b/internal/schema/doc.go @@ -0,0 +1,2 @@ +// Package schema provides the embedded JSON schemas for Upsun configuration (Flex, and Fixed via an overlay). +package schema diff --git a/internal/lint/schema/fixed-overlay.json b/internal/schema/fixed-overlay.json similarity index 100% rename from internal/lint/schema/fixed-overlay.json rename to internal/schema/fixed-overlay.json diff --git a/internal/lint/schema/fixed.go b/internal/schema/fixed.go similarity index 100% rename from internal/lint/schema/fixed.go rename to internal/schema/fixed.go diff --git a/internal/lint/schema/gen.go b/internal/schema/gen.go similarity index 92% rename from internal/lint/schema/gen.go rename to internal/schema/gen.go index a6965a21a..024eb3ae5 100644 --- a/internal/lint/schema/gen.go +++ b/internal/schema/gen.go @@ -3,7 +3,7 @@ // Command gen fetches the Upsun configuration schema from meta.upsun.com and // writes it, indented, to upsun-config-schema.json. // -// Run it with: go run gen.go (or `go generate ./internal/lint/schema`). +// Run it with: go run gen.go (or `go generate ./internal/schema`). package main import ( diff --git a/internal/lint/schema/schema.go b/internal/schema/schema.go similarity index 100% rename from internal/lint/schema/schema.go rename to internal/schema/schema.go diff --git a/internal/lint/schema/schema_test.go b/internal/schema/schema_test.go similarity index 100% rename from internal/lint/schema/schema_test.go rename to internal/schema/schema_test.go diff --git a/internal/lint/schema/upsun-config-schema.json b/internal/schema/upsun-config-schema.json similarity index 100% rename from internal/lint/schema/upsun-config-schema.json rename to internal/schema/upsun-config-schema.json diff --git a/internal/lint/config_schema.go b/pkg/lint/config_schema.go similarity index 77% rename from internal/lint/config_schema.go rename to pkg/lint/config_schema.go index 7a37fbe16..7c032a0b2 100644 --- a/internal/lint/config_schema.go +++ b/pkg/lint/config_schema.go @@ -7,11 +7,11 @@ import ( "gopkg.in/yaml.v3" ) -type Config struct { +type config struct { Applications map[string]struct { Type string `yaml:"type"` Stack any `yaml:"stack,omitempty"` - Image *OCIImage `yaml:"image,omitempty"` + Image *ociImage `yaml:"image,omitempty"` Hooks struct { Build string `yaml:"build,omitempty"` @@ -19,24 +19,24 @@ type Config struct { PostDeploy string `yaml:"post_deploy,omitempty"` } `yaml:"hooks,omitempty"` - Authorizations []Authorization `yaml:"authorizations,omitempty"` + Authorizations []authorization `yaml:"authorizations,omitempty"` Web struct { - Authorizations []Authorization `yaml:"authorizations,omitempty"` + Authorizations []authorization `yaml:"authorizations,omitempty"` Commands struct { Start string `yaml:"start,omitempty"` PostStart string `yaml:"post_start,omitempty"` } `yaml:"commands,omitempty"` - Locations map[string]WebLocation `yaml:"locations,omitempty"` + Locations map[string]webLocation `yaml:"locations,omitempty"` Relationships map[string]any `yaml:"relationships,omitempty"` - Mounts map[string]Mount `yaml:"mounts,omitempty"` + Mounts map[string]mount `yaml:"mounts,omitempty"` } `yaml:"web,omitempty"` Relationships map[string]any `yaml:"relationships,omitempty"` - Mounts map[string]Mount `yaml:"mounts,omitempty"` + Mounts map[string]mount `yaml:"mounts,omitempty"` Crons map[string]struct { Commands struct { @@ -47,8 +47,8 @@ type Config struct { Workers map[string]struct { Type string `yaml:"type,omitempty"` - Authorizations []Authorization `yaml:"authorizations,omitempty"` - Mounts map[string]Mount `yaml:"mounts,omitempty"` + Authorizations []authorization `yaml:"authorizations,omitempty"` + Mounts map[string]mount `yaml:"mounts,omitempty"` Relationships map[string]any `yaml:"relationships,omitempty"` Commands struct { @@ -71,21 +71,21 @@ type Config struct { To string `yaml:"to,omitempty"` } `yaml:"routes,omitempty"` - Tasks map[string]Task `yaml:"tasks,omitempty"` + Tasks map[string]task `yaml:"tasks,omitempty"` } -// WebLocation configures how requests under a web location are served. -type WebLocation struct { +// webLocation configures how requests under a web location are served. +type webLocation struct { Root string `yaml:"root,omitempty"` Rules map[string]any `yaml:"rules,omitempty"` } -// Task is an on-demand, run-to-completion workload (Flex only). -type Task struct { +// task is an on-demand, run-to-completion workload (Flex only). +type task struct { Type string `yaml:"type,omitempty"` Base string `yaml:"base,omitempty"` Stack any `yaml:"stack,omitempty"` - Image *OCIImage `yaml:"image,omitempty"` + Image *ociImage `yaml:"image,omitempty"` Run struct { Command string `yaml:"command,omitempty"` @@ -97,8 +97,8 @@ type Task struct { } `yaml:"hooks,omitempty"` Relationships map[string]any `yaml:"relationships,omitempty"` - Mounts map[string]Mount `yaml:"mounts,omitempty"` - Authorizations []Authorization `yaml:"authorizations,omitempty"` + Mounts map[string]mount `yaml:"mounts,omitempty"` + Authorizations []authorization `yaml:"authorizations,omitempty"` // fields lists the keys set on the task, to check that "base" is used alone. // As on the platform, this includes keys starting with ".". @@ -106,8 +106,8 @@ type Task struct { } // UnmarshalYAML decodes a task and records which keys it sets. -func (t *Task) UnmarshalYAML(node *yaml.Node) error { - type plain Task +func (t *task) UnmarshalYAML(node *yaml.Node) error { + type plain task if err := node.Decode((*plain)(t)); err != nil { return err } @@ -117,31 +117,31 @@ func (t *Task) UnmarshalYAML(node *yaml.Node) error { return nil } -// OCIImage configures the image of a container with a docker: type. -type OCIImage struct { +// ociImage configures the image of a container with a docker: type. +type ociImage struct { Name string `yaml:"name,omitempty"` Buildfile string `yaml:"buildfile,omitempty"` } -// Mount is a writable directory in a container. -type Mount struct { +// mount is a writable directory in a container. +type mount struct { Source string `yaml:"source,omitempty"` Service string `yaml:"service,omitempty"` } -// Authorization grants a workload access to the Upsun API at runtime. -type Authorization struct { +// authorization grants a workload access to the Upsun API at runtime. +type authorization struct { Type string `yaml:"type"` Action string `yaml:"action"` Resource string `yaml:"resource,omitempty"` } -func DecodeConfig(content string) (*Config, error) { +func decodeConfig(content string) (*config, error) { var doc yaml.Node if err := yaml.Unmarshal([]byte(content), &doc); err != nil { return nil, fmt.Errorf("failed to parse YAML: %w", err) } - var c Config + var c config if len(doc.Content) == 0 { return &c, nil } diff --git a/internal/lint/dependencies.go b/pkg/lint/dependencies.go similarity index 77% rename from internal/lint/dependencies.go rename to pkg/lint/dependencies.go index 104d1468c..465ceacec 100644 --- a/internal/lint/dependencies.go +++ b/pkg/lint/dependencies.go @@ -2,8 +2,8 @@ package lint import "fmt" -// CheckDependencies validates the application dependencies section. -func CheckDependencies(cfg *Config) *Result { +// checkDependencies validates the application dependencies section. +func checkDependencies(cfg *config) *Result { result := &Result{} validTypes := map[string]bool{ @@ -21,7 +21,7 @@ func CheckDependencies(cfg *Config) *Result { if !validTypes[depType] { path := "applications." + appName + ".dependencies." + depType msg := fmt.Sprintf("invalid dependency type '%s'; must be one of: nodejs, php, python, python3, ruby", depType) - result.AddError(path, msg) + result.addError(path, msg) continue } @@ -29,11 +29,11 @@ func CheckDependencies(cfg *Config) *Result { for pkgName, version := range packages { if pkgName == "" { path := "applications." + appName + ".dependencies." + depType - result.AddError(path, "package name cannot be empty") + result.addError(path, "package name cannot be empty") } if version == "" { path := "applications." + appName + ".dependencies." + depType + "." + pkgName - result.AddError(path, "package version cannot be empty") + result.addError(path, "package version cannot be empty") } } } diff --git a/internal/lint/dependencies_test.go b/pkg/lint/dependencies_test.go similarity index 97% rename from internal/lint/dependencies_test.go rename to pkg/lint/dependencies_test.go index c7c982495..bef52f23f 100644 --- a/internal/lint/dependencies_test.go +++ b/pkg/lint/dependencies_test.go @@ -102,12 +102,12 @@ applications: for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cfg, err := DecodeConfig(tt.yaml) + cfg, err := decodeConfig(tt.yaml) if err != nil { t.Fatalf("failed to decode YAML: %v", err) } - result := CheckDependencies(cfg) + result := checkDependencies(cfg) if len(tt.wantErrs) == 0 { if result.HasErrors() { diff --git a/pkg/lint/doc.go b/pkg/lint/doc.go new file mode 100644 index 000000000..9fd47339d --- /dev/null +++ b/pkg/lint/doc.go @@ -0,0 +1,5 @@ +// Package lint validates Upsun configuration (Flex and Fixed). +// +// Other Upsun projects import this package. It is not covered by semantic versioning: the module has no /v5 +// path, so importers use pseudo-versions, and the API may change in any commit. +package lint diff --git a/pkg/lint/export_test.go b/pkg/lint/export_test.go new file mode 100644 index 000000000..9089c4742 --- /dev/null +++ b/pkg/lint/export_test.go @@ -0,0 +1,14 @@ +package lint + +// Exports for the external lint_test package. +var ( + CheckNames = checkNames + CheckRelationships = checkRelationships + CheckRoutes = checkRoutes + CheckScripts = checkScripts + CheckTasks = checkTasks + CheckTypes = checkTypes + CheckWebConfig = checkWebConfig + CheckYAMLSchema = checkYAMLSchema + DecodeConfig = decodeConfig +) diff --git a/internal/lint/fixed.go b/pkg/lint/fixed.go similarity index 85% rename from internal/lint/fixed.go rename to pkg/lint/fixed.go index bcee5df3e..78ba7b5ea 100644 --- a/internal/lint/fixed.go +++ b/pkg/lint/fixed.go @@ -11,7 +11,7 @@ import ( "github.com/xeipuuv/gojsonschema" "gopkg.in/yaml.v3" - "github.com/upsun/cli/internal/lint/schema" + "github.com/upsun/cli/internal/schema" ) // Configuration section keys. @@ -25,10 +25,10 @@ const ( // flexTopKeys are the top-level keys of Flex-style configuration. var flexTopKeys = []string{keyApplications, keyServices, keyRoutes, keyTasks} -// lintFixed lints Fixed-style configuration in dir using the resolved names in -// cfg: per-app config files (cfg.app) and/or cfg.dir/applications.yaml, plus -// optional cfg.dir/routes.yaml and cfg.dir/services.yaml. -func lintFixed(dir string, cfg fixedNames) (*Result, error) { +// lintFixed lints Fixed-style configuration in dir: per-app config files +// (.platform.app.yaml) and/or .platform/applications.yaml, plus optional +// .platform/routes.yaml and .platform/services.yaml. +func lintFixed(dir string) (*Result, error) { root, err := os.OpenRoot(dir) if err != nil { return nil, err @@ -36,22 +36,22 @@ func lintFixed(dir string, cfg fixedNames) (*Result, error) { defer root.Close() l := &fixedLoader{dir: dir, fsys: root.FS(), result: &Result{}, sources: sourceIndex{}} - apps, err := l.loadApplications(cfg) + apps, err := l.loadApplications() if err != nil { return nil, err } - services, err := l.loadSection(cfg.dir, keyServices, schema.LoadServices) + services, err := l.loadSection(fixedDir, keyServices, schema.LoadServices) if err != nil { return nil, err } - routes, err := l.loadSection(cfg.dir, keyRoutes, schema.LoadRoutes) + routes, err := l.loadSection(fixedDir, keyRoutes, schema.LoadRoutes) if err != nil { return nil, err } result := l.result if len(apps) == 0 && !result.HasErrors() { - result.AddError("", "no application configuration found") + result.addError("", "no application configuration found") } // If the structure is invalid, don't run semantic checks over a broken config. @@ -71,7 +71,7 @@ func lintFixed(dir string, cfg fixedNames) (*Result, error) { if err != nil { return nil, err } - decoded, err := DecodeConfig(string(mergedYAML)) + decoded, err := decodeConfig(string(mergedYAML)) if err != nil { return nil, err } @@ -81,7 +81,7 @@ func lintFixed(dir string, cfg fixedNames) (*Result, error) { return nil, err } l.sources.locate(checks) - result.Merge(checks) + result.merge(checks) return result, nil } @@ -112,14 +112,14 @@ func (l *fixedLoader) load(abs string) (file string, node *yaml.Node) { // check validates data against a schema, locating issues within node. func (l *fixedLoader) check(data any, sch *gojsonschema.Schema, key string, src source) { - checked := CheckSchemaAt(data, sch, key) + checked := checkSchemaAt(data, sch, key) sourceIndex{key: src}.locate(checked) - l.result.Merge(checked) + l.result.merge(checked) } // loadApplications collects applications from per-app config files and -// cfg.dir/applications.yaml, validating each against the application schema. -func (l *fixedLoader) loadApplications(cfg fixedNames) (map[string]any, error) { +// .platform/applications.yaml, validating each against the application schema. +func (l *fixedLoader) loadApplications() (map[string]any, error) { appSchema, err := schema.LoadApplication() if err != nil { return nil, fmt.Errorf("failed to load application schema: %w", err) @@ -151,7 +151,7 @@ func (l *fixedLoader) loadApplications(cfg fixedNames) (map[string]any, error) { } // Individual per-app config files (e.g. .platform.app.yaml). - for _, abs := range findFixedAppFiles(l.dir, cfg.app) { + for _, abs := range findFixedAppFiles(l.dir, fixedAppFile) { file, node := l.load(abs) if node == nil { continue @@ -168,8 +168,8 @@ func (l *fixedLoader) loadApplications(cfg fixedNames) (map[string]any, error) { add(fixedAppName(data), data, source{file: file, node: node}) } - // cfg.dir/applications.yaml (a list of apps, or a map keyed by app name). - appsFile, ok := firstExistingYAML(l.dir, cfg.dir, keyApplications) + // .platform/applications.yaml (a list of apps, or a map keyed by app name). + appsFile, ok := firstExistingYAML(l.dir, fixedDir, keyApplications) if !ok { return apps, nil } @@ -241,9 +241,9 @@ func (l *fixedLoader) loadSection( for _, entry := range mappingEntries(node) { l.sources[section+"."+entry.key.Value] = source{file: file, line: entry.key.Line, node: entry.value} } - checked := CheckSchemaAt(data, sch, section) + checked := checkSchemaAt(data, sch, section) l.sources.locate(checked) - l.result.Merge(checked) + l.result.merge(checked) return data, nil } diff --git a/internal/lint/fixed_test.go b/pkg/lint/fixed_test.go similarity index 98% rename from internal/lint/fixed_test.go rename to pkg/lint/fixed_test.go index c444fc750..b3fe95dba 100644 --- a/internal/lint/fixed_test.go +++ b/pkg/lint/fixed_test.go @@ -291,9 +291,9 @@ type: "php:999"`, writeFile(t, filepath.Join(dir, name), content) } - result, style, err := CheckDir(dir, upsunVendor()) + result, style, err := checkDir(dir) require.NoError(t, err) - assert.Equal(t, StyleFixed, style) + assert.Equal(t, styleFixed, style) if tc.wantNoErr { assert.False(t, result.HasErrors(), "expected no errors, got: %s", result) @@ -313,7 +313,7 @@ func TestLintFixed_DuplicateAppName(t *testing.T) { writeFile(t, filepath.Join(dir, ".platform", "routes.yaml"), "{}") writeFile(t, filepath.Join(dir, "a", ".platform.app.yaml"), "name: same\ntype: \"php:8.3\"") writeFile(t, filepath.Join(dir, "b", ".platform.app.yaml"), "name: same\ntype: \"php:8.3\"") - result, _, err := CheckDir(dir, upsunVendor()) + result, _, err := checkDir(dir) require.NoError(t, err) // The error should name both source files so it is actionable. assert.Contains(t, result.String(), `duplicate application name "same"`) diff --git a/pkg/lint/linter.go b/pkg/lint/linter.go new file mode 100644 index 000000000..411e0fdc7 --- /dev/null +++ b/pkg/lint/linter.go @@ -0,0 +1,57 @@ +package lint + +import ( + "errors" + "fmt" + + "github.com/upsun/cli/internal/schema" + "github.com/upsun/cli/pkg/registry" +) + +// ErrEmptyContent is returned by CheckContent when the content is empty. +var ErrEmptyContent = errors.New("empty content") + +// CheckContent lints Flex configuration given as a single YAML document, i.e. +// the .upsun/*.yaml files merged. Configuration problems are reported in the +// Result; the error is for content that cannot be checked at all. +func CheckContent(content string) (*Result, error) { + if len(content) == 0 { + return nil, ErrEmptyContent + } + + yamlSchema, err := schema.Load() + if err != nil { + return nil, fmt.Errorf("failed to load schema: %w", err) + } + + // Check YAML validity and schema compliance. + if result := checkYAMLSchema(content, yamlSchema); result.HasErrors() { + return result, nil + } + + cfg, err := decodeConfig(content) + if err != nil { + return nil, err + } + + return runChecks(cfg) +} + +// runChecks runs the semantic checks over a decoded config. +func runChecks(cfg *config) (*Result, error) { + reg, err := registry.Load() + if err != nil { + return nil, fmt.Errorf("failed to load registry: %w", err) + } + + return combine( + checkRelationships(cfg), + checkNames(cfg), + checkTypes(cfg, reg), + checkScripts(cfg), + checkWebConfig(cfg), + checkDependencies(cfg), + checkRoutes(cfg), + checkTasks(cfg), + ), nil +} diff --git a/internal/lint/linter_test.go b/pkg/lint/linter_test.go similarity index 96% rename from internal/lint/linter_test.go rename to pkg/lint/linter_test.go index 82854a096..23cea30fd 100644 --- a/internal/lint/linter_test.go +++ b/pkg/lint/linter_test.go @@ -29,7 +29,7 @@ applications: services: {} `, wantErr: true, - wantErrMsg: `linter errors: + wantErrMsg: `Linter errors: - applications.Foo_.relationships.missing_service: relationship 'missing_service' in application 'Foo_' does not match any service (or app) (did you forget to define services?) - applications.Foo_.type: type not found: 'invalid'; it must be one of: composable, dotnet, elixir, frankenphp, golang, java, nodejs, php, python, ruby, rust (check the Registry for supported types, or make an application using a composable image) - applications.Foo_: "Foo_" is not a valid application name, it can only contain lowercase alphanumeric characters, dashes, or underscores.`, //nolint:lll @@ -65,7 +65,7 @@ services: database: {} `, wantErr: true, - wantErrMsg: "linter errors:\n - services.database: type is required", + wantErrMsg: "Linter errors:\n - services.database: type is required", }, { name: "invalid worker name", @@ -87,7 +87,7 @@ services: type: mariadb:11.4 `, wantErr: true, - wantErrMsg: `linter errors: + wantErrMsg: `Linter errors: - applications.foo.workers._badworker: "_badworker" is not a valid worker name, it should start and end with alphanumeric characters.`, }, } @@ -99,9 +99,9 @@ services: if tc.wantErr { assert.True(t, result.HasErrors(), "expected errors but got none") - assert.Equal(t, tc.wantErrMsg, result.Error()) + assert.Equal(t, tc.wantErrMsg, result.String()) } else { - assert.False(t, result.HasErrors(), "expected no errors but got: %s", result.Error()) + assert.False(t, result.HasErrors(), "expected no errors but got: %s", result.String()) } }) } @@ -208,7 +208,7 @@ tasks: commands: start: python agent.py `, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.myapp.authorizations.0.type: must be one of the following: "env", "task" - applications.myapp.authorizations.0: Additional property scope is not allowed - tasks.myagent.run.timeout: Must be less than or equal to 86400 @@ -222,7 +222,7 @@ applications: type: php:8.4 workers: {} `, - wantErrors: `linter errors: + wantErrors: `Linter errors: - Additional property workers is not allowed`, }, } @@ -236,7 +236,7 @@ workers: {} assert.False(t, result.HasWarnings(), "unexpected warnings: %s", result) return } - assert.Equal(t, c.wantErrors, result.Error()) + assert.Equal(t, c.wantErrors, result.String()) }) } } @@ -393,7 +393,7 @@ applications: commands: start: php report.php `, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.app.crons.report.timeout: Must be less than or equal to 86400 - applications.app.egress.runtime.allowed_domains.0: host is required`, }, @@ -405,7 +405,7 @@ applications: type: composable:25.11 stack: ["php@8.4"] `, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.app.stack: Array must have at most 0 items`, }, { @@ -422,7 +422,7 @@ services: type: postgresql:16 disk: 1024 `, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.app: Additional property disk is not allowed - applications.app: Additional property firewall is not allowed - services.db: Additional property disk is not allowed`, @@ -447,7 +447,7 @@ routes: type: upstream upstream: "classic:http" `, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.both.image: either name or buildfile must be provided, not both - applications.classic.image: 'image' is only used with a docker: type - applications.none.image: either name or buildfile must be provided, both are missing`, @@ -526,7 +526,7 @@ services: firewall: outbound: [{ips: ["192.0.2.0/24"]}] `, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.app.web.additional_hosts.api.example.com: Invalid type. Expected: string, given: integer - applications.app.web.operations.warm: commands is required - applications.app.web: Additional property firewall is not allowed @@ -545,7 +545,7 @@ applications: egress: build: {allowed_domains: [{host: repo.packagist.org}]} `, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.app.web.egress: Additional property build is not allowed`, }, { @@ -557,7 +557,7 @@ applications: web: container_profile: 5 `, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.app.web.container_profile: Invalid type. Expected: [string,null], given: integer`, }, } @@ -571,7 +571,7 @@ applications: assert.False(t, result.HasWarnings(), "unexpected warnings: %s", result) return } - assert.Equal(t, c.wantErrors, result.Error()) + assert.Equal(t, c.wantErrors, result.String()) }) } } @@ -656,7 +656,7 @@ applications: locations: /: {allow: null} `, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.app.mounts: Invalid type. Expected: object, given: null - applications.app.web.locations./.allow: Invalid type. Expected: boolean, given: null`, }, @@ -726,7 +726,7 @@ applications: tasks: .note: x `, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.app.crons..note: Invalid type. Expected: object, given: string - tasks..note: Invalid type. Expected: object, given: string`, }, @@ -780,7 +780,7 @@ services: assert.False(t, result.HasWarnings(), "unexpected warnings: %s", result) return } - assert.Equal(t, c.wantErrors, result.Error()) + assert.Equal(t, c.wantErrors, result.String()) }) } } diff --git a/internal/lint/merge.go b/pkg/lint/merge.go similarity index 100% rename from internal/lint/merge.go rename to pkg/lint/merge.go diff --git a/internal/lint/merge_test.go b/pkg/lint/merge_test.go similarity index 100% rename from internal/lint/merge_test.go rename to pkg/lint/merge_test.go diff --git a/internal/lint/names.go b/pkg/lint/names.go similarity index 87% rename from internal/lint/names.go rename to pkg/lint/names.go index a5f0bb34d..7f30fc298 100644 --- a/internal/lint/names.go +++ b/pkg/lint/names.go @@ -14,29 +14,29 @@ var ( isAlphanumeric = regexp.MustCompile(`^[a-z0-9]+$`).MatchString ) -// CheckNames checks that application, service, worker and task names are correct. -func CheckNames(cfg *Config) *Result { +// checkNames checks that application, service, worker and task names are correct. +func checkNames(cfg *config) *Result { result := &Result{} for name := range cfg.Applications { app := cfg.Applications[name] if err := validateServiceName(name, "application"); err != "" { - result.AddError("applications."+name, err) + result.addError("applications."+name, err) } for workerName := range app.Workers { if err := validateServiceName(workerName, "worker"); err != "" { - result.AddError("applications."+name+".workers."+workerName, err) + result.addError("applications."+name+".workers."+workerName, err) } } } for name := range cfg.Services { if err := validateServiceName(name, "service"); err != "" { - result.AddError("services."+name, err) + result.addError("services."+name, err) } } for name := range cfg.Tasks { if err := validateServiceName(name, "task"); err != "" { - result.AddError("tasks."+name, err) + result.addError("tasks."+name, err) } } diff --git a/internal/lint/names_test.go b/pkg/lint/names_test.go similarity index 98% rename from internal/lint/names_test.go rename to pkg/lint/names_test.go index 980304c37..5ff083477 100644 --- a/internal/lint/names_test.go +++ b/pkg/lint/names_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/upsun/cli/internal/lint" + "github.com/upsun/cli/pkg/lint" ) func TestCheckNames(t *testing.T) { @@ -156,7 +156,7 @@ tasks: if len(c.expectErrorValues) > 0 { assert.True(t, result.HasErrors()) for _, v := range c.expectErrorValues { - assert.ErrorContains(t, result, v) + assert.Contains(t, result.String(), v) } } else { assert.False(t, result.HasErrors()) diff --git a/pkg/lint/normalize.go b/pkg/lint/normalize.go new file mode 100644 index 000000000..7d33998b0 --- /dev/null +++ b/pkg/lint/normalize.go @@ -0,0 +1,205 @@ +package lint + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" +) + +// style identifies which configuration style a project uses. +type style int + +const ( + styleUnknown style = iota + // styleFlex is the Upsun unified configuration (.upsun/*.yaml). + styleFlex + // styleFixed is the legacy Platform.sh configuration (.platform.app.yaml, .platform/*.yaml). + styleFixed +) + +// Configuration locations, relative to the project root. +const ( + flexDir = ".upsun" // Flex: .upsun/*.yaml + fixedDir = ".platform" // Fixed: applications, routes and services + fixedAppFile = ".platform.app.yaml" // Fixed: per-app config file +) + +// CheckDir detects the configuration style in dir and lints it: Flex +// (.upsun/*.yaml) when present, else Fixed (.platform/ or .platform.app.yaml). +// Configuration problems are reported in the Result; the error is for +// configuration that cannot be found, read or parsed. +func CheckDir(dir string) (*Result, error) { + result, _, err := checkDir(dir) + return result, err +} + +// checkDir is CheckDir, also returning the detected style. +func checkDir(dir string) (*Result, style, error) { + switch { + case isFlex(dir): + root, err := os.OpenRoot(dir) + if err != nil { + return nil, styleFlex, err + } + defer root.Close() + // The root keeps !include tags from following symbolic links out of the project. + content, sources, err := getMergedConfigFiles(root.FS(), ".", flexDir) + var result *Result + var srcErr *sourceError + switch { + case errors.As(err, &srcErr): + result = &Result{Errors: []Issue{srcErr.issue()}} + case err != nil: + return nil, styleFlex, err + default: + if result, err = CheckContent(content); err != nil { + return nil, styleFlex, err + } + sources.locate(result) + } + addStrayWarnings(result, dir) + if isFixed(dir) { + result.addWarning("", fmt.Sprintf( + "both %s and %s configuration found; linting the %s (Flex) configuration", + flexDir, fixedDir, flexDir)) + } + return result, styleFlex, nil + case isFixed(dir): + result, err := lintFixed(dir) + if err != nil { + return nil, styleFixed, err + } + addStrayWarnings(result, dir) + return result, styleFixed, nil + default: + return nil, styleUnknown, fmt.Errorf("no configuration found in %q (looked for %s/*.yaml, %s, %s/)", + dir, flexDir, fixedAppFile, fixedDir) + } +} + +// isFlex reports whether the Flex config directory under root contains YAML files. +func isFlex(root string) bool { + files, err := findFlexConfigFiles(os.DirFS(root), ".", flexDir) + return err == nil && len(files) > 0 +} + +// isFixed reports whether root has a Fixed layout: the config directory exists, +// or a per-app config file sits at the top level. The anchor is required so a +// stray nested config file (e.g. a test fixture) does not turn an unrelated +// repository into a project; nested per-app files are still collected once a +// project is confirmed (see loadApplications). +func isFixed(root string) bool { + if fi, err := os.Stat(filepath.Join(root, fixedDir)); err == nil && fi.IsDir() { + return true + } + _, ok := firstExistingYAML(root, "", fixedAppFile) + return ok +} + +// addStrayWarnings warns about nested config directories. The platform only +// reads a config directory at the project root, so any below it are ignored. +func addStrayWarnings(result *Result, root string) { + walkProject(root, func(path string, d fs.DirEntry) { + // A config directory below the root (not a root-level copy, which + // detection already handles) is ignored by the platform. + if d.IsDir() && (d.Name() == flexDir || d.Name() == fixedDir) && filepath.Dir(path) != root { + result.Warnings = append(result.Warnings, Issue{ + File: filepath.ToSlash(relTo(root, path)), + Message: fmt.Sprintf("this %s directory is not at the project root and will be ignored", d.Name()), + }) + } + }) +} + +// maxFixedAppDepth bounds how deep the walk descends looking for per-app config +// files. It mirrors the legacy CLI's safeguard against slow, over-broad searches. +const maxFixedAppDepth = 5 + +// walkProject walks root, pruning VCS/dependency directories, separate Git +// checkouts, and limiting depth, invoking visit for each remaining entry (files +// and directories). +func walkProject(root string, visit func(path string, d fs.DirEntry)) { + _ = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return nil + } + if d.IsDir() && path != root { + switch d.Name() { + case ".git", ".idea", "node_modules", "vendor", "builds": + return fs.SkipDir + } + if rel, err := filepath.Rel(root, path); err == nil && + len(strings.Split(filepath.ToSlash(rel), "/")) >= maxFixedAppDepth { + return fs.SkipDir + } + if isSeparateCheckout(path) { + return fs.SkipDir + } + } + visit(path, d) + return nil + }) +} + +// findFixedAppFiles returns the paths of per-app config files (appFile, or its +// .yml variant) under dir, descending into subdirectories up to maxFixedAppDepth. +func findFixedAppFiles(dir, appFile string) []string { + names := yamlVariants(appFile) + var found []string + walkProject(dir, func(path string, d fs.DirEntry) { + if d.IsDir() { + return + } + for _, name := range names { + if d.Name() == name { + found = append(found, path) + break + } + } + }) + return found +} + +// firstExistingYAML returns the first existing path among the .yaml/.yml variants +// of base within dir under root, and whether one was found. +func firstExistingYAML(root, dir, base string) (string, bool) { + for _, name := range yamlVariants(base) { + p := filepath.Join(root, dir, name) + if _, err := os.Stat(p); err == nil { + return p, true + } + } + return "", false +} + +// yamlVariants returns base with both .yaml and .yml extensions, stripping any +// existing .yaml/.yml suffix first. +func yamlVariants(base string) []string { + base = strings.TrimSuffix(strings.TrimSuffix(base, ".yaml"), ".yml") + return []string{base + ".yaml", base + ".yml"} +} + +// isSeparateCheckout reports whether dir is a Git checkout other than a +// submodule: a separate clone (a .git directory) or a linked worktree (a .git +// file pointing into another repository's "worktrees" directory). These are not +// part of the project. Submodules also have a .git file, pointing into +// "modules", and are kept since they are deployed with the project. +func isSeparateCheckout(dir string) bool { + gitPath := filepath.Join(dir, ".git") + fi, err := os.Lstat(gitPath) + if err != nil { + return false + } + if fi.IsDir() { + return true + } + b, err := os.ReadFile(gitPath) + if err != nil { + return false + } + gitDir, ok := strings.CutPrefix(strings.TrimSpace(string(b)), "gitdir:") + return ok && strings.Contains(filepath.ToSlash(strings.TrimSpace(gitDir)), "/worktrees/") +} diff --git a/internal/lint/normalize_test.go b/pkg/lint/normalize_test.go similarity index 69% rename from internal/lint/normalize_test.go rename to pkg/lint/normalize_test.go index 146f0e739..bbd2650cf 100644 --- a/internal/lint/normalize_test.go +++ b/pkg/lint/normalize_test.go @@ -15,72 +15,23 @@ func writeFile(t *testing.T, path, content string) { require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) } -// upsunVendor is the first-party Upsun configuration used by most tests. -func upsunVendor() Vendor { - return Vendor{Flavor: "upsun", ConfigDir: ".upsun"} -} - func TestDetectStyle(t *testing.T) { cases := []struct { name string files map[string]string - want Style + want style wantErr bool }{ - {"flex", map[string]string{".upsun/config.yaml": "applications: {}"}, StyleFlex, false}, - {"fixed via app file", map[string]string{".platform.app.yaml": "name: app\ntype: \"php:8.3\""}, StyleFixed, false}, - {"fixed via .platform dir", map[string]string{".platform/routes.yaml": "{}"}, StyleFixed, false}, - {"none", nil, StyleUnknown, true}, + {"flex", map[string]string{".upsun/config.yaml": "applications: {}"}, styleFlex, false}, + {"fixed via app file", map[string]string{".platform.app.yaml": "name: app\ntype: \"php:8.3\""}, styleFixed, false}, + {"fixed via .platform dir", map[string]string{".platform/routes.yaml": "{}"}, styleFixed, false}, + {"flex wins over fixed", map[string]string{ + ".upsun/config.yaml": "applications: {}", ".platform.app.yaml": "name: app\ntype: \"php:8.3\""}, styleFlex, false}, + {"none", nil, styleUnknown, true}, // A per-app file buried in a subdirectory (e.g. a test fixture) must not, // on its own, mark the directory as a Fixed-style project. {"nested app file alone is not a project", - map[string]string{"fixtures/example/.platform.app.yaml": "name: app"}, StyleUnknown, true}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - dir := t.TempDir() - for p, c := range tc.files { - writeFile(t, filepath.Join(dir, p), c) - } - _, style, err := CheckDir(dir, upsunVendor()) - if tc.wantErr { - require.Error(t, err) - } else { - require.NoError(t, err) - } - assert.Equal(t, tc.want, style) - }) - } -} - -func TestCheckDir_Vendors(t *testing.T) { - const app = "name: app\ntype: \"php:8.3\"\n" - platformVendor := Vendor{Flavor: "platform", ConfigDir: ".platform", AppFile: ".platform.app.yaml"} - acmeFixed := Vendor{Flavor: "platform", ConfigDir: ".acme", AppFile: "acme.app.yaml"} - acmeFlex := Vendor{Flavor: "upsun", ConfigDir: ".acme"} - - cases := []struct { - name string - vendor Vendor - files map[string]string - want Style - wantErr bool - }{ - // Scenario B: Platform.sh CLI recognizes the Upsun Flex counterpart. - {"platform: .upsun present wins", platformVendor, - map[string]string{".upsun/config.yaml": "applications: {}", ".platform.app.yaml": app}, StyleFlex, false}, - {"platform: no .upsun is Fixed", platformVendor, - map[string]string{".platform.app.yaml": app}, StyleFixed, false}, - // Scenario C: white-label Fixed uses its own names, no counterpart. - {"white-label fixed via app file", acmeFixed, - map[string]string{"acme.app.yaml": app}, StyleFixed, false}, - {"white-label fixed ignores .upsun counterpart", acmeFixed, - map[string]string{".upsun/config.yaml": "applications: {}"}, StyleUnknown, true}, - // Scenario D: white-label Flex uses its own dir, no counterpart. - {"white-label flex via its dir", acmeFlex, - map[string]string{".acme/config.yaml": "applications: {}"}, StyleFlex, false}, - {"white-label flex ignores .platform counterpart", acmeFlex, - map[string]string{".platform.app.yaml": app}, StyleUnknown, true}, + map[string]string{"fixtures/example/.platform.app.yaml": "name: app"}, styleUnknown, true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -88,7 +39,7 @@ func TestCheckDir_Vendors(t *testing.T) { for p, c := range tc.files { writeFile(t, filepath.Join(dir, p), c) } - _, style, err := CheckDir(dir, tc.vendor) + _, style, err := checkDir(dir) if tc.wantErr { require.Error(t, err) } else { @@ -99,31 +50,13 @@ func TestCheckDir_Vendors(t *testing.T) { } } -func TestFindProjectRoot(t *testing.T) { - t.Run("ascends to the enclosing git root", func(t *testing.T) { - root := t.TempDir() - writeFile(t, filepath.Join(root, ".git", "HEAD"), "ref: refs/heads/main") - sub := filepath.Join(root, "services", "api") - require.NoError(t, os.MkdirAll(sub, 0o755)) - assert.Equal(t, root, FindProjectRoot(sub)) - }) - - t.Run("nearest git root wins over an outer one", func(t *testing.T) { - outer := t.TempDir() - writeFile(t, filepath.Join(outer, ".git", "HEAD"), "ref: refs/heads/main") - inner := filepath.Join(outer, "vendor", "pkg") - writeFile(t, filepath.Join(inner, ".git", "HEAD"), "ref: refs/heads/main") - assert.Equal(t, inner, FindProjectRoot(inner)) - }) -} - func TestLintFixed_StrayPlatformDir(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, ".platform.app.yaml"), "name: app\ntype: \"php:8.3\"") writeFile(t, filepath.Join(dir, "legacy", ".platform", "routes.yaml"), "{}") - result, style, err := CheckDir(dir, upsunVendor()) + result, style, err := checkDir(dir) require.NoError(t, err) - assert.Equal(t, StyleFixed, style) + assert.Equal(t, styleFixed, style) assert.Contains(t, result.String(), "legacy/.platform") assert.Contains(t, result.String(), "not at the project root and will be ignored") } @@ -134,9 +67,9 @@ func TestLintFixed_StrayUpsunDir(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, ".platform.app.yaml"), "name: app\ntype: \"php:8.3\"") writeFile(t, filepath.Join(dir, "sub", ".upsun", "config.yaml"), "applications: {}") - result, style, err := CheckDir(dir, upsunVendor()) + result, style, err := checkDir(dir) require.NoError(t, err) - assert.Equal(t, StyleFixed, style) + assert.Equal(t, styleFixed, style) assert.Contains(t, result.String(), "sub/.upsun") } @@ -155,17 +88,17 @@ routes: type: upstream upstream: "app:http" `) - result, style, err := CheckDir(dir, upsunVendor()) + result, style, err := checkDir(dir) require.NoError(t, err) - assert.Equal(t, StyleFlex, style) + assert.Equal(t, styleFlex, style) assert.False(t, result.HasErrors(), "expected no errors, got: %s", result) } func TestLintDir_NoConfig(t *testing.T) { dir := t.TempDir() - _, style, err := CheckDir(dir, upsunVendor()) + _, style, err := checkDir(dir) require.Error(t, err) - assert.Equal(t, StyleUnknown, style) + assert.Equal(t, styleUnknown, style) assert.Contains(t, err.Error(), "no configuration found") } @@ -176,9 +109,9 @@ func TestLintDir_BothPresentPrefersFlex(t *testing.T) { type: "php:8.3" `) writeFile(t, filepath.Join(dir, ".platform.app.yaml"), "name: app") - result, style, err := CheckDir(dir, upsunVendor()) + result, style, err := checkDir(dir) require.NoError(t, err) - assert.Equal(t, StyleFlex, style) + assert.Equal(t, styleFlex, style) assert.True(t, result.HasWarnings()) assert.Contains(t, result.String(), "both .upsun and .platform") } @@ -206,7 +139,7 @@ func TestLintDir_FlexSourceFiles(t *testing.T) { type: upstream upstream: "missing:http" `) - result, _, err := CheckDir(dir, upsunVendor()) + result, _, err := checkDir(dir) require.NoError(t, err) assert.Equal(t, `Linter errors: - .upsun/tasks.yaml:2: tasks.agent: Additional property extra is not allowed`, result.String()) @@ -218,7 +151,7 @@ func TestLintDir_FlexSourceFiles(t *testing.T) { run: command: ./run `) - result, _, err = CheckDir(dir, upsunVendor()) + result, _, err = checkDir(dir) require.NoError(t, err) assert.Contains(t, result.String(), ".upsun/apps.yaml:6: applications.app.authorizations.0.resource: task 'missing' is not found") assert.Contains(t, result.String(), `.upsun/routes.yaml:4: routes["https://{default}/api"].upstream: upstream target 'missing' does not exist`) @@ -236,7 +169,7 @@ func TestLintDir_SkipsSeparateCheckouts(t *testing.T) { writeFile(t, filepath.Join(dir, "sub", ".git"), "gitdir: ../.git/modules/sub\n") writeFile(t, filepath.Join(dir, "sub", ".upsun", "config.yaml"), "applications: {}") - result, _, err := CheckDir(dir, upsunVendor()) + result, _, err := checkDir(dir) require.NoError(t, err) assert.Equal(t, []Issue{{ File: "sub/.upsun", @@ -252,9 +185,9 @@ func TestLintFixed_WorktreeAppFileIgnored(t *testing.T) { writeFile(t, filepath.Join(dir, "wt", ".git"), "gitdir: /elsewhere/.git/worktrees/wt\n") writeFile(t, filepath.Join(dir, "wt", ".platform.app.yaml"), app) - result, style, err := CheckDir(dir, upsunVendor()) + result, style, err := checkDir(dir) require.NoError(t, err) - assert.Equal(t, StyleFixed, style) + assert.Equal(t, styleFixed, style) assert.False(t, result.HasErrors(), "unexpected errors: %s", result) } @@ -271,7 +204,7 @@ hooks: `) writeFile(t, filepath.Join(dir, ".upsun", "build.sh"), "set -e\ncomposer install\n") - result, _, err := CheckDir(dir, upsunVendor()) + result, _, err := checkDir(dir) require.NoError(t, err) assert.Equal(t, []Issue{{ File: ".upsun/config.yaml", Line: 3, Message: "'apps/missing.yaml' doesn't exist in the repository", @@ -279,7 +212,7 @@ hooks: writeFile(t, filepath.Join(dir, ".upsun", "config.yaml"), "applications:\n app: !include apps/app.yaml\n") writeFile(t, filepath.Join(dir, ".upsun", "build.sh"), "set -e\ncomposer install (\n") - result, _, err = CheckDir(dir, upsunVendor()) + result, _, err = checkDir(dir) require.NoError(t, err) require.Len(t, result.Errors, 1) assert.Equal(t, ".upsun/config.yaml", result.Errors[0].File) @@ -302,7 +235,7 @@ hooks: upstream: "missing:http" `) - result, _, err := CheckDir(dir, upsunVendor()) + result, _, err := checkDir(dir) require.NoError(t, err) assert.Equal(t, []Issue{{ File: ".platform/routes.yaml", @@ -320,7 +253,7 @@ func TestLintDir_Aliases(t *testing.T) { type: "php:1.0" applications: *apps `) - result, _, err := CheckDir(dir, upsunVendor()) + result, _, err := checkDir(dir) require.NoError(t, err) require.Len(t, result.Errors, 1) assert.Equal(t, Issue{ @@ -338,7 +271,7 @@ func TestLintDir_IncludeSymlinkOutsideProject(t *testing.T) { app: type: !include {type: string, path: ../link/secret.txt} `) - result, _, err := CheckDir(dir, upsunVendor()) + result, _, err := checkDir(dir) require.NoError(t, err) require.Len(t, result.Errors, 1) assert.Equal(t, Issue{ diff --git a/internal/lint/relationships.go b/pkg/lint/relationships.go similarity index 91% rename from internal/lint/relationships.go rename to pkg/lint/relationships.go index dc2a3a62c..bd329e2c7 100644 --- a/internal/lint/relationships.go +++ b/pkg/lint/relationships.go @@ -5,17 +5,17 @@ import ( "strings" ) -// CheckRelationships checks that application, service and task names are unique, +// checkRelationships checks that application, service and task names are unique, // and that application and task relationships correspond to existing services or // applications. -func CheckRelationships(cfg *Config) *Result { +func checkRelationships(cfg *config) *Result { result := &Result{} // Applications, services and tasks share a namespace. sections := make(map[string]string) addName := func(name, section string) { if prev, exists := sections[name]; exists { - result.AddError(section+"."+name, + result.addError(section+"."+name, fmt.Sprintf("duplicate name found: '%s' in '%s' (previous in '%s')", name, section, prev)) return } @@ -50,7 +50,7 @@ func CheckRelationships(cfg *Config) *Result { _, isWorker := workerNames[target] switch { case section == keyTasks: - result.AddError(relPath, fmt.Sprintf( + result.addError(relPath, fmt.Sprintf( "relationship '%s' in %s '%s' points to task '%s', but a task cannot be a relationship target", relName, kind, name, target)) case isNamed || isWorker: @@ -64,7 +64,7 @@ func CheckRelationships(cfg *Config) *Result { if len(cfg.Services) == 0 { msg += " (did you forget to define services?)" } - result.AddError(relPath, msg) + result.addError(relPath, msg) } } } @@ -82,7 +82,7 @@ func CheckRelationships(cfg *Config) *Result { } // A service can also be used through a mount (e.g. network storage). - linkMounts := func(mounts map[string]Mount) { + linkMounts := func(mounts map[string]mount) { for _, m := range mounts { if m.Source == "service" && m.Service != "" { linkedServices[m.Service] = struct{}{} @@ -103,7 +103,7 @@ func CheckRelationships(cfg *Config) *Result { for name := range cfg.Services { if _, linked := linkedServices[name]; !linked { - result.AddWarning("services."+name, fmt.Sprintf("no application or task has a relationship to service '%s'", name)) + result.addWarning("services."+name, fmt.Sprintf("no application or task has a relationship to service '%s'", name)) } } diff --git a/internal/lint/relationships_test.go b/pkg/lint/relationships_test.go similarity index 92% rename from internal/lint/relationships_test.go rename to pkg/lint/relationships_test.go index 420c7775c..db89b16bf 100644 --- a/internal/lint/relationships_test.go +++ b/pkg/lint/relationships_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/upsun/cli/internal/lint" + "github.com/upsun/cli/pkg/lint" ) //nolint:lll @@ -35,7 +35,7 @@ applications: foo: relationships: database:`, - expectErrorMessage: "linter errors:\n - applications.foo.relationships.database: relationship 'database' in application 'foo' " + //nolint:lll + expectErrorMessage: "Linter errors:\n - applications.foo.relationships.database: relationship 'database' in application 'foo' " + //nolint:lll "does not match any service (or app) (did you forget to define services?)", }, { @@ -46,7 +46,7 @@ applications: relationships: database: service: mydb`, - expectErrorMessage: "linter errors:\n - applications.foo.relationships.database: relationship 'database' in application 'foo' " + //nolint:lll + expectErrorMessage: "Linter errors:\n - applications.foo.relationships.database: relationship 'database' in application 'foo' " + //nolint:lll "points to a service (or app) named 'mydb' which is not found (did you forget to define services?)", }, { @@ -57,9 +57,9 @@ applications: foo: {} services: foo: {}`, - expectErrorMessage: "linter errors:" + + expectErrorMessage: "Linter errors:" + "\n - services.foo: duplicate name found: 'foo' in 'services' (previous in 'applications')" + - "\nlinter warnings:" + + "\nLinter warnings:" + "\n - services.foo: no application or task has a relationship to service 'foo'", }, { @@ -98,7 +98,7 @@ services: type: mariadb:11.4 kv: type: valkey:8.0`, - expectErrorMessage: "linter errors:\n - applications.bar.relationships.postgres: relationship 'postgres' in application 'bar' " + //nolint:lll + expectErrorMessage: "Linter errors:\n - applications.bar.relationships.postgres: relationship 'postgres' in application 'bar' " + //nolint:lll "does not match any service (or app)", }, { @@ -212,7 +212,7 @@ services: type: mariadb:11.4 files: type: network-storage:2.0`, - expectErrorMessage: `linter errors: + expectErrorMessage: `Linter errors: - applications.app.workers.queue.relationships.missing: relationship 'missing' in application 'app' does not match any service (or app)`, //nolint:lll }, { @@ -226,7 +226,7 @@ tasks: type: python:3.14 relationships: cache:`, - expectErrorMessage: `linter errors: + expectErrorMessage: `Linter errors: - tasks.agent.relationships.cache: relationship 'cache' in task 'agent' does not match any service (or app) (did you forget to define services?)`, //nolint:lll }, { @@ -244,7 +244,7 @@ tasks: type: python:3.14 relationships: agent:`, - expectErrorMessage: `linter errors: + expectErrorMessage: `Linter errors: - applications.app.relationships.agent: relationship 'agent' in application 'app' points to task 'agent', but a task cannot be a relationship target - tasks.other.relationships.agent: relationship 'agent' in task 'other' points to task 'agent', but a task cannot be a relationship target`, //nolint:lll }, @@ -264,7 +264,7 @@ tasks: type: python:3.14 db: type: python:3.14`, - expectErrorMessage: `linter errors: + expectErrorMessage: `Linter errors: - tasks.app: duplicate name found: 'app' in 'tasks' (previous in 'applications') - tasks.db: duplicate name found: 'db' in 'tasks' (previous in 'services')`, }, @@ -280,7 +280,7 @@ tasks: result := lint.CheckRelationships(cfg) if c.expectErrorMessage != "" { assert.True(t, result.HasErrors() || result.HasWarnings()) - assert.Equal(t, c.expectErrorMessage, result.Error()) + assert.Equal(t, c.expectErrorMessage, result.String()) } else { assert.False(t, result.HasErrors()) } diff --git a/internal/lint/result.go b/pkg/lint/result.go similarity index 54% rename from internal/lint/result.go rename to pkg/lint/result.go index 7b6c01d9b..2ddd3229b 100644 --- a/internal/lint/result.go +++ b/pkg/lint/result.go @@ -12,11 +12,11 @@ type Result struct { Warnings []Issue } -// Issue represents a single linting problem with its location in the content. +// Issue is a single linting problem and its location. type Issue struct { File string `json:"file,omitempty"` // The file, relative to the project root, if known. Line int `json:"line,omitempty"` // The line in the file, if known. - Path string `json:"path"` // e.g., "applications.foo.type", "services.database" + Path string `json:"path"` // The config key, e.g. "applications.foo.type". Message string `json:"message"` } @@ -28,28 +28,28 @@ func (i Issue) Location() string { return i.File } -// AddError adds an error to the linter result. -func (r *Result) AddError(path, message string) { +// addError adds an error to the result. +func (r *Result) addError(path, message string) { r.Errors = append(r.Errors, Issue{ Path: path, Message: message, }) } -// AddWarning adds a warning to the linter result. -func (r *Result) AddWarning(path, message string) { +// addWarning adds a warning to the result. +func (r *Result) addWarning(path, message string) { r.Warnings = append(r.Warnings, Issue{ Path: path, Message: message, }) } -// HasErrors returns true if the validation result contains any errors. +// HasErrors reports whether the result contains any errors. func (r *Result) HasErrors() bool { return len(r.Errors) > 0 } -// HasWarnings returns true if the validation result contains any warnings. +// HasWarnings reports whether the result contains any warnings. func (r *Result) HasWarnings() bool { return len(r.Warnings) > 0 } @@ -77,12 +77,6 @@ func issueLines(issues []Issue) []string { return messages } -// ErrorLines returns the formatted, sorted error lines without a heading. -func (r *Result) ErrorLines() []string { return issueLines(r.Errors) } - -// WarningLines returns the formatted, sorted warning lines without a heading. -func (r *Result) WarningLines() []string { return issueLines(r.Warnings) } - // formatIssues formats a list of validation issues with a given prefix. func formatIssues(issues []Issue, prefix string) []string { lines := issueLines(issues) @@ -92,40 +86,14 @@ func formatIssues(issues []Issue, prefix string) []string { return append([]string{prefix}, lines...) } -// formatResult formats all lint issues with appropriate capitalization. -func (r *Result) formatResult(capitalize bool) string { - var messages []string - - errorPrefix := "linter errors:" - warningPrefix := "linter warnings:" - if capitalize { - errorPrefix = "Linter errors:" - warningPrefix = "Linter warnings:" - } - - if errorMessages := formatIssues(r.Errors, errorPrefix); errorMessages != nil { - messages = append(messages, errorMessages...) - } - - if warningMessages := formatIssues(r.Warnings, warningPrefix); warningMessages != nil { - messages = append(messages, warningMessages...) - } - - return strings.Join(messages, "\n") -} - -// Error returns a formatted string representation of all validation issues. -func (r *Result) Error() string { - return r.formatResult(false) -} - -// String returns a formatted string representation of all validation issues. +// String formats all issues: sorted errors, then sorted warnings, each under a heading. func (r *Result) String() string { - return r.formatResult(true) + messages := append(formatIssues(r.Errors, "Linter errors:"), formatIssues(r.Warnings, "Linter warnings:")...) + return strings.Join(messages, "\n") } -// Merge appends another result's errors and warnings into this one. -func (r *Result) Merge(other *Result) { +// merge appends another result's errors and warnings into this one. +func (r *Result) merge(other *Result) { if other == nil { return } @@ -133,8 +101,8 @@ func (r *Result) Merge(other *Result) { r.Warnings = append(r.Warnings, other.Warnings...) } -// Combine combines a list of validation results. -func Combine(results ...*Result) *Result { +// combine combines a list of validation results. +func combine(results ...*Result) *Result { result := &Result{} for _, r := range results { if r != nil { diff --git a/internal/lint/routes.go b/pkg/lint/routes.go similarity index 82% rename from internal/lint/routes.go rename to pkg/lint/routes.go index 0aea07a60..8d4c4d52f 100644 --- a/internal/lint/routes.go +++ b/pkg/lint/routes.go @@ -6,13 +6,13 @@ import ( "strings" ) -// CheckRoutes validates that upstream routes link to existing applications or services. -func CheckRoutes(cfg *Config) *Result { +// checkRoutes validates that upstream routes link to existing applications or services. +func checkRoutes(cfg *config) *Result { result := &Result{} // Check if at least 1 route is defined when multiple applications exist if len(cfg.Applications) > 1 && len(cfg.Routes) == 0 { - result.AddError("routes", "at least 1 route must be defined when multiple applications are defined") + result.addError("routes", "at least 1 route must be defined when multiple applications are defined") } // Build a set of valid upstream targets (app names and service names) @@ -34,7 +34,7 @@ func CheckRoutes(cfg *Config) *Result { continue } if route.Upstream == "" { - result.AddError(fmt.Sprintf("routes[%q].upstream", routeURL), + result.addError(fmt.Sprintf("routes[%q].upstream", routeURL), "upstream property is required for routes with type 'upstream'") continue } @@ -43,7 +43,7 @@ func CheckRoutes(cfg *Config) *Result { upstream := route.Upstream parts := strings.SplitN(upstream, ":", 2) if len(parts) != 2 { - result.AddError(fmt.Sprintf("routes[%q].upstream", routeURL), + result.addError(fmt.Sprintf("routes[%q].upstream", routeURL), fmt.Sprintf("upstream '%s' must be in format 'name:protocol'", upstream)) continue } @@ -61,10 +61,10 @@ func CheckRoutes(cfg *Config) *Result { sort.Strings(availableTargets) if len(availableTargets) == 0 { - result.AddError(fmt.Sprintf("routes[%q].upstream", routeURL), + result.addError(fmt.Sprintf("routes[%q].upstream", routeURL), fmt.Sprintf("upstream target '%s' does not exist (no applications or services defined)", targetName)) } else { - result.AddError(fmt.Sprintf("routes[%q].upstream", routeURL), + result.addError(fmt.Sprintf("routes[%q].upstream", routeURL), fmt.Sprintf("upstream target '%s' does not exist, available targets: %s", targetName, strings.Join(availableTargets, ", "))) } @@ -73,7 +73,7 @@ func CheckRoutes(cfg *Config) *Result { if _, isApp := cfg.Applications[targetName]; isApp { // For applications, protocol must be 'http' if protocol != "http" { - result.AddError(fmt.Sprintf("routes[%q].upstream", routeURL), + result.addError(fmt.Sprintf("routes[%q].upstream", routeURL), fmt.Sprintf("protocol '%s' is not valid for application '%s'; must be 'http'", protocol, targetName)) } } diff --git a/internal/lint/routes_test.go b/pkg/lint/routes_test.go similarity index 88% rename from internal/lint/routes_test.go rename to pkg/lint/routes_test.go index ccfc38d77..094c15f6e 100644 --- a/internal/lint/routes_test.go +++ b/pkg/lint/routes_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/upsun/cli/internal/lint" + "github.com/upsun/cli/pkg/lint" ) func TestCheckRoutes(t *testing.T) { @@ -81,7 +81,7 @@ applications: routes: "https://{default}/": type: upstream`, - expectErrorMessage: "linter errors:\n - routes[\"https://{default}/\"].upstream: upstream property is required for routes with type 'upstream'", //nolint:lll + expectErrorMessage: "Linter errors:\n - routes[\"https://{default}/\"].upstream: upstream property is required for routes with type 'upstream'", //nolint:lll }, { name: "invalid_upstream_format", @@ -93,7 +93,7 @@ routes: "https://{default}/": type: upstream upstream: myapp_without_protocol`, - expectErrorMessage: "linter errors:\n - routes[\"https://{default}/\"].upstream: upstream 'myapp_without_protocol' must be in format 'name:protocol'", //nolint:lll + expectErrorMessage: "Linter errors:\n - routes[\"https://{default}/\"].upstream: upstream 'myapp_without_protocol' must be in format 'name:protocol'", //nolint:lll }, { name: "nonexistent_upstream_target", @@ -105,7 +105,7 @@ routes: "https://{default}/": type: upstream upstream: nonexistent:http`, - expectErrorMessage: "linter errors:\n - routes[\"https://{default}/\"].upstream: upstream target 'nonexistent' does not exist, available targets: myapp", //nolint:lll + expectErrorMessage: "Linter errors:\n - routes[\"https://{default}/\"].upstream: upstream target 'nonexistent' does not exist, available targets: myapp", //nolint:lll }, { name: "no_apps_or_services", @@ -114,7 +114,7 @@ routes: "https://{default}/": type: upstream upstream: myapp:http`, - expectErrorMessage: "linter errors:\n - routes[\"https://{default}/\"].upstream: upstream target 'myapp' does not exist (no applications or services defined)", //nolint:lll + expectErrorMessage: "Linter errors:\n - routes[\"https://{default}/\"].upstream: upstream target 'myapp' does not exist (no applications or services defined)", //nolint:lll }, { name: "multiple_invalid_routes", @@ -129,7 +129,7 @@ routes: "https://{default}/api": type: upstream upstream: nonexistent2:http`, - expectErrorMessage: "linter errors:\n - routes[\"https://{default}/\"].upstream: upstream target 'nonexistent1' does not exist, available targets: myapp\n - routes[\"https://{default}/api\"].upstream: upstream target 'nonexistent2' does not exist, available targets: myapp", //nolint:lll + expectErrorMessage: "Linter errors:\n - routes[\"https://{default}/\"].upstream: upstream target 'nonexistent1' does not exist, available targets: myapp\n - routes[\"https://{default}/api\"].upstream: upstream target 'nonexistent2' does not exist, available targets: myapp", //nolint:lll }, { name: "invalid_protocol_for_app", @@ -141,7 +141,7 @@ routes: "https://{default}/": type: upstream upstream: myapp:fastcgi`, - expectErrorMessage: "linter errors:\n - routes[\"https://{default}/\"].upstream: protocol 'fastcgi' is not valid for application 'myapp'; must be 'http'", //nolint:lll + expectErrorMessage: "Linter errors:\n - routes[\"https://{default}/\"].upstream: protocol 'fastcgi' is not valid for application 'myapp'; must be 'http'", //nolint:lll }, { name: "any_protocol_valid_for_service", @@ -165,7 +165,7 @@ routes: "https://{default}/": type: upstream upstream: myapp:custom_protocol`, - expectErrorMessage: "linter errors:\n - routes[\"https://{default}/\"].upstream: protocol 'custom_protocol' is not valid for application 'myapp'; must be 'http'", //nolint:lll + expectErrorMessage: "Linter errors:\n - routes[\"https://{default}/\"].upstream: protocol 'custom_protocol' is not valid for application 'myapp'; must be 'http'", //nolint:lll }, { name: "complex_route_urls", @@ -198,7 +198,7 @@ applications: type: nodejs:22 app2: type: php:8.4`, - expectErrorMessage: "linter errors:\n - routes: at least 1 route must be defined when multiple applications are defined", //nolint:lll + expectErrorMessage: "Linter errors:\n - routes: at least 1 route must be defined when multiple applications are defined", //nolint:lll }, { name: "multiple_apps_with_routes_valid", @@ -224,7 +224,7 @@ routes: result := lint.CheckRoutes(cfg) if c.expectErrorMessage != "" { assert.True(t, result.HasErrors() || result.HasWarnings()) - assert.Equal(t, c.expectErrorMessage, result.Error()) + assert.Equal(t, c.expectErrorMessage, result.String()) } else { assert.False(t, result.HasErrors()) assert.False(t, result.HasWarnings()) diff --git a/internal/lint/scripts.go b/pkg/lint/scripts.go similarity index 94% rename from internal/lint/scripts.go rename to pkg/lint/scripts.go index d92547da7..8130ac280 100644 --- a/internal/lint/scripts.go +++ b/pkg/lint/scripts.go @@ -7,7 +7,7 @@ import ( "mvdan.cc/sh/v3/syntax" ) -func CheckScripts(cfg *Config) *Result { +func checkScripts(cfg *config) *Result { result := &Result{} var scripts = make(map[string]string) @@ -48,7 +48,7 @@ func CheckScripts(cfg *Config) *Result { } r := strings.NewReader(v) if _, err := syntax.NewParser(syntax.Variant(syntax.LangPOSIX)).Parse(r, ""); err != nil { - result.AddError(k, fmt.Sprintf("invalid syntax: %s", err)) + result.addError(k, fmt.Sprintf("invalid syntax: %s", err)) } } diff --git a/internal/lint/scripts_test.go b/pkg/lint/scripts_test.go similarity index 88% rename from internal/lint/scripts_test.go rename to pkg/lint/scripts_test.go index 74b7a5782..347807ca5 100644 --- a/internal/lint/scripts_test.go +++ b/pkg/lint/scripts_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/upsun/cli/internal/lint" + "github.com/upsun/cli/pkg/lint" ) func TestCheckScripts(t *testing.T) { @@ -43,7 +43,7 @@ applications: commands: start: "node server.js" `, - expectErrorMessage: "linter errors:\n - applications.app1.hooks.build: invalid syntax: 1:6: " + + expectErrorMessage: "Linter errors:\n - applications.app1.hooks.build: invalid syntax: 1:6: " + "reached EOF without closing quote `'`", }, { @@ -56,7 +56,7 @@ applications: build: "echo 'Building app1'" deploy: "if (true; then echo 'deployed'; fi" `, - expectErrorMessage: "linter errors:\n - applications.app1.hooks.deploy: invalid syntax: 1:11: " + + expectErrorMessage: "Linter errors:\n - applications.app1.hooks.deploy: invalid syntax: 1:11: " + "`then` can only be used in an `if`", }, { @@ -71,7 +71,7 @@ applications: commands: start: "echo 'Missing quote" `, - expectErrorMessage: "linter errors:\n - applications.app1.crons.backup.commands.start: invalid syntax: 1:6: " + + expectErrorMessage: "Linter errors:\n - applications.app1.crons.backup.commands.start: invalid syntax: 1:6: " + "reached EOF without closing quote `'`", }, { @@ -84,7 +84,7 @@ applications: commands: start: "node server.js | | grep output" `, - expectErrorMessage: "linter errors:\n - applications.app1.web.commands.start: invalid syntax: 1:16: " + + expectErrorMessage: "Linter errors:\n - applications.app1.web.commands.start: invalid syntax: 1:16: " + "`|` must be followed by a statement", }, { @@ -106,7 +106,7 @@ applications: commands: start: "python app.py" `, - expectErrorMessage: "linter errors:\n - applications.app2.hooks.build: invalid syntax: 1:5: " + + expectErrorMessage: "Linter errors:\n - applications.app2.hooks.build: invalid syntax: 1:5: " + "c-style fors are a bash/zsh feature; tried parsing as posix", }, { @@ -123,7 +123,7 @@ applications: commands: start: "node worker.js | | grep output" `, - expectErrorMessage: "linter errors:\n - applications.app1.workers.queue.commands.start: " + + expectErrorMessage: "Linter errors:\n - applications.app1.workers.queue.commands.start: " + "invalid syntax: 1:16: `|` must be followed by a statement", }, { @@ -141,7 +141,7 @@ applications: pre_start: "echo 'unterminated" start: "node worker.js" `, - expectErrorMessage: "linter errors:\n - applications.app1.workers.queue.commands.pre_start: " + + expectErrorMessage: "Linter errors:\n - applications.app1.workers.queue.commands.pre_start: " + "invalid syntax: 1:6: reached EOF without closing quote `'`", }, { @@ -159,7 +159,7 @@ tasks: run: command: "python agent.py && (" `, - expectErrorMessage: "linter errors:\n" + + expectErrorMessage: "Linter errors:\n" + " - tasks.agent.hooks.deploy: invalid syntax: 1:6: reached EOF without closing quote `'`\n" + " - tasks.agent.run.command: invalid syntax: 1:20: `(` must be followed by a statement list", }, @@ -202,7 +202,7 @@ applications: if tt.expectErrorMessage != "" { assert.True(t, result.HasErrors()) - assert.Equal(t, tt.expectErrorMessage, result.Error()) + assert.Equal(t, tt.expectErrorMessage, result.String()) } else { assert.False(t, result.HasErrors()) } diff --git a/internal/lint/source.go b/pkg/lint/source.go similarity index 100% rename from internal/lint/source.go rename to pkg/lint/source.go diff --git a/internal/lint/source_test.go b/pkg/lint/source_test.go similarity index 100% rename from internal/lint/source_test.go rename to pkg/lint/source_test.go diff --git a/internal/lint/tasks.go b/pkg/lint/tasks.go similarity index 76% rename from internal/lint/tasks.go rename to pkg/lint/tasks.go index bc10af35d..6970be3f0 100644 --- a/internal/lint/tasks.go +++ b/pkg/lint/tasks.go @@ -14,9 +14,9 @@ var authorizationActions = map[string]string{ "env": "view", } -// CheckTasks checks task definitions and the authorizations of applications, +// checkTasks checks task definitions and the authorizations of applications, // their web and worker containers, and tasks. -func CheckTasks(cfg *Config) *Result { +func checkTasks(cfg *config) *Result { result := &Result{} for name := range cfg.Tasks { @@ -24,45 +24,45 @@ func CheckTasks(cfg *Config) *Result { path := "tasks." + name if task.Base != "" { if len(task.fields) > 1 { - result.AddError(path+".base", "when 'base' is set, no other property is allowed") + result.addError(path+".base", "when 'base' is set, no other property is allowed") } continue } if task.Run.Command == "" { - result.AddError(path+".run.command", "a run command is required") + result.addError(path+".run.command", "a run command is required") } for mountName, mount := range task.Mounts { mountPath := path + ".mounts." + mountName switch { case mount.Source == "storage" && mount.Service == "": - result.AddError(mountPath, "a task has no storage of its own, so a 'storage' mount must "+ + result.addError(mountPath, "a task has no storage of its own, so a 'storage' mount must "+ "name an application in 'service'") case mount.Source == "storage": if _, ok := cfg.Applications[mount.Service]; !ok { - result.AddError(mountPath+".service", fmt.Sprintf("application '%s' is not found", mount.Service)) + result.addError(mountPath+".service", fmt.Sprintf("application '%s' is not found", mount.Service)) } case mount.Source == "service": if _, ok := cfg.Services[mount.Service]; !ok { - result.AddError(mountPath+".service", fmt.Sprintf("service '%s' is not found", mount.Service)) + result.addError(mountPath+".service", fmt.Sprintf("service '%s' is not found", mount.Service)) } } } } - checkAuthorizations := func(path string, authorizations []Authorization) { + checkAuthorizations := func(path string, authorizations []authorization) { for i, a := range authorizations { itemPath := path + ".authorizations." + strconv.Itoa(i) if want, ok := authorizationActions[a.Type]; ok && a.Action != want { - result.AddError(itemPath+".action", + result.addError(itemPath+".action", fmt.Sprintf("authorization type '%s' only allows the action '%s'", a.Type, want)) } if a.Type != "task" { continue } if a.Resource == "" { - result.AddError(itemPath+".resource", "a task authorization requires a resource (the task name)") + result.addError(itemPath+".resource", "a task authorization requires a resource (the task name)") } else if _, ok := cfg.Tasks[a.Resource]; !ok { - result.AddError(itemPath+".resource", fmt.Sprintf("task '%s' is not found%s", + result.addError(itemPath+".resource", fmt.Sprintf("task '%s' is not found%s", a.Resource, availableHint(cfg.Tasks))) } } @@ -84,7 +84,7 @@ func CheckTasks(cfg *Config) *Result { } // availableHint lists the defined task names for an error message. -func availableHint(tasks map[string]Task) string { +func availableHint(tasks map[string]task) string { if len(tasks) == 0 { return " (no tasks are defined)" } diff --git a/internal/lint/tasks_test.go b/pkg/lint/tasks_test.go similarity index 93% rename from internal/lint/tasks_test.go rename to pkg/lint/tasks_test.go index 72abc3b56..7b99116d9 100644 --- a/internal/lint/tasks_test.go +++ b/pkg/lint/tasks_test.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/upsun/cli/internal/lint" + "github.com/upsun/cli/pkg/lint" ) //nolint:lll @@ -59,7 +59,7 @@ tasks: base: performance-agent run: command: ./run`, - wantErrors: `linter errors: + wantErrors: `Linter errors: - tasks.perf.base: when 'base' is set, no other property is allowed`, }, { @@ -71,7 +71,7 @@ applications: tasks: myagent: type: python:3.14`, - wantErrors: `linter errors: + wantErrors: `Linter errors: - tasks.myagent.run.command: a run command is required`, }, { @@ -92,7 +92,7 @@ tasks: - type: task resource: myagent action: view`, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.myapp.authorizations.0.action: authorization type 'env' only allows the action 'view' - tasks.myagent.authorizations.0.action: authorization type 'task' only allows the action 'operate'`, }, @@ -105,7 +105,7 @@ applications: authorizations: - type: task action: operate`, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.myapp.authorizations.0.resource: a task authorization requires a resource (the task name)`, }, { @@ -142,7 +142,7 @@ tasks: type: python:3.14 run: command: ./b`, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.myapp.authorizations.0.resource: task 'missing' is not found (defined tasks: a, b) - applications.myapp.web.authorizations.0.resource: task 'missing' is not found (defined tasks: a, b) - applications.myapp.workers.queue.authorizations.1.resource: task 'missing' is not found (defined tasks: a, b)`, @@ -157,7 +157,7 @@ applications: - type: task resource: myagent action: operate`, - wantErrors: `linter errors: + wantErrors: `Linter errors: - applications.myapp.authorizations.0.resource: task 'myagent' is not found (no tasks are defined)`, }, { @@ -191,7 +191,7 @@ tasks: wrong_service: source: service service: other`, - wantErrors: `linter errors: + wantErrors: `Linter errors: - tasks.myagent.mounts.own: a task has no storage of its own, so a 'storage' mount must name an application in 'service' - tasks.myagent.mounts.wrong_app.service: application 'other' is not found - tasks.myagent.mounts.wrong_service.service: service 'other' is not found`, @@ -207,7 +207,7 @@ tasks: assert.False(t, result.HasErrors(), "unexpected errors: %s", result) return } - assert.Equal(t, c.wantErrors, result.Error()) + assert.Equal(t, c.wantErrors, result.String()) }) } } diff --git a/internal/lint/testdata/registry.json b/pkg/lint/testdata/registry.json similarity index 100% rename from internal/lint/testdata/registry.json rename to pkg/lint/testdata/registry.json diff --git a/internal/lint/types.go b/pkg/lint/types.go similarity index 84% rename from internal/lint/types.go rename to pkg/lint/types.go index 023ec0f36..03989e38a 100644 --- a/internal/lint/types.go +++ b/pkg/lint/types.go @@ -5,47 +5,47 @@ import ( "slices" "strings" - "github.com/upsun/cli/internal/lint/registry" + "github.com/upsun/cli/pkg/registry" ) -// CheckTypes checks that application, worker, task and service types are supported images and versions. -func CheckTypes(cfg *Config, reg registry.Registry) *Result { +// checkTypes checks that application, worker, task and service types are supported images and versions. +func checkTypes(cfg *config, reg registry.Registry) *Result { result := &Result{} // check reports an error, or a warning for a retired version, at path. check := func(path, t string, runtime bool) { warning, err := checkType(t, reg, runtime) if err != nil { - result.AddError(path, err.Error()) + result.addError(path, err.Error()) } else if warning != "" { - result.AddWarning(path, warning) + result.addWarning(path, warning) } } // checkRuntime checks the type, stack and image of an application or task at path. - checkRuntime := func(path, t string, stack any, image *OCIImage) { + checkRuntime := func(path, t string, stack any, image *ociImage) { // A docker: type is built from an OCI image rather than a registry image. if strings.HasPrefix(t, "docker:") { switch { case image == nil || image.Name == "" && image.Buildfile == "": - result.AddError(path+".image", "either name or buildfile must be provided, both are missing") + result.addError(path+".image", "either name or buildfile must be provided, both are missing") case image.Name != "" && image.Buildfile != "": - result.AddError(path+".image", "either name or buildfile must be provided, not both") + result.addError(path+".image", "either name or buildfile must be provided, not both") } if !isStackEmpty(stack) { - result.AddWarning(path+".stack", "'stack' is only used with a composable image type") + result.addWarning(path+".stack", "'stack' is only used with a composable image type") } return } if image != nil { - result.AddError(path+".image", "'image' is only used with a docker: type") + result.addError(path+".image", "'image' is only used with a docker: type") } check(path+".type", t, true) isComposable := strings.HasPrefix(t, "composable") if isComposable && stack == nil { - result.AddWarning(path, "'stack' should be specified when using a composable image") + result.addWarning(path, "'stack' should be specified when using a composable image") } else if !isComposable && !isStackEmpty(stack) { - result.AddWarning(path+".stack", "'stack' is only used with a composable image type") + result.addWarning(path+".stack", "'stack' is only used with a composable image type") } } diff --git a/internal/lint/types_test.go b/pkg/lint/types_test.go similarity index 87% rename from internal/lint/types_test.go rename to pkg/lint/types_test.go index 0c0b69506..4b8d2457b 100644 --- a/internal/lint/types_test.go +++ b/pkg/lint/types_test.go @@ -8,8 +8,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/upsun/cli/internal/lint" - "github.com/upsun/cli/internal/lint/registry" + "github.com/upsun/cli/pkg/lint" + "github.com/upsun/cli/pkg/registry" ) //go:embed testdata/registry.json @@ -54,7 +54,7 @@ applications: services: database: type: mariadb:11.4`, - expectErrorMessage: "linter errors:\n - applications.foo.type: version '5.3' is not supported for type 'php'; " + //nolint:lll + expectErrorMessage: "Linter errors:\n - applications.foo.type: version '5.3' is not supported for type 'php'; " + //nolint:lll "it must be exactly one of: 8.4, 8.3, 8.2, 8.1", }, { @@ -63,7 +63,7 @@ services: applications: foo: type: strapi:latest`, - expectErrorMessage: "linter errors:\n - applications.foo.type: type not found: 'strapi'; it must be one of: " + + expectErrorMessage: "Linter errors:\n - applications.foo.type: type not found: 'strapi'; it must be one of: " + "composable, dotnet, elixir, golang, java, nodejs, php, python, ruby, rust " + "(check the Registry for supported types, or make an application using a composable image)", }, @@ -76,7 +76,7 @@ applications: services: myservice: type: nodejs:22`, - expectErrorMessage: "linter errors:\n - services.myservice.type: type 'nodejs' is a runtime type, not a service type", //nolint:lll + expectErrorMessage: "Linter errors:\n - services.myservice.type: type 'nodejs' is a runtime type, not a service type", //nolint:lll }, { name: "composable_without_stack", @@ -84,7 +84,7 @@ services: applications: foo: type: composable:%s`, registry.ChannelStable), - expectErrorMessage: `linter warnings: + expectErrorMessage: `Linter warnings: - applications.foo: 'stack' should be specified when using a composable image`, }, { @@ -101,7 +101,7 @@ tasks: base: performance-agent bad: type: php:1.0`, - expectErrorMessage: `linter errors: + expectErrorMessage: `Linter errors: - tasks.bad.type: version '1.0' is not supported for type 'php'; it must be exactly one of: 8.4, 8.3, 8.2, 8.1`, }, { @@ -113,9 +113,9 @@ applications: tasks: bad: stack: {runtimes: ["php@8.4"]}`, - expectErrorMessage: `linter errors: + expectErrorMessage: `Linter errors: - tasks.bad.type: type cannot be empty -linter warnings: +Linter warnings: - tasks.bad.stack: 'stack' is only used with a composable image type`, }, { @@ -125,7 +125,7 @@ applications: foo: type: php:8.4 stack: {runtimes: ["php@8.4"]}`, - expectErrorMessage: `linter warnings: + expectErrorMessage: `Linter warnings: - applications.foo.stack: 'stack' is only used with a composable image type`, }, } @@ -139,7 +139,7 @@ applications: result := lint.CheckTypes(cfg, testRegistry) if c.expectErrorMessage != "" { assert.True(t, result.HasErrors() || result.HasWarnings()) - assert.Equal(t, c.expectErrorMessage, result.Error()) + assert.Equal(t, c.expectErrorMessage, result.String()) } else { assert.False(t, result.HasErrors()) } @@ -162,8 +162,8 @@ applications: result := lint.CheckTypes(cfg, reg) assert.False(t, result.HasErrors()) - assert.Equal(t, `linter warnings: - - applications.foo.type: version '1.25' of type 'golang' is retired; use one of: 1.27, 1.26`, result.Error()) + assert.Equal(t, `Linter warnings: + - applications.foo.type: version '1.25' of type 'golang' is retired; use one of: 1.27, 1.26`, result.String()) } func TestCheckTypes_NoSupportedVersions(t *testing.T) { diff --git a/internal/lint/web.go b/pkg/lint/web.go similarity index 85% rename from internal/lint/web.go rename to pkg/lint/web.go index 99c27e440..bf0a14819 100644 --- a/internal/lint/web.go +++ b/pkg/lint/web.go @@ -33,7 +33,7 @@ func validateLocationPath(path string) string { return "" } -func CheckWebConfig(cfg *Config) *Result { +func checkWebConfig(cfg *config) *Result { result := &Result{} for appName := range cfg.Applications { @@ -42,7 +42,7 @@ func CheckWebConfig(cfg *Config) *Result { // Validate location key format. locKeyPath := "applications." + appName + ".web.locations" if errMsg := validateLocationPath(locName); errMsg != "" { - result.AddError(locKeyPath, fmt.Sprintf("location key %q %s", locName, errMsg)) + result.addError(locKeyPath, fmt.Sprintf("location key %q %s", locName, errMsg)) continue } @@ -52,10 +52,10 @@ func CheckWebConfig(cfg *Config) *Result { if loc.Root != "" { // Check for leading slash (not allowed) if loc.Root == "/" { - result.AddError(path, "the root cannot begin with a slash (use an empty string instead)") + result.addError(path, "the root cannot begin with a slash (use an empty string instead)") continue } else if strings.HasPrefix(loc.Root, "/") { - result.AddError(path, "the root cannot begin with a slash") + result.addError(path, "the root cannot begin with a slash") continue } @@ -64,7 +64,7 @@ func CheckWebConfig(cfg *Config) *Result { for part := range parts { if part == "" || part == "." || part == ".." { path := "applications." + appName + ".web.locations[\"" + locName + "\"].root" - result.AddError(path, "the root path cannot include empty parts, '.' or '..'") + result.addError(path, "the root path cannot include empty parts, '.' or '..'") break } } @@ -74,7 +74,7 @@ func CheckWebConfig(cfg *Config) *Result { for rulePattern := range loc.Rules { if rulePattern == "" { path := "applications." + appName + ".web.locations[\"" + locName + "\"].rules" - result.AddError(path, "rule pattern cannot be empty") + result.addError(path, "rule pattern cannot be empty") continue } @@ -83,7 +83,7 @@ func CheckWebConfig(cfg *Config) *Result { _, err := regexp2.Compile(rulePattern, regexp2.RE2) if err != nil { path := "applications." + appName + ".web.locations[\"" + locName + "\"].rules" - result.AddError(path, "invalid regular expression: "+err.Error()) + result.addError(path, "invalid regular expression: "+err.Error()) } } } diff --git a/internal/lint/web_test.go b/pkg/lint/web_test.go similarity index 85% rename from internal/lint/web_test.go rename to pkg/lint/web_test.go index 2cf0a5652..2cdef5e48 100644 --- a/internal/lint/web_test.go +++ b/pkg/lint/web_test.go @@ -3,7 +3,7 @@ package lint_test import ( "testing" - "github.com/upsun/cli/internal/lint" + "github.com/upsun/cli/pkg/lint" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -42,7 +42,7 @@ applications: root: "public" `, wantErr: true, - errMatch: "linter errors:\n - applications.app1.web.locations: location key \"api\" must start with a slash", + errMatch: "Linter errors:\n - applications.app1.web.locations: location key \"api\" must start with a slash", }, { name: "location key with empty segment", @@ -57,7 +57,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.app1.web.locations: location key \"/api//v1\" cannot include empty parts, '.' or '..'", + errMatch: "Linter errors:\n - applications.app1.web.locations: location key \"/api//v1\" cannot include empty parts, '.' or '..'", }, { name: "location key with dot segment", @@ -72,7 +72,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.app1.web.locations: location key \"/api/./v1\" cannot include empty parts, '.' or '..'", + errMatch: "Linter errors:\n - applications.app1.web.locations: location key \"/api/./v1\" cannot include empty parts, '.' or '..'", }, { name: "location key with dotdot segment", @@ -87,7 +87,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.app1.web.locations: location key \"/api/../v1\" cannot include empty parts, '.' or '..'", + errMatch: "Linter errors:\n - applications.app1.web.locations: location key \"/api/../v1\" cannot include empty parts, '.' or '..'", }, { name: "location key with regex pattern from issue AI-105", @@ -104,7 +104,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.testdemo.web.locations: location key \"^/(api|health)\" must start with a slash", + errMatch: "Linter errors:\n - applications.testdemo.web.locations: location key \"^/(api|health)\" must start with a slash", }, { name: "location key with query string character", @@ -119,7 +119,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.app1.web.locations: location key \"/api?version=1\" contains invalid characters", + errMatch: "Linter errors:\n - applications.app1.web.locations: location key \"/api?version=1\" contains invalid characters", }, { name: "location key with fragment character", @@ -134,7 +134,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.app1.web.locations: location key \"/page#section\" contains invalid characters", + errMatch: "Linter errors:\n - applications.app1.web.locations: location key \"/page#section\" contains invalid characters", }, { name: "valid location keys", @@ -182,7 +182,7 @@ applications: root: "/public" `, wantErr: true, - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", }, { name: "invalid root with leading slash and trailing slash", @@ -196,7 +196,7 @@ applications: root: "/public/" `, wantErr: true, - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", }, { name: "empty root", @@ -235,7 +235,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash (use an empty string instead)", + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash (use an empty string instead)", }, { name: "invalid root with leading slash and empty part", @@ -249,7 +249,7 @@ applications: root: "/foo//bar" `, wantErr: true, - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", }, { name: "invalid root with empty part (no leading slash)", @@ -264,7 +264,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].root: the root path cannot include empty parts, '.' or '..'", + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].root: the root path cannot include empty parts, '.' or '..'", }, { name: "invalid root with leading slash and dot", @@ -278,7 +278,7 @@ applications: root: "/foo/./bar" `, wantErr: true, - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", }, { name: "invalid root with dot (no leading slash)", @@ -293,7 +293,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].root: the root path cannot include empty parts, '.' or '..'", + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].root: the root path cannot include empty parts, '.' or '..'", }, { name: "invalid root with leading slash and dotdot", @@ -307,7 +307,7 @@ applications: root: "/foo/../bar" `, wantErr: true, - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", }, { name: "invalid root with dotdot (no leading slash)", @@ -322,7 +322,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].root: the root path cannot include empty parts, '.' or '..'", + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].root: the root path cannot include empty parts, '.' or '..'", }, { name: "multiple failing locations with leading slashes", @@ -339,7 +339,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.app1.web.locations[\"/bar\"].root: the root cannot begin with a slash\n" + + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/bar\"].root: the root cannot begin with a slash\n" + " - applications.app1.web.locations[\"/foo\"].root: the root cannot begin with a slash", }, { @@ -361,7 +361,7 @@ applications: `, wantErr: true, //nolint:lll - errMatch: "linter errors:\n - applications.app1.web.locations[\"/foo\"].root: the root cannot begin with a slash\n" + + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/foo\"].root: the root cannot begin with a slash\n" + " - applications.app2.web.locations[\"/bar\"].root: the root cannot begin with a slash", }, { @@ -398,7 +398,7 @@ applications: allow: false `, wantErr: true, - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].rules: " + + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].rules: " + "invalid regular expression: error parsing regexp: missing closing ) in `^/api/(unclosed`", }, { @@ -416,7 +416,7 @@ applications: allow: false `, wantErr: true, - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].rules: " + + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].rules: " + "invalid regular expression: error parsing regexp: insufficient hexadecimal digits in `\\x`", }, { @@ -434,7 +434,7 @@ applications: allow: false `, wantErr: true, - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].rules: rule pattern cannot be empty", + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].rules: rule pattern cannot be empty", }, { name: "multiple invalid regex rules", @@ -453,7 +453,7 @@ applications: cache: true `, wantErr: true, - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].rules: " + + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].rules: " + "invalid regular expression: error parsing regexp: missing closing ) in `^/api/(unclosed`\n" + " - applications.app1.web.locations[\"/\"].rules: " + "invalid regular expression: error parsing regexp: unterminated [] set in `[invalid`", @@ -473,7 +473,7 @@ applications: allow: false `, wantErr: true, - errMatch: "linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", + errMatch: "Linter errors:\n - applications.app1.web.locations[\"/\"].root: the root cannot begin with a slash", }, } @@ -484,7 +484,7 @@ applications: result := lint.CheckWebConfig(cfg) if c.wantErr { require.True(t, result.HasErrors()) - assert.Equal(t, c.errMatch, result.Error()) + assert.Equal(t, c.errMatch, result.String()) } else { assert.False(t, result.HasErrors()) } diff --git a/internal/lint/yaml.go b/pkg/lint/yaml.go similarity index 80% rename from internal/lint/yaml.go rename to pkg/lint/yaml.go index 148fd6d25..fc930b193 100644 --- a/internal/lint/yaml.go +++ b/pkg/lint/yaml.go @@ -8,25 +8,25 @@ import ( "gopkg.in/yaml.v3" ) -// CheckYAMLSchema checks that YAML content matches a JSON schema. -func CheckYAMLSchema(content string, schema *gojsonschema.Schema) *Result { +// checkYAMLSchema checks that YAML content matches a JSON schema. +func checkYAMLSchema(content string, schema *gojsonschema.Schema) *Result { var data = make(map[string]any) if err := yaml.Unmarshal([]byte(content), &data); err != nil { result := &Result{} - result.AddError("", interpretYAMLError(err)) + result.addError("", interpretYAMLError(err)) return result } - return CheckSchemaAt(data, schema, "") + return checkSchemaAt(data, schema, "") } -// CheckSchemaAt validates already-parsed data against a JSON schema, prefixing +// checkSchemaAt validates already-parsed data against a JSON schema, prefixing // each issue's path with pathPrefix (e.g. "applications.app"). -func CheckSchemaAt(data any, schema *gojsonschema.Schema, pathPrefix string) *Result { +func checkSchemaAt(data any, schema *gojsonschema.Schema, pathPrefix string) *Result { result := &Result{} schemaResult, err := schema.Validate(gojsonschema.NewGoLoader(data)) if err != nil { - result.AddError(pathPrefix, err.Error()) + result.addError(pathPrefix, err.Error()) return result } errs := schemaResult.Errors() @@ -47,7 +47,7 @@ func CheckSchemaAt(data any, schema *gojsonschema.Schema, pathPrefix string) *Re default: field = pathPrefix + "." + field } - result.AddError(field, msg) + result.addError(field, msg) } return result diff --git a/internal/lint/yaml_test.go b/pkg/lint/yaml_test.go similarity index 90% rename from internal/lint/yaml_test.go rename to pkg/lint/yaml_test.go index 094669a41..5252c88e9 100644 --- a/internal/lint/yaml_test.go +++ b/pkg/lint/yaml_test.go @@ -7,7 +7,7 @@ import ( "github.com/stretchr/testify/require" "github.com/xeipuuv/gojsonschema" - "github.com/upsun/cli/internal/lint" + "github.com/upsun/cli/pkg/lint" ) func TestCheckYAMLSchema(t *testing.T) { @@ -30,7 +30,7 @@ list: name: "valid YAML but not matching schema", content: `invalidKey: someValue`, wantErr: true, - errorMsg: "linter errors:", + errorMsg: "Linter errors:", }, { name: "invalid YAML malformed syntax", @@ -46,7 +46,7 @@ list: { name: "empty YAML content", wantErr: true, - errorMsg: "linter errors:", + errorMsg: "Linter errors:", }, { name: "YAML with invalid types against schema", @@ -56,7 +56,7 @@ list: - item1 - item2`, wantErr: true, - errorMsg: "linter errors:", + errorMsg: "Linter errors:", }, } @@ -68,7 +68,7 @@ list: if c.wantErr { assert.True(t, result.HasErrors()) if c.errorMsg != "" { - assert.Contains(t, result.Error(), c.errorMsg) + assert.Contains(t, result.String(), c.errorMsg) } } else { assert.False(t, result.HasErrors()) diff --git a/pkg/registry/doc.go b/pkg/registry/doc.go new file mode 100644 index 000000000..57349ea3f --- /dev/null +++ b/pkg/registry/doc.go @@ -0,0 +1,5 @@ +// Package registry provides the embedded Upsun image registry (runtime and service types and versions). +// +// Other Upsun projects import this package. It is not covered by semantic versioning: the module has no /v5 +// path, so importers use pseudo-versions, and the API may change in any commit. +package registry diff --git a/internal/lint/registry/gen.go b/pkg/registry/gen.go similarity index 95% rename from internal/lint/registry/gen.go rename to pkg/registry/gen.go index c6da174c9..eed7c58fa 100644 --- a/internal/lint/registry/gen.go +++ b/pkg/registry/gen.go @@ -3,7 +3,7 @@ // Command gen fetches the container image registry from meta.upsun.com and // writes registry.json in the format consumed by the registry package. // -// Run it with: go run gen.go (or `go generate ./internal/lint/registry`). +// Run it with: go run gen.go (or `go generate ./pkg/registry`). package main import ( @@ -16,7 +16,7 @@ import ( "strings" "time" - "github.com/upsun/cli/internal/lint/registry" + "github.com/upsun/cli/pkg/registry" ) const imagesURL = "https://meta.upsun.com/images" diff --git a/pkg/registry/model.go b/pkg/registry/model.go new file mode 100644 index 000000000..21bcb86ff --- /dev/null +++ b/pkg/registry/model.go @@ -0,0 +1,59 @@ +package registry + +import ( + "encoding/json" + "slices" +) + +// Registry maps image types (e.g. "php", "postgresql") to their images. +type Registry map[string]Image + +// AllTypes returns the sorted image types: runtimes if runtime is true, otherwise services. +func (r Registry) AllTypes(runtime bool) []string { + types := make([]string, 0, len(r)) + for k := range r { + if r[k].IsRuntime == runtime { + types = append(types, k) + } + } + slices.Sort(types) + return types +} + +// Image describes a single service/runtime container image. +type Image struct { + Name string `json:"name" yaml:"name,omitempty"` + Type string `json:"type" yaml:"type"` + IsRuntime bool `json:"runtime" yaml:"is_runtime"` + Versions VersionInfo `json:"versions,omitzero" yaml:"versions,omitempty"` +} + +// VersionInfo lists deprecated, supported, and optional legacy and retired versions. +type VersionInfo struct { + Supported []string `json:"supported"` + + Deprecated []string `json:"deprecated,omitempty" yaml:"deprecated,omitempty"` + Legacy []string `json:"legacy,omitempty" yaml:"legacy,omitempty"` + Retired []string `json:"retired,omitempty" yaml:"retired,omitempty"` +} + +// UnmarshalJSON handles both string and object formats for versions field. +// Some registry entries have versions as a string (e.g. "25.05") instead of an object. +func (v *VersionInfo) UnmarshalJSON(data []byte) error { + // Try unmarshaling as a string first. + var str string + if err := json.Unmarshal(data, &str); err == nil { + // It's a string, treat it as a single supported version. + v.Supported = []string{str} + return nil + } + + // Try unmarshaling as the normal object structure. + type versionInfoAlias VersionInfo + var obj versionInfoAlias + if err := json.Unmarshal(data, &obj); err != nil { + return err + } + *v = VersionInfo(obj) + return nil +} diff --git a/internal/lint/registry/model_test.go b/pkg/registry/model_test.go similarity index 95% rename from internal/lint/registry/model_test.go rename to pkg/registry/model_test.go index 435903290..d96c1e9df 100644 --- a/internal/lint/registry/model_test.go +++ b/pkg/registry/model_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/upsun/cli/internal/lint/registry" + "github.com/upsun/cli/pkg/registry" ) func TestVersionInfo_UnmarshalJSON(t *testing.T) { diff --git a/internal/lint/registry/registry.go b/pkg/registry/registry.go similarity index 90% rename from internal/lint/registry/registry.go rename to pkg/registry/registry.go index 8716d217a..9ccbf3e50 100644 --- a/internal/lint/registry/registry.go +++ b/pkg/registry/registry.go @@ -13,7 +13,7 @@ const ChannelStable = "25.11" // //go:generate go run gen.go //go:embed registry.json -var Data []byte +var data []byte var ( parsedRegistry Registry @@ -21,6 +21,7 @@ var ( parsedOnce sync.Once ) +// Parse decodes a registry from JSON in the format of registry.json. func Parse(b []byte) (reg Registry, err error) { err = json.Unmarshal(b, ®) if err == nil { @@ -29,9 +30,11 @@ func Parse(b []byte) (reg Registry, err error) { return } -func Parsed() (Registry, error) { +// Load returns the embedded registry, parsed once and cached. The returned map +// is shared between callers, so it must not be modified. +func Load() (Registry, error) { parsedOnce.Do(func() { - parsedRegistry, parsedErr = Parse(Data) + parsedRegistry, parsedErr = Parse(data) }) return parsedRegistry, parsedErr } @@ -42,8 +45,6 @@ func clean(reg Registry) { img := reg[k] // Remove deprecated version info. img.Versions.Deprecated = nil - // Remove descriptions. - img.Description = "" reg[k] = img } diff --git a/internal/lint/registry/registry.json b/pkg/registry/registry.json similarity index 100% rename from internal/lint/registry/registry.json rename to pkg/registry/registry.json diff --git a/internal/lint/registry/registry_test.go b/pkg/registry/registry_test.go similarity index 91% rename from internal/lint/registry/registry_test.go rename to pkg/registry/registry_test.go index db84e6f1e..310ea679d 100644 --- a/internal/lint/registry/registry_test.go +++ b/pkg/registry/registry_test.go @@ -5,12 +5,12 @@ import ( "github.com/stretchr/testify/assert" - "github.com/upsun/cli/internal/lint/registry" + "github.com/upsun/cli/pkg/registry" ) // Basic test to ensure that the registry can be parsed. func TestRegistry(t *testing.T) { - reg, err := registry.Parsed() + reg, err := registry.Load() assert.NoError(t, err) assert.NotEmpty(t, reg) assert.Contains(t, reg, "golang") @@ -24,7 +24,7 @@ func TestRegistry(t *testing.T) { // The persistent variants are aliases added by clean(), tracking the versions // of the type they copy. func TestRegistry_PersistentAliases(t *testing.T) { - reg, err := registry.Parsed() + reg, err := registry.Load() assert.NoError(t, err) for _, c := range []struct{ base, alias string }{ @@ -42,7 +42,7 @@ func TestRegistry_PersistentAliases(t *testing.T) { // Replica images are listed upstream without versions of their own, so clean() // takes them from the base type. Without this they reject every version. func TestRegistry_ReplicaVersions(t *testing.T) { - reg, err := registry.Parsed() + reg, err := registry.Load() assert.NoError(t, err) for _, base := range []string{"mariadb", "postgresql"} {