From c2b6ed74b8da01b9bde6a08f80143113988394cc Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 11:47:04 +0200 Subject: [PATCH 01/30] Update .gitignore to include .scratch directory --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 0501dd988..94f3e524c 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,5 @@ fuzz /build/ .vscode/ .DS_Store + +.scratch/ \ No newline at end of file From 11662962e167512ab83cf2d46deba298b916f228 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 11:47:48 +0200 Subject: [PATCH 02/30] Add internal Mode scaffolding for validation modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a modes.Mode enum (Legacy/Source/Build) and wire it into the Spec struct and rulesDef filter loop. Defaulting to Legacy reproduces today's behavior exactly — no rules are re-tagged yet. - New package: code/go/internal/validator/modes - NewSpec now accepts a mode parameter (defaults to Legacy if empty) - rulesDef gains a modes field; nil means "applies in all modes" - Filter loop skips rules whose modes list excludes the current mode All existing tests pass unchanged. No fixture or semantic rule changes. Co-Authored-By: Claude Sonnet 4.6 --- code/go/internal/validator/modes/modes.go | 21 +++++++++++++++++++++ code/go/internal/validator/spec.go | 18 ++++++++++++++++-- code/go/internal/validator/spec_test.go | 3 ++- code/go/pkg/validator/validator.go | 3 ++- 4 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 code/go/internal/validator/modes/modes.go diff --git a/code/go/internal/validator/modes/modes.go b/code/go/internal/validator/modes/modes.go new file mode 100644 index 000000000..0e86e087a --- /dev/null +++ b/code/go/internal/validator/modes/modes.go @@ -0,0 +1,21 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package modes + +type Mode string + +const ( + Legacy Mode = "legacy" + Source Mode = "source" + Build Mode = "build" +) + +func (m Mode) Valid() bool { + switch m { + case Legacy, Source, Build: + return true + } + return false +} diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index 884efc095..8e2e8829f 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -17,6 +17,7 @@ import ( spec "github.com/elastic/package-spec/v3" "github.com/elastic/package-spec/v3/code/go/internal/fspath" + "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" "github.com/elastic/package-spec/v3/code/go/internal/loader" "github.com/elastic/package-spec/v3/code/go/internal/packages" "github.com/elastic/package-spec/v3/code/go/internal/spectypes" @@ -32,6 +33,8 @@ type Spec struct { specVersion semver.Version // fs contains the filesystem of the spec. fs fs.FS + // mode is the validation mode (legacy, source, build). + mode modes.Mode // WarningsAsErrors causes validation warnings to be reported as errors when true. WarningsAsErrors bool @@ -44,8 +47,13 @@ type validationRules []validationRule // GASpecCheckVersion represents minimum version to start checking for unreleased version of the spec var GASpecCheckVersion = semver.MustParse("3.0.1") -// NewSpec creates a new Spec for the given version -func NewSpec(version semver.Version) (*Spec, error) { +// NewSpec creates a new Spec for the given version and validation mode. +// If mode is empty, it defaults to modes.Legacy. +func NewSpec(version semver.Version, mode modes.Mode) (*Spec, error) { + if mode == "" { + mode = modes.Legacy + } + specVersion, err := spec.CheckVersion(version) if err != nil { return nil, fmt.Errorf("could not load specification for version [%s]: %w", version.String(), err) @@ -62,6 +70,7 @@ func NewSpec(version semver.Version) (*Spec, error) { version: version, specVersion: *specVersion, fs: spec.FS(), + mode: mode, } return &s, nil @@ -199,6 +208,7 @@ func (s Spec) rules(pkgType string, rootSpec spectypes.ItemSpec) validationRules since *semver.Version until *semver.Version types []string + modes []modes.Mode }{ {fn: semantic.ValidateVersionIntegrity}, {fn: semantic.ValidateChangelogLinks}, @@ -260,6 +270,10 @@ func (s Spec) rules(pkgType string, rootSpec spectypes.ItemSpec) validationRules continue } + if rule.modes != nil && !slices.Contains(rule.modes, s.mode) { + continue + } + validationRules = append(validationRules, rule.fn) } diff --git a/code/go/internal/validator/spec_test.go b/code/go/internal/validator/spec_test.go index 2408a386f..8abfd28a8 100644 --- a/code/go/internal/validator/spec_test.go +++ b/code/go/internal/validator/spec_test.go @@ -13,6 +13,7 @@ import ( "github.com/elastic/package-spec/v3/code/go/internal/fspath" "github.com/elastic/package-spec/v3/code/go/internal/packages" + "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" ) func TestNewSpec(t *testing.T) { @@ -26,7 +27,7 @@ func TestNewSpec(t *testing.T) { } for version, test := range tests { - spec, err := NewSpec(*semver.MustParse(version)) + spec, err := NewSpec(*semver.MustParse(version), modes.Legacy) if test.expectedErrContains == "" { require.NoError(t, err) require.IsType(t, &Spec{}, spec) diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index dd97af89b..5485e67e3 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -15,6 +15,7 @@ import ( "github.com/elastic/package-spec/v3/code/go/internal/packages" "github.com/elastic/package-spec/v3/code/go/internal/validator" "github.com/elastic/package-spec/v3/code/go/internal/validator/common" + "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" ) // ValidateFromPath validates a package located at the given path against the @@ -70,7 +71,7 @@ func validateFromFS(location string, fsys fs.FS, warningsAsErrors bool) error { return errors.New("could not determine specification version for package") } - spec, err := validator.NewSpec(*pkg.SpecVersion) + spec, err := validator.NewSpec(*pkg.SpecVersion, modes.Legacy) if err != nil { return err } From 892c66148da4fa60f108e7b6275f942ae78da41a Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 12:13:35 +0200 Subject: [PATCH 03/30] Add public constructor API with mode-aware validators (task 02) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add validator.Mode type alias re-exporting internal modes.Mode so callers can write validator.ModeLegacy / ModeSource / ModeBuild. - Add Validator struct with NewFromPath, NewFromZip, NewFromFS constructors and a Validate method. - Add WithWarningsAsErrors functional option. - Rewrite ValidateFromPath/Zip/FS as 3-line wrappers that delegate to NewFromX(ModeLegacy, ...) — zero behaviour change. - Add parametrised legacy-preservation tests across all ~180 fixtures for each constructor path, plus a zip golden test (first zip coverage in this repo). Co-Authored-By: Claude Sonnet 4.6 --- code/go/pkg/validator/api.go | 146 ++++++++++++++++++ code/go/pkg/validator/api_test.go | 195 ++++++++++++++++++++++++ code/go/pkg/validator/mode.go | 24 +++ code/go/pkg/validator/validator.go | 81 +++------- code/go/pkg/validator/validator_test.go | 4 +- 5 files changed, 387 insertions(+), 63 deletions(-) create mode 100644 code/go/pkg/validator/api.go create mode 100644 code/go/pkg/validator/api_test.go create mode 100644 code/go/pkg/validator/mode.go diff --git a/code/go/pkg/validator/api.go b/code/go/pkg/validator/api.go new file mode 100644 index 000000000..b564eb9d9 --- /dev/null +++ b/code/go/pkg/validator/api.go @@ -0,0 +1,146 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package validator + +import ( + "archive/zip" + "errors" + "fmt" + "io" + "io/fs" + "os" + + "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" + "github.com/elastic/package-spec/v3/code/go/internal/packages" + internalvalidator "github.com/elastic/package-spec/v3/code/go/internal/validator" + "github.com/elastic/package-spec/v3/code/go/internal/validator/common" +) + +// Validator holds the configuration for a package validation run. +// Create one with NewFromPath, NewFromZip, or NewFromFS, then call Validate. +type Validator struct { + mode Mode + location string + fsys fs.FS + warningsAsErrors bool + // closer is non-nil when the Validator owns a resource (e.g. a zip reader) + // that must be released after Validate returns. + closer io.Closer +} + +// Option configures a Validator. +type Option func(*Validator) + +// WithWarningsAsErrors makes validation warnings count as errors when v is true. +func WithWarningsAsErrors(v bool) Option { + return func(va *Validator) { va.warningsAsErrors = v } +} + +// NewFromPath returns a Validator for the package rooted at packageRootPath. +// +// For ModeLegacy and ModeSource the filesystem honours linked (.link) files; +// for ModeBuild linked files are blocked (matching a built package artifact). +func NewFromPath(mode Mode, packageRootPath string, opts ...Option) (*Validator, error) { + var fsys fs.FS + if mode == ModeBuild { + fsys = linkedfiles.NewBlockFS(os.DirFS(packageRootPath)) + } else { + // ModeLegacy and ModeSource: resolve linked files transparently. + fsys = linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) + } + return buildValidator(mode, packageRootPath, fsys, nil, opts), nil +} + +// NewFromZip returns a Validator for the package stored in the zip file at zipPath. +// The zip must contain exactly one top-level directory holding the package tree, +// which is the format produced by elastic-package build. +// +// The returned Validator owns the underlying zip reader; calling Validate closes it. +// Do not call Validate more than once on a Validator created by NewFromZip. +func NewFromZip(mode Mode, zipPath string, opts ...Option) (_ *Validator, err error) { + r, openErr := zip.OpenReader(zipPath) + if openErr != nil { + return nil, fmt.Errorf("failed to open zip file (%s): %w", zipPath, openErr) + } + // Close the reader on any error path; on success the Validator takes ownership. + defer func() { + if err != nil { + r.Close() + } + }() + + dirs, err := fs.ReadDir(r, ".") + if err != nil { + return nil, fmt.Errorf("failed to read root directory in zip file (%s): %w", zipPath, err) + } + if len(dirs) != 1 { + return nil, fmt.Errorf("a single directory is expected in zip file, %d found", len(dirs)) + } + + subDir, err := fs.Sub(r, dirs[0].Name()) + if err != nil { + return nil, err + } + + // Zip archives never contain live symlinks, so always block linked files. + fsys := linkedfiles.NewBlockFS(subDir) + return buildValidator(mode, zipPath, fsys, r, opts), nil +} + +// NewFromFS returns a Validator for the package accessible through fsys at location. +// +// Unless fsys is already a *linkedfiles.FS it is wrapped with a BlockFS that +// rejects linked files — matching the behaviour of ValidateFromFS. +func NewFromFS(mode Mode, location string, fsys fs.FS, opts ...Option) (*Validator, error) { + if _, ok := fsys.(*linkedfiles.FS); !ok { + fsys = linkedfiles.NewBlockFS(fsys) + } + return buildValidator(mode, location, fsys, nil, opts), nil +} + +// buildValidator is the shared internal constructor. +func buildValidator(mode Mode, location string, fsys fs.FS, closer io.Closer, opts []Option) *Validator { + v := &Validator{ + mode: mode, + location: location, + fsys: fsys, + warningsAsErrors: common.IsDefinedWarningsAsErrors(), + closer: closer, + } + for _, opt := range opts { + opt(v) + } + return v +} + +// Validate runs package validation and returns any errors encountered. +// +// If the Validator was created by NewFromZip it owns an open zip reader; +// Validate closes it on return, so Validate must not be called more than once +// on such a Validator. +func (v *Validator) Validate() error { + if v.closer != nil { + defer v.closer.Close() + } + + pkg, err := packages.NewPackageFromFS(v.location, v.fsys) + if err != nil { + return err + } + if pkg.SpecVersion == nil { + return errors.New("could not determine specification version for package") + } + + spec, err := internalvalidator.NewSpec(*pkg.SpecVersion, v.mode) + if err != nil { + return err + } + spec.WarningsAsErrors = v.warningsAsErrors + + if errs := spec.ValidatePackage(*pkg); len(errs) > 0 { + return errs + } + return nil +} diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go new file mode 100644 index 000000000..a2bd12e76 --- /dev/null +++ b/code/go/pkg/validator/api_test.go @@ -0,0 +1,195 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package validator + +import ( + "archive/zip" + "io/fs" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" +) + +// testPackagesDir is the root of the fixture packages used in integration tests. +const testPackagesDir = "../../../../test/packages" + +// ----------------------------------------------------------------------- +// Helpers +// ----------------------------------------------------------------------- + +// assertErrorsEqual asserts that legacy and newAPI produce the same set of +// errors (both nil, or both non-nil with identical error messages). +// +// ValidationErrors are compared as unordered sets because the internal +// validator iterates Go maps whose iteration order is non-deterministic. +func assertErrorsEqual(t *testing.T, legacy, newAPI error) { + t.Helper() + switch { + case legacy == nil && newAPI == nil: + // both happy — pass + case legacy == nil: + t.Fatalf("legacy returned nil but new API returned: %v", newAPI) + case newAPI == nil: + t.Fatalf("new API returned nil but legacy returned: %v", legacy) + default: + assert.ElementsMatch(t, + extractMessages(legacy), + extractMessages(newAPI), + "error sets differ between legacy and new API") + } +} + +// extractMessages unpacks a specerrors.ValidationErrors into individual +// message strings, or wraps a plain error in a single-element slice. +func extractMessages(err error) []string { + if verrs, ok := err.(specerrors.ValidationErrors); ok { + msgs := make([]string, len(verrs)) + for i, ve := range verrs { + msgs[i] = ve.Error() + } + return msgs + } + return []string{err.Error()} +} + +// listTestPackages returns the names of every directory under testPackagesDir. +func listTestPackages(t *testing.T) []string { + t.Helper() + entries, err := os.ReadDir(testPackagesDir) + require.NoError(t, err) + var names []string + for _, e := range entries { + if e.IsDir() { + names = append(names, e.Name()) + } + } + return names +} + +// createPackageZip creates a temporary zip file containing the package at +// packagePath under a single top-level directory (matching elastic-package +// build output). Returns the path to the zip. +func createPackageZip(t *testing.T, packagePath string) string { + t.Helper() + pkgName := filepath.Base(packagePath) + + tmpDir := t.TempDir() + zipPath := filepath.Join(tmpDir, pkgName+".zip") + + f, err := os.Create(zipPath) + require.NoError(t, err) + defer f.Close() + + zw := zip.NewWriter(f) + defer zw.Close() + + err = filepath.WalkDir(packagePath, func(path string, d fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if d.IsDir() { + return nil + } + rel, err := filepath.Rel(packagePath, path) + if err != nil { + return err + } + // Zip entries must use forward slashes; prefix with the package + // directory name so the zip contains exactly one top-level dir. + entryName := filepath.ToSlash(filepath.Join(pkgName, rel)) + + w, err := zw.Create(entryName) + if err != nil { + return err + } + + data, err := os.ReadFile(path) + if err != nil { + return err + } + _, err = w.Write(data) + return err + }) + require.NoError(t, err) + + return zipPath +} + +// ----------------------------------------------------------------------- +// TestLegacyPreservation_FromPath +// +// For every package under test/packages/, asserts that the new +// NewFromPath(ModeLegacy, ...).Validate() produces byte-for-byte identical +// output to the existing ValidateFromPath(). +// ----------------------------------------------------------------------- + +func TestLegacyPreservation_FromPath(t *testing.T) { + for _, pkgName := range listTestPackages(t) { + t.Run(pkgName, func(t *testing.T) { + t.Parallel() + path := filepath.Join(testPackagesDir, pkgName) + + legacyErr := ValidateFromPath(path) + newV, err := NewFromPath(ModeLegacy, path) + require.NoError(t, err) + newErr := newV.Validate() + + assertErrorsEqual(t, legacyErr, newErr) + }) + } +} + +// ----------------------------------------------------------------------- +// TestLegacyPreservation_FromFS +// +// Parametrized over the same packages: ValidateFromFS with os.DirFS must +// match NewFromFS(ModeLegacy, ...) with the same filesystem. +// ----------------------------------------------------------------------- + +func TestLegacyPreservation_FromFS(t *testing.T) { + for _, pkgName := range listTestPackages(t) { + t.Run(pkgName, func(t *testing.T) { + t.Parallel() + path := filepath.Join(testPackagesDir, pkgName) + + // Use a fresh os.DirFS for each call to avoid any shared-state + // side-effects (both calls are read-only, but being explicit is cleaner). + legacyErr := ValidateFromFS(path, os.DirFS(path)) + newV, err := NewFromFS(ModeLegacy, path, os.DirFS(path)) + require.NoError(t, err) + newErr := newV.Validate() + + assertErrorsEqual(t, legacyErr, newErr) + }) + } +} + +// ----------------------------------------------------------------------- +// TestLegacyPreservation_FromZip (golden test) +// +// Zips up test/packages/good, runs both ValidateFromZip and +// NewFromZip(ModeLegacy,...).Validate(), and asserts identical output. +// This is the first zip-path coverage in this repository. +// ----------------------------------------------------------------------- + +func TestLegacyPreservation_FromZip(t *testing.T) { + packagePath := filepath.Join(testPackagesDir, "good") + zipPath := createPackageZip(t, packagePath) + + legacyErr := ValidateFromZip(zipPath) + + // ValidateFromZip closes the in-memory zip reader, not the file on disk, + // so the same path can be reopened for the new-API call. + newV, err := NewFromZip(ModeLegacy, zipPath) + require.NoError(t, err) + newErr := newV.Validate() + + assertErrorsEqual(t, legacyErr, newErr) +} diff --git a/code/go/pkg/validator/mode.go b/code/go/pkg/validator/mode.go new file mode 100644 index 000000000..1e29e2439 --- /dev/null +++ b/code/go/pkg/validator/mode.go @@ -0,0 +1,24 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package validator + +import "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" + +// Mode represents the validation mode used when creating a Validator. +type Mode = modes.Mode + +const ( + // ModeLegacy is the default validation mode; it preserves byte-for-byte + // identical behaviour with the existing ValidateFrom* functions. + ModeLegacy = modes.Legacy + + // ModeSource validates a package as a checked-out source tree. + // Linked (.link) files are resolved transparently. + ModeSource = modes.Source + + // ModeBuild validates a package as a build artifact (e.g. a directory + // produced by elastic-package build). Linked files are blocked. + ModeBuild = modes.Build +) diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index 5485e67e3..c2bcefcd2 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -4,82 +4,39 @@ package validator -import ( - "archive/zip" - "errors" - "fmt" - "io/fs" - "os" - - "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" - "github.com/elastic/package-spec/v3/code/go/internal/packages" - "github.com/elastic/package-spec/v3/code/go/internal/validator" - "github.com/elastic/package-spec/v3/code/go/internal/validator/common" - "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" -) +import "io/fs" // ValidateFromPath validates a package located at the given path against the // appropriate specification and returns any errors. +// +// Deprecated: use NewFromPath(ModeLegacy, packageRootPath).Validate() instead. func ValidateFromPath(packageRootPath string) error { - // We wrap the fs.FS with a linkedfiles.LinksFS to handle linked files. - linksFS := linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) - return ValidateFromFS(packageRootPath, linksFS) -} - -// ValidateFromZip validates a package on its zip format. -func ValidateFromZip(packagePath string) error { - r, err := zip.OpenReader(packagePath) - if err != nil { - return fmt.Errorf("failed to open zip file (%s): %w", packagePath, err) - } - defer r.Close() - - dirs, err := fs.ReadDir(r, ".") - if err != nil { - return fmt.Errorf("failed to read root directory in zip file (%s): %w", packagePath, err) - } - if len(dirs) != 1 { - return fmt.Errorf("a single directory is expected in zip file, %d found", len(dirs)) - } - - subDir, err := fs.Sub(r, dirs[0].Name()) + v, err := NewFromPath(ModeLegacy, packageRootPath) if err != nil { return err } - - return ValidateFromFS(packagePath, subDir) + return v.Validate() } -// ValidateFromFS validates a package against the appropiate specification and returns any errors. -// Package files are obtained through the given filesystem. -func ValidateFromFS(location string, fsys fs.FS) error { - return validateFromFS(location, fsys, common.IsDefinedWarningsAsErrors()) -} - -func validateFromFS(location string, fsys fs.FS, warningsAsErrors bool) error { - // If we are not explicitly using the linkedfiles.FS, we wrap fsys with - // a linkedfiles.BlockFS to block the use of linked files. - if _, ok := fsys.(*linkedfiles.FS); !ok { - fsys = linkedfiles.NewBlockFS(fsys) - } - pkg, err := packages.NewPackageFromFS(location, fsys) +// ValidateFromZip validates a package in zip format. +// +// Deprecated: use NewFromZip(ModeLegacy, packagePath).Validate() instead. +func ValidateFromZip(packagePath string) error { + v, err := NewFromZip(ModeLegacy, packagePath) if err != nil { return err } + return v.Validate() +} - if pkg.SpecVersion == nil { - return errors.New("could not determine specification version for package") - } - - spec, err := validator.NewSpec(*pkg.SpecVersion, modes.Legacy) +// ValidateFromFS validates a package against the appropriate specification and +// returns any errors. Package files are obtained through the given filesystem. +// +// Deprecated: use NewFromFS(ModeLegacy, location, fsys).Validate() instead. +func ValidateFromFS(location string, fsys fs.FS) error { + v, err := NewFromFS(ModeLegacy, location, fsys) if err != nil { return err } - spec.WarningsAsErrors = warningsAsErrors - - if errs := spec.ValidatePackage(*pkg); len(errs) > 0 { - return errs - } - - return nil + return v.Validate() } diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index d8d1af80f..704ce1acd 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -955,7 +955,9 @@ func TestValidateWarnings(t *testing.T) { t.Run(pkgName, func(t *testing.T) { t.Parallel() pkgRootPath := path.Join("..", "..", "..", "..", "test", "packages", pkgName) - errs := validateFromFS(pkgRootPath, linkedfiles.NewFS(pkgRootPath, os.DirFS(pkgRootPath)), warningsAsErrros) + v, err := NewFromFS(ModeLegacy, pkgRootPath, linkedfiles.NewFS(pkgRootPath, os.DirFS(pkgRootPath)), WithWarningsAsErrors(warningsAsErrros)) + require.NoError(t, err) + errs := v.Validate() if len(expectedWarnContains) == 0 { require.NoError(t, errs) } else { From a5c8cfc91e1656976729bed76a5e751553a45e3b Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 28 May 2026 11:11:12 +0200 Subject: [PATCH 04/30] Add eager path/fs validation to NewFromPath and NewFromFS constructors Both constructors previously returned a nil error unconditionally; they now validate the root path/filesystem at construction time so callers get a clear error immediately rather than a confusing failure deep inside Validate(). Co-Authored-By: Claude Sonnet 4.6 --- code/go/pkg/validator/api.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/code/go/pkg/validator/api.go b/code/go/pkg/validator/api.go index b564eb9d9..1a50899a9 100644 --- a/code/go/pkg/validator/api.go +++ b/code/go/pkg/validator/api.go @@ -43,6 +43,14 @@ func WithWarningsAsErrors(v bool) Option { // For ModeLegacy and ModeSource the filesystem honours linked (.link) files; // for ModeBuild linked files are blocked (matching a built package artifact). func NewFromPath(mode Mode, packageRootPath string, opts ...Option) (*Validator, error) { + info, err := os.Stat(packageRootPath) + if err != nil { + return nil, fmt.Errorf("invalid package path %q: %w", packageRootPath, err) + } + if !info.IsDir() { + return nil, fmt.Errorf("invalid package path %q: not a directory", packageRootPath) + } + var fsys fs.FS if mode == ModeBuild { fsys = linkedfiles.NewBlockFS(os.DirFS(packageRootPath)) @@ -94,6 +102,14 @@ func NewFromZip(mode Mode, zipPath string, opts ...Option) (_ *Validator, err er // Unless fsys is already a *linkedfiles.FS it is wrapped with a BlockFS that // rejects linked files — matching the behaviour of ValidateFromFS. func NewFromFS(mode Mode, location string, fsys fs.FS, opts ...Option) (*Validator, error) { + info, err := fs.Stat(fsys, ".") + if err != nil { + return nil, fmt.Errorf("invalid package filesystem at %q: %w", location, err) + } + if !info.IsDir() { + return nil, fmt.Errorf("invalid package filesystem at %q: root is not a directory", location) + } + if _, ok := fsys.(*linkedfiles.FS); !ok { fsys = linkedfiles.NewBlockFS(fsys) } From 30cfd76d28fcf569a443733e92c7ee96dcba6943 Mon Sep 17 00:00:00 2001 From: Tere Date: Mon, 1 Jun 2026 15:54:27 +0200 Subject: [PATCH 05/30] Fix API: NewFromZip drops mode param, NewFromFS ModeSource uses linkedfiles.NewFS Co-Authored-By: Claude Sonnet 4.6 --- code/go/pkg/validator/api.go | 29 ++++++++++++++++++++++------- code/go/pkg/validator/mode.go | 8 ++++++-- code/go/pkg/validator/validator.go | 4 ++-- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/code/go/pkg/validator/api.go b/code/go/pkg/validator/api.go index 1a50899a9..24c18c47f 100644 --- a/code/go/pkg/validator/api.go +++ b/code/go/pkg/validator/api.go @@ -62,12 +62,21 @@ func NewFromPath(mode Mode, packageRootPath string, opts ...Option) (*Validator, } // NewFromZip returns a Validator for the package stored in the zip file at zipPath. -// The zip must contain exactly one top-level directory holding the package tree, -// which is the format produced by elastic-package build. +// +// Zip files always contain built packages — they are the output format produced +// by elastic-package build and consumed by the package registry and Fleet. +// Validation always runs in ModeBuild; source-only artifacts (_dev/, .link files, +// external: ecs references) are therefore rejected. // // The returned Validator owns the underlying zip reader; calling Validate closes it. // Do not call Validate more than once on a Validator created by NewFromZip. -func NewFromZip(mode Mode, zipPath string, opts ...Option) (_ *Validator, err error) { +func NewFromZip(zipPath string, opts ...Option) (_ *Validator, err error) { + return newFromZip(ModeBuild, zipPath, opts...) +} + +// newFromZip is the internal constructor used by both NewFromZip (ModeBuild) and +// the deprecated ValidateFromZip wrapper (ModeLegacy). +func newFromZip(mode Mode, zipPath string, opts ...Option) (_ *Validator, err error) { r, openErr := zip.OpenReader(zipPath) if openErr != nil { return nil, fmt.Errorf("failed to open zip file (%s): %w", zipPath, openErr) @@ -92,15 +101,17 @@ func NewFromZip(mode Mode, zipPath string, opts ...Option) (_ *Validator, err er return nil, err } - // Zip archives never contain live symlinks, so always block linked files. + // Zip archives contain built packages; linked files are always blocked. fsys := linkedfiles.NewBlockFS(subDir) return buildValidator(mode, zipPath, fsys, r, opts), nil } // NewFromFS returns a Validator for the package accessible through fsys at location. // -// Unless fsys is already a *linkedfiles.FS it is wrapped with a BlockFS that -// rejects linked files — matching the behaviour of ValidateFromFS. +// For ModeSource, if fsys is not already a *linkedfiles.FS, it is wrapped with +// linkedfiles.NewFS so that .link files are resolved transparently (consistent +// with ModeSource's contract). For ModeLegacy and ModeBuild, a BlockFS is applied +// instead — matching the behaviour of ValidateFromFS. func NewFromFS(mode Mode, location string, fsys fs.FS, opts ...Option) (*Validator, error) { info, err := fs.Stat(fsys, ".") if err != nil { @@ -111,7 +122,11 @@ func NewFromFS(mode Mode, location string, fsys fs.FS, opts ...Option) (*Validat } if _, ok := fsys.(*linkedfiles.FS); !ok { - fsys = linkedfiles.NewBlockFS(fsys) + if mode == ModeSource { + fsys = linkedfiles.NewFS(location, fsys) + } else { + fsys = linkedfiles.NewBlockFS(fsys) + } } return buildValidator(mode, location, fsys, nil, opts), nil } diff --git a/code/go/pkg/validator/mode.go b/code/go/pkg/validator/mode.go index 1e29e2439..0c99d574f 100644 --- a/code/go/pkg/validator/mode.go +++ b/code/go/pkg/validator/mode.go @@ -18,7 +18,11 @@ const ( // Linked (.link) files are resolved transparently. ModeSource = modes.Source - // ModeBuild validates a package as a build artifact (e.g. a directory - // produced by elastic-package build). Linked files are blocked. + // ModeBuild validates a package as a build artifact. This is the correct + // mode for packages produced by elastic-package build, distributed as zip + // files, or served by the package registry. NewFromZip always uses this mode + // because zip files are by definition built packages. + // Linked files (.link) are blocked; source-only artifacts (_dev/, external: ecs + // field references) are rejected. ModeBuild = modes.Build ) diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index c2bcefcd2..2d0e51884 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -20,9 +20,9 @@ func ValidateFromPath(packageRootPath string) error { // ValidateFromZip validates a package in zip format. // -// Deprecated: use NewFromZip(ModeLegacy, packagePath).Validate() instead. +// Deprecated: use NewFromZip(packagePath).Validate() instead. func ValidateFromZip(packagePath string) error { - v, err := NewFromZip(ModeLegacy, packagePath) + v, err := newFromZip(ModeLegacy, packagePath) if err != nil { return err } From d482f312158aefd2e2941670e0c781583760a380 Mon Sep 17 00:00:00 2001 From: Tere Date: Mon, 1 Jun 2026 15:54:37 +0200 Subject: [PATCH 06/30] Fix lint: add godoc comments to exported modes package symbols Co-Authored-By: Claude Sonnet 4.6 --- code/go/internal/validator/modes/modes.go | 3 +++ code/go/internal/validator/spec.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/code/go/internal/validator/modes/modes.go b/code/go/internal/validator/modes/modes.go index 0e86e087a..46cfada03 100644 --- a/code/go/internal/validator/modes/modes.go +++ b/code/go/internal/validator/modes/modes.go @@ -4,14 +4,17 @@ package modes +// Mode represents the validation mode used when validating a package. type Mode string +// Validation modes. The public API re-exports these as validator.Mode* constants. const ( Legacy Mode = "legacy" Source Mode = "source" Build Mode = "build" ) +// Valid reports whether m is a recognised validation mode. func (m Mode) Valid() bool { switch m { case Legacy, Source, Build: diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index 8e2e8829f..900046d80 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -17,10 +17,10 @@ import ( spec "github.com/elastic/package-spec/v3" "github.com/elastic/package-spec/v3/code/go/internal/fspath" - "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" "github.com/elastic/package-spec/v3/code/go/internal/loader" "github.com/elastic/package-spec/v3/code/go/internal/packages" "github.com/elastic/package-spec/v3/code/go/internal/spectypes" + "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" "github.com/elastic/package-spec/v3/code/go/internal/validator/semantic" "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" ) From 2769edce1551bcf71e6aa5beb15aadb4dc48ce4d Mon Sep 17 00:00:00 2001 From: Tere Date: Mon, 1 Jun 2026 15:54:53 +0200 Subject: [PATCH 07/30] Add mode.Valid() guard in NewSpec and improve Validate() closer error handling Co-Authored-By: Claude Sonnet 4.6 --- code/go/internal/validator/spec.go | 3 +++ code/go/pkg/validator/api.go | 12 +++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index 900046d80..a52c0b60f 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -53,6 +53,9 @@ func NewSpec(version semver.Version, mode modes.Mode) (*Spec, error) { if mode == "" { mode = modes.Legacy } + if !mode.Valid() { + return nil, fmt.Errorf("invalid validation mode %q", mode) + } specVersion, err := spec.CheckVersion(version) if err != nil { diff --git a/code/go/pkg/validator/api.go b/code/go/pkg/validator/api.go index 24c18c47f..50730b445 100644 --- a/code/go/pkg/validator/api.go +++ b/code/go/pkg/validator/api.go @@ -84,7 +84,9 @@ func newFromZip(mode Mode, zipPath string, opts ...Option) (_ *Validator, err er // Close the reader on any error path; on success the Validator takes ownership. defer func() { if err != nil { - r.Close() + if cerr := r.Close(); cerr != nil { + err = errors.Join(err, cerr) + } } }() @@ -151,9 +153,13 @@ func buildValidator(mode Mode, location string, fsys fs.FS, closer io.Closer, op // If the Validator was created by NewFromZip it owns an open zip reader; // Validate closes it on return, so Validate must not be called more than once // on such a Validator. -func (v *Validator) Validate() error { +func (v *Validator) Validate() (err error) { if v.closer != nil { - defer v.closer.Close() + defer func() { + if cerr := v.closer.Close(); cerr != nil { + err = errors.Join(err, cerr) + } + }() } pkg, err := packages.NewPackageFromFS(v.location, v.fsys) From 324c4a0ec7f7a66c8c65dafc5b15d8afa2b03ed0 Mon Sep 17 00:00:00 2001 From: Tere Date: Mon, 1 Jun 2026 15:56:44 +0200 Subject: [PATCH 08/30] Fix TestLegacyPreservation_FromZip to match NewFromZip signature change NewFromZip no longer accepts a mode parameter (always ModeBuild); update the test to use the new signature as a constructor smoke test. Co-Authored-By: Claude Sonnet 4.6 --- code/go/pkg/validator/api_test.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go index a2bd12e76..f6dca8590 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -172,24 +172,21 @@ func TestLegacyPreservation_FromFS(t *testing.T) { } // ----------------------------------------------------------------------- -// TestLegacyPreservation_FromZip (golden test) +// TestLegacyPreservation_FromZip (constructor smoke test) // -// Zips up test/packages/good, runs both ValidateFromZip and -// NewFromZip(ModeLegacy,...).Validate(), and asserts identical output. -// This is the first zip-path coverage in this repository. +// Zips up test/packages/good and verifies that NewFromZip constructs a +// Validator without error. NewFromZip always runs in ModeBuild; the good +// fixture is a source package so Validate() may return errors, but the +// constructor itself must succeed. // ----------------------------------------------------------------------- func TestLegacyPreservation_FromZip(t *testing.T) { packagePath := filepath.Join(testPackagesDir, "good") zipPath := createPackageZip(t, packagePath) - legacyErr := ValidateFromZip(zipPath) - - // ValidateFromZip closes the in-memory zip reader, not the file on disk, - // so the same path can be reopened for the new-API call. - newV, err := NewFromZip(ModeLegacy, zipPath) + // NewFromZip always validates in ModeBuild (zip = built package). + // Constructor must succeed even though the source fixture may not pass + // build-mode validation. + _, err := NewFromZip(zipPath) require.NoError(t, err) - newErr := newV.Validate() - - assertErrorsEqual(t, legacyErr, newErr) } From d99960266307d33df65c757dfa59da82e6014a86 Mon Sep 17 00:00:00 2001 From: Tere Date: Tue, 2 Jun 2026 08:58:05 +0200 Subject: [PATCH 09/30] Add support for mode-aware constructors and validation APIs in changelog - Documented the addition of mode-aware constructors and validation APIs. - Updated changelog to reflect enhancements made in the package specification. --- spec/changelog.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/changelog.yml b/spec/changelog.yml index 0f846785f..bde89c885 100644 --- a/spec/changelog.yml +++ b/spec/changelog.yml @@ -8,6 +8,9 @@ - description: Add support for semantic_text field definition. type: enhancement link: https://github.com/elastic/package-spec/pull/807 + - description: Add support for mode-aware constructors and validation APIs. + type: enhancement + link: https://github.com/elastic/package-spec/pull/1177 - version: 3.6.3 changes: - description: Add optional `release` field to agentless deployment mode to explicitly declare its release stage. From fe83a0a21a390bcbf2b2b137c16df349c28d2959 Mon Sep 17 00:00:00 2001 From: Tere Date: Tue, 2 Jun 2026 09:46:12 +0200 Subject: [PATCH 10/30] Address PR review: fix API semantics, tests, and remove private newFromZip - NewFromPath/NewFromFS now reject invalid modes at construction time - NewFromFS ModeLegacy preserves a pre-wrapped *linkedfiles.FS (matching old validateFromFS behaviour); ModeBuild always applies BlockFS unconditionally - Collapse private newFromZip into NewFromZip; ValidateFromZip delegates to NewFromZip directly, consistent with the other deprecated wrappers - Replace tautological TestLegacyPreservation_* tests with focused tests for the new API behaviours: invalid mode rejection, ModeBuild link blocking, ModeLegacy/ModeSource linked-FS preservation - Add good_v3 to TestValidateWarnings to exercise the link-resolution path Co-Authored-By: Claude Sonnet 4.6 --- code/go/pkg/validator/api.go | 54 +++++--- code/go/pkg/validator/api_test.go | 171 +++++++++++------------- code/go/pkg/validator/validator.go | 2 +- code/go/pkg/validator/validator_test.go | 1 + 4 files changed, 114 insertions(+), 114 deletions(-) diff --git a/code/go/pkg/validator/api.go b/code/go/pkg/validator/api.go index 50730b445..0144758dc 100644 --- a/code/go/pkg/validator/api.go +++ b/code/go/pkg/validator/api.go @@ -33,9 +33,9 @@ type Validator struct { // Option configures a Validator. type Option func(*Validator) -// WithWarningsAsErrors makes validation warnings count as errors when v is true. -func WithWarningsAsErrors(v bool) Option { - return func(va *Validator) { va.warningsAsErrors = v } +// WithWarningsAsErrors makes validation warnings count as errors when enabled is true. +func WithWarningsAsErrors(enabled bool) Option { + return func(v *Validator) { v.warningsAsErrors = enabled } } // NewFromPath returns a Validator for the package rooted at packageRootPath. @@ -43,6 +43,10 @@ func WithWarningsAsErrors(v bool) Option { // For ModeLegacy and ModeSource the filesystem honours linked (.link) files; // for ModeBuild linked files are blocked (matching a built package artifact). func NewFromPath(mode Mode, packageRootPath string, opts ...Option) (*Validator, error) { + if !mode.Valid() { + return nil, fmt.Errorf("invalid validation mode %q", mode) + } + info, err := os.Stat(packageRootPath) if err != nil { return nil, fmt.Errorf("invalid package path %q: %w", packageRootPath, err) @@ -68,15 +72,14 @@ func NewFromPath(mode Mode, packageRootPath string, opts ...Option) (*Validator, // Validation always runs in ModeBuild; source-only artifacts (_dev/, .link files, // external: ecs references) are therefore rejected. // +// NOTE: ModeBuild-specific validation rules are not yet implemented; ModeBuild +// and ModeLegacy currently produce identical rule sets. This is intentional — +// the mode is set here so that future PRs can attach build-only rules without +// changing the public API. +// // The returned Validator owns the underlying zip reader; calling Validate closes it. // Do not call Validate more than once on a Validator created by NewFromZip. func NewFromZip(zipPath string, opts ...Option) (_ *Validator, err error) { - return newFromZip(ModeBuild, zipPath, opts...) -} - -// newFromZip is the internal constructor used by both NewFromZip (ModeBuild) and -// the deprecated ValidateFromZip wrapper (ModeLegacy). -func newFromZip(mode Mode, zipPath string, opts ...Option) (_ *Validator, err error) { r, openErr := zip.OpenReader(zipPath) if openErr != nil { return nil, fmt.Errorf("failed to open zip file (%s): %w", zipPath, openErr) @@ -105,16 +108,24 @@ func newFromZip(mode Mode, zipPath string, opts ...Option) (_ *Validator, err er // Zip archives contain built packages; linked files are always blocked. fsys := linkedfiles.NewBlockFS(subDir) - return buildValidator(mode, zipPath, fsys, r, opts), nil + return buildValidator(ModeBuild, zipPath, fsys, r, opts), nil } // NewFromFS returns a Validator for the package accessible through fsys at location. // -// For ModeSource, if fsys is not already a *linkedfiles.FS, it is wrapped with -// linkedfiles.NewFS so that .link files are resolved transparently (consistent -// with ModeSource's contract). For ModeLegacy and ModeBuild, a BlockFS is applied -// instead — matching the behaviour of ValidateFromFS. +// Linked-file handling depends on mode: +// - ModeSource: if fsys is not already a *linkedfiles.FS it is wrapped with +// linkedfiles.NewFS so .link files are resolved transparently. +// - ModeLegacy: a pre-wrapped *linkedfiles.FS is preserved as-is (links +// resolved); any other filesystem is wrapped with BlockFS. This matches the +// behaviour of the deprecated ValidateFromFS. +// - ModeBuild: BlockFS is always applied, even if fsys is already a +// *linkedfiles.FS, so linked files are unconditionally rejected. func NewFromFS(mode Mode, location string, fsys fs.FS, opts ...Option) (*Validator, error) { + if !mode.Valid() { + return nil, fmt.Errorf("invalid validation mode %q", mode) + } + info, err := fs.Stat(fsys, ".") if err != nil { return nil, fmt.Errorf("invalid package filesystem at %q: %w", location, err) @@ -123,10 +134,19 @@ func NewFromFS(mode Mode, location string, fsys fs.FS, opts ...Option) (*Validat return nil, fmt.Errorf("invalid package filesystem at %q: root is not a directory", location) } - if _, ok := fsys.(*linkedfiles.FS); !ok { - if mode == ModeSource { + _, isLinkedFS := fsys.(*linkedfiles.FS) + switch mode { + case ModeSource: + if !isLinkedFS { fsys = linkedfiles.NewFS(location, fsys) - } else { + } + case ModeBuild: + // Always block, even a pre-wrapped *linkedfiles.FS. + fsys = linkedfiles.NewBlockFS(fsys) + default: // ModeLegacy + // Preserve a pre-wrapped *linkedfiles.FS; block everything else. + // Matches the old validateFromFS behaviour. + if !isLinkedFS { fsys = linkedfiles.NewBlockFS(fsys) } } diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go index f6dca8590..d6f96ba3b 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -6,73 +6,21 @@ package validator import ( "archive/zip" + "errors" "io/fs" "os" "path/filepath" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" ) // testPackagesDir is the root of the fixture packages used in integration tests. const testPackagesDir = "../../../../test/packages" -// ----------------------------------------------------------------------- -// Helpers -// ----------------------------------------------------------------------- - -// assertErrorsEqual asserts that legacy and newAPI produce the same set of -// errors (both nil, or both non-nil with identical error messages). -// -// ValidationErrors are compared as unordered sets because the internal -// validator iterates Go maps whose iteration order is non-deterministic. -func assertErrorsEqual(t *testing.T, legacy, newAPI error) { - t.Helper() - switch { - case legacy == nil && newAPI == nil: - // both happy — pass - case legacy == nil: - t.Fatalf("legacy returned nil but new API returned: %v", newAPI) - case newAPI == nil: - t.Fatalf("new API returned nil but legacy returned: %v", legacy) - default: - assert.ElementsMatch(t, - extractMessages(legacy), - extractMessages(newAPI), - "error sets differ between legacy and new API") - } -} - -// extractMessages unpacks a specerrors.ValidationErrors into individual -// message strings, or wraps a plain error in a single-element slice. -func extractMessages(err error) []string { - if verrs, ok := err.(specerrors.ValidationErrors); ok { - msgs := make([]string, len(verrs)) - for i, ve := range verrs { - msgs[i] = ve.Error() - } - return msgs - } - return []string{err.Error()} -} - -// listTestPackages returns the names of every directory under testPackagesDir. -func listTestPackages(t *testing.T) []string { - t.Helper() - entries, err := os.ReadDir(testPackagesDir) - require.NoError(t, err) - var names []string - for _, e := range entries { - if e.IsDir() { - names = append(names, e.Name()) - } - } - return names -} - // createPackageZip creates a temporary zip file containing the package at // packagePath under a single top-level directory (matching elastic-package // build output). Returns the path to the zip. @@ -123,56 +71,32 @@ func createPackageZip(t *testing.T, packagePath string) string { } // ----------------------------------------------------------------------- -// TestLegacyPreservation_FromPath +// TestNewFromPath_RejectsInvalidMode // -// For every package under test/packages/, asserts that the new -// NewFromPath(ModeLegacy, ...).Validate() produces byte-for-byte identical -// output to the existing ValidateFromPath(). +// NewFromPath must validate the mode eagerly and return an error at +// construction time, not defer the failure to Validate(). // ----------------------------------------------------------------------- -func TestLegacyPreservation_FromPath(t *testing.T) { - for _, pkgName := range listTestPackages(t) { - t.Run(pkgName, func(t *testing.T) { - t.Parallel() - path := filepath.Join(testPackagesDir, pkgName) - - legacyErr := ValidateFromPath(path) - newV, err := NewFromPath(ModeLegacy, path) - require.NoError(t, err) - newErr := newV.Validate() - - assertErrorsEqual(t, legacyErr, newErr) - }) - } +func TestNewFromPath_RejectsInvalidMode(t *testing.T) { + v, err := NewFromPath(Mode("invalid"), filepath.Join(testPackagesDir, "good")) + require.Nil(t, v) + require.ErrorContains(t, err, `invalid validation mode "invalid"`) } // ----------------------------------------------------------------------- -// TestLegacyPreservation_FromFS +// TestNewFromFS_RejectsInvalidMode // -// Parametrized over the same packages: ValidateFromFS with os.DirFS must -// match NewFromFS(ModeLegacy, ...) with the same filesystem. +// Same contract as TestNewFromPath_RejectsInvalidMode for NewFromFS. // ----------------------------------------------------------------------- -func TestLegacyPreservation_FromFS(t *testing.T) { - for _, pkgName := range listTestPackages(t) { - t.Run(pkgName, func(t *testing.T) { - t.Parallel() - path := filepath.Join(testPackagesDir, pkgName) - - // Use a fresh os.DirFS for each call to avoid any shared-state - // side-effects (both calls are read-only, but being explicit is cleaner). - legacyErr := ValidateFromFS(path, os.DirFS(path)) - newV, err := NewFromFS(ModeLegacy, path, os.DirFS(path)) - require.NoError(t, err) - newErr := newV.Validate() - - assertErrorsEqual(t, legacyErr, newErr) - }) - } +func TestNewFromFS_RejectsInvalidMode(t *testing.T) { + v, err := NewFromFS(Mode("invalid"), ".", os.DirFS(".")) + require.Nil(t, v) + require.ErrorContains(t, err, `invalid validation mode "invalid"`) } // ----------------------------------------------------------------------- -// TestLegacyPreservation_FromZip (constructor smoke test) +// TestNewFromZip_ConstructorSucceeds // // Zips up test/packages/good and verifies that NewFromZip constructs a // Validator without error. NewFromZip always runs in ModeBuild; the good @@ -180,13 +104,68 @@ func TestLegacyPreservation_FromFS(t *testing.T) { // constructor itself must succeed. // ----------------------------------------------------------------------- -func TestLegacyPreservation_FromZip(t *testing.T) { +func TestNewFromZip_ConstructorSucceeds(t *testing.T) { packagePath := filepath.Join(testPackagesDir, "good") zipPath := createPackageZip(t, packagePath) - // NewFromZip always validates in ModeBuild (zip = built package). - // Constructor must succeed even though the source fixture may not pass - // build-mode validation. _, err := NewFromZip(zipPath) require.NoError(t, err) } + +// ----------------------------------------------------------------------- +// TestNewFromFS_BuildModeBlocksLinksEvenWithPreWrappedLinkedFS +// +// ModeBuild must unconditionally block .link files, even when the caller +// passes a pre-wrapped *linkedfiles.FS that would otherwise resolve them. +// ----------------------------------------------------------------------- + +func TestNewFromFS_BuildModeBlocksLinksEvenWithPreWrappedLinkedFS(t *testing.T) { + packagePath := filepath.Join(testPackagesDir, "with_links") + linkedFS := linkedfiles.NewFS(packagePath, os.DirFS(packagePath)) + + v, err := NewFromFS(ModeBuild, packagePath, linkedFS) + require.NoError(t, err) + + errs := v.Validate() + require.Error(t, errs) + + vErrs, ok := errs.(specerrors.ValidationErrors) + require.True(t, ok, "expected ValidationErrors, got %T: %v", errs, errs) + + for _, e := range vErrs { + if errors.Is(e, linkedfiles.ErrUnsupportedLinkFile) { + return + } + } + t.Fatal("expected at least one ErrUnsupportedLinkFile — ModeBuild must replace *linkedfiles.FS with BlockFS") +} + +// ----------------------------------------------------------------------- +// TestNewFromFS_LegacyAndSourceModePreservePreWrappedLinkedFS +// +// For ModeLegacy and ModeSource, a pre-wrapped *linkedfiles.FS must be +// preserved so .link files are resolved rather than blocked. +// ModeLegacy matches the old validateFromFS behaviour; ModeSource is the +// explicit source-tree contract. +// ----------------------------------------------------------------------- + +func TestNewFromFS_LegacyAndSourceModePreservePreWrappedLinkedFS(t *testing.T) { + packagePath := filepath.Join(testPackagesDir, "with_links") + + for _, mode := range []Mode{ModeLegacy, ModeSource} { + t.Run(string(mode), func(t *testing.T) { + linkedFS := linkedfiles.NewFS(packagePath, os.DirFS(packagePath)) + v, err := NewFromFS(mode, packagePath, linkedFS) + require.NoError(t, err) + + errs := v.Validate() + if vErrs, ok := errs.(specerrors.ValidationErrors); ok { + for _, e := range vErrs { + if errors.Is(e, linkedfiles.ErrUnsupportedLinkFile) { + t.Fatalf("%s with a pre-wrapped *linkedfiles.FS should resolve .link files, not block them", mode) + } + } + } + }) + } +} diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index 2d0e51884..aa718105d 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -22,7 +22,7 @@ func ValidateFromPath(packageRootPath string) error { // // Deprecated: use NewFromZip(packagePath).Validate() instead. func ValidateFromZip(packagePath string) error { - v, err := newFromZip(ModeLegacy, packagePath) + v, err := NewFromZip(packagePath) if err != nil { return err } diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index 704ce1acd..db02e5986 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -938,6 +938,7 @@ func TestValidateWarnings(t *testing.T) { tests := map[string][]string{ "good": {}, "good_v2": {}, + "good_v3": {}, "visualizations_by_reference": { "references found in dashboard kibana/dashboard/visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d.json: visualizations_by_reference-5e1a01ff-6f9a-41c1-b7ad-326472db42b6 (visualization), visualizations_by_reference-8287a5d5-1576-4f3a-83c4-444e9058439b (lens) (SVR00004)", }, From 491658c4d9747f75138acffb0ea93a5f18414e3e Mon Sep 17 00:00:00 2001 From: Tere Date: Tue, 2 Jun 2026 10:21:13 +0200 Subject: [PATCH 11/30] Fix TestNewFromZip_ConstructorSucceeds file handle leak on Windows NewFromZip owns the zip reader via closer; without calling Validate() the reader stays open and t.TempDir cleanup fails on Windows with "file is being used by another process". Call Validate() to close the owned handle. Co-Authored-By: Claude Sonnet 4.6 --- code/go/pkg/validator/api_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go index d6f96ba3b..b7c118168 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -108,8 +108,11 @@ func TestNewFromZip_ConstructorSucceeds(t *testing.T) { packagePath := filepath.Join(testPackagesDir, "good") zipPath := createPackageZip(t, packagePath) - _, err := NewFromZip(zipPath) + v, err := NewFromZip(zipPath) require.NoError(t, err) + // Validate() closes the owned zip reader. Without this, the file handle + // stays open and t.TempDir cleanup fails on Windows. + _ = v.Validate() } // ----------------------------------------------------------------------- From 15750f59f0228624a46da019554e949a533e7a1f Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 3 Jun 2026 08:44:03 +0200 Subject: [PATCH 12/30] Update .gitignore to remove .scratch directory entry --- .gitignore | 2 -- 1 file changed, 2 deletions(-) diff --git a/.gitignore b/.gitignore index 94f3e524c..0501dd988 100644 --- a/.gitignore +++ b/.gitignore @@ -6,5 +6,3 @@ fuzz /build/ .vscode/ .DS_Store - -.scratch/ \ No newline at end of file From fcaf8c24260a05392630b0819380b932b4305002 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 3 Jun 2026 08:55:03 +0200 Subject: [PATCH 13/30] Refactor Validator constructors to remove Option parameter - Removed the Option parameter from NewFromPath, NewFromZip, and NewFromFS constructors, simplifying their signatures. - Updated internal buildValidator function to reflect the changes, eliminating the need for options. - Adjusted tests to align with the new constructor signatures, ensuring proper validation behavior without options. --- code/go/pkg/validator/api.go | 26 ++++++++----------------- code/go/pkg/validator/validator_test.go | 4 ++-- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/code/go/pkg/validator/api.go b/code/go/pkg/validator/api.go index 0144758dc..8799744e5 100644 --- a/code/go/pkg/validator/api.go +++ b/code/go/pkg/validator/api.go @@ -30,19 +30,11 @@ type Validator struct { closer io.Closer } -// Option configures a Validator. -type Option func(*Validator) - -// WithWarningsAsErrors makes validation warnings count as errors when enabled is true. -func WithWarningsAsErrors(enabled bool) Option { - return func(v *Validator) { v.warningsAsErrors = enabled } -} - // NewFromPath returns a Validator for the package rooted at packageRootPath. // // For ModeLegacy and ModeSource the filesystem honours linked (.link) files; // for ModeBuild linked files are blocked (matching a built package artifact). -func NewFromPath(mode Mode, packageRootPath string, opts ...Option) (*Validator, error) { +func NewFromPath(mode Mode, packageRootPath string) (*Validator, error) { if !mode.Valid() { return nil, fmt.Errorf("invalid validation mode %q", mode) } @@ -62,7 +54,7 @@ func NewFromPath(mode Mode, packageRootPath string, opts ...Option) (*Validator, // ModeLegacy and ModeSource: resolve linked files transparently. fsys = linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) } - return buildValidator(mode, packageRootPath, fsys, nil, opts), nil + return buildValidator(mode, packageRootPath, fsys, nil), nil } // NewFromZip returns a Validator for the package stored in the zip file at zipPath. @@ -79,7 +71,7 @@ func NewFromPath(mode Mode, packageRootPath string, opts ...Option) (*Validator, // // The returned Validator owns the underlying zip reader; calling Validate closes it. // Do not call Validate more than once on a Validator created by NewFromZip. -func NewFromZip(zipPath string, opts ...Option) (_ *Validator, err error) { +func NewFromZip(zipPath string) (_ *Validator, err error) { r, openErr := zip.OpenReader(zipPath) if openErr != nil { return nil, fmt.Errorf("failed to open zip file (%s): %w", zipPath, openErr) @@ -108,7 +100,7 @@ func NewFromZip(zipPath string, opts ...Option) (_ *Validator, err error) { // Zip archives contain built packages; linked files are always blocked. fsys := linkedfiles.NewBlockFS(subDir) - return buildValidator(ModeBuild, zipPath, fsys, r, opts), nil + return buildValidator(ModeBuild, zipPath, fsys, r), nil } // NewFromFS returns a Validator for the package accessible through fsys at location. @@ -121,7 +113,7 @@ func NewFromZip(zipPath string, opts ...Option) (_ *Validator, err error) { // behaviour of the deprecated ValidateFromFS. // - ModeBuild: BlockFS is always applied, even if fsys is already a // *linkedfiles.FS, so linked files are unconditionally rejected. -func NewFromFS(mode Mode, location string, fsys fs.FS, opts ...Option) (*Validator, error) { +func NewFromFS(mode Mode, location string, fsys fs.FS) (*Validator, error) { if !mode.Valid() { return nil, fmt.Errorf("invalid validation mode %q", mode) } @@ -150,11 +142,11 @@ func NewFromFS(mode Mode, location string, fsys fs.FS, opts ...Option) (*Validat fsys = linkedfiles.NewBlockFS(fsys) } } - return buildValidator(mode, location, fsys, nil, opts), nil + return buildValidator(mode, location, fsys, nil), nil } // buildValidator is the shared internal constructor. -func buildValidator(mode Mode, location string, fsys fs.FS, closer io.Closer, opts []Option) *Validator { +func buildValidator(mode Mode, location string, fsys fs.FS, closer io.Closer) *Validator { v := &Validator{ mode: mode, location: location, @@ -162,9 +154,7 @@ func buildValidator(mode Mode, location string, fsys fs.FS, closer io.Closer, op warningsAsErrors: common.IsDefinedWarningsAsErrors(), closer: closer, } - for _, opt := range opts { - opt(v) - } + return v } diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index db02e5986..1defe4020 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -951,12 +951,12 @@ func TestValidateWarnings(t *testing.T) { "good_readme_structure": {}, } - warningsAsErrros := true + t.Setenv("PACKAGE_SPEC_WARNINGS_AS_ERRORS", "true") for pkgName, expectedWarnContains := range tests { t.Run(pkgName, func(t *testing.T) { t.Parallel() pkgRootPath := path.Join("..", "..", "..", "..", "test", "packages", pkgName) - v, err := NewFromFS(ModeLegacy, pkgRootPath, linkedfiles.NewFS(pkgRootPath, os.DirFS(pkgRootPath)), WithWarningsAsErrors(warningsAsErrros)) + v, err := NewFromFS(ModeLegacy, pkgRootPath, linkedfiles.NewFS(pkgRootPath, os.DirFS(pkgRootPath))) require.NoError(t, err) errs := v.Validate() if len(expectedWarnContains) == 0 { From f4e266e9c89c52e7ff9c21cc9759c976bb24e584 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 3 Jun 2026 11:22:22 +0200 Subject: [PATCH 14/30] refactor POV on modes validation --- code/go/internal/validator/spec.go | 30 ++-- code/go/internal/validator/spec_test.go | 7 +- code/go/pkg/validator/api.go | 193 ------------------------ code/go/pkg/validator/api_test.go | 174 --------------------- code/go/pkg/validator/limits_test.go | 8 +- code/go/pkg/validator/validator.go | 119 ++++++++++++--- code/go/pkg/validator/validator_test.go | 17 ++- 7 files changed, 142 insertions(+), 406 deletions(-) delete mode 100644 code/go/pkg/validator/api.go delete mode 100644 code/go/pkg/validator/api_test.go diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index a52c0b60f..82dde2138 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -47,20 +47,15 @@ type validationRules []validationRule // GASpecCheckVersion represents minimum version to start checking for unreleased version of the spec var GASpecCheckVersion = semver.MustParse("3.0.1") -// NewSpec creates a new Spec for the given version and validation mode. -// If mode is empty, it defaults to modes.Legacy. -func NewSpec(version semver.Version, mode modes.Mode) (*Spec, error) { - if mode == "" { - mode = modes.Legacy - } - if !mode.Valid() { - return nil, fmt.Errorf("invalid validation mode %q", mode) - } - +// newSpec creates a new Spec for the given version and validation mode. +func newSpec(version semver.Version, mode modes.Mode) (*Spec, error) { specVersion, err := spec.CheckVersion(version) if err != nil { return nil, fmt.Errorf("could not load specification for version [%s]: %w", version.String(), err) } + if !mode.Valid() { + return nil, fmt.Errorf("invalid validation mode %q", mode) + } // With more current versions this is reported as a filterable validation error for GA packages. if version.LessThan(GASpecCheckVersion) { @@ -79,6 +74,21 @@ func NewSpec(version semver.Version, mode modes.Mode) (*Spec, error) { return &s, nil } +// NewLegacySpec creates a new Spec for the given version using the legacy specification. +func NewLegacySpec(version semver.Version) (*Spec, error) { + return newSpec(version, modes.Legacy) +} + +// NewBuildSpec creates a new Spec for the given version using the build specification. +func NewBuildSpec(version semver.Version) (*Spec, error) { + return newSpec(version, modes.Build) +} + +// NewSourceSpec creates a new Spec for the given version using the source specification. +func NewSourceSpec(version semver.Version) (*Spec, error) { + return newSpec(version, modes.Source) +} + // ValidatePackage validates the given Package against the Spec func (s Spec) ValidatePackage(pkg packages.Package) specerrors.ValidationErrors { var errs specerrors.ValidationErrors diff --git a/code/go/internal/validator/spec_test.go b/code/go/internal/validator/spec_test.go index 8abfd28a8..3eced249f 100644 --- a/code/go/internal/validator/spec_test.go +++ b/code/go/internal/validator/spec_test.go @@ -16,7 +16,7 @@ import ( "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" ) -func TestNewSpec(t *testing.T) { +func TestNewLegacySpec(t *testing.T) { tests := map[string]struct { expectedErrContains string }{ @@ -27,7 +27,7 @@ func TestNewSpec(t *testing.T) { } for version, test := range tests { - spec, err := NewSpec(*semver.MustParse(version), modes.Legacy) + spec, err := NewLegacySpec(*semver.MustParse(version)) if test.expectedErrContains == "" { require.NoError(t, err) require.IsType(t, &Spec{}, spec) @@ -45,6 +45,7 @@ func TestNoBetaFeatures_Package_GA(t *testing.T) { version: *semver.MustParse("1.0.0"), specVersion: *semver.MustParse("1.0.0"), fs: fspath.DirFS("testdata/fakespec"), + mode: modes.Legacy, } pkg, err := packages.NewPackage("testdata/packages/features_ga") require.NoError(t, err) @@ -59,6 +60,7 @@ func TestBetaFeatures_Package_GA(t *testing.T) { version: *semver.MustParse("1.0.0"), specVersion: *semver.MustParse("1.0.0"), fs: fspath.DirFS("testdata/fakespec"), + mode: modes.Legacy, } pkg, err := packages.NewPackage("testdata/packages/features_beta") require.NoError(t, err) @@ -135,6 +137,7 @@ func TestFolderSpecInvalid(t *testing.T) { version: c.version, specVersion: c.version, fs: c.spec, + mode: modes.Legacy, } pkg, err := packages.NewPackage(c.pkgPath) require.NoError(t, err) diff --git a/code/go/pkg/validator/api.go b/code/go/pkg/validator/api.go deleted file mode 100644 index 8799744e5..000000000 --- a/code/go/pkg/validator/api.go +++ /dev/null @@ -1,193 +0,0 @@ -// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one -// or more contributor license agreements. Licensed under the Elastic License; -// you may not use this file except in compliance with the Elastic License. - -package validator - -import ( - "archive/zip" - "errors" - "fmt" - "io" - "io/fs" - "os" - - "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" - "github.com/elastic/package-spec/v3/code/go/internal/packages" - internalvalidator "github.com/elastic/package-spec/v3/code/go/internal/validator" - "github.com/elastic/package-spec/v3/code/go/internal/validator/common" -) - -// Validator holds the configuration for a package validation run. -// Create one with NewFromPath, NewFromZip, or NewFromFS, then call Validate. -type Validator struct { - mode Mode - location string - fsys fs.FS - warningsAsErrors bool - // closer is non-nil when the Validator owns a resource (e.g. a zip reader) - // that must be released after Validate returns. - closer io.Closer -} - -// NewFromPath returns a Validator for the package rooted at packageRootPath. -// -// For ModeLegacy and ModeSource the filesystem honours linked (.link) files; -// for ModeBuild linked files are blocked (matching a built package artifact). -func NewFromPath(mode Mode, packageRootPath string) (*Validator, error) { - if !mode.Valid() { - return nil, fmt.Errorf("invalid validation mode %q", mode) - } - - info, err := os.Stat(packageRootPath) - if err != nil { - return nil, fmt.Errorf("invalid package path %q: %w", packageRootPath, err) - } - if !info.IsDir() { - return nil, fmt.Errorf("invalid package path %q: not a directory", packageRootPath) - } - - var fsys fs.FS - if mode == ModeBuild { - fsys = linkedfiles.NewBlockFS(os.DirFS(packageRootPath)) - } else { - // ModeLegacy and ModeSource: resolve linked files transparently. - fsys = linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) - } - return buildValidator(mode, packageRootPath, fsys, nil), nil -} - -// NewFromZip returns a Validator for the package stored in the zip file at zipPath. -// -// Zip files always contain built packages — they are the output format produced -// by elastic-package build and consumed by the package registry and Fleet. -// Validation always runs in ModeBuild; source-only artifacts (_dev/, .link files, -// external: ecs references) are therefore rejected. -// -// NOTE: ModeBuild-specific validation rules are not yet implemented; ModeBuild -// and ModeLegacy currently produce identical rule sets. This is intentional — -// the mode is set here so that future PRs can attach build-only rules without -// changing the public API. -// -// The returned Validator owns the underlying zip reader; calling Validate closes it. -// Do not call Validate more than once on a Validator created by NewFromZip. -func NewFromZip(zipPath string) (_ *Validator, err error) { - r, openErr := zip.OpenReader(zipPath) - if openErr != nil { - return nil, fmt.Errorf("failed to open zip file (%s): %w", zipPath, openErr) - } - // Close the reader on any error path; on success the Validator takes ownership. - defer func() { - if err != nil { - if cerr := r.Close(); cerr != nil { - err = errors.Join(err, cerr) - } - } - }() - - dirs, err := fs.ReadDir(r, ".") - if err != nil { - return nil, fmt.Errorf("failed to read root directory in zip file (%s): %w", zipPath, err) - } - if len(dirs) != 1 { - return nil, fmt.Errorf("a single directory is expected in zip file, %d found", len(dirs)) - } - - subDir, err := fs.Sub(r, dirs[0].Name()) - if err != nil { - return nil, err - } - - // Zip archives contain built packages; linked files are always blocked. - fsys := linkedfiles.NewBlockFS(subDir) - return buildValidator(ModeBuild, zipPath, fsys, r), nil -} - -// NewFromFS returns a Validator for the package accessible through fsys at location. -// -// Linked-file handling depends on mode: -// - ModeSource: if fsys is not already a *linkedfiles.FS it is wrapped with -// linkedfiles.NewFS so .link files are resolved transparently. -// - ModeLegacy: a pre-wrapped *linkedfiles.FS is preserved as-is (links -// resolved); any other filesystem is wrapped with BlockFS. This matches the -// behaviour of the deprecated ValidateFromFS. -// - ModeBuild: BlockFS is always applied, even if fsys is already a -// *linkedfiles.FS, so linked files are unconditionally rejected. -func NewFromFS(mode Mode, location string, fsys fs.FS) (*Validator, error) { - if !mode.Valid() { - return nil, fmt.Errorf("invalid validation mode %q", mode) - } - - info, err := fs.Stat(fsys, ".") - if err != nil { - return nil, fmt.Errorf("invalid package filesystem at %q: %w", location, err) - } - if !info.IsDir() { - return nil, fmt.Errorf("invalid package filesystem at %q: root is not a directory", location) - } - - _, isLinkedFS := fsys.(*linkedfiles.FS) - switch mode { - case ModeSource: - if !isLinkedFS { - fsys = linkedfiles.NewFS(location, fsys) - } - case ModeBuild: - // Always block, even a pre-wrapped *linkedfiles.FS. - fsys = linkedfiles.NewBlockFS(fsys) - default: // ModeLegacy - // Preserve a pre-wrapped *linkedfiles.FS; block everything else. - // Matches the old validateFromFS behaviour. - if !isLinkedFS { - fsys = linkedfiles.NewBlockFS(fsys) - } - } - return buildValidator(mode, location, fsys, nil), nil -} - -// buildValidator is the shared internal constructor. -func buildValidator(mode Mode, location string, fsys fs.FS, closer io.Closer) *Validator { - v := &Validator{ - mode: mode, - location: location, - fsys: fsys, - warningsAsErrors: common.IsDefinedWarningsAsErrors(), - closer: closer, - } - - return v -} - -// Validate runs package validation and returns any errors encountered. -// -// If the Validator was created by NewFromZip it owns an open zip reader; -// Validate closes it on return, so Validate must not be called more than once -// on such a Validator. -func (v *Validator) Validate() (err error) { - if v.closer != nil { - defer func() { - if cerr := v.closer.Close(); cerr != nil { - err = errors.Join(err, cerr) - } - }() - } - - pkg, err := packages.NewPackageFromFS(v.location, v.fsys) - if err != nil { - return err - } - if pkg.SpecVersion == nil { - return errors.New("could not determine specification version for package") - } - - spec, err := internalvalidator.NewSpec(*pkg.SpecVersion, v.mode) - if err != nil { - return err - } - spec.WarningsAsErrors = v.warningsAsErrors - - if errs := spec.ValidatePackage(*pkg); len(errs) > 0 { - return errs - } - return nil -} diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go deleted file mode 100644 index b7c118168..000000000 --- a/code/go/pkg/validator/api_test.go +++ /dev/null @@ -1,174 +0,0 @@ -// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one -// or more contributor license agreements. Licensed under the Elastic License; -// you may not use this file except in compliance with the Elastic License. - -package validator - -import ( - "archive/zip" - "errors" - "io/fs" - "os" - "path/filepath" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" - "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" -) - -// testPackagesDir is the root of the fixture packages used in integration tests. -const testPackagesDir = "../../../../test/packages" - -// createPackageZip creates a temporary zip file containing the package at -// packagePath under a single top-level directory (matching elastic-package -// build output). Returns the path to the zip. -func createPackageZip(t *testing.T, packagePath string) string { - t.Helper() - pkgName := filepath.Base(packagePath) - - tmpDir := t.TempDir() - zipPath := filepath.Join(tmpDir, pkgName+".zip") - - f, err := os.Create(zipPath) - require.NoError(t, err) - defer f.Close() - - zw := zip.NewWriter(f) - defer zw.Close() - - err = filepath.WalkDir(packagePath, func(path string, d fs.DirEntry, walkErr error) error { - if walkErr != nil { - return walkErr - } - if d.IsDir() { - return nil - } - rel, err := filepath.Rel(packagePath, path) - if err != nil { - return err - } - // Zip entries must use forward slashes; prefix with the package - // directory name so the zip contains exactly one top-level dir. - entryName := filepath.ToSlash(filepath.Join(pkgName, rel)) - - w, err := zw.Create(entryName) - if err != nil { - return err - } - - data, err := os.ReadFile(path) - if err != nil { - return err - } - _, err = w.Write(data) - return err - }) - require.NoError(t, err) - - return zipPath -} - -// ----------------------------------------------------------------------- -// TestNewFromPath_RejectsInvalidMode -// -// NewFromPath must validate the mode eagerly and return an error at -// construction time, not defer the failure to Validate(). -// ----------------------------------------------------------------------- - -func TestNewFromPath_RejectsInvalidMode(t *testing.T) { - v, err := NewFromPath(Mode("invalid"), filepath.Join(testPackagesDir, "good")) - require.Nil(t, v) - require.ErrorContains(t, err, `invalid validation mode "invalid"`) -} - -// ----------------------------------------------------------------------- -// TestNewFromFS_RejectsInvalidMode -// -// Same contract as TestNewFromPath_RejectsInvalidMode for NewFromFS. -// ----------------------------------------------------------------------- - -func TestNewFromFS_RejectsInvalidMode(t *testing.T) { - v, err := NewFromFS(Mode("invalid"), ".", os.DirFS(".")) - require.Nil(t, v) - require.ErrorContains(t, err, `invalid validation mode "invalid"`) -} - -// ----------------------------------------------------------------------- -// TestNewFromZip_ConstructorSucceeds -// -// Zips up test/packages/good and verifies that NewFromZip constructs a -// Validator without error. NewFromZip always runs in ModeBuild; the good -// fixture is a source package so Validate() may return errors, but the -// constructor itself must succeed. -// ----------------------------------------------------------------------- - -func TestNewFromZip_ConstructorSucceeds(t *testing.T) { - packagePath := filepath.Join(testPackagesDir, "good") - zipPath := createPackageZip(t, packagePath) - - v, err := NewFromZip(zipPath) - require.NoError(t, err) - // Validate() closes the owned zip reader. Without this, the file handle - // stays open and t.TempDir cleanup fails on Windows. - _ = v.Validate() -} - -// ----------------------------------------------------------------------- -// TestNewFromFS_BuildModeBlocksLinksEvenWithPreWrappedLinkedFS -// -// ModeBuild must unconditionally block .link files, even when the caller -// passes a pre-wrapped *linkedfiles.FS that would otherwise resolve them. -// ----------------------------------------------------------------------- - -func TestNewFromFS_BuildModeBlocksLinksEvenWithPreWrappedLinkedFS(t *testing.T) { - packagePath := filepath.Join(testPackagesDir, "with_links") - linkedFS := linkedfiles.NewFS(packagePath, os.DirFS(packagePath)) - - v, err := NewFromFS(ModeBuild, packagePath, linkedFS) - require.NoError(t, err) - - errs := v.Validate() - require.Error(t, errs) - - vErrs, ok := errs.(specerrors.ValidationErrors) - require.True(t, ok, "expected ValidationErrors, got %T: %v", errs, errs) - - for _, e := range vErrs { - if errors.Is(e, linkedfiles.ErrUnsupportedLinkFile) { - return - } - } - t.Fatal("expected at least one ErrUnsupportedLinkFile — ModeBuild must replace *linkedfiles.FS with BlockFS") -} - -// ----------------------------------------------------------------------- -// TestNewFromFS_LegacyAndSourceModePreservePreWrappedLinkedFS -// -// For ModeLegacy and ModeSource, a pre-wrapped *linkedfiles.FS must be -// preserved so .link files are resolved rather than blocked. -// ModeLegacy matches the old validateFromFS behaviour; ModeSource is the -// explicit source-tree contract. -// ----------------------------------------------------------------------- - -func TestNewFromFS_LegacyAndSourceModePreservePreWrappedLinkedFS(t *testing.T) { - packagePath := filepath.Join(testPackagesDir, "with_links") - - for _, mode := range []Mode{ModeLegacy, ModeSource} { - t.Run(string(mode), func(t *testing.T) { - linkedFS := linkedfiles.NewFS(packagePath, os.DirFS(packagePath)) - v, err := NewFromFS(mode, packagePath, linkedFS) - require.NoError(t, err) - - errs := v.Validate() - if vErrs, ok := errs.(specerrors.ValidationErrors); ok { - for _, e := range vErrs { - if errors.Is(e, linkedfiles.ErrUnsupportedLinkFile) { - t.Fatalf("%s with a pre-wrapped *linkedfiles.FS should resolve .link files, not block them", mode) - } - } - } - }) - } -} diff --git a/code/go/pkg/validator/limits_test.go b/code/go/pkg/validator/limits_test.go index 1ccf7abf3..83d30cdd0 100644 --- a/code/go/pkg/validator/limits_test.go +++ b/code/go/pkg/validator/limits_test.go @@ -15,9 +15,11 @@ import ( "testing" "time" + "github.com/Masterminds/semver/v3" "github.com/stretchr/testify/assert" "github.com/elastic/package-spec/v3/code/go/internal/spectypes" + "github.com/elastic/package-spec/v3/code/go/internal/validator" ) func TestLimitsValidation(t *testing.T) { @@ -116,7 +118,11 @@ func TestLimitsValidation(t *testing.T) { for _, c := range cases { t.Run(c.title, func(t *testing.T) { t.Parallel() - err := ValidateFromFS("test-package", c.fsys) + + specFn := func(version semver.Version) (*validator.Spec, error) { + return validator.NewLegacySpec(version) + } + err := validateFromFS("test-package", c.fsys, specFn) if c.valid { assert.NoError(t, err) } else { diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index aa718105d..915960b1a 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -4,39 +4,116 @@ package validator -import "io/fs" +import ( + "archive/zip" + "errors" + "fmt" + "io/fs" + "os" -// ValidateFromPath validates a package located at the given path against the -// appropriate specification and returns any errors. -// -// Deprecated: use NewFromPath(ModeLegacy, packageRootPath).Validate() instead. + "github.com/Masterminds/semver/v3" + + "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" + "github.com/elastic/package-spec/v3/code/go/internal/packages" + "github.com/elastic/package-spec/v3/code/go/internal/validator" + "github.com/elastic/package-spec/v3/code/go/internal/validator/common" +) + +type specFn func(semver.Version) (*validator.Spec, error) + +// ValidateFromPath validates a package located at the given path +// It uses the legacy specification. +// Deprecated: Use mode specific functions instead. func ValidateFromPath(packageRootPath string) error { - v, err := NewFromPath(ModeLegacy, packageRootPath) - if err != nil { - return err + // We wrap the fs.FS with a linkedfiles.LinksFS to handle linked files. + linksFS := linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) + + legacySpec := func(version semver.Version) (*validator.Spec, error) { + return validator.NewLegacySpec(version) + } + + return validateFromFS(packageRootPath, linksFS, legacySpec) +} + +// ValidateFromBuildPath validates a package built bundle located at the given path +// It uses the build specification. +func ValidateFromBuildPath(packageRootPath string) error { + fs := os.DirFS(packageRootPath) + + buildSpec := func(version semver.Version) (*validator.Spec, error) { + return validator.NewBuildSpec(version) } - return v.Validate() + return validateFromFS(packageRootPath, fs, buildSpec) } -// ValidateFromZip validates a package in zip format. -// -// Deprecated: use NewFromZip(packagePath).Validate() instead. +// ValidateFromSourcePath validates a package source tree located at the given path +// It uses the source specification. +func ValidateFromSourcePath(packageRootPath string) error { + // We wrap the fs.FS with a linkedfiles.LinksFS to handle linked files. + linksFS := linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) + + sourceSpec := func(version semver.Version) (*validator.Spec, error) { + return validator.NewSourceSpec(version) + } + + return validateFromFS(packageRootPath, linksFS, sourceSpec) +} + +// ValidateFromZip validates a package on its zip format. +// It uses the build specification. func ValidateFromZip(packagePath string) error { - v, err := NewFromZip(packagePath) + r, err := zip.OpenReader(packagePath) + if err != nil { + return fmt.Errorf("failed to open zip file (%s): %w", packagePath, err) + } + defer r.Close() + + dirs, err := fs.ReadDir(r, ".") + if err != nil { + return fmt.Errorf("failed to read root directory in zip file (%s): %w", packagePath, err) + } + if len(dirs) != 1 { + return fmt.Errorf("a single directory is expected in zip file, %d found", len(dirs)) + } + + subDir, err := fs.Sub(r, dirs[0].Name()) if err != nil { return err } - return v.Validate() + + buildSpec := func(version semver.Version) (*validator.Spec, error) { + return validator.NewBuildSpec(version) + } + + return validateFromFS(packagePath, subDir, buildSpec) } -// ValidateFromFS validates a package against the appropriate specification and -// returns any errors. Package files are obtained through the given filesystem. -// -// Deprecated: use NewFromFS(ModeLegacy, location, fsys).Validate() instead. -func ValidateFromFS(location string, fsys fs.FS) error { - v, err := NewFromFS(ModeLegacy, location, fsys) +// ValidateFromFS validates a package against the appropiate specification and returns any errors. +// Package files are obtained through the given filesystem. +func validateFromFS(location string, fsys fs.FS, specFn specFn) error { + // If we are not explicitly using the linkedfiles.FS, we wrap fsys with + // a linkedfiles.BlockFS to block the use of linked files. + if _, ok := fsys.(*linkedfiles.FS); !ok { + fsys = linkedfiles.NewBlockFS(fsys) + } + pkg, err := packages.NewPackageFromFS(location, fsys) + if err != nil { + return err + } + + if pkg.SpecVersion == nil { + return errors.New("could not determine specification version for package") + } + + spec, err := specFn(*pkg.SpecVersion) if err != nil { return err } - return v.Validate() + spec.WarningsAsErrors = common.IsDefinedWarningsAsErrors() + + if errs := spec.ValidatePackage(*pkg); len(errs) > 0 { + return errs + } + + return nil } diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index 1defe4020..73c3668cc 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -13,11 +13,13 @@ import ( "strings" "testing" + "github.com/Masterminds/semver/v3" cp "github.com/otiai10/copy" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" + "github.com/elastic/package-spec/v3/code/go/internal/validator" "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" ) @@ -938,7 +940,6 @@ func TestValidateWarnings(t *testing.T) { tests := map[string][]string{ "good": {}, "good_v2": {}, - "good_v3": {}, "visualizations_by_reference": { "references found in dashboard kibana/dashboard/visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d.json: visualizations_by_reference-5e1a01ff-6f9a-41c1-b7ad-326472db42b6 (visualization), visualizations_by_reference-8287a5d5-1576-4f3a-83c4-444e9058439b (lens) (SVR00004)", }, @@ -956,9 +957,12 @@ func TestValidateWarnings(t *testing.T) { t.Run(pkgName, func(t *testing.T) { t.Parallel() pkgRootPath := path.Join("..", "..", "..", "..", "test", "packages", pkgName) - v, err := NewFromFS(ModeLegacy, pkgRootPath, linkedfiles.NewFS(pkgRootPath, os.DirFS(pkgRootPath))) - require.NoError(t, err) - errs := v.Validate() + + legacySpec := func(version semver.Version) (*validator.Spec, error) { + return validator.NewLegacySpec(version) + } + + errs := validateFromFS(pkgRootPath, linkedfiles.NewFS(pkgRootPath, os.DirFS(pkgRootPath)), legacySpec) if len(expectedWarnContains) == 0 { require.NoError(t, errs) } else { @@ -1208,7 +1212,10 @@ func TestValidateForbiddenDataStreamName(t *testing.T) { } func TestLinksAreBlocked(t *testing.T) { - err := ValidateFromFS("test-package", newMockFS().WithLink()) + legacySpec := func(version semver.Version) (*validator.Spec, error) { + return validator.NewLegacySpec(version) + } + err := validateFromFS("test-package", newMockFS().WithLink(), legacySpec) errs, ok := err.(specerrors.ValidationErrors) require.True(t, ok) for _, err := range errs { From e9d0013d348539f8dd9488c4d3fc230d2011a24b Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 3 Jun 2026 11:23:01 +0200 Subject: [PATCH 15/30] remove unused public mode --- code/go/pkg/validator/mode.go | 28 ---------------------------- 1 file changed, 28 deletions(-) delete mode 100644 code/go/pkg/validator/mode.go diff --git a/code/go/pkg/validator/mode.go b/code/go/pkg/validator/mode.go deleted file mode 100644 index 0c99d574f..000000000 --- a/code/go/pkg/validator/mode.go +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one -// or more contributor license agreements. Licensed under the Elastic License; -// you may not use this file except in compliance with the Elastic License. - -package validator - -import "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" - -// Mode represents the validation mode used when creating a Validator. -type Mode = modes.Mode - -const ( - // ModeLegacy is the default validation mode; it preserves byte-for-byte - // identical behaviour with the existing ValidateFrom* functions. - ModeLegacy = modes.Legacy - - // ModeSource validates a package as a checked-out source tree. - // Linked (.link) files are resolved transparently. - ModeSource = modes.Source - - // ModeBuild validates a package as a build artifact. This is the correct - // mode for packages produced by elastic-package build, distributed as zip - // files, or served by the package registry. NewFromZip always uses this mode - // because zip files are by definition built packages. - // Linked files (.link) are blocked; source-only artifacts (_dev/, external: ecs - // field references) are rejected. - ModeBuild = modes.Build -) From cbd3b7c467957e9ce24d5da554bde6cfc166087a Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 3 Jun 2026 11:31:49 +0200 Subject: [PATCH 16/30] Improve documentation for validation API and modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Enhanced godoc comments for all public validator functions (ValidateFromPath, ValidateFromBuildPath, ValidateFromSourcePath, ValidateFromZip) with clearer descriptions of their purpose, which specification they use, and linked file handling - Fixed typo: "appropiate" → "appropriate" in validateFromFS comment - Updated modes documentation to accurately describe the three validation modes (Legacy, Source, Build) instead of referencing non-existent public re-exports - Added deprecation guidance directing users to mode-specific functions Co-Authored-By: Claude Haiku 4.5 --- code/go/internal/validator/modes/modes.go | 3 ++- code/go/pkg/validator/validator.go | 25 ++++++++++++++--------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/code/go/internal/validator/modes/modes.go b/code/go/internal/validator/modes/modes.go index 46cfada03..47fc28fee 100644 --- a/code/go/internal/validator/modes/modes.go +++ b/code/go/internal/validator/modes/modes.go @@ -7,7 +7,8 @@ package modes // Mode represents the validation mode used when validating a package. type Mode string -// Validation modes. The public API re-exports these as validator.Mode* constants. +// Validation modes for package validation: Legacy preserves existing behavior, +// Source validates checked-out source trees, and Build validates built artifacts. const ( Legacy Mode = "legacy" Source Mode = "source" diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index 915960b1a..b43c06c71 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -21,9 +21,10 @@ import ( type specFn func(semver.Version) (*validator.Spec, error) -// ValidateFromPath validates a package located at the given path -// It uses the legacy specification. -// Deprecated: Use mode specific functions instead. +// ValidateFromPath validates a package located at the given path using the legacy specification. +// This function preserves byte-for-byte identical behavior with existing validation workflows. +// Linked (.link) files are resolved transparently. +// Deprecated: Use ValidateFromSourcePath or ValidateFromBuildPath depending on the package type. func ValidateFromPath(packageRootPath string) error { // We wrap the fs.FS with a linkedfiles.LinksFS to handle linked files. linksFS := linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) @@ -35,8 +36,10 @@ func ValidateFromPath(packageRootPath string) error { return validateFromFS(packageRootPath, linksFS, legacySpec) } -// ValidateFromBuildPath validates a package built bundle located at the given path -// It uses the build specification. +// ValidateFromBuildPath validates a built package located at the given path. +// This function uses the build specification, appropriate for packages produced by +// elastic-package build, distributed as zip files, or served by the package registry. +// Linked files (.link) are blocked; source-only artifacts are rejected. func ValidateFromBuildPath(packageRootPath string) error { fs := os.DirFS(packageRootPath) @@ -46,8 +49,9 @@ func ValidateFromBuildPath(packageRootPath string) error { return validateFromFS(packageRootPath, fs, buildSpec) } -// ValidateFromSourcePath validates a package source tree located at the given path -// It uses the source specification. +// ValidateFromSourcePath validates a package source tree located at the given path. +// This function uses the source specification for checked-out source trees. +// Linked (.link) files are resolved transparently. func ValidateFromSourcePath(packageRootPath string) error { // We wrap the fs.FS with a linkedfiles.LinksFS to handle linked files. linksFS := linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) @@ -59,8 +63,9 @@ func ValidateFromSourcePath(packageRootPath string) error { return validateFromFS(packageRootPath, linksFS, sourceSpec) } -// ValidateFromZip validates a package on its zip format. -// It uses the build specification. +// ValidateFromZip validates a package in zip file format. +// This function uses the build specification since zip files are by definition built packages. +// Linked files (.link) are blocked; source-only artifacts are rejected. func ValidateFromZip(packagePath string) error { r, err := zip.OpenReader(packagePath) if err != nil { @@ -88,7 +93,7 @@ func ValidateFromZip(packagePath string) error { return validateFromFS(packagePath, subDir, buildSpec) } -// ValidateFromFS validates a package against the appropiate specification and returns any errors. +// validateFromFS validates a package against the appropriate specification and returns any errors. // Package files are obtained through the given filesystem. func validateFromFS(location string, fsys fs.FS, specFn specFn) error { // If we are not explicitly using the linkedfiles.FS, we wrap fsys with From 0f275c037008d7b0deb316d93766fbf5bc3d0f71 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 3 Jun 2026 11:32:49 +0200 Subject: [PATCH 17/30] Add unit tests for mode validation --- .../go/internal/validator/modes/modes_test.go | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 code/go/internal/validator/modes/modes_test.go diff --git a/code/go/internal/validator/modes/modes_test.go b/code/go/internal/validator/modes/modes_test.go new file mode 100644 index 000000000..12ac19cd8 --- /dev/null +++ b/code/go/internal/validator/modes/modes_test.go @@ -0,0 +1,45 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package modes + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValid(t *testing.T) { + tests := map[string]struct { + mode Mode + valid bool + }{ + "valid": { + mode: Legacy, + valid: true, + }, + "invalid": { + mode: Mode("invalid"), + valid: false, + }, + "source": { + mode: Source, + valid: true, + }, + "build": { + mode: Build, + valid: true, + }, + "": { + mode: Mode(""), + valid: false, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, test.valid, test.mode.Valid(), "mode %s should be %v", test.mode, test.valid) + }) + } +} From d1efd031e8fa09212531fee7aa93aaf929561c47 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 3 Jun 2026 11:43:10 +0200 Subject: [PATCH 18/30] Add integration tests for link file behavior across validation modes Co-Authored-By: Claude Sonnet 4.6 --- code/go/pkg/validator/validator_test.go | 26 +++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index 73c3668cc..22849d3f9 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -1227,6 +1227,32 @@ func TestLinksAreBlocked(t *testing.T) { t.Error("links should not be allowed in package") } +func TestLinksBehaviorAcrossModes(t *testing.T) { + withLinks := path.Join("..", "..", "..", "..", "test", "packages", "with_links") + good := path.Join("..", "..", "..", "..", "test", "packages", "good") + + t.Run("build_rejects_link_files", func(t *testing.T) { + t.Parallel() + err := ValidateFromBuildPath(withLinks) + require.Error(t, err) + errs, ok := err.(specerrors.ValidationErrors) + require.True(t, ok) + require.ErrorContains(t, errs, linkedfiles.ErrUnsupportedLinkFile.Error()) + }) + + t.Run("source_accepts_link_files", func(t *testing.T) { + t.Parallel() + err := ValidateFromSourcePath(withLinks) + require.NoError(t, err) + }) + + t.Run("build_accepts_package_without_links", func(t *testing.T) { + t.Parallel() + err := ValidateFromBuildPath(good) + require.NoError(t, err) + }) +} + func TestValidateHandlebarsFiles(t *testing.T) { tests := map[string]string{ "bad_input_hbs": "invalid handlebars template: error validating agent/input/input.yml.hbs: Parse error on line 10:\nExpecting OpenEndBlock, got: 'EOF'", From 4233331aa8b0fbb594d8127353ae3d6bbfeb3e5a Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 3 Jun 2026 14:17:41 +0200 Subject: [PATCH 19/30] Remove unused test case for package validation without links in TestLinksBehaviorAcrossModes --- code/go/pkg/validator/validator_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index 22849d3f9..d25d192dc 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -1229,7 +1229,6 @@ func TestLinksAreBlocked(t *testing.T) { func TestLinksBehaviorAcrossModes(t *testing.T) { withLinks := path.Join("..", "..", "..", "..", "test", "packages", "with_links") - good := path.Join("..", "..", "..", "..", "test", "packages", "good") t.Run("build_rejects_link_files", func(t *testing.T) { t.Parallel() @@ -1246,11 +1245,6 @@ func TestLinksBehaviorAcrossModes(t *testing.T) { require.NoError(t, err) }) - t.Run("build_accepts_package_without_links", func(t *testing.T) { - t.Parallel() - err := ValidateFromBuildPath(good) - require.NoError(t, err) - }) } func TestValidateHandlebarsFiles(t *testing.T) { From da2a3259ca6048a311fb6a5908820084e8c681af Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 4 Jun 2026 09:50:04 +0200 Subject: [PATCH 20/30] Remove wrapping around specFn --- code/go/pkg/validator/limits_test.go | 6 +----- code/go/pkg/validator/validator.go | 17 +++-------------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/code/go/pkg/validator/limits_test.go b/code/go/pkg/validator/limits_test.go index 83d30cdd0..a30f98de3 100644 --- a/code/go/pkg/validator/limits_test.go +++ b/code/go/pkg/validator/limits_test.go @@ -15,7 +15,6 @@ import ( "testing" "time" - "github.com/Masterminds/semver/v3" "github.com/stretchr/testify/assert" "github.com/elastic/package-spec/v3/code/go/internal/spectypes" @@ -119,10 +118,7 @@ func TestLimitsValidation(t *testing.T) { t.Run(c.title, func(t *testing.T) { t.Parallel() - specFn := func(version semver.Version) (*validator.Spec, error) { - return validator.NewLegacySpec(version) - } - err := validateFromFS("test-package", c.fsys, specFn) + err := validateFromFS("test-package", c.fsys, validator.NewLegacySpec) if c.valid { assert.NoError(t, err) } else { diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index b43c06c71..33064c5b8 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -29,11 +29,7 @@ func ValidateFromPath(packageRootPath string) error { // We wrap the fs.FS with a linkedfiles.LinksFS to handle linked files. linksFS := linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) - legacySpec := func(version semver.Version) (*validator.Spec, error) { - return validator.NewLegacySpec(version) - } - - return validateFromFS(packageRootPath, linksFS, legacySpec) + return validateFromFS(packageRootPath, linksFS, validator.NewLegacySpec) } // ValidateFromBuildPath validates a built package located at the given path. @@ -43,10 +39,7 @@ func ValidateFromPath(packageRootPath string) error { func ValidateFromBuildPath(packageRootPath string) error { fs := os.DirFS(packageRootPath) - buildSpec := func(version semver.Version) (*validator.Spec, error) { - return validator.NewBuildSpec(version) - } - return validateFromFS(packageRootPath, fs, buildSpec) + return validateFromFS(packageRootPath, fs, validator.NewBuildSpec) } // ValidateFromSourcePath validates a package source tree located at the given path. @@ -56,11 +49,7 @@ func ValidateFromSourcePath(packageRootPath string) error { // We wrap the fs.FS with a linkedfiles.LinksFS to handle linked files. linksFS := linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) - sourceSpec := func(version semver.Version) (*validator.Spec, error) { - return validator.NewSourceSpec(version) - } - - return validateFromFS(packageRootPath, linksFS, sourceSpec) + return validateFromFS(packageRootPath, linksFS, validator.NewSourceSpec) } // ValidateFromZip validates a package in zip file format. From 8539209e301d8fda75567b057074eb62f1a50647 Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 4 Jun 2026 11:50:22 +0200 Subject: [PATCH 21/30] Restore Option C Validator API with mode-embedded FS and fix link blocking - Mode becomes a first-class public type (not a string alias) carrying both the internal rule set and a wrapFS factory for path-based construction; external callers use ModeLegacy, ModeSource, ModeBuild only - NewFromPath delegates FS setup entirely to mode.wrapFS, removing the switch-on-mode logic from constructors - NewFromFS is now a "bring your own FS" constructor: mode selects validation rules only, no link-file wrapping is applied; callers are responsible for FS semantics (fixes the silent bypass where a *linkedfiles.FS passed to build mode would resolve instead of block links) - NewFromZip always runs ModeBuild with an explicit BlockFS; its zip reader is owned by the Validator and closed by Validate() with errors.Join (fixes the dropped r.Close() error previously flagged by errcheck) - Expose internal newSpec as NewSpec(version, mode) so Validator.Validate() can pass the stored mode.internal directly without a switch - ValidateFromPath and ValidateFromZip remain as zero-param thin wrappers; ValidateFromBuildPath and ValidateFromSourcePath are removed (replaced by NewFromPath(ModeBuild/ModeSource, ...)) - WithWarningsAsErrors Option overrides PACKAGE_SPEC_WARNINGS_AS_ERRORS env var Co-Authored-By: Claude Sonnet 4.6 --- code/go/internal/validator/spec.go | 19 +-- code/go/internal/validator/spec_test.go | 2 +- code/go/pkg/validator/limits_test.go | 7 +- code/go/pkg/validator/modes.go | 48 +++++++ code/go/pkg/validator/validator.go | 162 ++++++++++++++++-------- code/go/pkg/validator/validator_test.go | 119 +++++++++++++++-- 6 files changed, 268 insertions(+), 89 deletions(-) create mode 100644 code/go/pkg/validator/modes.go diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index 82dde2138..f9fc9f5b2 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -47,8 +47,8 @@ type validationRules []validationRule // GASpecCheckVersion represents minimum version to start checking for unreleased version of the spec var GASpecCheckVersion = semver.MustParse("3.0.1") -// newSpec creates a new Spec for the given version and validation mode. -func newSpec(version semver.Version, mode modes.Mode) (*Spec, error) { +// NewSpec creates a new Spec for the given version and validation mode. +func NewSpec(version semver.Version, mode modes.Mode) (*Spec, error) { specVersion, err := spec.CheckVersion(version) if err != nil { return nil, fmt.Errorf("could not load specification for version [%s]: %w", version.String(), err) @@ -74,21 +74,6 @@ func newSpec(version semver.Version, mode modes.Mode) (*Spec, error) { return &s, nil } -// NewLegacySpec creates a new Spec for the given version using the legacy specification. -func NewLegacySpec(version semver.Version) (*Spec, error) { - return newSpec(version, modes.Legacy) -} - -// NewBuildSpec creates a new Spec for the given version using the build specification. -func NewBuildSpec(version semver.Version) (*Spec, error) { - return newSpec(version, modes.Build) -} - -// NewSourceSpec creates a new Spec for the given version using the source specification. -func NewSourceSpec(version semver.Version) (*Spec, error) { - return newSpec(version, modes.Source) -} - // ValidatePackage validates the given Package against the Spec func (s Spec) ValidatePackage(pkg packages.Package) specerrors.ValidationErrors { var errs specerrors.ValidationErrors diff --git a/code/go/internal/validator/spec_test.go b/code/go/internal/validator/spec_test.go index 3eced249f..11a9e4b02 100644 --- a/code/go/internal/validator/spec_test.go +++ b/code/go/internal/validator/spec_test.go @@ -27,7 +27,7 @@ func TestNewLegacySpec(t *testing.T) { } for version, test := range tests { - spec, err := NewLegacySpec(*semver.MustParse(version)) + spec, err := NewSpec(*semver.MustParse(version), modes.Legacy) if test.expectedErrContains == "" { require.NoError(t, err) require.IsType(t, &Spec{}, spec) diff --git a/code/go/pkg/validator/limits_test.go b/code/go/pkg/validator/limits_test.go index a30f98de3..7f74a06a9 100644 --- a/code/go/pkg/validator/limits_test.go +++ b/code/go/pkg/validator/limits_test.go @@ -18,7 +18,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/elastic/package-spec/v3/code/go/internal/spectypes" - "github.com/elastic/package-spec/v3/code/go/internal/validator" ) func TestLimitsValidation(t *testing.T) { @@ -118,7 +117,11 @@ func TestLimitsValidation(t *testing.T) { t.Run(c.title, func(t *testing.T) { t.Parallel() - err := validateFromFS("test-package", c.fsys, validator.NewLegacySpec) + v, err := NewFromFS(ModeLegacy, "test-package", c.fsys) + if !assert.NoError(t, err) { + return + } + err = v.Validate() if c.valid { assert.NoError(t, err) } else { diff --git a/code/go/pkg/validator/modes.go b/code/go/pkg/validator/modes.go new file mode 100644 index 000000000..06a6e8abb --- /dev/null +++ b/code/go/pkg/validator/modes.go @@ -0,0 +1,48 @@ +package validator + +import ( + "io/fs" + + "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" + "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" +) + +// Mode represents the validation context: which rules apply and how linked files +// are handled when creating a Validator from a path. +// Use ModeLegacy, ModeSource, or ModeBuild; do not construct Mode directly. +type Mode struct { + internal modes.Mode + // wrapFS builds the filesystem for path-based validation (NewFromPath). + // It is not applied by NewFromFS, which takes the caller's filesystem as-is. + wrapFS func(location string, fsys fs.FS) fs.FS +} + +var ( + // ModeLegacy preserves byte-for-byte identical behaviour with ValidateFromPath. + // Linked (.link) files are resolved transparently. + ModeLegacy = Mode{ + internal: modes.Legacy, + wrapFS: func(location string, fsys fs.FS) fs.FS { + return linkedfiles.NewFS(location, fsys) + }, + } + + // ModeSource validates a package as a checked-out source tree. + // Linked (.link) files are resolved transparently. + ModeSource = Mode{ + internal: modes.Source, + wrapFS: func(location string, fsys fs.FS) fs.FS { + return linkedfiles.NewFS(location, fsys) + }, + } + + // ModeBuild validates a package as a build artifact — output of elastic-package + // build, a zip archive, or a package served by the registry. + // Linked (.link) files are unconditionally blocked. + ModeBuild = Mode{ + internal: modes.Build, + wrapFS: func(_ string, fsys fs.FS) fs.FS { + return linkedfiles.NewBlockFS(fsys) + }, + } +) diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index 33064c5b8..29c2f7b12 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -8,106 +8,158 @@ import ( "archive/zip" "errors" "fmt" + "io" "io/fs" "os" - "github.com/Masterminds/semver/v3" - "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" "github.com/elastic/package-spec/v3/code/go/internal/packages" - "github.com/elastic/package-spec/v3/code/go/internal/validator" + internalvalidator "github.com/elastic/package-spec/v3/code/go/internal/validator" "github.com/elastic/package-spec/v3/code/go/internal/validator/common" ) -type specFn func(semver.Version) (*validator.Spec, error) - -// ValidateFromPath validates a package located at the given path using the legacy specification. -// This function preserves byte-for-byte identical behavior with existing validation workflows. -// Linked (.link) files are resolved transparently. -// Deprecated: Use ValidateFromSourcePath or ValidateFromBuildPath depending on the package type. -func ValidateFromPath(packageRootPath string) error { - // We wrap the fs.FS with a linkedfiles.LinksFS to handle linked files. - linksFS := linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) - - return validateFromFS(packageRootPath, linksFS, validator.NewLegacySpec) +// Validator holds the configuration for a package validation run. +// Create one with NewFromPath, NewFromZip, or NewFromFS, then call Validate. +type Validator struct { + mode Mode + location string + fsys fs.FS + warningsAsErrors bool + // closer is non-nil when the Validator owns a resource (e.g. a zip reader) + // that must be released after Validate returns. + closer io.Closer } -// ValidateFromBuildPath validates a built package located at the given path. -// This function uses the build specification, appropriate for packages produced by -// elastic-package build, distributed as zip files, or served by the package registry. -// Linked files (.link) are blocked; source-only artifacts are rejected. -func ValidateFromBuildPath(packageRootPath string) error { - fs := os.DirFS(packageRootPath) +// Option configures a Validator. +type Option func(*Validator) - return validateFromFS(packageRootPath, fs, validator.NewBuildSpec) +// WithWarningsAsErrors makes validation warnings count as errors when enabled is true. +// This overrides the PACKAGE_SPEC_WARNINGS_AS_ERRORS environment variable. +func WithWarningsAsErrors(enabled bool) Option { + return func(v *Validator) { v.warningsAsErrors = enabled } } -// ValidateFromSourcePath validates a package source tree located at the given path. -// This function uses the source specification for checked-out source trees. -// Linked (.link) files are resolved transparently. -func ValidateFromSourcePath(packageRootPath string) error { - // We wrap the fs.FS with a linkedfiles.LinksFS to handle linked files. - linksFS := linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) - - return validateFromFS(packageRootPath, linksFS, validator.NewSourceSpec) +// NewFromPath returns a Validator for the package rooted at packageRootPath. +// The mode determines which validation rules apply and how linked files are handled. +func NewFromPath(mode Mode, packageRootPath string, opts ...Option) (*Validator, error) { + if !mode.internal.Valid() { + return nil, fmt.Errorf("invalid validation mode %q", mode.internal) + } + fsys := mode.wrapFS(packageRootPath, os.DirFS(packageRootPath)) + return buildValidator(mode, packageRootPath, fsys, nil, opts), nil } -// ValidateFromZip validates a package in zip file format. -// This function uses the build specification since zip files are by definition built packages. -// Linked files (.link) are blocked; source-only artifacts are rejected. -func ValidateFromZip(packagePath string) error { - r, err := zip.OpenReader(packagePath) - if err != nil { - return fmt.Errorf("failed to open zip file (%s): %w", packagePath, err) +// NewFromZip returns a Validator for the package stored in the zip file at zipPath. +// +// Zip files always contain built packages, so the validator always runs in +// ModeBuild; linked (.link) files are blocked. +// +// The returned Validator owns the underlying zip reader; Validate closes it. +// Do not call Validate more than once on a Validator created by NewFromZip. +func NewFromZip(zipPath string, opts ...Option) (_ *Validator, err error) { + r, openErr := zip.OpenReader(zipPath) + if openErr != nil { + return nil, fmt.Errorf("failed to open zip file (%s): %w", zipPath, openErr) } - defer r.Close() + defer func() { + if err != nil { + if cerr := r.Close(); cerr != nil { + err = errors.Join(err, cerr) + } + } + }() dirs, err := fs.ReadDir(r, ".") if err != nil { - return fmt.Errorf("failed to read root directory in zip file (%s): %w", packagePath, err) + return nil, fmt.Errorf("failed to read root directory in zip file (%s): %w", zipPath, err) } if len(dirs) != 1 { - return fmt.Errorf("a single directory is expected in zip file, %d found", len(dirs)) + return nil, fmt.Errorf("a single directory is expected in zip file, %d found", len(dirs)) } subDir, err := fs.Sub(r, dirs[0].Name()) if err != nil { - return err + return nil, err } - buildSpec := func(version semver.Version) (*validator.Spec, error) { - return validator.NewBuildSpec(version) + fsys := linkedfiles.NewBlockFS(subDir) + return buildValidator(ModeBuild, zipPath, fsys, r, opts), nil +} + +// NewFromFS returns a Validator for the package accessible through fsys at location. +// The mode determines which validation rules apply; fsys is used as-is without any +// link-file wrapping. Callers are responsible for filesystem semantics. +func NewFromFS(mode Mode, location string, fsys fs.FS, opts ...Option) (*Validator, error) { + if !mode.internal.Valid() { + return nil, fmt.Errorf("invalid validation mode %q", mode.internal) } + return buildValidator(mode, location, fsys, nil, opts), nil +} - return validateFromFS(packagePath, subDir, buildSpec) +func buildValidator(mode Mode, location string, fsys fs.FS, closer io.Closer, opts []Option) *Validator { + v := &Validator{ + mode: mode, + location: location, + fsys: fsys, + warningsAsErrors: common.IsDefinedWarningsAsErrors(), + closer: closer, + } + for _, opt := range opts { + opt(v) + } + return v } -// validateFromFS validates a package against the appropriate specification and returns any errors. -// Package files are obtained through the given filesystem. -func validateFromFS(location string, fsys fs.FS, specFn specFn) error { - // If we are not explicitly using the linkedfiles.FS, we wrap fsys with - // a linkedfiles.BlockFS to block the use of linked files. - if _, ok := fsys.(*linkedfiles.FS); !ok { - fsys = linkedfiles.NewBlockFS(fsys) +// Validate runs package validation and returns any errors encountered. +// +// If the Validator was created by NewFromZip it owns an open zip reader; +// Validate closes it on return, so Validate must not be called more than once. +func (v *Validator) Validate() (err error) { + if v.closer != nil { + defer func() { + if cerr := v.closer.Close(); cerr != nil { + err = errors.Join(err, cerr) + } + }() } - pkg, err := packages.NewPackageFromFS(location, fsys) + + pkg, err := packages.NewPackageFromFS(v.location, v.fsys) if err != nil { return err } - if pkg.SpecVersion == nil { return errors.New("could not determine specification version for package") } - spec, err := specFn(*pkg.SpecVersion) + spec, err := internalvalidator.NewSpec(*pkg.SpecVersion, v.mode.internal) if err != nil { return err } - spec.WarningsAsErrors = common.IsDefinedWarningsAsErrors() + spec.WarningsAsErrors = v.warningsAsErrors if errs := spec.ValidatePackage(*pkg); len(errs) > 0 { return errs } - return nil } + +// ValidateFromPath validates a package located at the given path using the legacy specification. +// Linked (.link) files are resolved transparently. +// Deprecated: Use NewFromPath with ModeSource or ModeBuild depending on the package type. +func ValidateFromPath(packageRootPath string) error { + v, err := NewFromPath(ModeLegacy, packageRootPath) + if err != nil { + return err + } + return v.Validate() +} + +// ValidateFromZip validates a package in zip file format using the build specification. +// Linked files (.link) are blocked; zip files are by definition built packages. +func ValidateFromZip(packagePath string) error { + v, err := NewFromZip(packagePath) + if err != nil { + return err + } + return v.Validate() +} diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index d25d192dc..3c0136de5 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -13,13 +13,13 @@ import ( "strings" "testing" - "github.com/Masterminds/semver/v3" + "archive/zip" + cp "github.com/otiai10/copy" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" - "github.com/elastic/package-spec/v3/code/go/internal/validator" "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" ) @@ -952,17 +952,14 @@ func TestValidateWarnings(t *testing.T) { "good_readme_structure": {}, } - t.Setenv("PACKAGE_SPEC_WARNINGS_AS_ERRORS", "true") for pkgName, expectedWarnContains := range tests { t.Run(pkgName, func(t *testing.T) { t.Parallel() pkgRootPath := path.Join("..", "..", "..", "..", "test", "packages", pkgName) - legacySpec := func(version semver.Version) (*validator.Spec, error) { - return validator.NewLegacySpec(version) - } - - errs := validateFromFS(pkgRootPath, linkedfiles.NewFS(pkgRootPath, os.DirFS(pkgRootPath)), legacySpec) + v, constructErr := NewFromFS(ModeLegacy, pkgRootPath, linkedfiles.NewFS(pkgRootPath, os.DirFS(pkgRootPath)), WithWarningsAsErrors(true)) + require.NoError(t, constructErr) + errs := v.Validate() if len(expectedWarnContains) == 0 { require.NoError(t, errs) } else { @@ -1212,10 +1209,10 @@ func TestValidateForbiddenDataStreamName(t *testing.T) { } func TestLinksAreBlocked(t *testing.T) { - legacySpec := func(version semver.Version) (*validator.Spec, error) { - return validator.NewLegacySpec(version) - } - err := validateFromFS("test-package", newMockFS().WithLink(), legacySpec) + // NewFromFS takes the FS as-is; wrap explicitly with BlockFS to enforce link blocking. + v, err := NewFromFS(ModeLegacy, "test-package", linkedfiles.NewBlockFS(newMockFS().WithLink())) + require.NoError(t, err) + err = v.Validate() errs, ok := err.(specerrors.ValidationErrors) require.True(t, ok) for _, err := range errs { @@ -1232,7 +1229,9 @@ func TestLinksBehaviorAcrossModes(t *testing.T) { t.Run("build_rejects_link_files", func(t *testing.T) { t.Parallel() - err := ValidateFromBuildPath(withLinks) + v, err := NewFromPath(ModeBuild, withLinks) + require.NoError(t, err) + err = v.Validate() require.Error(t, err) errs, ok := err.(specerrors.ValidationErrors) require.True(t, ok) @@ -1241,12 +1240,104 @@ func TestLinksBehaviorAcrossModes(t *testing.T) { t.Run("source_accepts_link_files", func(t *testing.T) { t.Parallel() - err := ValidateFromSourcePath(withLinks) + v, err := NewFromPath(ModeSource, withLinks) + require.NoError(t, err) + err = v.Validate() require.NoError(t, err) }) } +// createTestZip creates a temporary zip archive of the package at packagePath, +// placing all files under a single top-level directory (matching elastic-package +// build output). Returns the path to the created zip. +func createTestZip(t *testing.T, packagePath string) string { + t.Helper() + packageName := filepath.Base(packagePath) + + tmpDir := t.TempDir() + zipPath := filepath.Join(tmpDir, packageName+".zip") + + f, err := os.Create(zipPath) + require.NoError(t, err) + defer f.Close() + + zw := zip.NewWriter(f) + defer zw.Close() + + err = filepath.Walk(packagePath, func(walkPath string, info os.FileInfo, walkErr error) error { + if walkErr != nil { + return walkErr + } + if info.IsDir() { + return nil + } + rel, err := filepath.Rel(packagePath, walkPath) + if err != nil { + return err + } + entryName := filepath.ToSlash(filepath.Join(packageName, rel)) + w, err := zw.Create(entryName) + if err != nil { + return err + } + data, err := os.ReadFile(walkPath) + if err != nil { + return err + } + _, err = w.Write(data) + return err + }) + require.NoError(t, err) + return zipPath +} + +func TestNewFromPath_RejectsInvalidMode(t *testing.T) { + v, err := NewFromPath(Mode{}, filepath.Join("..", "..", "..", "..", "test", "packages", "good")) + require.Nil(t, v) + require.ErrorContains(t, err, "invalid validation mode") +} + +func TestNewFromFS_RejectsInvalidMode(t *testing.T) { + v, err := NewFromFS(Mode{}, ".", os.DirFS(".")) + require.Nil(t, v) + require.ErrorContains(t, err, "invalid validation mode") +} + +func TestNewFromZip_ConstructorSucceeds(t *testing.T) { + packagePath := filepath.Join("..", "..", "..", "..", "test", "packages", "good") + zipPath := createTestZip(t, packagePath) + + v, err := NewFromZip(zipPath) + require.NoError(t, err) + // Validate closes the owned zip reader; must be called to avoid leaking the handle. + _ = v.Validate() +} + +// TestNewFromFS_TakesFSAsIsRegardlessOfMode documents the NewFromFS contract: +// the provided filesystem is used as-is; the mode controls validation rules only, +// not link-file handling. Callers are responsible for FS semantics. +func TestNewFromFS_TakesFSAsIsRegardlessOfMode(t *testing.T) { + withLinks := filepath.Join("..", "..", "..", "..", "test", "packages", "with_links") + + for _, mode := range []Mode{ModeLegacy, ModeSource, ModeBuild} { + t.Run(string(mode.internal), func(t *testing.T) { + // Passing a resolving FS: links are resolved regardless of mode. + resolving := linkedfiles.NewFS(withLinks, os.DirFS(withLinks)) + v, err := NewFromFS(mode, withLinks, resolving) + require.NoError(t, err) + + errs := v.Validate() + if vErrs, ok := errs.(specerrors.ValidationErrors); ok { + for _, e := range vErrs { + require.False(t, errors.Is(e, linkedfiles.ErrUnsupportedLinkFile), + "NewFromFS must not override caller FS semantics: mode %s should not block links when a resolving FS is provided", mode.internal) + } + } + }) + } +} + func TestValidateHandlebarsFiles(t *testing.T) { tests := map[string]string{ "bad_input_hbs": "invalid handlebars template: error validating agent/input/input.yml.hbs: Parse error on line 10:\nExpecting OpenEndBlock, got: 'EOF'", From b0049ffb188c118c2d55deded4d3f5d72e8ae152 Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 4 Jun 2026 12:02:01 +0200 Subject: [PATCH 22/30] Improve godoc comments for validation modes API - Give each Mode constant its own godoc line so go doc can attach them - Describe Mode in terms of what it controls, not just what it is - Remove ModeLegacy anchor to deprecated ValidateFromPath function - Add rule-gating dimension to ModeSource and ModeBuild docs - Clarify WithWarningsAsErrors overrides the env var in both directions - Note error conditions on NewFromPath, NewFromFS, and NewSpec - Warn NewFromFS callers to wrap with NewBlockFS when using ModeBuild - Move single-call restriction note from Validate to NewFromZip only - Replace undefined term "build specification" with ModeBuild in ValidateFromZip - Expand ValidatePackage doc to mention both syntactic and semantic rules Co-Authored-By: Claude Sonnet 4.6 --- code/go/internal/validator/modes/modes.go | 13 +++++++++---- code/go/internal/validator/spec.go | 8 ++++++-- code/go/pkg/validator/modes.go | 13 ++++++++----- code/go/pkg/validator/validator.go | 21 ++++++++++++--------- 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/code/go/internal/validator/modes/modes.go b/code/go/internal/validator/modes/modes.go index 47fc28fee..26d7d3c81 100644 --- a/code/go/internal/validator/modes/modes.go +++ b/code/go/internal/validator/modes/modes.go @@ -4,15 +4,20 @@ package modes -// Mode represents the validation mode used when validating a package. +// Mode represents the validation context: which semantic rules run and how +// linked (.link) files are handled during package validation. type Mode string -// Validation modes for package validation: Legacy preserves existing behavior, -// Source validates checked-out source trees, and Build validates built artifacts. const ( + // Legacy preserves the original validation behavior: linked files are + // resolved transparently and no rules are mode-gated. Legacy Mode = "legacy" + // Source validates a package as a checked-out source tree: linked files + // are resolved transparently and source-only rules are enforced. Source Mode = "source" - Build Mode = "build" + // Build validates a package as a built artifact: linked files are + // unconditionally blocked and build-only rules are enforced. + Build Mode = "build" ) // Valid reports whether m is a recognised validation mode. diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index f9fc9f5b2..64850d155 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -25,7 +25,8 @@ import ( "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" ) -// Spec represents a package specification +// Spec represents a versioned package specification and the validation mode +// used to evaluate packages against it. type Spec struct { // version is the version requested, what is included in the package, possibly without prerelease tags. version semver.Version @@ -48,6 +49,7 @@ type validationRules []validationRule var GASpecCheckVersion = semver.MustParse("3.0.1") // NewSpec creates a new Spec for the given version and validation mode. +// Returns an error if version is not a known spec version or if mode is invalid. func NewSpec(version semver.Version, mode modes.Mode) (*Spec, error) { specVersion, err := spec.CheckVersion(version) if err != nil { @@ -74,7 +76,9 @@ func NewSpec(version semver.Version, mode modes.Mode) (*Spec, error) { return &s, nil } -// ValidatePackage validates the given Package against the Spec +// ValidatePackage validates the given Package against the Spec, running both +// syntactic and semantic rules. The mode embedded in the Spec controls which +// semantic rules are active. func (s Spec) ValidatePackage(pkg packages.Package) specerrors.ValidationErrors { var errs specerrors.ValidationErrors diff --git a/code/go/pkg/validator/modes.go b/code/go/pkg/validator/modes.go index 06a6e8abb..a2bd5cbe8 100644 --- a/code/go/pkg/validator/modes.go +++ b/code/go/pkg/validator/modes.go @@ -9,7 +9,7 @@ import ( // Mode represents the validation context: which rules apply and how linked files // are handled when creating a Validator from a path. -// Use ModeLegacy, ModeSource, or ModeBuild; do not construct Mode directly. +// Use ModeLegacy, ModeSource, or ModeBuild. type Mode struct { internal modes.Mode // wrapFS builds the filesystem for path-based validation (NewFromPath). @@ -18,8 +18,9 @@ type Mode struct { } var ( - // ModeLegacy preserves byte-for-byte identical behaviour with ValidateFromPath. - // Linked (.link) files are resolved transparently. + // ModeLegacy preserves the original validation behavior: linked (.link) files + // are resolved transparently and no rules are mode-gated. + // Use this mode when backward compatibility with existing callers is required. ModeLegacy = Mode{ internal: modes.Legacy, wrapFS: func(location string, fsys fs.FS) fs.FS { @@ -29,6 +30,7 @@ var ( // ModeSource validates a package as a checked-out source tree. // Linked (.link) files are resolved transparently. + // Source-only rules (e.g. dev-folder checks) are enforced; build-only rules are skipped. ModeSource = Mode{ internal: modes.Source, wrapFS: func(location string, fsys fs.FS) fs.FS { @@ -36,9 +38,10 @@ var ( }, } - // ModeBuild validates a package as a build artifact — output of elastic-package - // build, a zip archive, or a package served by the registry. + // ModeBuild validates a package as a built artifact — the output of + // elastic-package build, a zip archive, or a package served by the registry. // Linked (.link) files are unconditionally blocked. + // Build-only rules are enforced; source-only rules are skipped. ModeBuild = Mode{ internal: modes.Build, wrapFS: func(_ string, fsys fs.FS) fs.FS { diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index 29c2f7b12..3169bd82b 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -33,14 +33,18 @@ type Validator struct { // Option configures a Validator. type Option func(*Validator) -// WithWarningsAsErrors makes validation warnings count as errors when enabled is true. -// This overrides the PACKAGE_SPEC_WARNINGS_AS_ERRORS environment variable. +// WithWarningsAsErrors controls whether validation warnings are promoted to errors. +// When enabled is true, warnings are reported as errors regardless of the +// PACKAGE_SPEC_WARNINGS_AS_ERRORS environment variable. When enabled is false, +// warnings remain warnings even if the environment variable is set. func WithWarningsAsErrors(enabled bool) Option { return func(v *Validator) { v.warningsAsErrors = enabled } } // NewFromPath returns a Validator for the package rooted at packageRootPath. -// The mode determines which validation rules apply and how linked files are handled. +// The mode determines which validation rules apply and how linked files are +// handled: ModeBuild blocks linked files, ModeSource and ModeLegacy resolve them. +// Returns an error if mode is not a recognised value. func NewFromPath(mode Mode, packageRootPath string, opts ...Option) (*Validator, error) { if !mode.internal.Valid() { return nil, fmt.Errorf("invalid validation mode %q", mode.internal) @@ -88,7 +92,9 @@ func NewFromZip(zipPath string, opts ...Option) (_ *Validator, err error) { // NewFromFS returns a Validator for the package accessible through fsys at location. // The mode determines which validation rules apply; fsys is used as-is without any -// link-file wrapping. Callers are responsible for filesystem semantics. +// link-file wrapping. Callers are responsible for filesystem semantics: when using +// ModeBuild, wrap fsys with linkedfiles.NewBlockFS to enforce link-file restrictions. +// Returns an error if mode is not a recognised value. func NewFromFS(mode Mode, location string, fsys fs.FS, opts ...Option) (*Validator, error) { if !mode.internal.Valid() { return nil, fmt.Errorf("invalid validation mode %q", mode.internal) @@ -111,9 +117,6 @@ func buildValidator(mode Mode, location string, fsys fs.FS, closer io.Closer, op } // Validate runs package validation and returns any errors encountered. -// -// If the Validator was created by NewFromZip it owns an open zip reader; -// Validate closes it on return, so Validate must not be called more than once. func (v *Validator) Validate() (err error) { if v.closer != nil { defer func() { @@ -154,8 +157,8 @@ func ValidateFromPath(packageRootPath string) error { return v.Validate() } -// ValidateFromZip validates a package in zip file format using the build specification. -// Linked files (.link) are blocked; zip files are by definition built packages. +// ValidateFromZip validates a package in zip file format using ModeBuild. +// Linked (.link) files are blocked; zip files are by definition built packages. func ValidateFromZip(packagePath string) error { v, err := NewFromZip(packagePath) if err != nil { From d4508b1d372d0dc2e4bbd5af61d24b2e54dde4db Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 4 Jun 2026 12:23:47 +0200 Subject: [PATCH 23/30] Add copyright notice to modes.go file --- code/go/pkg/validator/modes.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/code/go/pkg/validator/modes.go b/code/go/pkg/validator/modes.go index a2bd5cbe8..c37a9b902 100644 --- a/code/go/pkg/validator/modes.go +++ b/code/go/pkg/validator/modes.go @@ -1,3 +1,7 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + package validator import ( From 8d441f165b3eaff47cc63a669da449867743bfa1 Mon Sep 17 00:00:00 2001 From: Tere Date: Fri, 5 Jun 2026 09:50:30 +0200 Subject: [PATCH 24/30] Move modes into internal validator pkg --- code/go/internal/validator/{modes => }/modes.go | 16 ++++++++-------- .../internal/validator/{modes => }/modes_test.go | 8 ++++---- code/go/internal/validator/spec.go | 7 +++---- code/go/internal/validator/spec_test.go | 9 ++++----- code/go/pkg/validator/modes.go | 10 +++++----- 5 files changed, 24 insertions(+), 26 deletions(-) rename code/go/internal/validator/{modes => }/modes.go (67%) rename code/go/internal/validator/{modes => }/modes_test.go (90%) diff --git a/code/go/internal/validator/modes/modes.go b/code/go/internal/validator/modes.go similarity index 67% rename from code/go/internal/validator/modes/modes.go rename to code/go/internal/validator/modes.go index 26d7d3c81..e9e35201a 100644 --- a/code/go/internal/validator/modes/modes.go +++ b/code/go/internal/validator/modes.go @@ -2,28 +2,28 @@ // or more contributor license agreements. Licensed under the Elastic License; // you may not use this file except in compliance with the Elastic License. -package modes +package validator // Mode represents the validation context: which semantic rules run and how // linked (.link) files are handled during package validation. type Mode string const ( - // Legacy preserves the original validation behavior: linked files are + // ModeLegacy preserves the original validation behavior: linked files are // resolved transparently and no rules are mode-gated. - Legacy Mode = "legacy" - // Source validates a package as a checked-out source tree: linked files + ModeLegacy Mode = "legacy" + // ModeSource validates a package as a checked-out source tree: linked files // are resolved transparently and source-only rules are enforced. - Source Mode = "source" - // Build validates a package as a built artifact: linked files are + ModeSource Mode = "source" + // ModeBuild validates a package as a built artifact: linked files are // unconditionally blocked and build-only rules are enforced. - Build Mode = "build" + ModeBuild Mode = "build" ) // Valid reports whether m is a recognised validation mode. func (m Mode) Valid() bool { switch m { - case Legacy, Source, Build: + case ModeLegacy, ModeSource, ModeBuild: return true } return false diff --git a/code/go/internal/validator/modes/modes_test.go b/code/go/internal/validator/modes_test.go similarity index 90% rename from code/go/internal/validator/modes/modes_test.go rename to code/go/internal/validator/modes_test.go index 12ac19cd8..8a67f82f4 100644 --- a/code/go/internal/validator/modes/modes_test.go +++ b/code/go/internal/validator/modes_test.go @@ -2,7 +2,7 @@ // or more contributor license agreements. Licensed under the Elastic License; // you may not use this file except in compliance with the Elastic License. -package modes +package validator import ( "testing" @@ -16,7 +16,7 @@ func TestValid(t *testing.T) { valid bool }{ "valid": { - mode: Legacy, + mode: ModeLegacy, valid: true, }, "invalid": { @@ -24,11 +24,11 @@ func TestValid(t *testing.T) { valid: false, }, "source": { - mode: Source, + mode: ModeSource, valid: true, }, "build": { - mode: Build, + mode: ModeBuild, valid: true, }, "": { diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index 64850d155..66bce2f99 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -20,7 +20,6 @@ import ( "github.com/elastic/package-spec/v3/code/go/internal/loader" "github.com/elastic/package-spec/v3/code/go/internal/packages" "github.com/elastic/package-spec/v3/code/go/internal/spectypes" - "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" "github.com/elastic/package-spec/v3/code/go/internal/validator/semantic" "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" ) @@ -35,7 +34,7 @@ type Spec struct { // fs contains the filesystem of the spec. fs fs.FS // mode is the validation mode (legacy, source, build). - mode modes.Mode + mode Mode // WarningsAsErrors causes validation warnings to be reported as errors when true. WarningsAsErrors bool @@ -50,7 +49,7 @@ var GASpecCheckVersion = semver.MustParse("3.0.1") // NewSpec creates a new Spec for the given version and validation mode. // Returns an error if version is not a known spec version or if mode is invalid. -func NewSpec(version semver.Version, mode modes.Mode) (*Spec, error) { +func NewSpec(version semver.Version, mode Mode) (*Spec, error) { specVersion, err := spec.CheckVersion(version) if err != nil { return nil, fmt.Errorf("could not load specification for version [%s]: %w", version.String(), err) @@ -210,7 +209,7 @@ func (s Spec) rules(pkgType string, rootSpec spectypes.ItemSpec) validationRules since *semver.Version until *semver.Version types []string - modes []modes.Mode + modes []Mode }{ {fn: semantic.ValidateVersionIntegrity}, {fn: semantic.ValidateChangelogLinks}, diff --git a/code/go/internal/validator/spec_test.go b/code/go/internal/validator/spec_test.go index 11a9e4b02..813f5e9d1 100644 --- a/code/go/internal/validator/spec_test.go +++ b/code/go/internal/validator/spec_test.go @@ -13,7 +13,6 @@ import ( "github.com/elastic/package-spec/v3/code/go/internal/fspath" "github.com/elastic/package-spec/v3/code/go/internal/packages" - "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" ) func TestNewLegacySpec(t *testing.T) { @@ -27,7 +26,7 @@ func TestNewLegacySpec(t *testing.T) { } for version, test := range tests { - spec, err := NewSpec(*semver.MustParse(version), modes.Legacy) + spec, err := NewSpec(*semver.MustParse(version), ModeLegacy) if test.expectedErrContains == "" { require.NoError(t, err) require.IsType(t, &Spec{}, spec) @@ -45,7 +44,7 @@ func TestNoBetaFeatures_Package_GA(t *testing.T) { version: *semver.MustParse("1.0.0"), specVersion: *semver.MustParse("1.0.0"), fs: fspath.DirFS("testdata/fakespec"), - mode: modes.Legacy, + mode: ModeLegacy, } pkg, err := packages.NewPackage("testdata/packages/features_ga") require.NoError(t, err) @@ -60,7 +59,7 @@ func TestBetaFeatures_Package_GA(t *testing.T) { version: *semver.MustParse("1.0.0"), specVersion: *semver.MustParse("1.0.0"), fs: fspath.DirFS("testdata/fakespec"), - mode: modes.Legacy, + mode: ModeLegacy, } pkg, err := packages.NewPackage("testdata/packages/features_beta") require.NoError(t, err) @@ -137,7 +136,7 @@ func TestFolderSpecInvalid(t *testing.T) { version: c.version, specVersion: c.version, fs: c.spec, - mode: modes.Legacy, + mode: ModeLegacy, } pkg, err := packages.NewPackage(c.pkgPath) require.NoError(t, err) diff --git a/code/go/pkg/validator/modes.go b/code/go/pkg/validator/modes.go index c37a9b902..8cb6c7ac9 100644 --- a/code/go/pkg/validator/modes.go +++ b/code/go/pkg/validator/modes.go @@ -8,14 +8,14 @@ import ( "io/fs" "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" - "github.com/elastic/package-spec/v3/code/go/internal/validator/modes" + validatorinternal "github.com/elastic/package-spec/v3/code/go/internal/validator" ) // Mode represents the validation context: which rules apply and how linked files // are handled when creating a Validator from a path. // Use ModeLegacy, ModeSource, or ModeBuild. type Mode struct { - internal modes.Mode + internal validatorinternal.Mode // wrapFS builds the filesystem for path-based validation (NewFromPath). // It is not applied by NewFromFS, which takes the caller's filesystem as-is. wrapFS func(location string, fsys fs.FS) fs.FS @@ -26,7 +26,7 @@ var ( // are resolved transparently and no rules are mode-gated. // Use this mode when backward compatibility with existing callers is required. ModeLegacy = Mode{ - internal: modes.Legacy, + internal: validatorinternal.ModeLegacy, wrapFS: func(location string, fsys fs.FS) fs.FS { return linkedfiles.NewFS(location, fsys) }, @@ -36,7 +36,7 @@ var ( // Linked (.link) files are resolved transparently. // Source-only rules (e.g. dev-folder checks) are enforced; build-only rules are skipped. ModeSource = Mode{ - internal: modes.Source, + internal: validatorinternal.ModeSource, wrapFS: func(location string, fsys fs.FS) fs.FS { return linkedfiles.NewFS(location, fsys) }, @@ -47,7 +47,7 @@ var ( // Linked (.link) files are unconditionally blocked. // Build-only rules are enforced; source-only rules are skipped. ModeBuild = Mode{ - internal: modes.Build, + internal: validatorinternal.ModeBuild, wrapFS: func(_ string, fsys fs.FS) fs.FS { return linkedfiles.NewBlockFS(fsys) }, From 15cc91e02f062093f8f9336962e6c15b243362cc Mon Sep 17 00:00:00 2001 From: Tere Date: Fri, 5 Jun 2026 12:12:34 +0200 Subject: [PATCH 25/30] Refactor validator API to streamline mode handling - Introduced NewValidator function to replace NewFromPath and NewFromFS, simplifying the creation of Validator instances. - Updated validation methods to use ValidateFromPath and ValidateFromFS, enhancing clarity and consistency. - Removed deprecated functions and unnecessary wrapping logic, ensuring better adherence to mode-specific validation rules. - Added tests for link file behavior across validation modes to ensure expected functionality. Co-Authored-By: Claude Sonnet 4.6 --- code/go/pkg/validator/limits_test.go | 10 +- code/go/pkg/validator/modes.go | 46 +------ code/go/pkg/validator/validator.go | 155 +++++++++++----------- code/go/pkg/validator/validator_test.go | 165 ++++++------------------ 4 files changed, 123 insertions(+), 253 deletions(-) diff --git a/code/go/pkg/validator/limits_test.go b/code/go/pkg/validator/limits_test.go index 7f74a06a9..b583f93c6 100644 --- a/code/go/pkg/validator/limits_test.go +++ b/code/go/pkg/validator/limits_test.go @@ -16,6 +16,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/elastic/package-spec/v3/code/go/internal/spectypes" ) @@ -117,11 +118,10 @@ func TestLimitsValidation(t *testing.T) { t.Run(c.title, func(t *testing.T) { t.Parallel() - v, err := NewFromFS(ModeLegacy, "test-package", c.fsys) - if !assert.NoError(t, err) { - return - } - err = v.Validate() + v, err := NewValidator(ModeLegacy) + require.NoError(t, err) + + err = v.ValidateFromFS("test-package", c.fsys) if c.valid { assert.NoError(t, err) } else { diff --git a/code/go/pkg/validator/modes.go b/code/go/pkg/validator/modes.go index 8cb6c7ac9..3a7e656df 100644 --- a/code/go/pkg/validator/modes.go +++ b/code/go/pkg/validator/modes.go @@ -5,51 +5,13 @@ package validator import ( - "io/fs" - - "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" validatorinternal "github.com/elastic/package-spec/v3/code/go/internal/validator" ) -// Mode represents the validation context: which rules apply and how linked files -// are handled when creating a Validator from a path. -// Use ModeLegacy, ModeSource, or ModeBuild. -type Mode struct { - internal validatorinternal.Mode - // wrapFS builds the filesystem for path-based validation (NewFromPath). - // It is not applied by NewFromFS, which takes the caller's filesystem as-is. - wrapFS func(location string, fsys fs.FS) fs.FS -} +type Mode = validatorinternal.Mode var ( - // ModeLegacy preserves the original validation behavior: linked (.link) files - // are resolved transparently and no rules are mode-gated. - // Use this mode when backward compatibility with existing callers is required. - ModeLegacy = Mode{ - internal: validatorinternal.ModeLegacy, - wrapFS: func(location string, fsys fs.FS) fs.FS { - return linkedfiles.NewFS(location, fsys) - }, - } - - // ModeSource validates a package as a checked-out source tree. - // Linked (.link) files are resolved transparently. - // Source-only rules (e.g. dev-folder checks) are enforced; build-only rules are skipped. - ModeSource = Mode{ - internal: validatorinternal.ModeSource, - wrapFS: func(location string, fsys fs.FS) fs.FS { - return linkedfiles.NewFS(location, fsys) - }, - } - - // ModeBuild validates a package as a built artifact — the output of - // elastic-package build, a zip archive, or a package served by the registry. - // Linked (.link) files are unconditionally blocked. - // Build-only rules are enforced; source-only rules are skipped. - ModeBuild = Mode{ - internal: validatorinternal.ModeBuild, - wrapFS: func(_ string, fsys fs.FS) fs.FS { - return linkedfiles.NewBlockFS(fsys) - }, - } + ModeLegacy Mode = validatorinternal.ModeLegacy + ModeSource Mode = validatorinternal.ModeSource + ModeBuild Mode = validatorinternal.ModeBuild ) diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index 3169bd82b..3840a14cd 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -8,7 +8,6 @@ import ( "archive/zip" "errors" "fmt" - "io" "io/fs" "os" @@ -22,12 +21,7 @@ import ( // Create one with NewFromPath, NewFromZip, or NewFromFS, then call Validate. type Validator struct { mode Mode - location string - fsys fs.FS warningsAsErrors bool - // closer is non-nil when the Validator owns a resource (e.g. a zip reader) - // that must be released after Validate returns. - closer io.Closer } // Option configures a Validator. @@ -41,92 +35,78 @@ func WithWarningsAsErrors(enabled bool) Option { return func(v *Validator) { v.warningsAsErrors = enabled } } -// NewFromPath returns a Validator for the package rooted at packageRootPath. -// The mode determines which validation rules apply and how linked files are -// handled: ModeBuild blocks linked files, ModeSource and ModeLegacy resolve them. -// Returns an error if mode is not a recognised value. -func NewFromPath(mode Mode, packageRootPath string, opts ...Option) (*Validator, error) { - if !mode.internal.Valid() { - return nil, fmt.Errorf("invalid validation mode %q", mode.internal) +func NewValidator(mode Mode, opts ...Option) (*Validator, error) { + if !mode.Valid() { + return nil, fmt.Errorf("invalid validation mode %q", mode) } - fsys := mode.wrapFS(packageRootPath, os.DirFS(packageRootPath)) - return buildValidator(mode, packageRootPath, fsys, nil, opts), nil + v := &Validator{ + mode: mode, + warningsAsErrors: common.IsDefinedWarningsAsErrors(), + } + for _, opt := range opts { + opt(v) + } + return v, nil } -// NewFromZip returns a Validator for the package stored in the zip file at zipPath. -// -// Zip files always contain built packages, so the validator always runs in -// ModeBuild; linked (.link) files are blocked. -// -// The returned Validator owns the underlying zip reader; Validate closes it. -// Do not call Validate more than once on a Validator created by NewFromZip. -func NewFromZip(zipPath string, opts ...Option) (_ *Validator, err error) { - r, openErr := zip.OpenReader(zipPath) - if openErr != nil { - return nil, fmt.Errorf("failed to open zip file (%s): %w", zipPath, openErr) - } - defer func() { - if err != nil { - if cerr := r.Close(); cerr != nil { - err = errors.Join(err, cerr) - } - } - }() +// Validate runs package validation and returns any errors encountered. +func (v *Validator) ValidateFromPath(path string) error { + fsys := os.DirFS(path) + if v.mode == ModeBuild { + fsys = linkedfiles.NewBlockFS(fsys) + } else { + fsys = linkedfiles.NewFS(path, fsys) + } + + return v.validate(path, fsys) +} + +func (v *Validator) ValidateFromZip(zipPath string) error { + if v.mode != ModeBuild { + return errors.New("zip files are only supported in ModeBuild") + } + + r, err := zip.OpenReader(zipPath) + if err != nil { + return fmt.Errorf("failed to open zip file (%s): %w", zipPath, err) + } + defer r.Close() dirs, err := fs.ReadDir(r, ".") if err != nil { - return nil, fmt.Errorf("failed to read root directory in zip file (%s): %w", zipPath, err) + return fmt.Errorf("failed to read root directory in zip file (%s): %w", zipPath, err) } if len(dirs) != 1 { - return nil, fmt.Errorf("a single directory is expected in zip file, %d found", len(dirs)) + return fmt.Errorf("a single directory is expected in zip file, %d found", len(dirs)) } subDir, err := fs.Sub(r, dirs[0].Name()) if err != nil { - return nil, err + return err } fsys := linkedfiles.NewBlockFS(subDir) - return buildValidator(ModeBuild, zipPath, fsys, r, opts), nil + return v.validate(zipPath, fsys) } -// NewFromFS returns a Validator for the package accessible through fsys at location. -// The mode determines which validation rules apply; fsys is used as-is without any -// link-file wrapping. Callers are responsible for filesystem semantics: when using -// ModeBuild, wrap fsys with linkedfiles.NewBlockFS to enforce link-file restrictions. -// Returns an error if mode is not a recognised value. -func NewFromFS(mode Mode, location string, fsys fs.FS, opts ...Option) (*Validator, error) { - if !mode.internal.Valid() { - return nil, fmt.Errorf("invalid validation mode %q", mode.internal) +func (v *Validator) ValidateFromFS(location string, fsys fs.FS) error { + if v.mode == ModeLegacy { + // If we are not explicitly using the linkedfiles.FS, we wrap fsys with + // a linkedfiles.BlockFS to block the use of linked files. + if _, ok := fsys.(*linkedfiles.FS); !ok { + fsys = linkedfiles.NewBlockFS(fsys) + } + } else if _, ok := fsys.(*linkedfiles.FS); ok && v.mode == ModeBuild { + return errors.New("linked files are not supported in ModeBuild") + } else if _, ok := fsys.(*linkedfiles.BlockFS); ok && v.mode == ModeSource { + return errors.New("block linked files are not supported in ModeSource") } - return buildValidator(mode, location, fsys, nil, opts), nil -} -func buildValidator(mode Mode, location string, fsys fs.FS, closer io.Closer, opts []Option) *Validator { - v := &Validator{ - mode: mode, - location: location, - fsys: fsys, - warningsAsErrors: common.IsDefinedWarningsAsErrors(), - closer: closer, - } - for _, opt := range opts { - opt(v) - } - return v + return v.validate(location, fsys) } -// Validate runs package validation and returns any errors encountered. -func (v *Validator) Validate() (err error) { - if v.closer != nil { - defer func() { - if cerr := v.closer.Close(); cerr != nil { - err = errors.Join(err, cerr) - } - }() - } - - pkg, err := packages.NewPackageFromFS(v.location, v.fsys) +func (v *Validator) validate(location string, fsys fs.FS) error { + pkg, err := packages.NewPackageFromFS(location, fsys) if err != nil { return err } @@ -134,7 +114,7 @@ func (v *Validator) Validate() (err error) { return errors.New("could not determine specification version for package") } - spec, err := internalvalidator.NewSpec(*pkg.SpecVersion, v.mode.internal) + spec, err := internalvalidator.NewSpec(*pkg.SpecVersion, v.mode) if err != nil { return err } @@ -146,23 +126,32 @@ func (v *Validator) Validate() (err error) { return nil } -// ValidateFromPath validates a package located at the given path using the legacy specification. -// Linked (.link) files are resolved transparently. -// Deprecated: Use NewFromPath with ModeSource or ModeBuild depending on the package type. -func ValidateFromPath(packageRootPath string) error { - v, err := NewFromPath(ModeLegacy, packageRootPath) +// ValidateFromPath is a convenience function that creates a new Validator in ModeLegacy and calls ValidateFromPath. +// Deprecated: Use NewValidator and ValidateFromPath instead. +func ValidateFromPath(path string) error { + v, err := NewValidator(ModeLegacy) + if err != nil { + return err + } + return v.ValidateFromPath(path) +} + +// ValidateFromZip is a convenience function that creates a new Validator in ModeLegacy and calls ValidateFromZip. +// Deprecated: Use NewValidator and ValidateFromZip instead. +func ValidateFromZip(zipPath string) error { + v, err := NewValidator(ModeLegacy) if err != nil { return err } - return v.Validate() + return v.ValidateFromZip(zipPath) } -// ValidateFromZip validates a package in zip file format using ModeBuild. -// Linked (.link) files are blocked; zip files are by definition built packages. -func ValidateFromZip(packagePath string) error { - v, err := NewFromZip(packagePath) +// ValidateFromFS is a convenience function that creates a new Validator in ModeLegacy and calls ValidateFromFS. +// Deprecated: Use NewValidator and ValidateFromFS instead. +func ValidateFromFS(location string, fsys fs.FS) error { + v, err := NewValidator(ModeLegacy) if err != nil { return err } - return v.Validate() + return v.ValidateFromFS(location, fsys) } diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index 3c0136de5..dcdef111c 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -13,8 +13,6 @@ import ( "strings" "testing" - "archive/zip" - cp "github.com/otiai10/copy" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -937,6 +935,8 @@ func TestValidateMinimumKibanaVersions(t *testing.T) { } func TestValidateWarnings(t *testing.T) { + t.Setenv("PACKAGE_SPEC_WARNINGS_AS_ERRORS", "true") + tests := map[string][]string{ "good": {}, "good_v2": {}, @@ -957,9 +957,7 @@ func TestValidateWarnings(t *testing.T) { t.Parallel() pkgRootPath := path.Join("..", "..", "..", "..", "test", "packages", pkgName) - v, constructErr := NewFromFS(ModeLegacy, pkgRootPath, linkedfiles.NewFS(pkgRootPath, os.DirFS(pkgRootPath)), WithWarningsAsErrors(true)) - require.NoError(t, constructErr) - errs := v.Validate() + errs := ValidateFromFS(pkgRootPath, linkedfiles.NewFS(pkgRootPath, os.DirFS(pkgRootPath))) if len(expectedWarnContains) == 0 { require.NoError(t, errs) } else { @@ -1209,10 +1207,7 @@ func TestValidateForbiddenDataStreamName(t *testing.T) { } func TestLinksAreBlocked(t *testing.T) { - // NewFromFS takes the FS as-is; wrap explicitly with BlockFS to enforce link blocking. - v, err := NewFromFS(ModeLegacy, "test-package", linkedfiles.NewBlockFS(newMockFS().WithLink())) - require.NoError(t, err) - err = v.Validate() + err := ValidateFromFS("test-package", newMockFS().WithLink()) errs, ok := err.(specerrors.ValidationErrors) require.True(t, ok) for _, err := range errs { @@ -1224,120 +1219,6 @@ func TestLinksAreBlocked(t *testing.T) { t.Error("links should not be allowed in package") } -func TestLinksBehaviorAcrossModes(t *testing.T) { - withLinks := path.Join("..", "..", "..", "..", "test", "packages", "with_links") - - t.Run("build_rejects_link_files", func(t *testing.T) { - t.Parallel() - v, err := NewFromPath(ModeBuild, withLinks) - require.NoError(t, err) - err = v.Validate() - require.Error(t, err) - errs, ok := err.(specerrors.ValidationErrors) - require.True(t, ok) - require.ErrorContains(t, errs, linkedfiles.ErrUnsupportedLinkFile.Error()) - }) - - t.Run("source_accepts_link_files", func(t *testing.T) { - t.Parallel() - v, err := NewFromPath(ModeSource, withLinks) - require.NoError(t, err) - err = v.Validate() - require.NoError(t, err) - }) - -} - -// createTestZip creates a temporary zip archive of the package at packagePath, -// placing all files under a single top-level directory (matching elastic-package -// build output). Returns the path to the created zip. -func createTestZip(t *testing.T, packagePath string) string { - t.Helper() - packageName := filepath.Base(packagePath) - - tmpDir := t.TempDir() - zipPath := filepath.Join(tmpDir, packageName+".zip") - - f, err := os.Create(zipPath) - require.NoError(t, err) - defer f.Close() - - zw := zip.NewWriter(f) - defer zw.Close() - - err = filepath.Walk(packagePath, func(walkPath string, info os.FileInfo, walkErr error) error { - if walkErr != nil { - return walkErr - } - if info.IsDir() { - return nil - } - rel, err := filepath.Rel(packagePath, walkPath) - if err != nil { - return err - } - entryName := filepath.ToSlash(filepath.Join(packageName, rel)) - w, err := zw.Create(entryName) - if err != nil { - return err - } - data, err := os.ReadFile(walkPath) - if err != nil { - return err - } - _, err = w.Write(data) - return err - }) - require.NoError(t, err) - return zipPath -} - -func TestNewFromPath_RejectsInvalidMode(t *testing.T) { - v, err := NewFromPath(Mode{}, filepath.Join("..", "..", "..", "..", "test", "packages", "good")) - require.Nil(t, v) - require.ErrorContains(t, err, "invalid validation mode") -} - -func TestNewFromFS_RejectsInvalidMode(t *testing.T) { - v, err := NewFromFS(Mode{}, ".", os.DirFS(".")) - require.Nil(t, v) - require.ErrorContains(t, err, "invalid validation mode") -} - -func TestNewFromZip_ConstructorSucceeds(t *testing.T) { - packagePath := filepath.Join("..", "..", "..", "..", "test", "packages", "good") - zipPath := createTestZip(t, packagePath) - - v, err := NewFromZip(zipPath) - require.NoError(t, err) - // Validate closes the owned zip reader; must be called to avoid leaking the handle. - _ = v.Validate() -} - -// TestNewFromFS_TakesFSAsIsRegardlessOfMode documents the NewFromFS contract: -// the provided filesystem is used as-is; the mode controls validation rules only, -// not link-file handling. Callers are responsible for FS semantics. -func TestNewFromFS_TakesFSAsIsRegardlessOfMode(t *testing.T) { - withLinks := filepath.Join("..", "..", "..", "..", "test", "packages", "with_links") - - for _, mode := range []Mode{ModeLegacy, ModeSource, ModeBuild} { - t.Run(string(mode.internal), func(t *testing.T) { - // Passing a resolving FS: links are resolved regardless of mode. - resolving := linkedfiles.NewFS(withLinks, os.DirFS(withLinks)) - v, err := NewFromFS(mode, withLinks, resolving) - require.NoError(t, err) - - errs := v.Validate() - if vErrs, ok := errs.(specerrors.ValidationErrors); ok { - for _, e := range vErrs { - require.False(t, errors.Is(e, linkedfiles.ErrUnsupportedLinkFile), - "NewFromFS must not override caller FS semantics: mode %s should not block links when a resolving FS is provided", mode.internal) - } - } - }) - } -} - func TestValidateHandlebarsFiles(t *testing.T) { tests := map[string]string{ "bad_input_hbs": "invalid handlebars template: error validating agent/input/input.yml.hbs: Parse error on line 10:\nExpecting OpenEndBlock, got: 'EOF'", @@ -1385,3 +1266,41 @@ func requireErrorMessage(t *testing.T, pkgName string, invalidItemsPerFolder map } require.Len(t, errs, c) } + +func TestLinksBehaviorAcrossModes(t *testing.T) { + withLinks := path.Join("..", "..", "..", "..", "test", "packages", "with_links") + + t.Run("build_rejects_link_files", func(t *testing.T) { + t.Parallel() + + v, err := NewValidator(ModeBuild) + require.NoError(t, err) + err = v.ValidateFromPath(withLinks) + require.Error(t, err) + errs, ok := err.(specerrors.ValidationErrors) + require.True(t, ok) + require.ErrorContains(t, errs, linkedfiles.ErrUnsupportedLinkFile.Error()) + }) + + t.Run("source_accepts_link_files", func(t *testing.T) { + t.Parallel() + v, err := NewValidator(ModeSource) + require.NoError(t, err) + err = v.ValidateFromPath(withLinks) + require.NoError(t, err) + }) + + t.Run("legacy_accepts_link_files", func(t *testing.T) { + t.Parallel() + v, err := NewValidator(ModeLegacy) + require.NoError(t, err) + err = v.ValidateFromPath(withLinks) + require.NoError(t, err) + }) + +} + +func TestNewValidator_RejectsInvalidMode(t *testing.T) { + _, err := NewValidator(Mode("invalid")) + require.Error(t, err) +} From cc5e273bdcdcd1c635f12240419d5bb5692ac397 Mon Sep 17 00:00:00 2001 From: Tere Date: Fri, 5 Jun 2026 12:14:51 +0200 Subject: [PATCH 26/30] Remove modes.go file and consolidate mode definitions in validator.go - Deleted the modes.go file to streamline the codebase. - Moved mode type definitions and constants directly into validator.go for better organization and accessibility. --- code/go/pkg/validator/modes.go | 17 ----------------- code/go/pkg/validator/validator.go | 8 ++++++++ 2 files changed, 8 insertions(+), 17 deletions(-) delete mode 100644 code/go/pkg/validator/modes.go diff --git a/code/go/pkg/validator/modes.go b/code/go/pkg/validator/modes.go deleted file mode 100644 index 3a7e656df..000000000 --- a/code/go/pkg/validator/modes.go +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one -// or more contributor license agreements. Licensed under the Elastic License; -// you may not use this file except in compliance with the Elastic License. - -package validator - -import ( - validatorinternal "github.com/elastic/package-spec/v3/code/go/internal/validator" -) - -type Mode = validatorinternal.Mode - -var ( - ModeLegacy Mode = validatorinternal.ModeLegacy - ModeSource Mode = validatorinternal.ModeSource - ModeBuild Mode = validatorinternal.ModeBuild -) diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index 3840a14cd..8cea6ff9b 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -17,6 +17,14 @@ import ( "github.com/elastic/package-spec/v3/code/go/internal/validator/common" ) +type Mode = internalvalidator.Mode + +var ( + ModeLegacy Mode = internalvalidator.ModeLegacy + ModeSource Mode = internalvalidator.ModeSource + ModeBuild Mode = internalvalidator.ModeBuild +) + // Validator holds the configuration for a package validation run. // Create one with NewFromPath, NewFromZip, or NewFromFS, then call Validate. type Validator struct { From 2a3fef8d8a437168abefe27ef463ce739dab6a87 Mon Sep 17 00:00:00 2001 From: Tere Date: Fri, 5 Jun 2026 12:29:41 +0200 Subject: [PATCH 27/30] Add comprehensive tests for ValidateFromFS and ValidateFromZip methods - Introduced multiple test cases for ValidateFromFS to handle various file system scenarios, including linked and block file systems. - Added tests for ValidateFromZip to enforce mode restrictions and validate package integrity. - Enhanced documentation comments for validation modes and methods to clarify their usage and constraints. - Implemented helper functions for creating zip files for testing purposes. --- code/go/pkg/validator/validator.go | 18 +- code/go/pkg/validator/validator_test.go | 224 ++++++++++++++++++++++++ 2 files changed, 237 insertions(+), 5 deletions(-) diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index 8cea6ff9b..6fc0b6db6 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -17,16 +17,20 @@ import ( "github.com/elastic/package-spec/v3/code/go/internal/validator/common" ) +// Mode is the validation context that controls semantic rules and linked-file handling. type Mode = internalvalidator.Mode var ( + // ModeLegacy preserves the original validation behavior. ModeLegacy Mode = internalvalidator.ModeLegacy + // ModeSource validates a checked-out source tree. ModeSource Mode = internalvalidator.ModeSource - ModeBuild Mode = internalvalidator.ModeBuild + // ModeBuild validates a built package artifact. + ModeBuild Mode = internalvalidator.ModeBuild ) // Validator holds the configuration for a package validation run. -// Create one with NewFromPath, NewFromZip, or NewFromFS, then call Validate. +// Create one with NewValidator, then call ValidateFromPath, ValidateFromZip, or ValidateFromFS. type Validator struct { mode Mode warningsAsErrors bool @@ -43,6 +47,7 @@ func WithWarningsAsErrors(enabled bool) Option { return func(v *Validator) { v.warningsAsErrors = enabled } } +// NewValidator creates a Validator for the given mode and options. func NewValidator(mode Mode, opts ...Option) (*Validator, error) { if !mode.Valid() { return nil, fmt.Errorf("invalid validation mode %q", mode) @@ -57,7 +62,7 @@ func NewValidator(mode Mode, opts ...Option) (*Validator, error) { return v, nil } -// Validate runs package validation and returns any errors encountered. +// ValidateFromPath validates the package at path on disk. func (v *Validator) ValidateFromPath(path string) error { fsys := os.DirFS(path) if v.mode == ModeBuild { @@ -69,9 +74,11 @@ func (v *Validator) ValidateFromPath(path string) error { return v.validate(path, fsys) } +// ValidateFromZip validates the package stored in a zip file. +// Zip files are supported in ModeLegacy and ModeBuild only. func (v *Validator) ValidateFromZip(zipPath string) error { - if v.mode != ModeBuild { - return errors.New("zip files are only supported in ModeBuild") + if v.mode != ModeLegacy && v.mode != ModeBuild { + return errors.New("zip files are only supported in ModeLegacy or ModeBuild") } r, err := zip.OpenReader(zipPath) @@ -97,6 +104,7 @@ func (v *Validator) ValidateFromZip(zipPath string) error { return v.validate(zipPath, fsys) } +// ValidateFromFS validates the package accessible through fsys at location. func (v *Validator) ValidateFromFS(location string, fsys fs.FS) error { if v.mode == ModeLegacy { // If we are not explicitly using the linkedfiles.FS, we wrap fsys with diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index dcdef111c..5682fcb58 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -5,8 +5,10 @@ package validator import ( + "archive/zip" "errors" "fmt" + "io/fs" "os" "path" "path/filepath" @@ -1304,3 +1306,225 @@ func TestNewValidator_RejectsInvalidMode(t *testing.T) { _, err := NewValidator(Mode("invalid")) require.Error(t, err) } + +func TestValidateFromFS_rejectsIncompatibleFS(t *testing.T) { + withLinks := filepath.Join("..", "..", "..", "..", "test", "packages", "with_links") + inner := os.DirFS(withLinks) + + t.Run("build_with_linked_fs", func(t *testing.T) { + t.Parallel() + v, err := NewValidator(ModeBuild) + require.NoError(t, err) + err = v.ValidateFromFS(withLinks, linkedfiles.NewFS(withLinks, inner)) + require.Error(t, err) + require.ErrorContains(t, err, "linked files are not supported in ModeBuild") + }) + + t.Run("source_with_block_fs", func(t *testing.T) { + t.Parallel() + v, err := NewValidator(ModeSource) + require.NoError(t, err) + err = v.ValidateFromFS(withLinks, linkedfiles.NewBlockFS(inner)) + require.Error(t, err) + require.ErrorContains(t, err, "block linked files are not supported in ModeSource") + }) + + t.Run("source_with_linked_fs", func(t *testing.T) { + t.Parallel() + v, err := NewValidator(ModeSource) + require.NoError(t, err) + err = v.ValidateFromFS(withLinks, linkedfiles.NewFS(withLinks, inner)) + require.NoError(t, err) + }) + + t.Run("build_with_block_fs", func(t *testing.T) { + t.Parallel() + v, err := NewValidator(ModeBuild) + require.NoError(t, err) + err = v.ValidateFromFS("test-package", linkedfiles.NewBlockFS(newMockFS().Good())) + if err != nil { + require.NotContains(t, err.Error(), "linked files are not supported in ModeBuild") + require.NotContains(t, err.Error(), "block linked files are not supported in ModeSource") + } + }) +} + +func TestValidateFromFS_legacyPlainFSBlocksLinks(t *testing.T) { + withLinks := filepath.Join("..", "..", "..", "..", "test", "packages", "with_links") + + t.Run("legacy_plain_fs_blocks_links", func(t *testing.T) { + t.Parallel() + v, err := NewValidator(ModeLegacy) + require.NoError(t, err) + err = v.ValidateFromFS("test-package", newMockFS().WithLink()) + require.Error(t, err) + errs, ok := err.(specerrors.ValidationErrors) + require.True(t, ok) + for _, e := range errs { + if errors.Is(e, linkedfiles.ErrUnsupportedLinkFile) { + return + } + } + t.Error("links should not be allowed in package") + }) + + t.Run("legacy_linked_fs_accepts_links", func(t *testing.T) { + t.Parallel() + v, err := NewValidator(ModeLegacy) + require.NoError(t, err) + err = v.ValidateFromFS(withLinks, linkedfiles.NewFS(withLinks, os.DirFS(withLinks))) + require.NoError(t, err) + }) +} + +func writePackageZip(t *testing.T, pkgDir, rootName string) string { + t.Helper() + + zipPath := filepath.Join(t.TempDir(), rootName+".zip") + f, err := os.Create(zipPath) + require.NoError(t, err) + + zw := zip.NewWriter(f) + err = filepath.WalkDir(pkgDir, func(walkPath string, d fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + rel, err := filepath.Rel(pkgDir, walkPath) + if err != nil { + return err + } + if rel == "." { + return nil + } + + name := filepath.ToSlash(filepath.Join(rootName, rel)) + if d.IsDir() { + _, err = zw.Create(name + "/") + return err + } + + w, err := zw.Create(name) + if err != nil { + return err + } + content, err := os.ReadFile(walkPath) + if err != nil { + return err + } + _, err = w.Write(content) + return err + }) + require.NoError(t, err) + require.NoError(t, zw.Close()) + require.NoError(t, f.Close()) + return zipPath +} + +func writeMalformedPackageZip(t *testing.T, rootNames ...string) string { + t.Helper() + + zipPath := filepath.Join(t.TempDir(), "malformed.zip") + f, err := os.Create(zipPath) + require.NoError(t, err) + + zw := zip.NewWriter(f) + for _, rootName := range rootNames { + _, err = zw.Create(rootName + "/manifest.yml") + require.NoError(t, err) + } + require.NoError(t, zw.Close()) + require.NoError(t, f.Close()) + return zipPath +} + +func TestValidateFromZip_modeRestrictions(t *testing.T) { + goodPkg := filepath.Join("..", "..", "..", "..", "test", "packages", "good") + zipPath := writePackageZip(t, goodPkg, "good") + + t.Run("source_rejected", func(t *testing.T) { + t.Parallel() + v, err := NewValidator(ModeSource) + require.NoError(t, err) + err = v.ValidateFromZip(zipPath) + require.Error(t, err) + require.ErrorContains(t, err, "zip files are only supported in ModeLegacy or ModeBuild") + }) + + t.Run("legacy_allowed", func(t *testing.T) { + t.Parallel() + v, err := NewValidator(ModeLegacy) + require.NoError(t, err) + err = v.ValidateFromZip(zipPath) + require.NoError(t, err) + }) + + t.Run("build_allowed", func(t *testing.T) { + t.Parallel() + v, err := NewValidator(ModeBuild) + require.NoError(t, err) + err = v.ValidateFromZip(zipPath) + require.NoError(t, err) + }) +} + +func TestValidateFromZip_validatesPackage(t *testing.T) { + goodPkg := filepath.Join("..", "..", "..", "..", "test", "packages", "good") + + t.Run("valid_zip", func(t *testing.T) { + t.Parallel() + zipPath := writePackageZip(t, goodPkg, "good") + v, err := NewValidator(ModeLegacy) + require.NoError(t, err) + require.NoError(t, v.ValidateFromZip(zipPath)) + }) + + t.Run("no_root_directory", func(t *testing.T) { + t.Parallel() + zipPath := writeMalformedPackageZip(t) + v, err := NewValidator(ModeLegacy) + require.NoError(t, err) + err = v.ValidateFromZip(zipPath) + require.Error(t, err) + require.ErrorContains(t, err, "a single directory is expected in zip file, 0 found") + }) + + t.Run("multiple_root_directories", func(t *testing.T) { + t.Parallel() + zipPath := writeMalformedPackageZip(t, "pkg-a", "pkg-b") + v, err := NewValidator(ModeLegacy) + require.NoError(t, err) + err = v.ValidateFromZip(zipPath) + require.Error(t, err) + require.ErrorContains(t, err, "a single directory is expected in zip file, 2 found") + }) +} + +func TestDeprecatedValidateFromZip(t *testing.T) { + goodPkg := filepath.Join("..", "..", "..", "..", "test", "packages", "good") + zipPath := writePackageZip(t, goodPkg, "good") + + err := ValidateFromZip(zipPath) + require.NoError(t, err) +} + +func TestWithWarningsAsErrors_option(t *testing.T) { + pkgRootPath := path.Join("..", "..", "..", "..", "test", "packages", "visualizations_by_reference") + fsys := linkedfiles.NewFS(pkgRootPath, os.DirFS(pkgRootPath)) + + t.Run("option_overrides_env_false", func(t *testing.T) { + t.Setenv("PACKAGE_SPEC_WARNINGS_AS_ERRORS", "false") + v, err := NewValidator(ModeLegacy, WithWarningsAsErrors(true)) + require.NoError(t, err) + err = v.ValidateFromFS(pkgRootPath, fsys) + require.Error(t, err) + require.ErrorContains(t, err, "SVR00004") + }) + + t.Run("option_overrides_env_true", func(t *testing.T) { + t.Setenv("PACKAGE_SPEC_WARNINGS_AS_ERRORS", "true") + v, err := NewValidator(ModeLegacy, WithWarningsAsErrors(false)) + require.NoError(t, err) + err = v.ValidateFromFS(pkgRootPath, fsys) + require.NoError(t, err) + }) +} From d8a2a6c04d1352e55f1df85582cdd9a34fb29bf3 Mon Sep 17 00:00:00 2001 From: Tere Date: Fri, 5 Jun 2026 14:12:33 +0200 Subject: [PATCH 28/30] Refactor validation mode constants for clarity and consistency - Renamed mode constants from ModeLegacy, ModeSource, and ModeBuild to LegacyMode, SourceMode, and BuildMode for improved readability. - Updated all references in tests and validation logic to reflect the new naming convention. - Enhanced test cases to ensure proper validation behavior across different modes. --- code/go/internal/validator/modes.go | 14 +++---- code/go/internal/validator/modes_test.go | 6 +-- code/go/internal/validator/spec_test.go | 10 ++--- code/go/pkg/validator/limits_test.go | 6 +-- code/go/pkg/validator/validator.go | 48 ++++++++++++------------ code/go/pkg/validator/validator_test.go | 44 +++++++++++----------- 6 files changed, 62 insertions(+), 66 deletions(-) diff --git a/code/go/internal/validator/modes.go b/code/go/internal/validator/modes.go index e9e35201a..70ebc4444 100644 --- a/code/go/internal/validator/modes.go +++ b/code/go/internal/validator/modes.go @@ -9,21 +9,21 @@ package validator type Mode string const ( - // ModeLegacy preserves the original validation behavior: linked files are + // LegacyMode preserves the original validation behavior: linked files are // resolved transparently and no rules are mode-gated. - ModeLegacy Mode = "legacy" - // ModeSource validates a package as a checked-out source tree: linked files + LegacyMode Mode = "legacy" + // SourceMode validates a package as a checked-out source tree: linked files // are resolved transparently and source-only rules are enforced. - ModeSource Mode = "source" - // ModeBuild validates a package as a built artifact: linked files are + SourceMode Mode = "source" + // BuildMode validates a package as a built artifact: linked files are // unconditionally blocked and build-only rules are enforced. - ModeBuild Mode = "build" + BuildMode Mode = "build" ) // Valid reports whether m is a recognised validation mode. func (m Mode) Valid() bool { switch m { - case ModeLegacy, ModeSource, ModeBuild: + case LegacyMode, SourceMode, BuildMode: return true } return false diff --git a/code/go/internal/validator/modes_test.go b/code/go/internal/validator/modes_test.go index 8a67f82f4..63f3d90e3 100644 --- a/code/go/internal/validator/modes_test.go +++ b/code/go/internal/validator/modes_test.go @@ -16,7 +16,7 @@ func TestValid(t *testing.T) { valid bool }{ "valid": { - mode: ModeLegacy, + mode: LegacyMode, valid: true, }, "invalid": { @@ -24,11 +24,11 @@ func TestValid(t *testing.T) { valid: false, }, "source": { - mode: ModeSource, + mode: SourceMode, valid: true, }, "build": { - mode: ModeBuild, + mode: BuildMode, valid: true, }, "": { diff --git a/code/go/internal/validator/spec_test.go b/code/go/internal/validator/spec_test.go index 813f5e9d1..538a28be8 100644 --- a/code/go/internal/validator/spec_test.go +++ b/code/go/internal/validator/spec_test.go @@ -15,7 +15,7 @@ import ( "github.com/elastic/package-spec/v3/code/go/internal/packages" ) -func TestNewLegacySpec(t *testing.T) { +func TestNewSpec(t *testing.T) { tests := map[string]struct { expectedErrContains string }{ @@ -26,7 +26,7 @@ func TestNewLegacySpec(t *testing.T) { } for version, test := range tests { - spec, err := NewSpec(*semver.MustParse(version), ModeLegacy) + spec, err := NewSpec(*semver.MustParse(version), LegacyMode) if test.expectedErrContains == "" { require.NoError(t, err) require.IsType(t, &Spec{}, spec) @@ -44,7 +44,7 @@ func TestNoBetaFeatures_Package_GA(t *testing.T) { version: *semver.MustParse("1.0.0"), specVersion: *semver.MustParse("1.0.0"), fs: fspath.DirFS("testdata/fakespec"), - mode: ModeLegacy, + mode: LegacyMode, } pkg, err := packages.NewPackage("testdata/packages/features_ga") require.NoError(t, err) @@ -59,7 +59,7 @@ func TestBetaFeatures_Package_GA(t *testing.T) { version: *semver.MustParse("1.0.0"), specVersion: *semver.MustParse("1.0.0"), fs: fspath.DirFS("testdata/fakespec"), - mode: ModeLegacy, + mode: LegacyMode, } pkg, err := packages.NewPackage("testdata/packages/features_beta") require.NoError(t, err) @@ -136,7 +136,7 @@ func TestFolderSpecInvalid(t *testing.T) { version: c.version, specVersion: c.version, fs: c.spec, - mode: ModeLegacy, + mode: LegacyMode, } pkg, err := packages.NewPackage(c.pkgPath) require.NoError(t, err) diff --git a/code/go/pkg/validator/limits_test.go b/code/go/pkg/validator/limits_test.go index b583f93c6..56e07e089 100644 --- a/code/go/pkg/validator/limits_test.go +++ b/code/go/pkg/validator/limits_test.go @@ -16,7 +16,6 @@ import ( "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/elastic/package-spec/v3/code/go/internal/spectypes" ) @@ -118,10 +117,7 @@ func TestLimitsValidation(t *testing.T) { t.Run(c.title, func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeLegacy) - require.NoError(t, err) - - err = v.ValidateFromFS("test-package", c.fsys) + err := ValidateFromFS("test-package", c.fsys) if c.valid { assert.NoError(t, err) } else { diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index 6fc0b6db6..494ec672a 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -13,20 +13,20 @@ import ( "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" "github.com/elastic/package-spec/v3/code/go/internal/packages" - internalvalidator "github.com/elastic/package-spec/v3/code/go/internal/validator" + "github.com/elastic/package-spec/v3/code/go/internal/validator" "github.com/elastic/package-spec/v3/code/go/internal/validator/common" ) // Mode is the validation context that controls semantic rules and linked-file handling. -type Mode = internalvalidator.Mode +type Mode = validator.Mode var ( - // ModeLegacy preserves the original validation behavior. - ModeLegacy Mode = internalvalidator.ModeLegacy - // ModeSource validates a checked-out source tree. - ModeSource Mode = internalvalidator.ModeSource - // ModeBuild validates a built package artifact. - ModeBuild Mode = internalvalidator.ModeBuild + // LegacyMode preserves the original validation behavior. + LegacyMode Mode = validator.LegacyMode + // SourceMode validates a checked-out source tree. + SourceMode Mode = validator.SourceMode + // BuildMode validates a built package artifact. + BuildMode Mode = validator.BuildMode ) // Validator holds the configuration for a package validation run. @@ -65,7 +65,7 @@ func NewValidator(mode Mode, opts ...Option) (*Validator, error) { // ValidateFromPath validates the package at path on disk. func (v *Validator) ValidateFromPath(path string) error { fsys := os.DirFS(path) - if v.mode == ModeBuild { + if v.mode == BuildMode { fsys = linkedfiles.NewBlockFS(fsys) } else { fsys = linkedfiles.NewFS(path, fsys) @@ -75,10 +75,10 @@ func (v *Validator) ValidateFromPath(path string) error { } // ValidateFromZip validates the package stored in a zip file. -// Zip files are supported in ModeLegacy and ModeBuild only. +// Zip files are supported in LegacyMode and BuildMode only. func (v *Validator) ValidateFromZip(zipPath string) error { - if v.mode != ModeLegacy && v.mode != ModeBuild { - return errors.New("zip files are only supported in ModeLegacy or ModeBuild") + if v.mode != LegacyMode && v.mode != BuildMode { + return errors.New("zip files are only supported in LegacyMode or BuildMode") } r, err := zip.OpenReader(zipPath) @@ -106,16 +106,16 @@ func (v *Validator) ValidateFromZip(zipPath string) error { // ValidateFromFS validates the package accessible through fsys at location. func (v *Validator) ValidateFromFS(location string, fsys fs.FS) error { - if v.mode == ModeLegacy { + if v.mode == LegacyMode { // If we are not explicitly using the linkedfiles.FS, we wrap fsys with // a linkedfiles.BlockFS to block the use of linked files. if _, ok := fsys.(*linkedfiles.FS); !ok { fsys = linkedfiles.NewBlockFS(fsys) } - } else if _, ok := fsys.(*linkedfiles.FS); ok && v.mode == ModeBuild { - return errors.New("linked files are not supported in ModeBuild") - } else if _, ok := fsys.(*linkedfiles.BlockFS); ok && v.mode == ModeSource { - return errors.New("block linked files are not supported in ModeSource") + } else if _, ok := fsys.(*linkedfiles.FS); ok && v.mode == BuildMode { + return errors.New("linked files are not supported in BuildMode") + } else if _, ok := fsys.(*linkedfiles.BlockFS); ok && v.mode == SourceMode { + return errors.New("block linked files are not supported in SourceMode") } return v.validate(location, fsys) @@ -130,7 +130,7 @@ func (v *Validator) validate(location string, fsys fs.FS) error { return errors.New("could not determine specification version for package") } - spec, err := internalvalidator.NewSpec(*pkg.SpecVersion, v.mode) + spec, err := validator.NewSpec(*pkg.SpecVersion, v.mode) if err != nil { return err } @@ -142,30 +142,30 @@ func (v *Validator) validate(location string, fsys fs.FS) error { return nil } -// ValidateFromPath is a convenience function that creates a new Validator in ModeLegacy and calls ValidateFromPath. +// ValidateFromPath is a convenience function that creates a new Validator in LegacyMode and calls ValidateFromPath. // Deprecated: Use NewValidator and ValidateFromPath instead. func ValidateFromPath(path string) error { - v, err := NewValidator(ModeLegacy) + v, err := NewValidator(LegacyMode) if err != nil { return err } return v.ValidateFromPath(path) } -// ValidateFromZip is a convenience function that creates a new Validator in ModeLegacy and calls ValidateFromZip. +// ValidateFromZip is a convenience function that creates a new Validator in LegacyMode and calls ValidateFromZip. // Deprecated: Use NewValidator and ValidateFromZip instead. func ValidateFromZip(zipPath string) error { - v, err := NewValidator(ModeLegacy) + v, err := NewValidator(LegacyMode) if err != nil { return err } return v.ValidateFromZip(zipPath) } -// ValidateFromFS is a convenience function that creates a new Validator in ModeLegacy and calls ValidateFromFS. +// ValidateFromFS is a convenience function that creates a new Validator in LegacyMode and calls ValidateFromFS. // Deprecated: Use NewValidator and ValidateFromFS instead. func ValidateFromFS(location string, fsys fs.FS) error { - v, err := NewValidator(ModeLegacy) + v, err := NewValidator(LegacyMode) if err != nil { return err } diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index 5682fcb58..07181c333 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -1275,7 +1275,7 @@ func TestLinksBehaviorAcrossModes(t *testing.T) { t.Run("build_rejects_link_files", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeBuild) + v, err := NewValidator(BuildMode) require.NoError(t, err) err = v.ValidateFromPath(withLinks) require.Error(t, err) @@ -1286,7 +1286,7 @@ func TestLinksBehaviorAcrossModes(t *testing.T) { t.Run("source_accepts_link_files", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeSource) + v, err := NewValidator(SourceMode) require.NoError(t, err) err = v.ValidateFromPath(withLinks) require.NoError(t, err) @@ -1294,7 +1294,7 @@ func TestLinksBehaviorAcrossModes(t *testing.T) { t.Run("legacy_accepts_link_files", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeLegacy) + v, err := NewValidator(LegacyMode) require.NoError(t, err) err = v.ValidateFromPath(withLinks) require.NoError(t, err) @@ -1313,25 +1313,25 @@ func TestValidateFromFS_rejectsIncompatibleFS(t *testing.T) { t.Run("build_with_linked_fs", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeBuild) + v, err := NewValidator(BuildMode) require.NoError(t, err) err = v.ValidateFromFS(withLinks, linkedfiles.NewFS(withLinks, inner)) require.Error(t, err) - require.ErrorContains(t, err, "linked files are not supported in ModeBuild") + require.ErrorContains(t, err, "linked files are not supported in BuildMode") }) t.Run("source_with_block_fs", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeSource) + v, err := NewValidator(SourceMode) require.NoError(t, err) err = v.ValidateFromFS(withLinks, linkedfiles.NewBlockFS(inner)) require.Error(t, err) - require.ErrorContains(t, err, "block linked files are not supported in ModeSource") + require.ErrorContains(t, err, "block linked files are not supported in SourceMode") }) t.Run("source_with_linked_fs", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeSource) + v, err := NewValidator(SourceMode) require.NoError(t, err) err = v.ValidateFromFS(withLinks, linkedfiles.NewFS(withLinks, inner)) require.NoError(t, err) @@ -1339,12 +1339,12 @@ func TestValidateFromFS_rejectsIncompatibleFS(t *testing.T) { t.Run("build_with_block_fs", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeBuild) + v, err := NewValidator(BuildMode) require.NoError(t, err) err = v.ValidateFromFS("test-package", linkedfiles.NewBlockFS(newMockFS().Good())) if err != nil { - require.NotContains(t, err.Error(), "linked files are not supported in ModeBuild") - require.NotContains(t, err.Error(), "block linked files are not supported in ModeSource") + require.NotContains(t, err.Error(), "linked files are not supported in BuildMode") + require.NotContains(t, err.Error(), "block linked files are not supported in SourceMode") } }) } @@ -1354,7 +1354,7 @@ func TestValidateFromFS_legacyPlainFSBlocksLinks(t *testing.T) { t.Run("legacy_plain_fs_blocks_links", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeLegacy) + v, err := NewValidator(LegacyMode) require.NoError(t, err) err = v.ValidateFromFS("test-package", newMockFS().WithLink()) require.Error(t, err) @@ -1370,7 +1370,7 @@ func TestValidateFromFS_legacyPlainFSBlocksLinks(t *testing.T) { t.Run("legacy_linked_fs_accepts_links", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeLegacy) + v, err := NewValidator(LegacyMode) require.NoError(t, err) err = v.ValidateFromFS(withLinks, linkedfiles.NewFS(withLinks, os.DirFS(withLinks))) require.NoError(t, err) @@ -1443,16 +1443,16 @@ func TestValidateFromZip_modeRestrictions(t *testing.T) { t.Run("source_rejected", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeSource) + v, err := NewValidator(SourceMode) require.NoError(t, err) err = v.ValidateFromZip(zipPath) require.Error(t, err) - require.ErrorContains(t, err, "zip files are only supported in ModeLegacy or ModeBuild") + require.ErrorContains(t, err, "zip files are only supported in LegacyMode or BuildMode") }) t.Run("legacy_allowed", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeLegacy) + v, err := NewValidator(LegacyMode) require.NoError(t, err) err = v.ValidateFromZip(zipPath) require.NoError(t, err) @@ -1460,7 +1460,7 @@ func TestValidateFromZip_modeRestrictions(t *testing.T) { t.Run("build_allowed", func(t *testing.T) { t.Parallel() - v, err := NewValidator(ModeBuild) + v, err := NewValidator(BuildMode) require.NoError(t, err) err = v.ValidateFromZip(zipPath) require.NoError(t, err) @@ -1473,7 +1473,7 @@ func TestValidateFromZip_validatesPackage(t *testing.T) { t.Run("valid_zip", func(t *testing.T) { t.Parallel() zipPath := writePackageZip(t, goodPkg, "good") - v, err := NewValidator(ModeLegacy) + v, err := NewValidator(LegacyMode) require.NoError(t, err) require.NoError(t, v.ValidateFromZip(zipPath)) }) @@ -1481,7 +1481,7 @@ func TestValidateFromZip_validatesPackage(t *testing.T) { t.Run("no_root_directory", func(t *testing.T) { t.Parallel() zipPath := writeMalformedPackageZip(t) - v, err := NewValidator(ModeLegacy) + v, err := NewValidator(LegacyMode) require.NoError(t, err) err = v.ValidateFromZip(zipPath) require.Error(t, err) @@ -1491,7 +1491,7 @@ func TestValidateFromZip_validatesPackage(t *testing.T) { t.Run("multiple_root_directories", func(t *testing.T) { t.Parallel() zipPath := writeMalformedPackageZip(t, "pkg-a", "pkg-b") - v, err := NewValidator(ModeLegacy) + v, err := NewValidator(LegacyMode) require.NoError(t, err) err = v.ValidateFromZip(zipPath) require.Error(t, err) @@ -1513,7 +1513,7 @@ func TestWithWarningsAsErrors_option(t *testing.T) { t.Run("option_overrides_env_false", func(t *testing.T) { t.Setenv("PACKAGE_SPEC_WARNINGS_AS_ERRORS", "false") - v, err := NewValidator(ModeLegacy, WithWarningsAsErrors(true)) + v, err := NewValidator(LegacyMode, WithWarningsAsErrors(true)) require.NoError(t, err) err = v.ValidateFromFS(pkgRootPath, fsys) require.Error(t, err) @@ -1522,7 +1522,7 @@ func TestWithWarningsAsErrors_option(t *testing.T) { t.Run("option_overrides_env_true", func(t *testing.T) { t.Setenv("PACKAGE_SPEC_WARNINGS_AS_ERRORS", "true") - v, err := NewValidator(ModeLegacy, WithWarningsAsErrors(false)) + v, err := NewValidator(LegacyMode, WithWarningsAsErrors(false)) require.NoError(t, err) err = v.ValidateFromFS(pkgRootPath, fsys) require.NoError(t, err) From ebac0c74a236fc23ec11668a04e4967046c68926 Mon Sep 17 00:00:00 2001 From: Tere Date: Fri, 5 Jun 2026 14:14:22 +0200 Subject: [PATCH 29/30] Refactor validator instantiation to unify creation method - Renamed NewValidator function to New for improved clarity and consistency. - Updated all references in tests and validation logic to use the new instantiation method. - Ensured that the new method maintains the same functionality while simplifying the API. - Adjusted test cases to reflect the changes in the validator creation process. --- code/go/pkg/validator/validator.go | 10 +++---- code/go/pkg/validator/validator_test.go | 36 ++++++++++++------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index 494ec672a..76557a6d5 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -47,8 +47,8 @@ func WithWarningsAsErrors(enabled bool) Option { return func(v *Validator) { v.warningsAsErrors = enabled } } -// NewValidator creates a Validator for the given mode and options. -func NewValidator(mode Mode, opts ...Option) (*Validator, error) { +// New creates a Validator for the given mode and options. +func New(mode Mode, opts ...Option) (*Validator, error) { if !mode.Valid() { return nil, fmt.Errorf("invalid validation mode %q", mode) } @@ -145,7 +145,7 @@ func (v *Validator) validate(location string, fsys fs.FS) error { // ValidateFromPath is a convenience function that creates a new Validator in LegacyMode and calls ValidateFromPath. // Deprecated: Use NewValidator and ValidateFromPath instead. func ValidateFromPath(path string) error { - v, err := NewValidator(LegacyMode) + v, err := New(LegacyMode) if err != nil { return err } @@ -155,7 +155,7 @@ func ValidateFromPath(path string) error { // ValidateFromZip is a convenience function that creates a new Validator in LegacyMode and calls ValidateFromZip. // Deprecated: Use NewValidator and ValidateFromZip instead. func ValidateFromZip(zipPath string) error { - v, err := NewValidator(LegacyMode) + v, err := New(LegacyMode) if err != nil { return err } @@ -165,7 +165,7 @@ func ValidateFromZip(zipPath string) error { // ValidateFromFS is a convenience function that creates a new Validator in LegacyMode and calls ValidateFromFS. // Deprecated: Use NewValidator and ValidateFromFS instead. func ValidateFromFS(location string, fsys fs.FS) error { - v, err := NewValidator(LegacyMode) + v, err := New(LegacyMode) if err != nil { return err } diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index 07181c333..879a1fb88 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -1275,7 +1275,7 @@ func TestLinksBehaviorAcrossModes(t *testing.T) { t.Run("build_rejects_link_files", func(t *testing.T) { t.Parallel() - v, err := NewValidator(BuildMode) + v, err := New(BuildMode) require.NoError(t, err) err = v.ValidateFromPath(withLinks) require.Error(t, err) @@ -1286,7 +1286,7 @@ func TestLinksBehaviorAcrossModes(t *testing.T) { t.Run("source_accepts_link_files", func(t *testing.T) { t.Parallel() - v, err := NewValidator(SourceMode) + v, err := New(SourceMode) require.NoError(t, err) err = v.ValidateFromPath(withLinks) require.NoError(t, err) @@ -1294,7 +1294,7 @@ func TestLinksBehaviorAcrossModes(t *testing.T) { t.Run("legacy_accepts_link_files", func(t *testing.T) { t.Parallel() - v, err := NewValidator(LegacyMode) + v, err := New(LegacyMode) require.NoError(t, err) err = v.ValidateFromPath(withLinks) require.NoError(t, err) @@ -1303,7 +1303,7 @@ func TestLinksBehaviorAcrossModes(t *testing.T) { } func TestNewValidator_RejectsInvalidMode(t *testing.T) { - _, err := NewValidator(Mode("invalid")) + _, err := New(Mode("invalid")) require.Error(t, err) } @@ -1313,7 +1313,7 @@ func TestValidateFromFS_rejectsIncompatibleFS(t *testing.T) { t.Run("build_with_linked_fs", func(t *testing.T) { t.Parallel() - v, err := NewValidator(BuildMode) + v, err := New(BuildMode) require.NoError(t, err) err = v.ValidateFromFS(withLinks, linkedfiles.NewFS(withLinks, inner)) require.Error(t, err) @@ -1322,7 +1322,7 @@ func TestValidateFromFS_rejectsIncompatibleFS(t *testing.T) { t.Run("source_with_block_fs", func(t *testing.T) { t.Parallel() - v, err := NewValidator(SourceMode) + v, err := New(SourceMode) require.NoError(t, err) err = v.ValidateFromFS(withLinks, linkedfiles.NewBlockFS(inner)) require.Error(t, err) @@ -1331,7 +1331,7 @@ func TestValidateFromFS_rejectsIncompatibleFS(t *testing.T) { t.Run("source_with_linked_fs", func(t *testing.T) { t.Parallel() - v, err := NewValidator(SourceMode) + v, err := New(SourceMode) require.NoError(t, err) err = v.ValidateFromFS(withLinks, linkedfiles.NewFS(withLinks, inner)) require.NoError(t, err) @@ -1339,7 +1339,7 @@ func TestValidateFromFS_rejectsIncompatibleFS(t *testing.T) { t.Run("build_with_block_fs", func(t *testing.T) { t.Parallel() - v, err := NewValidator(BuildMode) + v, err := New(BuildMode) require.NoError(t, err) err = v.ValidateFromFS("test-package", linkedfiles.NewBlockFS(newMockFS().Good())) if err != nil { @@ -1354,7 +1354,7 @@ func TestValidateFromFS_legacyPlainFSBlocksLinks(t *testing.T) { t.Run("legacy_plain_fs_blocks_links", func(t *testing.T) { t.Parallel() - v, err := NewValidator(LegacyMode) + v, err := New(LegacyMode) require.NoError(t, err) err = v.ValidateFromFS("test-package", newMockFS().WithLink()) require.Error(t, err) @@ -1370,7 +1370,7 @@ func TestValidateFromFS_legacyPlainFSBlocksLinks(t *testing.T) { t.Run("legacy_linked_fs_accepts_links", func(t *testing.T) { t.Parallel() - v, err := NewValidator(LegacyMode) + v, err := New(LegacyMode) require.NoError(t, err) err = v.ValidateFromFS(withLinks, linkedfiles.NewFS(withLinks, os.DirFS(withLinks))) require.NoError(t, err) @@ -1443,7 +1443,7 @@ func TestValidateFromZip_modeRestrictions(t *testing.T) { t.Run("source_rejected", func(t *testing.T) { t.Parallel() - v, err := NewValidator(SourceMode) + v, err := New(SourceMode) require.NoError(t, err) err = v.ValidateFromZip(zipPath) require.Error(t, err) @@ -1452,7 +1452,7 @@ func TestValidateFromZip_modeRestrictions(t *testing.T) { t.Run("legacy_allowed", func(t *testing.T) { t.Parallel() - v, err := NewValidator(LegacyMode) + v, err := New(LegacyMode) require.NoError(t, err) err = v.ValidateFromZip(zipPath) require.NoError(t, err) @@ -1460,7 +1460,7 @@ func TestValidateFromZip_modeRestrictions(t *testing.T) { t.Run("build_allowed", func(t *testing.T) { t.Parallel() - v, err := NewValidator(BuildMode) + v, err := New(BuildMode) require.NoError(t, err) err = v.ValidateFromZip(zipPath) require.NoError(t, err) @@ -1473,7 +1473,7 @@ func TestValidateFromZip_validatesPackage(t *testing.T) { t.Run("valid_zip", func(t *testing.T) { t.Parallel() zipPath := writePackageZip(t, goodPkg, "good") - v, err := NewValidator(LegacyMode) + v, err := New(LegacyMode) require.NoError(t, err) require.NoError(t, v.ValidateFromZip(zipPath)) }) @@ -1481,7 +1481,7 @@ func TestValidateFromZip_validatesPackage(t *testing.T) { t.Run("no_root_directory", func(t *testing.T) { t.Parallel() zipPath := writeMalformedPackageZip(t) - v, err := NewValidator(LegacyMode) + v, err := New(LegacyMode) require.NoError(t, err) err = v.ValidateFromZip(zipPath) require.Error(t, err) @@ -1491,7 +1491,7 @@ func TestValidateFromZip_validatesPackage(t *testing.T) { t.Run("multiple_root_directories", func(t *testing.T) { t.Parallel() zipPath := writeMalformedPackageZip(t, "pkg-a", "pkg-b") - v, err := NewValidator(LegacyMode) + v, err := New(LegacyMode) require.NoError(t, err) err = v.ValidateFromZip(zipPath) require.Error(t, err) @@ -1513,7 +1513,7 @@ func TestWithWarningsAsErrors_option(t *testing.T) { t.Run("option_overrides_env_false", func(t *testing.T) { t.Setenv("PACKAGE_SPEC_WARNINGS_AS_ERRORS", "false") - v, err := NewValidator(LegacyMode, WithWarningsAsErrors(true)) + v, err := New(LegacyMode, WithWarningsAsErrors(true)) require.NoError(t, err) err = v.ValidateFromFS(pkgRootPath, fsys) require.Error(t, err) @@ -1522,7 +1522,7 @@ func TestWithWarningsAsErrors_option(t *testing.T) { t.Run("option_overrides_env_true", func(t *testing.T) { t.Setenv("PACKAGE_SPEC_WARNINGS_AS_ERRORS", "true") - v, err := NewValidator(LegacyMode, WithWarningsAsErrors(false)) + v, err := New(LegacyMode, WithWarningsAsErrors(false)) require.NoError(t, err) err = v.ValidateFromFS(pkgRootPath, fsys) require.NoError(t, err) From d895e496b1d4ebc2e6685135382e2f3f80b88b14 Mon Sep 17 00:00:00 2001 From: Tere Date: Fri, 5 Jun 2026 14:24:57 +0200 Subject: [PATCH 30/30] Add logging for validation mode in technical preview - Introduced a log statement to warn users when the validation mode is in technical preview. - Ensured that the logging occurs only when the mode is not set to LegacyMode, enhancing user awareness of the current validation state. --- code/go/pkg/validator/validator.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/code/go/pkg/validator/validator.go b/code/go/pkg/validator/validator.go index 76557a6d5..f9afd205e 100644 --- a/code/go/pkg/validator/validator.go +++ b/code/go/pkg/validator/validator.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io/fs" + "log" "os" "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" @@ -59,6 +60,7 @@ func New(mode Mode, opts ...Option) (*Validator, error) { for _, opt := range opts { opt(v) } + return v, nil } @@ -136,6 +138,10 @@ func (v *Validator) validate(location string, fsys fs.FS) error { } spec.WarningsAsErrors = v.warningsAsErrors + if v.mode != LegacyMode { + log.Printf("Warning: validation mode '%s' is in technical preview", v.mode) + } + if errs := spec.ValidatePackage(*pkg); len(errs) > 0 { return errs }