From 3018aed2bfe272ca42a476f75d52b3570867074c Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Sun, 18 Oct 2026 10:34:20 +0100 Subject: [PATCH 1/7] refactor(lint): move lint package to pkg/lint for reuse Move internal/lint, with its registry and schema subpackages, test data and generators, to pkg/lint so that other Go modules can import the config linter. Update imports, Makefile targets and CLAUDE.md. Add a package doc comment stating that the API has no stability guarantee. No behavior changes. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- Makefile | 10 +++++----- commands/lint.go | 2 +- commands/lint_test.go | 2 +- {internal => pkg}/lint/config_schema.go | 0 {internal => pkg}/lint/dependencies.go | 0 {internal => pkg}/lint/dependencies_test.go | 0 pkg/lint/doc.go | 4 ++++ {internal => pkg}/lint/fixed.go | 2 +- {internal => pkg}/lint/fixed_test.go | 0 {internal => pkg}/lint/linter.go | 4 ++-- {internal => pkg}/lint/linter_test.go | 0 {internal => pkg}/lint/merge.go | 0 {internal => pkg}/lint/merge_test.go | 0 {internal => pkg}/lint/names.go | 0 {internal => pkg}/lint/names_test.go | 2 +- {internal => pkg}/lint/normalize.go | 0 {internal => pkg}/lint/normalize_test.go | 0 {internal => pkg}/lint/registry/gen.go | 4 ++-- {internal => pkg}/lint/registry/model.go | 0 {internal => pkg}/lint/registry/model_test.go | 2 +- {internal => pkg}/lint/registry/registry.go | 0 {internal => pkg}/lint/registry/registry.json | 0 {internal => pkg}/lint/registry/registry_test.go | 2 +- {internal => pkg}/lint/relationships.go | 0 {internal => pkg}/lint/relationships_test.go | 2 +- {internal => pkg}/lint/result.go | 0 {internal => pkg}/lint/routes.go | 0 {internal => pkg}/lint/routes_test.go | 2 +- {internal => pkg}/lint/schema/fixed-overlay.json | 0 {internal => pkg}/lint/schema/fixed.go | 0 {internal => pkg}/lint/schema/gen.go | 2 +- {internal => pkg}/lint/schema/schema.go | 0 {internal => pkg}/lint/schema/schema_test.go | 0 {internal => pkg}/lint/schema/upsun-config-schema.json | 0 {internal => pkg}/lint/scripts.go | 0 {internal => pkg}/lint/scripts_test.go | 2 +- {internal => pkg}/lint/source.go | 0 {internal => pkg}/lint/source_test.go | 0 {internal => pkg}/lint/tasks.go | 0 {internal => pkg}/lint/tasks_test.go | 2 +- {internal => pkg}/lint/testdata/registry.json | 0 {internal => pkg}/lint/types.go | 2 +- {internal => pkg}/lint/types_test.go | 4 ++-- {internal => pkg}/lint/web.go | 0 {internal => pkg}/lint/web_test.go | 2 +- {internal => pkg}/lint/yaml.go | 0 {internal => pkg}/lint/yaml_test.go | 2 +- 48 files changed, 30 insertions(+), 26 deletions(-) rename {internal => pkg}/lint/config_schema.go (100%) rename {internal => pkg}/lint/dependencies.go (100%) rename {internal => pkg}/lint/dependencies_test.go (100%) create mode 100644 pkg/lint/doc.go rename {internal => pkg}/lint/fixed.go (99%) rename {internal => pkg}/lint/fixed_test.go (100%) rename {internal => pkg}/lint/linter.go (92%) rename {internal => pkg}/lint/linter_test.go (100%) rename {internal => pkg}/lint/merge.go (100%) rename {internal => pkg}/lint/merge_test.go (100%) rename {internal => pkg}/lint/names.go (100%) rename {internal => pkg}/lint/names_test.go (99%) rename {internal => pkg}/lint/normalize.go (100%) rename {internal => pkg}/lint/normalize_test.go (100%) rename {internal => pkg}/lint/registry/gen.go (95%) rename {internal => pkg}/lint/registry/model.go (100%) rename {internal => pkg}/lint/registry/model_test.go (95%) rename {internal => pkg}/lint/registry/registry.go (100%) rename {internal => pkg}/lint/registry/registry.json (100%) rename {internal => pkg}/lint/registry/registry_test.go (97%) rename {internal => pkg}/lint/relationships.go (100%) rename {internal => pkg}/lint/relationships_test.go (99%) rename {internal => pkg}/lint/result.go (100%) rename {internal => pkg}/lint/routes.go (100%) rename {internal => pkg}/lint/routes_test.go (99%) rename {internal => pkg}/lint/schema/fixed-overlay.json (100%) rename {internal => pkg}/lint/schema/fixed.go (100%) rename {internal => pkg}/lint/schema/gen.go (92%) rename {internal => pkg}/lint/schema/schema.go (100%) rename {internal => pkg}/lint/schema/schema_test.go (100%) rename {internal => pkg}/lint/schema/upsun-config-schema.json (100%) rename {internal => pkg}/lint/scripts.go (100%) rename {internal => pkg}/lint/scripts_test.go (99%) rename {internal => pkg}/lint/source.go (100%) rename {internal => pkg}/lint/source_test.go (100%) rename {internal => pkg}/lint/tasks.go (100%) rename {internal => pkg}/lint/tasks_test.go (99%) rename {internal => pkg}/lint/testdata/registry.json (100%) rename {internal => pkg}/lint/types.go (98%) rename {internal => pkg}/lint/types_test.go (98%) rename {internal => pkg}/lint/web.go (100%) rename {internal => pkg}/lint/web_test.go (99%) rename {internal => pkg}/lint/yaml.go (100%) rename {internal => pkg}/lint/yaml_test.go (97%) diff --git a/CLAUDE.md b/CLAUDE.md index 9b5b2d58a..f1d434628 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. `pkg/lint` is public because other Upsun projects import it; its API has no stability guarantee. 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 `pkg/lint/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..8abe8ecea 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/lint/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/lint/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, pkg/lint/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 pkg/lint/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 -- pkg/lint/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..a07d409f8 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. diff --git a/commands/lint_test.go b/commands/lint_test.go index 919b3b547..89cf9c8a8 100644 --- a/commands/lint_test.go +++ b/commands/lint_test.go @@ -15,7 +15,7 @@ import ( "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 diff --git a/internal/lint/config_schema.go b/pkg/lint/config_schema.go similarity index 100% rename from internal/lint/config_schema.go rename to pkg/lint/config_schema.go diff --git a/internal/lint/dependencies.go b/pkg/lint/dependencies.go similarity index 100% rename from internal/lint/dependencies.go rename to pkg/lint/dependencies.go diff --git a/internal/lint/dependencies_test.go b/pkg/lint/dependencies_test.go similarity index 100% rename from internal/lint/dependencies_test.go rename to pkg/lint/dependencies_test.go diff --git a/pkg/lint/doc.go b/pkg/lint/doc.go new file mode 100644 index 000000000..70a379ea0 --- /dev/null +++ b/pkg/lint/doc.go @@ -0,0 +1,4 @@ +// Package lint validates Upsun configuration (Flex and Fixed). +// +// It is also imported by other Upsun projects. Its API has no stability guarantee and may change in any release. +package lint diff --git a/internal/lint/fixed.go b/pkg/lint/fixed.go similarity index 99% rename from internal/lint/fixed.go rename to pkg/lint/fixed.go index bcee5df3e..b7b273d89 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/pkg/lint/schema" ) // Configuration section keys. diff --git a/internal/lint/fixed_test.go b/pkg/lint/fixed_test.go similarity index 100% rename from internal/lint/fixed_test.go rename to pkg/lint/fixed_test.go diff --git a/internal/lint/linter.go b/pkg/lint/linter.go similarity index 92% rename from internal/lint/linter.go rename to pkg/lint/linter.go index 27e09cd05..b1bd9ce1b 100644 --- a/internal/lint/linter.go +++ b/pkg/lint/linter.go @@ -4,8 +4,8 @@ import ( "errors" "fmt" - "github.com/upsun/cli/internal/lint/registry" - "github.com/upsun/cli/internal/lint/schema" + "github.com/upsun/cli/pkg/lint/registry" + "github.com/upsun/cli/pkg/lint/schema" ) var ErrEmptyContent = errors.New("empty content") diff --git a/internal/lint/linter_test.go b/pkg/lint/linter_test.go similarity index 100% rename from internal/lint/linter_test.go rename to pkg/lint/linter_test.go 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 100% rename from internal/lint/names.go rename to pkg/lint/names.go diff --git a/internal/lint/names_test.go b/pkg/lint/names_test.go similarity index 99% rename from internal/lint/names_test.go rename to pkg/lint/names_test.go index 980304c37..0bd888656 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) { diff --git a/internal/lint/normalize.go b/pkg/lint/normalize.go similarity index 100% rename from internal/lint/normalize.go rename to pkg/lint/normalize.go diff --git a/internal/lint/normalize_test.go b/pkg/lint/normalize_test.go similarity index 100% rename from internal/lint/normalize_test.go rename to pkg/lint/normalize_test.go diff --git a/internal/lint/registry/gen.go b/pkg/lint/registry/gen.go similarity index 95% rename from internal/lint/registry/gen.go rename to pkg/lint/registry/gen.go index c6da174c9..2abd6f927 100644 --- a/internal/lint/registry/gen.go +++ b/pkg/lint/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/lint/registry`). package main import ( @@ -16,7 +16,7 @@ import ( "strings" "time" - "github.com/upsun/cli/internal/lint/registry" + "github.com/upsun/cli/pkg/lint/registry" ) const imagesURL = "https://meta.upsun.com/images" diff --git a/internal/lint/registry/model.go b/pkg/lint/registry/model.go similarity index 100% rename from internal/lint/registry/model.go rename to pkg/lint/registry/model.go diff --git a/internal/lint/registry/model_test.go b/pkg/lint/registry/model_test.go similarity index 95% rename from internal/lint/registry/model_test.go rename to pkg/lint/registry/model_test.go index 435903290..2ce27ff70 100644 --- a/internal/lint/registry/model_test.go +++ b/pkg/lint/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/lint/registry" ) func TestVersionInfo_UnmarshalJSON(t *testing.T) { diff --git a/internal/lint/registry/registry.go b/pkg/lint/registry/registry.go similarity index 100% rename from internal/lint/registry/registry.go rename to pkg/lint/registry/registry.go diff --git a/internal/lint/registry/registry.json b/pkg/lint/registry/registry.json similarity index 100% rename from internal/lint/registry/registry.json rename to pkg/lint/registry/registry.json diff --git a/internal/lint/registry/registry_test.go b/pkg/lint/registry/registry_test.go similarity index 97% rename from internal/lint/registry/registry_test.go rename to pkg/lint/registry/registry_test.go index db84e6f1e..99a62c17e 100644 --- a/internal/lint/registry/registry_test.go +++ b/pkg/lint/registry/registry_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/upsun/cli/internal/lint/registry" + "github.com/upsun/cli/pkg/lint/registry" ) // Basic test to ensure that the registry can be parsed. diff --git a/internal/lint/relationships.go b/pkg/lint/relationships.go similarity index 100% rename from internal/lint/relationships.go rename to pkg/lint/relationships.go diff --git a/internal/lint/relationships_test.go b/pkg/lint/relationships_test.go similarity index 99% rename from internal/lint/relationships_test.go rename to pkg/lint/relationships_test.go index 420c7775c..3d8fca13d 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 diff --git a/internal/lint/result.go b/pkg/lint/result.go similarity index 100% rename from internal/lint/result.go rename to pkg/lint/result.go diff --git a/internal/lint/routes.go b/pkg/lint/routes.go similarity index 100% rename from internal/lint/routes.go rename to pkg/lint/routes.go diff --git a/internal/lint/routes_test.go b/pkg/lint/routes_test.go similarity index 99% rename from internal/lint/routes_test.go rename to pkg/lint/routes_test.go index ccfc38d77..530308756 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) { diff --git a/internal/lint/schema/fixed-overlay.json b/pkg/lint/schema/fixed-overlay.json similarity index 100% rename from internal/lint/schema/fixed-overlay.json rename to pkg/lint/schema/fixed-overlay.json diff --git a/internal/lint/schema/fixed.go b/pkg/lint/schema/fixed.go similarity index 100% rename from internal/lint/schema/fixed.go rename to pkg/lint/schema/fixed.go diff --git a/internal/lint/schema/gen.go b/pkg/lint/schema/gen.go similarity index 92% rename from internal/lint/schema/gen.go rename to pkg/lint/schema/gen.go index a6965a21a..d46d572ed 100644 --- a/internal/lint/schema/gen.go +++ b/pkg/lint/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 ./pkg/lint/schema`). package main import ( diff --git a/internal/lint/schema/schema.go b/pkg/lint/schema/schema.go similarity index 100% rename from internal/lint/schema/schema.go rename to pkg/lint/schema/schema.go diff --git a/internal/lint/schema/schema_test.go b/pkg/lint/schema/schema_test.go similarity index 100% rename from internal/lint/schema/schema_test.go rename to pkg/lint/schema/schema_test.go diff --git a/internal/lint/schema/upsun-config-schema.json b/pkg/lint/schema/upsun-config-schema.json similarity index 100% rename from internal/lint/schema/upsun-config-schema.json rename to pkg/lint/schema/upsun-config-schema.json diff --git a/internal/lint/scripts.go b/pkg/lint/scripts.go similarity index 100% rename from internal/lint/scripts.go rename to pkg/lint/scripts.go diff --git a/internal/lint/scripts_test.go b/pkg/lint/scripts_test.go similarity index 99% rename from internal/lint/scripts_test.go rename to pkg/lint/scripts_test.go index 74b7a5782..f6bda27d7 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) { 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 100% rename from internal/lint/tasks.go rename to pkg/lint/tasks.go diff --git a/internal/lint/tasks_test.go b/pkg/lint/tasks_test.go similarity index 99% rename from internal/lint/tasks_test.go rename to pkg/lint/tasks_test.go index 72abc3b56..dbfa04601 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 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 98% rename from internal/lint/types.go rename to pkg/lint/types.go index 023ec0f36..8a7800452 100644 --- a/internal/lint/types.go +++ b/pkg/lint/types.go @@ -5,7 +5,7 @@ import ( "slices" "strings" - "github.com/upsun/cli/internal/lint/registry" + "github.com/upsun/cli/pkg/lint/registry" ) // CheckTypes checks that application, worker, task and service types are supported images and versions. diff --git a/internal/lint/types_test.go b/pkg/lint/types_test.go similarity index 98% rename from internal/lint/types_test.go rename to pkg/lint/types_test.go index 0c0b69506..f97d867e4 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/lint/registry" ) //go:embed testdata/registry.json diff --git a/internal/lint/web.go b/pkg/lint/web.go similarity index 100% rename from internal/lint/web.go rename to pkg/lint/web.go diff --git a/internal/lint/web_test.go b/pkg/lint/web_test.go similarity index 99% rename from internal/lint/web_test.go rename to pkg/lint/web_test.go index 2cf0a5652..6631c7149 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" diff --git a/internal/lint/yaml.go b/pkg/lint/yaml.go similarity index 100% rename from internal/lint/yaml.go rename to pkg/lint/yaml.go diff --git a/internal/lint/yaml_test.go b/pkg/lint/yaml_test.go similarity index 97% rename from internal/lint/yaml_test.go rename to pkg/lint/yaml_test.go index 094669a41..920ffde3e 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) { From 766e99bd3e3b31664bb9850144e470493f7e3547 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Sun, 18 Oct 2026 10:54:14 +0100 Subject: [PATCH 2/7] refactor(lint): move registry and schema to top-level packages Move pkg/lint/registry to pkg/registry and pkg/lint/schema to pkg/schema, since other projects use the image registry outside of linting. Add package docs to both, and reword the versioning note: the module has no /v5 path, so importers use pseudo-versions and the API is not covered by semantic versioning. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- Makefile | 10 +++++----- pkg/lint/doc.go | 3 ++- pkg/lint/fixed.go | 2 +- pkg/lint/linter.go | 4 ++-- pkg/lint/types.go | 2 +- pkg/lint/types_test.go | 2 +- pkg/registry/doc.go | 5 +++++ pkg/{lint => }/registry/gen.go | 4 ++-- pkg/{lint => }/registry/model.go | 0 pkg/{lint => }/registry/model_test.go | 2 +- pkg/{lint => }/registry/registry.go | 0 pkg/{lint => }/registry/registry.json | 0 pkg/{lint => }/registry/registry_test.go | 2 +- pkg/schema/doc.go | 5 +++++ pkg/{lint => }/schema/fixed-overlay.json | 0 pkg/{lint => }/schema/fixed.go | 0 pkg/{lint => }/schema/gen.go | 2 +- pkg/{lint => }/schema/schema.go | 0 pkg/{lint => }/schema/schema_test.go | 0 pkg/{lint => }/schema/upsun-config-schema.json | 0 21 files changed, 28 insertions(+), 17 deletions(-) create mode 100644 pkg/registry/doc.go rename pkg/{lint => }/registry/gen.go (96%) rename pkg/{lint => }/registry/model.go (100%) rename pkg/{lint => }/registry/model_test.go (96%) rename pkg/{lint => }/registry/registry.go (100%) rename pkg/{lint => }/registry/registry.json (100%) rename pkg/{lint => }/registry/registry_test.go (97%) create mode 100644 pkg/schema/doc.go rename pkg/{lint => }/schema/fixed-overlay.json (100%) rename pkg/{lint => }/schema/fixed.go (100%) rename pkg/{lint => }/schema/gen.go (93%) rename pkg/{lint => }/schema/schema.go (100%) rename pkg/{lint => }/schema/schema_test.go (100%) rename pkg/{lint => }/schema/upsun-config-schema.json (100%) diff --git a/CLAUDE.md b/CLAUDE.md index f1d434628..9fc838cf7 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 `pkg/lint`, reporting all errors at once. `pkg/lint` is public because other Upsun projects import it; its API has no stability guarantee. 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 `pkg/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. `pkg/lint`, `pkg/registry` and `pkg/schema` 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 `pkg/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 8abe8ecea..ef08ce374 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 pkg/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 -- pkg/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, pkg/lint/schema/fixed-overlay.json. +# Fixed config is checked with the same schema and an overlay, pkg/schema/fixed-overlay.json. .PHONY: update-schema update-schema: ## Refresh the embedded Flex config schema from meta.upsun.com - cd pkg/lint/schema && go run gen.go + cd pkg/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 -- pkg/lint/schema/upsun-config-schema.json + git diff --exit-code -- pkg/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/pkg/lint/doc.go b/pkg/lint/doc.go index 70a379ea0..9fd47339d 100644 --- a/pkg/lint/doc.go +++ b/pkg/lint/doc.go @@ -1,4 +1,5 @@ // Package lint validates Upsun configuration (Flex and Fixed). // -// It is also imported by other Upsun projects. Its API has no stability guarantee and may change in any release. +// 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/fixed.go b/pkg/lint/fixed.go index b7b273d89..f368b95dc 100644 --- a/pkg/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/pkg/lint/schema" + "github.com/upsun/cli/pkg/schema" ) // Configuration section keys. diff --git a/pkg/lint/linter.go b/pkg/lint/linter.go index b1bd9ce1b..0ece9f053 100644 --- a/pkg/lint/linter.go +++ b/pkg/lint/linter.go @@ -4,8 +4,8 @@ import ( "errors" "fmt" - "github.com/upsun/cli/pkg/lint/registry" - "github.com/upsun/cli/pkg/lint/schema" + "github.com/upsun/cli/pkg/registry" + "github.com/upsun/cli/pkg/schema" ) var ErrEmptyContent = errors.New("empty content") diff --git a/pkg/lint/types.go b/pkg/lint/types.go index 8a7800452..ca2dde530 100644 --- a/pkg/lint/types.go +++ b/pkg/lint/types.go @@ -5,7 +5,7 @@ import ( "slices" "strings" - "github.com/upsun/cli/pkg/lint/registry" + "github.com/upsun/cli/pkg/registry" ) // CheckTypes checks that application, worker, task and service types are supported images and versions. diff --git a/pkg/lint/types_test.go b/pkg/lint/types_test.go index f97d867e4..b8458d9a7 100644 --- a/pkg/lint/types_test.go +++ b/pkg/lint/types_test.go @@ -9,7 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/upsun/cli/pkg/lint" - "github.com/upsun/cli/pkg/lint/registry" + "github.com/upsun/cli/pkg/registry" ) //go:embed testdata/registry.json 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/pkg/lint/registry/gen.go b/pkg/registry/gen.go similarity index 96% rename from pkg/lint/registry/gen.go rename to pkg/registry/gen.go index 2abd6f927..eed7c58fa 100644 --- a/pkg/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 ./pkg/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/pkg/lint/registry" + "github.com/upsun/cli/pkg/registry" ) const imagesURL = "https://meta.upsun.com/images" diff --git a/pkg/lint/registry/model.go b/pkg/registry/model.go similarity index 100% rename from pkg/lint/registry/model.go rename to pkg/registry/model.go diff --git a/pkg/lint/registry/model_test.go b/pkg/registry/model_test.go similarity index 96% rename from pkg/lint/registry/model_test.go rename to pkg/registry/model_test.go index 2ce27ff70..d96c1e9df 100644 --- a/pkg/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/pkg/lint/registry" + "github.com/upsun/cli/pkg/registry" ) func TestVersionInfo_UnmarshalJSON(t *testing.T) { diff --git a/pkg/lint/registry/registry.go b/pkg/registry/registry.go similarity index 100% rename from pkg/lint/registry/registry.go rename to pkg/registry/registry.go diff --git a/pkg/lint/registry/registry.json b/pkg/registry/registry.json similarity index 100% rename from pkg/lint/registry/registry.json rename to pkg/registry/registry.json diff --git a/pkg/lint/registry/registry_test.go b/pkg/registry/registry_test.go similarity index 97% rename from pkg/lint/registry/registry_test.go rename to pkg/registry/registry_test.go index 99a62c17e..eb4da21d7 100644 --- a/pkg/lint/registry/registry_test.go +++ b/pkg/registry/registry_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/upsun/cli/pkg/lint/registry" + "github.com/upsun/cli/pkg/registry" ) // Basic test to ensure that the registry can be parsed. diff --git a/pkg/schema/doc.go b/pkg/schema/doc.go new file mode 100644 index 000000000..435956fd1 --- /dev/null +++ b/pkg/schema/doc.go @@ -0,0 +1,5 @@ +// Package schema provides the embedded JSON schemas for Upsun configuration (Flex, and Fixed via an overlay). +// +// 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 schema diff --git a/pkg/lint/schema/fixed-overlay.json b/pkg/schema/fixed-overlay.json similarity index 100% rename from pkg/lint/schema/fixed-overlay.json rename to pkg/schema/fixed-overlay.json diff --git a/pkg/lint/schema/fixed.go b/pkg/schema/fixed.go similarity index 100% rename from pkg/lint/schema/fixed.go rename to pkg/schema/fixed.go diff --git a/pkg/lint/schema/gen.go b/pkg/schema/gen.go similarity index 93% rename from pkg/lint/schema/gen.go rename to pkg/schema/gen.go index d46d572ed..07c76323a 100644 --- a/pkg/lint/schema/gen.go +++ b/pkg/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 ./pkg/lint/schema`). +// Run it with: go run gen.go (or `go generate ./pkg/schema`). package main import ( diff --git a/pkg/lint/schema/schema.go b/pkg/schema/schema.go similarity index 100% rename from pkg/lint/schema/schema.go rename to pkg/schema/schema.go diff --git a/pkg/lint/schema/schema_test.go b/pkg/schema/schema_test.go similarity index 100% rename from pkg/lint/schema/schema_test.go rename to pkg/schema/schema_test.go diff --git a/pkg/lint/schema/upsun-config-schema.json b/pkg/schema/upsun-config-schema.json similarity index 100% rename from pkg/lint/schema/upsun-config-schema.json rename to pkg/schema/upsun-config-schema.json From b37aafa093340805f33c95024cf3bc0f97913b03 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Sun, 18 Oct 2026 10:59:48 +0100 Subject: [PATCH 3/7] refactor(lint): shrink the exported API of pkg/lint Unexport the individual checks, Combine, CheckSchemaAt, Config, DecodeConfig and the config field types. Callers use CheckContent or CheckDir, and Result to add their own issues. The external tests reach the checks through export_test.go. Move the schema package to internal/schema, since only pkg/lint uses it. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- Makefile | 6 +-- internal/schema/doc.go | 2 + {pkg => internal}/schema/fixed-overlay.json | 0 {pkg => internal}/schema/fixed.go | 0 {pkg => internal}/schema/gen.go | 2 +- {pkg => internal}/schema/schema.go | 0 {pkg => internal}/schema/schema_test.go | 0 .../schema/upsun-config-schema.json | 0 pkg/lint/config_schema.go | 54 +++++++++---------- pkg/lint/dependencies.go | 4 +- pkg/lint/dependencies_test.go | 4 +- pkg/lint/export_test.go | 14 +++++ pkg/lint/fixed.go | 8 +-- pkg/lint/linter.go | 26 ++++----- pkg/lint/names.go | 4 +- pkg/lint/relationships.go | 6 +-- pkg/lint/result.go | 4 +- pkg/lint/routes.go | 4 +- pkg/lint/scripts.go | 2 +- pkg/lint/tasks.go | 8 +-- pkg/lint/types.go | 6 +-- pkg/lint/web.go | 2 +- pkg/lint/yaml.go | 10 ++-- pkg/schema/doc.go | 5 -- 25 files changed, 92 insertions(+), 81 deletions(-) create mode 100644 internal/schema/doc.go rename {pkg => internal}/schema/fixed-overlay.json (100%) rename {pkg => internal}/schema/fixed.go (100%) rename {pkg => internal}/schema/gen.go (93%) rename {pkg => internal}/schema/schema.go (100%) rename {pkg => internal}/schema/schema_test.go (100%) rename {pkg => internal}/schema/upsun-config-schema.json (100%) create mode 100644 pkg/lint/export_test.go delete mode 100644 pkg/schema/doc.go diff --git a/CLAUDE.md b/CLAUDE.md index 9fc838cf7..cc02ce331 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 `pkg/lint`, reporting all errors at once. `pkg/lint`, `pkg/registry` and `pkg/schema` 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 `pkg/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. `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 ef08ce374..6a6417154 100644 --- a/Makefile +++ b/Makefile @@ -244,15 +244,15 @@ check-registry: update-registry 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, pkg/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 pkg/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 -- pkg/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/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/pkg/schema/fixed-overlay.json b/internal/schema/fixed-overlay.json similarity index 100% rename from pkg/schema/fixed-overlay.json rename to internal/schema/fixed-overlay.json diff --git a/pkg/schema/fixed.go b/internal/schema/fixed.go similarity index 100% rename from pkg/schema/fixed.go rename to internal/schema/fixed.go diff --git a/pkg/schema/gen.go b/internal/schema/gen.go similarity index 93% rename from pkg/schema/gen.go rename to internal/schema/gen.go index 07c76323a..024eb3ae5 100644 --- a/pkg/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 ./pkg/schema`). +// Run it with: go run gen.go (or `go generate ./internal/schema`). package main import ( diff --git a/pkg/schema/schema.go b/internal/schema/schema.go similarity index 100% rename from pkg/schema/schema.go rename to internal/schema/schema.go diff --git a/pkg/schema/schema_test.go b/internal/schema/schema_test.go similarity index 100% rename from pkg/schema/schema_test.go rename to internal/schema/schema_test.go diff --git a/pkg/schema/upsun-config-schema.json b/internal/schema/upsun-config-schema.json similarity index 100% rename from pkg/schema/upsun-config-schema.json rename to internal/schema/upsun-config-schema.json diff --git a/pkg/lint/config_schema.go b/pkg/lint/config_schema.go index 7a37fbe16..7c032a0b2 100644 --- a/pkg/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/pkg/lint/dependencies.go b/pkg/lint/dependencies.go index 104d1468c..8781a98a0 100644 --- a/pkg/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{ diff --git a/pkg/lint/dependencies_test.go b/pkg/lint/dependencies_test.go index c7c982495..bef52f23f 100644 --- a/pkg/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/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/pkg/lint/fixed.go b/pkg/lint/fixed.go index f368b95dc..65d1028ad 100644 --- a/pkg/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/pkg/schema" + "github.com/upsun/cli/internal/schema" ) // Configuration section keys. @@ -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 } @@ -112,7 +112,7 @@ 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) } @@ -241,7 +241,7 @@ 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) return data, nil diff --git a/pkg/lint/linter.go b/pkg/lint/linter.go index 0ece9f053..465956433 100644 --- a/pkg/lint/linter.go +++ b/pkg/lint/linter.go @@ -4,8 +4,8 @@ import ( "errors" "fmt" + "github.com/upsun/cli/internal/schema" "github.com/upsun/cli/pkg/registry" - "github.com/upsun/cli/pkg/schema" ) var ErrEmptyContent = errors.New("empty content") @@ -22,11 +22,11 @@ func CheckContent(content string) (*Result, error) { } // Check YAML validity and schema compliance. - if result := CheckYAMLSchema(content, yamlSchema); result.HasErrors() { + if result := checkYAMLSchema(content, yamlSchema); result.HasErrors() { return result, nil } - cfg, err := DecodeConfig(content) + cfg, err := decodeConfig(content) if err != nil { return nil, err } @@ -35,20 +35,20 @@ func CheckContent(content string) (*Result, error) { } // runChecks runs the semantic checks over a decoded config. -func runChecks(cfg *Config) (*Result, error) { +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), + return combine( + checkRelationships(cfg), + checkNames(cfg), + checkTypes(cfg, reg), + checkScripts(cfg), + checkWebConfig(cfg), + checkDependencies(cfg), + checkRoutes(cfg), + checkTasks(cfg), ), nil } diff --git a/pkg/lint/names.go b/pkg/lint/names.go index a5f0bb34d..5de29b920 100644 --- a/pkg/lint/names.go +++ b/pkg/lint/names.go @@ -14,8 +14,8 @@ 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 { diff --git a/pkg/lint/relationships.go b/pkg/lint/relationships.go index dc2a3a62c..14901019f 100644 --- a/pkg/lint/relationships.go +++ b/pkg/lint/relationships.go @@ -5,10 +5,10 @@ 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. @@ -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{}{} diff --git a/pkg/lint/result.go b/pkg/lint/result.go index 7b6c01d9b..131366173 100644 --- a/pkg/lint/result.go +++ b/pkg/lint/result.go @@ -133,8 +133,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/pkg/lint/routes.go b/pkg/lint/routes.go index 0aea07a60..4782439c2 100644 --- a/pkg/lint/routes.go +++ b/pkg/lint/routes.go @@ -6,8 +6,8 @@ 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 diff --git a/pkg/lint/scripts.go b/pkg/lint/scripts.go index d92547da7..9526cdb1a 100644 --- a/pkg/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) diff --git a/pkg/lint/tasks.go b/pkg/lint/tasks.go index bc10af35d..698ce8db3 100644 --- a/pkg/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 { @@ -49,7 +49,7 @@ func CheckTasks(cfg *Config) *Result { } } - 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 { @@ -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/pkg/lint/types.go b/pkg/lint/types.go index ca2dde530..ef78c645a 100644 --- a/pkg/lint/types.go +++ b/pkg/lint/types.go @@ -8,8 +8,8 @@ import ( "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. @@ -23,7 +23,7 @@ func CheckTypes(cfg *Config, reg registry.Registry) *Result { } // 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 { diff --git a/pkg/lint/web.go b/pkg/lint/web.go index 99c27e440..1cd36d412 100644 --- a/pkg/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 { diff --git a/pkg/lint/yaml.go b/pkg/lint/yaml.go index 148fd6d25..f651e7ed2 100644 --- a/pkg/lint/yaml.go +++ b/pkg/lint/yaml.go @@ -8,20 +8,20 @@ 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)) 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)) diff --git a/pkg/schema/doc.go b/pkg/schema/doc.go deleted file mode 100644 index 435956fd1..000000000 --- a/pkg/schema/doc.go +++ /dev/null @@ -1,5 +0,0 @@ -// Package schema provides the embedded JSON schemas for Upsun configuration (Flex, and Fixed via an overlay). -// -// 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 schema From 57a4ca5d7efa7a5d6c7033a13678fdc5eaea459c Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Sun, 18 Oct 2026 11:09:17 +0100 Subject: [PATCH 4/7] refactor(lint): tidy the exported lint and registry APIs - Remove Result.Error, so a Result is not mistaken for an error; tests use String. - Return only the result and error from CheckDir, and unexport the configuration style. - Move FindProjectRoot to the lint command, its only caller. - Rename registry.Parsed to Load and unexport the embedded data. - Document ErrEmptyContent, CheckContent, Parse, Load, Registry and AllTypes, and reword other doc comments. Co-Authored-By: Claude Opus 5.5 --- commands/lint.go | 26 +++++++++- commands/lint_test.go | 23 +++++++++ pkg/lint/fixed_test.go | 6 +-- pkg/lint/linter.go | 7 ++- pkg/lint/linter_test.go | 38 +++++++-------- pkg/lint/names_test.go | 2 +- pkg/lint/normalize.go | 67 ++++++++++---------------- pkg/lint/normalize_test.go | 88 ++++++++++++++-------------------- pkg/lint/relationships_test.go | 20 ++++---- pkg/lint/result.go | 40 +++------------- pkg/lint/routes_test.go | 18 +++---- pkg/lint/scripts_test.go | 18 +++---- pkg/lint/tasks_test.go | 16 +++---- pkg/lint/types_test.go | 22 ++++----- pkg/lint/web_test.go | 48 +++++++++---------- pkg/lint/yaml_test.go | 8 ++-- pkg/registry/model.go | 4 +- pkg/registry/registry.go | 9 ++-- pkg/registry/registry_test.go | 6 +-- 19 files changed, 230 insertions(+), 236 deletions(-) diff --git a/commands/lint.go b/commands/lint.go index a07d409f8..947cf7f52 100644 --- a/commands/lint.go +++ b/commands/lint.go @@ -104,15 +104,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, vendor) 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 89cf9c8a8..101a00908 100644 --- a/commands/lint_test.go +++ b/commands/lint_test.go @@ -165,3 +165,26 @@ func TestPrintLintResult_ValidWithWarnings(t *testing.T) { ✓ 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)) + }) +} diff --git a/pkg/lint/fixed_test.go b/pkg/lint/fixed_test.go index c444fc750..f637322b1 100644 --- a/pkg/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, upsunVendor()) 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, upsunVendor()) 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 index 465956433..411e0fdc7 100644 --- a/pkg/lint/linter.go +++ b/pkg/lint/linter.go @@ -8,9 +8,12 @@ import ( "github.com/upsun/cli/pkg/registry" ) +// ErrEmptyContent is returned by CheckContent when the content is empty. var ErrEmptyContent = errors.New("empty content") -// CheckContent checks merged Flex-style configuration content and returns a Result. +// 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 @@ -36,7 +39,7 @@ func CheckContent(content string) (*Result, error) { // runChecks runs the semantic checks over a decoded config. func runChecks(cfg *config) (*Result, error) { - reg, err := registry.Parsed() + reg, err := registry.Load() if err != nil { return nil, fmt.Errorf("failed to load registry: %w", err) } diff --git a/pkg/lint/linter_test.go b/pkg/lint/linter_test.go index 82854a096..23cea30fd 100644 --- a/pkg/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/pkg/lint/names_test.go b/pkg/lint/names_test.go index 0bd888656..5ff083477 100644 --- a/pkg/lint/names_test.go +++ b/pkg/lint/names_test.go @@ -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 index 0fc4cbf35..b8a4e906f 100644 --- a/pkg/lint/normalize.go +++ b/pkg/lint/normalize.go @@ -9,15 +9,15 @@ import ( "strings" ) -// Style identifies which configuration style a project uses. -type Style int +// 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 + 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 @@ -72,10 +72,17 @@ func (v Vendor) candidates() (flexDirs []string, fixed []fixedNames) { 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) { +// CheckDir detects the configuration style in dir and lints it. Detection uses +// the vendor's name conventions and the files present: Flex wins when present, +// else Fixed. Configuration problems are reported in the Result; the error is +// for configuration that cannot be found, read or parsed. +func CheckDir(dir string, vendor Vendor) (*Result, error) { + result, _, err := checkDir(dir, vendor) + return result, err +} + +// checkDir is CheckDir, also returning the detected style. +func checkDir(dir string, vendor Vendor) (*Result, style, error) { flexDirs, fixedSet := vendor.candidates() flexDir, flexOK := detectFlex(dir, flexDirs) fixedCfg, fixedOK := detectFixed(dir, fixedSet) @@ -84,7 +91,7 @@ func CheckDir(dir string, vendor Vendor) (*Result, Style, error) { case flexOK: root, err := os.OpenRoot(dir) if err != nil { - return nil, StyleFlex, err + return nil, styleFlex, err } defer root.Close() // The root keeps !include tags from following symbolic links out of the project. @@ -95,10 +102,10 @@ func CheckDir(dir string, vendor Vendor) (*Result, Style, error) { case errors.As(err, &srcErr): result = &Result{Errors: []Issue{srcErr.issue()}} case err != nil: - return nil, StyleFlex, err + return nil, styleFlex, err default: if result, err = CheckContent(content); err != nil { - return nil, StyleFlex, err + return nil, styleFlex, err } sources.locate(result) } @@ -108,16 +115,16 @@ func CheckDir(dir string, vendor Vendor) (*Result, Style, error) { "both %s and %s configuration found; linting the %s (Flex) configuration", flexDir, fixedCfg.dir, flexDir)) } - return result, StyleFlex, nil + return result, styleFlex, nil case fixedOK: result, err := lintFixed(dir, fixedCfg) if err != nil { - return nil, StyleFixed, err + return nil, styleFixed, err } addStrayWarnings(result, dir, flexDirs, fixedSet) - return result, StyleFixed, nil + return result, styleFixed, nil default: - return nil, StyleUnknown, fmt.Errorf( + return nil, styleUnknown, fmt.Errorf( "no configuration found in %q (looked for %s)", dir, lookedFor(flexDirs, fixedSet)) } } @@ -149,7 +156,7 @@ func detectFixed(root string, set []fixedNames) (fixedNames, bool) { return fixedNames{}, false } -// lookedFor builds the hint listing the locations CheckDir searched. +// 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 { @@ -161,28 +168,6 @@ func lookedFor(flexDirs []string, fixed []fixedNames) string { 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. diff --git a/pkg/lint/normalize_test.go b/pkg/lint/normalize_test.go index 146f0e739..ce8b3a65b 100644 --- a/pkg/lint/normalize_test.go +++ b/pkg/lint/normalize_test.go @@ -24,17 +24,17 @@ 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}, + {"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}, + map[string]string{"fixtures/example/.platform.app.yaml": "name: app"}, styleUnknown, true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -42,7 +42,7 @@ func TestDetectStyle(t *testing.T) { for p, c := range tc.files { writeFile(t, filepath.Join(dir, p), c) } - _, style, err := CheckDir(dir, upsunVendor()) + _, style, err := checkDir(dir, upsunVendor()) if tc.wantErr { require.Error(t, err) } else { @@ -63,24 +63,24 @@ func TestCheckDir_Vendors(t *testing.T) { name string vendor Vendor files map[string]string - want Style + 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}, + 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}, + 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}, + 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}, + 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}, + 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{".platform.app.yaml": app}, styleUnknown, true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -88,7 +88,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, tc.vendor) if tc.wantErr { require.Error(t, err) } else { @@ -99,31 +99,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, upsunVendor()) 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 +116,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, upsunVendor()) require.NoError(t, err) - assert.Equal(t, StyleFixed, style) + assert.Equal(t, styleFixed, style) assert.Contains(t, result.String(), "sub/.upsun") } @@ -155,17 +137,17 @@ routes: type: upstream upstream: "app:http" `) - result, style, err := CheckDir(dir, upsunVendor()) + result, style, err := checkDir(dir, upsunVendor()) 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, upsunVendor()) require.Error(t, err) - assert.Equal(t, StyleUnknown, style) + assert.Equal(t, styleUnknown, style) assert.Contains(t, err.Error(), "no configuration found") } @@ -176,9 +158,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, upsunVendor()) 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 +188,7 @@ func TestLintDir_FlexSourceFiles(t *testing.T) { type: upstream upstream: "missing:http" `) - result, _, err := CheckDir(dir, upsunVendor()) + result, _, err := checkDir(dir, upsunVendor()) 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 +200,7 @@ func TestLintDir_FlexSourceFiles(t *testing.T) { run: command: ./run `) - result, _, err = CheckDir(dir, upsunVendor()) + result, _, err = checkDir(dir, upsunVendor()) 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 +218,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, upsunVendor()) require.NoError(t, err) assert.Equal(t, []Issue{{ File: "sub/.upsun", @@ -252,9 +234,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, upsunVendor()) 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 +253,7 @@ hooks: `) writeFile(t, filepath.Join(dir, ".upsun", "build.sh"), "set -e\ncomposer install\n") - result, _, err := CheckDir(dir, upsunVendor()) + result, _, err := checkDir(dir, upsunVendor()) 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 +261,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, upsunVendor()) require.NoError(t, err) require.Len(t, result.Errors, 1) assert.Equal(t, ".upsun/config.yaml", result.Errors[0].File) @@ -302,7 +284,7 @@ hooks: upstream: "missing:http" `) - result, _, err := CheckDir(dir, upsunVendor()) + result, _, err := checkDir(dir, upsunVendor()) require.NoError(t, err) assert.Equal(t, []Issue{{ File: ".platform/routes.yaml", @@ -320,7 +302,7 @@ func TestLintDir_Aliases(t *testing.T) { type: "php:1.0" applications: *apps `) - result, _, err := CheckDir(dir, upsunVendor()) + result, _, err := checkDir(dir, upsunVendor()) require.NoError(t, err) require.Len(t, result.Errors, 1) assert.Equal(t, Issue{ @@ -338,7 +320,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, upsunVendor()) require.NoError(t, err) require.Len(t, result.Errors, 1) assert.Equal(t, Issue{ diff --git a/pkg/lint/relationships_test.go b/pkg/lint/relationships_test.go index 3d8fca13d..db89b16bf 100644 --- a/pkg/lint/relationships_test.go +++ b/pkg/lint/relationships_test.go @@ -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/pkg/lint/result.go b/pkg/lint/result.go index 131366173..68666fc03 100644 --- a/pkg/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"` } @@ -44,12 +44,12 @@ func (r *Result) AddWarning(path, message string) { }) } -// 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 } @@ -92,36 +92,10 @@ 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. diff --git a/pkg/lint/routes_test.go b/pkg/lint/routes_test.go index 530308756..094c15f6e 100644 --- a/pkg/lint/routes_test.go +++ b/pkg/lint/routes_test.go @@ -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/pkg/lint/scripts_test.go b/pkg/lint/scripts_test.go index f6bda27d7..347807ca5 100644 --- a/pkg/lint/scripts_test.go +++ b/pkg/lint/scripts_test.go @@ -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/pkg/lint/tasks_test.go b/pkg/lint/tasks_test.go index dbfa04601..7b99116d9 100644 --- a/pkg/lint/tasks_test.go +++ b/pkg/lint/tasks_test.go @@ -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/pkg/lint/types_test.go b/pkg/lint/types_test.go index b8458d9a7..4b8d2457b 100644 --- a/pkg/lint/types_test.go +++ b/pkg/lint/types_test.go @@ -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/pkg/lint/web_test.go b/pkg/lint/web_test.go index 6631c7149..2cdef5e48 100644 --- a/pkg/lint/web_test.go +++ b/pkg/lint/web_test.go @@ -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/pkg/lint/yaml_test.go b/pkg/lint/yaml_test.go index 920ffde3e..5252c88e9 100644 --- a/pkg/lint/yaml_test.go +++ b/pkg/lint/yaml_test.go @@ -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/model.go b/pkg/registry/model.go index 8d985970e..57788c3d8 100644 --- a/pkg/registry/model.go +++ b/pkg/registry/model.go @@ -5,8 +5,10 @@ import ( "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 { @@ -75,7 +77,7 @@ type BuildConfig struct { Flavor string `json:"flavor,omitempty" yaml:"flavor,omitempty"` } -// Dependency version map under Docs. +// Dependency maps package names to version constraints, for one package manager. type Dependency map[string]string // VersionInfo lists deprecated, supported, and optional legacy and retired versions. diff --git a/pkg/registry/registry.go b/pkg/registry/registry.go index 8716d217a..17b4fbeb4 100644 --- a/pkg/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 } diff --git a/pkg/registry/registry_test.go b/pkg/registry/registry_test.go index eb4da21d7..310ea679d 100644 --- a/pkg/registry/registry_test.go +++ b/pkg/registry/registry_test.go @@ -10,7 +10,7 @@ import ( // 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"} { From 9fc7db762297adca568d310dd907015b5594a4d6 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Sun, 18 Oct 2026 11:17:26 +0100 Subject: [PATCH 5/7] refactor(lint): drop Vendor and unused registry types CheckDir now takes only a directory and looks for the first-party layouts: .upsun/*.yaml (Flex), else .platform/ or .platform.app.yaml (Fixed). Both first-party CLIs already recognized both layouts. A CLI configured with another project_config_dir is no longer supported by lint. Remove the registry's Docs types and the Image Description, Configuration and Docs fields. The embedded registry.json does not contain them. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- commands/lint.go | 21 ++--- pkg/lint/fixed.go | 24 +++--- pkg/lint/fixed_test.go | 4 +- pkg/lint/normalize.go | 155 ++++++++++--------------------------- pkg/lint/normalize_test.go | 83 ++++---------------- pkg/registry/model.go | 52 ------------- pkg/registry/registry.go | 2 - 8 files changed, 76 insertions(+), 267 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index cc02ce331..8adb1b79d 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 `pkg/lint`, reporting all errors at once. `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`) +- `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, not a vendor's `project_config_dir`. `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/commands/lint.go b/commands/lint.go index 947cf7f52..2a36f7f5d 100644 --- a/commands/lint.go +++ b/commands/lint.go @@ -41,26 +41,15 @@ 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, - } -} - -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 +64,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" { @@ -109,7 +98,7 @@ func lintInput(cmd *cobra.Command, args []string, vendor lint.Vendor) (*lint.Res 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 } diff --git a/pkg/lint/fixed.go b/pkg/lint/fixed.go index 65d1028ad..e992c8cc1 100644 --- a/pkg/lint/fixed.go +++ b/pkg/lint/fixed.go @@ -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,15 +36,15 @@ 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 } @@ -118,8 +118,8 @@ func (l *fixedLoader) check(data any, sch *gojsonschema.Schema, key string, src } // 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 } diff --git a/pkg/lint/fixed_test.go b/pkg/lint/fixed_test.go index f637322b1..b3fe95dba 100644 --- a/pkg/lint/fixed_test.go +++ b/pkg/lint/fixed_test.go @@ -291,7 +291,7 @@ 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) @@ -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/normalize.go b/pkg/lint/normalize.go index b8a4e906f..38bc85c5b 100644 --- a/pkg/lint/normalize.go +++ b/pkg/lint/normalize.go @@ -20,75 +20,26 @@ const ( 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. +// Configuration locations, relative to the project root. const ( - flavorUpsun = "upsun" // project_config_flavor for the Flex format - firstPartyFlexDir = ".upsun" // Upsun (Flex) project config directory - firstPartyFixedDir = ".platform" - firstPartyFixedApp = ".platform.app.yaml" + flexDir = ".upsun" // Flex: .upsun/*.yaml + fixedDir = ".platform" // Fixed: applications, routes and services + fixedAppFile = ".platform.app.yaml" // Fixed: per-app config file ) -// 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. Detection uses -// the vendor's name conventions and the files present: Flex wins when present, -// else Fixed. Configuration problems are reported in the Result; the error is -// for configuration that cannot be found, read or parsed. -func CheckDir(dir string, vendor Vendor) (*Result, error) { - result, _, err := checkDir(dir, vendor) +// 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, vendor Vendor) (*Result, style, error) { - flexDirs, fixedSet := vendor.candidates() - flexDir, flexOK := detectFlex(dir, flexDirs) - fixedCfg, fixedOK := detectFixed(dir, fixedSet) - +func checkDir(dir string) (*Result, style, error) { switch { - case flexOK: + case isFlex(dir): root, err := os.OpenRoot(dir) if err != nil { return nil, styleFlex, err @@ -109,80 +60,52 @@ func checkDir(dir string, vendor Vendor) (*Result, style, error) { } sources.locate(result) } - addStrayWarnings(result, dir, flexDirs, fixedSet) - if fixedOK { + addStrayWarnings(result, dir) + if isFixed(dir) { result.AddWarning("", fmt.Sprintf( "both %s and %s configuration found; linting the %s (Flex) configuration", - flexDir, fixedCfg.dir, flexDir)) + flexDir, fixedDir, flexDir)) } return result, styleFlex, nil - case fixedOK: - result, err := lintFixed(dir, fixedCfg) + case isFixed(dir): + result, err := lintFixed(dir) if err != nil { return nil, styleFixed, err } - addStrayWarnings(result, dir, flexDirs, fixedSet) + addStrayWarnings(result, dir) return result, styleFixed, nil default: - return nil, styleUnknown, fmt.Errorf( - "no configuration found in %q (looked for %s)", dir, lookedFor(flexDirs, fixedSet)) + return nil, styleUnknown, fmt.Errorf("no configuration found in %q (looked for %s/*.yaml, %s, %s/)", + dir, flexDir, fixedAppFile, fixedDir) } } -// 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 +// 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 } -// 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+"/") +// 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 } - return strings.Join(parts, ", ") + _, ok := firstExistingYAML(root, "", fixedAppFile) + return ok } -// 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 - } +// 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 known config directory below the root (not a root-level copy, which + // A 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 { + 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()), diff --git a/pkg/lint/normalize_test.go b/pkg/lint/normalize_test.go index ce8b3a65b..bbd2650cf 100644 --- a/pkg/lint/normalize_test.go +++ b/pkg/lint/normalize_test.go @@ -15,11 +15,6 @@ 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 @@ -30,6 +25,8 @@ func TestDetectStyle(t *testing.T) { {"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. @@ -42,53 +39,7 @@ func TestDetectStyle(t *testing.T) { 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}, - } - 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, tc.vendor) + _, style, err := checkDir(dir) if tc.wantErr { require.Error(t, err) } else { @@ -103,7 +54,7 @@ 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.Contains(t, result.String(), "legacy/.platform") @@ -116,7 +67,7 @@ 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.Contains(t, result.String(), "sub/.upsun") @@ -137,7 +88,7 @@ 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.False(t, result.HasErrors(), "expected no errors, got: %s", result) @@ -145,7 +96,7 @@ routes: 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.Contains(t, err.Error(), "no configuration found") @@ -158,7 +109,7 @@ 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.True(t, result.HasWarnings()) @@ -188,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()) @@ -200,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`) @@ -218,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", @@ -234,7 +185,7 @@ 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.False(t, result.HasErrors(), "unexpected errors: %s", result) @@ -253,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", @@ -261,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) @@ -284,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", @@ -302,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{ @@ -320,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/pkg/registry/model.go b/pkg/registry/model.go index 57788c3d8..21bcb86ff 100644 --- a/pkg/registry/model.go +++ b/pkg/registry/model.go @@ -26,60 +26,8 @@ type Image struct { 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 maps package names to version constraints, for one package manager. -type Dependency map[string]string - // VersionInfo lists deprecated, supported, and optional legacy and retired versions. type VersionInfo struct { Supported []string `json:"supported"` diff --git a/pkg/registry/registry.go b/pkg/registry/registry.go index 17b4fbeb4..9ccbf3e50 100644 --- a/pkg/registry/registry.go +++ b/pkg/registry/registry.go @@ -45,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 } From efb24bf0895e48207baec164cd49d4ff9be31482 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Sun, 18 Oct 2026 11:21:35 +0100 Subject: [PATCH 6/7] fix(lint): register lint only for first-party config names lint looks only for .upsun and .platform/.platform.app.yaml, so a CLI configured with other names would offer a command that cannot find its projects' configuration. Register the command only when the configured names match. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- commands/lint.go | 13 +++++++++++++ commands/lint_test.go | 26 ++++++++++++++++++++++++++ commands/root.go | 4 +++- 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8adb1b79d..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 `pkg/lint`, reporting all errors at once. It looks for those names only, not a vendor's `project_config_dir`. `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`) +- `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/commands/lint.go b/commands/lint.go index 2a36f7f5d..41ef92822 100644 --- a/commands/lint.go +++ b/commands/lint.go @@ -48,6 +48,19 @@ func newLintCommand(cnf *config.Config) *cobra.Command { return cmd } +// 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) error { result, format, err := lintInput(cmd, args) if err != nil { diff --git a/commands/lint_test.go b/commands/lint_test.go index 101a00908..f401e74f2 100644 --- a/commands/lint_test.go +++ b/commands/lint_test.go @@ -10,6 +10,7 @@ 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" @@ -188,3 +189,28 @@ func TestFindProjectRoot(t *testing.T) { 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))) } From 9e35a8c14031cda0a528bfc1cc81ba263f18bf48 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Sun, 18 Oct 2026 11:23:41 +0100 Subject: [PATCH 7/7] refactor(lint): unexport Result helpers Remove ErrorLines and WarningLines, which had no callers, and unexport AddError, AddWarning and Merge. Callers outside the package can append to the exported Errors and Warnings fields. HasErrors, HasWarnings and String stay exported. Co-Authored-By: Claude Opus 5.5 --- commands/lint_test.go | 10 +++++----- pkg/lint/dependencies.go | 6 +++--- pkg/lint/fixed.go | 8 ++++---- pkg/lint/names.go | 8 ++++---- pkg/lint/normalize.go | 2 +- pkg/lint/relationships.go | 8 ++++---- pkg/lint/result.go | 18 ++++++------------ pkg/lint/routes.go | 12 ++++++------ pkg/lint/scripts.go | 2 +- pkg/lint/tasks.go | 16 ++++++++-------- pkg/lint/types.go | 16 ++++++++-------- pkg/lint/web.go | 12 ++++++------ pkg/lint/yaml.go | 6 +++--- 13 files changed, 59 insertions(+), 65 deletions(-) diff --git a/commands/lint_test.go b/commands/lint_test.go index f401e74f2..ed8569411 100644 --- a/commands/lint_test.go +++ b/commands/lint_test.go @@ -47,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) @@ -158,8 +159,7 @@ 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 diff --git a/pkg/lint/dependencies.go b/pkg/lint/dependencies.go index 8781a98a0..465ceacec 100644 --- a/pkg/lint/dependencies.go +++ b/pkg/lint/dependencies.go @@ -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/pkg/lint/fixed.go b/pkg/lint/fixed.go index e992c8cc1..78ba7b5ea 100644 --- a/pkg/lint/fixed.go +++ b/pkg/lint/fixed.go @@ -51,7 +51,7 @@ func lintFixed(dir string) (*Result, error) { 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. @@ -81,7 +81,7 @@ func lintFixed(dir string) (*Result, error) { return nil, err } l.sources.locate(checks) - result.Merge(checks) + result.merge(checks) return result, nil } @@ -114,7 +114,7 @@ func (l *fixedLoader) load(abs string) (file string, node *yaml.Node) { func (l *fixedLoader) check(data any, sch *gojsonschema.Schema, key string, src source) { 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 @@ -243,7 +243,7 @@ func (l *fixedLoader) loadSection( } checked := checkSchemaAt(data, sch, section) l.sources.locate(checked) - l.result.Merge(checked) + l.result.merge(checked) return data, nil } diff --git a/pkg/lint/names.go b/pkg/lint/names.go index 5de29b920..7f30fc298 100644 --- a/pkg/lint/names.go +++ b/pkg/lint/names.go @@ -21,22 +21,22 @@ func checkNames(cfg *config) *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/pkg/lint/normalize.go b/pkg/lint/normalize.go index 38bc85c5b..7d33998b0 100644 --- a/pkg/lint/normalize.go +++ b/pkg/lint/normalize.go @@ -62,7 +62,7 @@ func checkDir(dir string) (*Result, style, error) { } addStrayWarnings(result, dir) if isFixed(dir) { - result.AddWarning("", fmt.Sprintf( + result.addWarning("", fmt.Sprintf( "both %s and %s configuration found; linting the %s (Flex) configuration", flexDir, fixedDir, flexDir)) } diff --git a/pkg/lint/relationships.go b/pkg/lint/relationships.go index 14901019f..bd329e2c7 100644 --- a/pkg/lint/relationships.go +++ b/pkg/lint/relationships.go @@ -15,7 +15,7 @@ func checkRelationships(cfg *config) *Result { 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) } } } @@ -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/pkg/lint/result.go b/pkg/lint/result.go index 68666fc03..2ddd3229b 100644 --- a/pkg/lint/result.go +++ b/pkg/lint/result.go @@ -28,16 +28,16 @@ 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, @@ -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) @@ -98,8 +92,8 @@ func (r *Result) String() string { 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 } diff --git a/pkg/lint/routes.go b/pkg/lint/routes.go index 4782439c2..8d4c4d52f 100644 --- a/pkg/lint/routes.go +++ b/pkg/lint/routes.go @@ -12,7 +12,7 @@ func checkRoutes(cfg *config) *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/pkg/lint/scripts.go b/pkg/lint/scripts.go index 9526cdb1a..8130ac280 100644 --- a/pkg/lint/scripts.go +++ b/pkg/lint/scripts.go @@ -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/pkg/lint/tasks.go b/pkg/lint/tasks.go index 698ce8db3..6970be3f0 100644 --- a/pkg/lint/tasks.go +++ b/pkg/lint/tasks.go @@ -24,26 +24,26 @@ 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)) } } } @@ -53,16 +53,16 @@ func checkTasks(cfg *config) *Result { 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))) } } diff --git a/pkg/lint/types.go b/pkg/lint/types.go index ef78c645a..03989e38a 100644 --- a/pkg/lint/types.go +++ b/pkg/lint/types.go @@ -16,9 +16,9 @@ func checkTypes(cfg *config, reg registry.Registry) *Result { 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) } } @@ -28,24 +28,24 @@ func checkTypes(cfg *config, reg registry.Registry) *Result { 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/pkg/lint/web.go b/pkg/lint/web.go index 1cd36d412..bf0a14819 100644 --- a/pkg/lint/web.go +++ b/pkg/lint/web.go @@ -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/pkg/lint/yaml.go b/pkg/lint/yaml.go index f651e7ed2..fc930b193 100644 --- a/pkg/lint/yaml.go +++ b/pkg/lint/yaml.go @@ -13,7 +13,7 @@ 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, "") @@ -26,7 +26,7 @@ func checkSchemaAt(data any, schema *gojsonschema.Schema, pathPrefix string) *Re 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