From e1a0cb2233e655adcd6699702f93eb506c8a7201 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 11:47:04 +0200 Subject: [PATCH 01/23] 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 08356fa8d0dc0db2c2edc88897f34b0d637e3220 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 11:47:48 +0200 Subject: [PATCH 02/23] 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 135f03aaa195d9dadc8e1e0406f3b1924e2a79e4 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 12:13:35 +0200 Subject: [PATCH 03/23] 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 9d454d57fa20f643827786407395232f130e5a8e Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 13:50:38 +0200 Subject: [PATCH 04/23] Add source mode: tag source-only rules and reject _embedded_ecs (task 03) - Tag ValidateExternalFieldsWithDevFolder and ValidateTestPackageRequirements with modes:[Legacy, Source] so they are skipped in build mode. - Add ValidateNoEmbeddedEcsInDynamicTemplates (source mode only, integration packages): walks data_stream/*/manifest.yml and rejects any dynamic_templates entry whose key starts with '_embedded_ecs'. These keys are auto-injected by elastic-package at build time and must not appear in source packages. - Register the new rule with modes:[Source] and types:[integration]; it does not run in legacy mode, preserving byte-for-byte legacy behaviour. - Resolve the TODO at spec/integration/data_stream/manifest.spec.yml:351: replace with a comment pointing to the new validator and issue #549. - Add fixture test/packages/bad_embedded_ecs/: minimal valid integration package containing _embedded_ecs-* entries; passes legacy, fails source. Also serves as JSON schema coverage for patternProperties '^_embedded_ecs'. - Update test/packages/good_v3/data_stream/ecs_import_mappings/manifest.yml: replace 90+ _embedded_ecs-* dynamic template entries with regular ones so good_v3 passes source-mode validation (build artifacts must not appear in source packages). - Add to api_test.go: TestSourceMode_GoodPackages - all good_* fixtures pass ModeSource TestSourceMode_BadEmbeddedEcs - bad_embedded_ecs: legacy pass, source fail (_embedded_ecs), build pass TestBuildMode_SkipsBuildExcludedRules - ValidateExternalFieldsWithDevFolder absent in build mode applyPackageFilter helper - applies validation.yml exclusions so packages with known-excluded codes (e.g. SVR00006) are treated consistently Co-Authored-By: Claude Sonnet 4.6 --- .../semantic/validate_no_embedded_ecs.go | 79 +++ .../semantic/validate_no_embedded_ecs_test.go | 172 +++++++ code/go/internal/validator/spec.go | 8 +- code/go/pkg/validator/api_test.go | 114 +++++ .../integration/data_stream/manifest.spec.yml | 3 +- test/packages/bad_embedded_ecs/changelog.yml | 5 + .../logs/agent/stream/stream.yml.hbs | 7 + .../data_stream/logs/fields/base-fields.yml | 12 + .../data_stream/logs/manifest.yml | 31 ++ test/packages/bad_embedded_ecs/docs/README.md | 7 + test/packages/bad_embedded_ecs/manifest.yml | 24 + .../ecs_import_mappings/manifest.yml | 466 +----------------- 12 files changed, 469 insertions(+), 459 deletions(-) create mode 100644 code/go/internal/validator/semantic/validate_no_embedded_ecs.go create mode 100644 code/go/internal/validator/semantic/validate_no_embedded_ecs_test.go create mode 100644 test/packages/bad_embedded_ecs/changelog.yml create mode 100644 test/packages/bad_embedded_ecs/data_stream/logs/agent/stream/stream.yml.hbs create mode 100644 test/packages/bad_embedded_ecs/data_stream/logs/fields/base-fields.yml create mode 100644 test/packages/bad_embedded_ecs/data_stream/logs/manifest.yml create mode 100644 test/packages/bad_embedded_ecs/docs/README.md create mode 100644 test/packages/bad_embedded_ecs/manifest.yml diff --git a/code/go/internal/validator/semantic/validate_no_embedded_ecs.go b/code/go/internal/validator/semantic/validate_no_embedded_ecs.go new file mode 100644 index 000000000..fa03303c2 --- /dev/null +++ b/code/go/internal/validator/semantic/validate_no_embedded_ecs.go @@ -0,0 +1,79 @@ +// 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 semantic + +import ( + "io/fs" + "path" + "strings" + + "gopkg.in/yaml.v3" + + "github.com/elastic/package-spec/v3/code/go/internal/fspath" + "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" +) + +// embeddedEcsDataStreamManifest is a minimal struct for unmarshalling just the +// elasticsearch.index_template.mappings.dynamic_templates section of a data stream manifest. +type embeddedEcsDataStreamManifest struct { + Elasticsearch struct { + IndexTemplate struct { + Mappings struct { + DynamicTemplates []map[string]any `yaml:"dynamic_templates"` + } `yaml:"mappings"` + } `yaml:"index_template"` + } `yaml:"elasticsearch"` +} + +// ValidateNoEmbeddedEcsInDynamicTemplates rejects data stream manifests that contain +// dynamic_templates entries whose keys match the "^_embedded_ecs" pattern. +// +// Keys starting with "_embedded_ecs" are auto-injected by elastic-package at build time +// when import_mappings is enabled. They must not appear in source packages and are +// rejected in source validation mode. Built packages (where these keys are expected) +// should be validated with ModeBuild, not ModeSource. +func ValidateNoEmbeddedEcsInDynamicTemplates(fsys fspath.FS) specerrors.ValidationErrors { + dataStreams, err := listDataStreams(fsys) + if err != nil { + return specerrors.ValidationErrors{specerrors.NewStructuredError(err, specerrors.UnassignedCode)} + } + + var errs specerrors.ValidationErrors + for _, dataStream := range dataStreams { + manifestPath := path.Join(dataStreamDir, dataStream, "manifest.yml") + streamErrs := checkDataStreamManifestForEmbeddedEcs(fsys, manifestPath) + errs = append(errs, streamErrs...) + } + return errs +} + +func checkDataStreamManifestForEmbeddedEcs(fsys fspath.FS, manifestPath string) specerrors.ValidationErrors { + data, err := fs.ReadFile(fsys, manifestPath) + if err != nil { + return specerrors.ValidationErrors{ + specerrors.NewStructuredErrorf("file \"%s\" is invalid: failed to read manifest: %w", fsys.Path(manifestPath), err), + } + } + + var manifest embeddedEcsDataStreamManifest + if err := yaml.Unmarshal(data, &manifest); err != nil { + return specerrors.ValidationErrors{ + specerrors.NewStructuredErrorf("file \"%s\" is invalid: failed to parse manifest: %w", fsys.Path(manifestPath), err), + } + } + + var errs specerrors.ValidationErrors + for _, entry := range manifest.Elasticsearch.IndexTemplate.Mappings.DynamicTemplates { + for key := range entry { + if strings.HasPrefix(key, "_embedded_ecs") { + errs = append(errs, specerrors.NewStructuredErrorf( + "file \"%s\" is invalid: dynamic template %q starts with \"_embedded_ecs\"; this key is auto-injected at build time and must not appear in source packages", + fsys.Path(manifestPath), key, + )) + } + } + } + return errs +} diff --git a/code/go/internal/validator/semantic/validate_no_embedded_ecs_test.go b/code/go/internal/validator/semantic/validate_no_embedded_ecs_test.go new file mode 100644 index 000000000..5b545a1f7 --- /dev/null +++ b/code/go/internal/validator/semantic/validate_no_embedded_ecs_test.go @@ -0,0 +1,172 @@ +// 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 semantic + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/elastic/package-spec/v3/code/go/internal/fspath" +) + +func TestValidateNoEmbeddedEcsInDynamicTemplates(t *testing.T) { + tests := []struct { + name string + manifestYAML string + skipWriteStream bool // when true, data_stream/ is not created (simulates input/content packages) + expectErrors bool + errorContains []string + }{ + { + name: "no data_stream directory", + skipWriteStream: true, + expectErrors: false, + }, + { + name: "no dynamic_templates", + manifestYAML: ` +title: Test stream +type: logs +streams: + - input: logfile + title: Sample + description: Sample +`, + expectErrors: false, + }, + { + name: "dynamic_templates with regular names", + manifestYAML: ` +title: Test stream +type: logs +streams: + - input: logfile + title: Sample + description: Sample +elasticsearch: + index_template: + mappings: + dynamic_templates: + - my_ip_template: + mapping: + type: ip + match: ip + - my_date_template: + mapping: + type: date + match: timestamp +`, + expectErrors: false, + }, + { + name: "dynamic_templates with _embedded_ecs key", + manifestYAML: ` +title: Test stream +type: logs +streams: + - input: logfile + title: Sample + description: Sample +elasticsearch: + index_template: + mappings: + dynamic_templates: + - _embedded_ecs-ip_to_ip: + mapping: + type: ip + match: ip +`, + expectErrors: true, + errorContains: []string{"_embedded_ecs-ip_to_ip", "_embedded_ecs"}, + }, + { + name: "dynamic_templates mixed: regular and _embedded_ecs", + manifestYAML: ` +title: Test stream +type: logs +streams: + - input: logfile + title: Sample + description: Sample +elasticsearch: + index_template: + mappings: + dynamic_templates: + - my_ip_template: + mapping: + type: ip + match: ip + - _embedded_ecs-date_to_date: + mapping: + type: date + match: timestamp +`, + expectErrors: true, + errorContains: []string{"_embedded_ecs-date_to_date"}, + }, + { + name: "multiple _embedded_ecs entries", + manifestYAML: ` +title: Test stream +type: logs +streams: + - input: logfile + title: Sample + description: Sample +elasticsearch: + index_template: + mappings: + dynamic_templates: + - _embedded_ecs-ip_to_ip: + mapping: + type: ip + match: ip + - _embedded_ecs-port_to_long: + mapping: + type: long + match: port +`, + expectErrors: true, + errorContains: []string{"_embedded_ecs-ip_to_ip", "_embedded_ecs-port_to_long"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tempDir := t.TempDir() + + if !tc.skipWriteStream { + // Create data_stream/test_stream/manifest.yml + dsManifestDir := filepath.Join(tempDir, "data_stream", "test_stream") + require.NoError(t, os.MkdirAll(dsManifestDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(dsManifestDir, "manifest.yml"), []byte(tc.manifestYAML), 0644)) + } + + fsys := fspath.DirFS(tempDir) + errs := ValidateNoEmbeddedEcsInDynamicTemplates(fsys) + + if !tc.expectErrors { + assert.Nil(t, errs, "expected no errors but got: %v", errs) + return + } + + require.NotNil(t, errs, "expected validation errors but got none") + var sb strings.Builder + for _, e := range errs { + sb.WriteString(e.Error()) + sb.WriteString("\n") + } + combined := sb.String() + for _, substr := range tc.errorContains { + assert.Contains(t, combined, substr) + } + }) + } +} diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index 8e2e8829f..3deb518d6 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -221,7 +221,8 @@ func (s Spec) rules(pkgType string, rootSpec spectypes.ItemSpec) validationRules {fn: semantic.ValidateDimensionFields, types: []string{"integration", "input"}}, {fn: semantic.ValidateDateFields, types: []string{"integration", "input"}}, {fn: semantic.ValidateRequiredFields, types: []string{"integration", "input"}}, - {fn: semantic.ValidateExternalFieldsWithDevFolder, types: []string{"integration", "input"}}, + {fn: semantic.ValidateExternalFieldsWithDevFolder, types: []string{"integration", "input"}, + modes: []modes.Mode{modes.Legacy, modes.Source}}, {fn: warnOn(semantic.ValidateVisualizationsUsedByValue), types: []string{"integration", "content"}, until: semver.MustParse("3.0.0")}, {fn: semantic.ValidateVisualizationsUsedByValue, types: []string{"integration", "content"}, since: semver.MustParse("3.0.0")}, {fn: semantic.ValidateILMPolicyPresent, since: semver.MustParse("2.0.0"), types: []string{"integration"}}, @@ -254,7 +255,10 @@ func (s Spec) rules(pkgType string, rootSpec spectypes.ItemSpec) validationRules {fn: semantic.ValidateIntegrationInputQualifier, types: []string{"integration"}, since: semver.MustParse("3.6.0")}, {fn: semantic.ValidateDeprecatedReplacedBy, since: semver.MustParse("3.6.0")}, {fn: semantic.ValidatePackageReferences, types: []string{"integration"}, since: semver.MustParse("3.6.0")}, - {fn: semantic.ValidateTestPackageRequirements, types: []string{"integration"}, since: semver.MustParse("3.6.0")}, + {fn: semantic.ValidateTestPackageRequirements, types: []string{"integration"}, since: semver.MustParse("3.6.0"), + modes: []modes.Mode{modes.Legacy, modes.Source}}, + {fn: semantic.ValidateNoEmbeddedEcsInDynamicTemplates, types: []string{"integration"}, + modes: []modes.Mode{modes.Source}}, } var validationRules validationRules diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go index a2bd12e76..2ead972ca 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -6,9 +6,11 @@ package validator import ( "archive/zip" + "errors" "io/fs" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -59,6 +61,32 @@ func extractMessages(err error) []string { return []string{err.Error()} } +// applyPackageFilter applies the validation.yml filter from pkgPath if it exists, +// returning the filtered error (or the original if no filter is present). +// +// This mirrors the filter logic in TestValidateFile so that packages with known +// excluded error codes (e.g. SVR00006 in good_deployer_system_benchmark) are +// treated the same way across all integration tests. +func applyPackageFilter(t *testing.T, pkgPath string, err error) error { + t.Helper() + if err == nil { + return nil + } + verrs, ok := err.(specerrors.ValidationErrors) + if !ok { + return err + } + filterConfig, filterErr := specerrors.LoadConfigFilter(os.DirFS(pkgPath)) + if errors.Is(filterErr, os.ErrNotExist) { + return err // no validation.yml — return original + } + require.NoError(t, filterErr, "loading validation.yml for %s", pkgPath) + filter := specerrors.NewFilter(filterConfig) + result, filterRunErr := filter.Run(verrs) + require.NoError(t, filterRunErr, "running filter for %s", pkgPath) + return result.Processed +} + // listTestPackages returns the names of every directory under testPackagesDir. func listTestPackages(t *testing.T) []string { t.Helper() @@ -171,6 +199,92 @@ func TestLegacyPreservation_FromFS(t *testing.T) { } } +// ----------------------------------------------------------------------- +// TestSourceMode_GoodPackages +// +// Asserts that every good_* fixture under test/packages/ passes source-mode +// validation. Source mode runs all the rules that legacy mode runs plus +// source-only checks (e.g. rejecting _embedded_ecs in dynamic_templates). +// All packages prefixed "good_" are expected to represent valid source trees. +// ----------------------------------------------------------------------- + +func TestSourceMode_GoodPackages(t *testing.T) { + for _, pkgName := range listTestPackages(t) { + if pkgName != "good" && !strings.HasPrefix(pkgName, "good_") { + continue + } + t.Run(pkgName, func(t *testing.T) { + t.Parallel() + pkgPath := filepath.Join(testPackagesDir, pkgName) + + v, err := NewFromPath(ModeSource, pkgPath) + require.NoError(t, err) + rawErr := v.Validate() + filteredErr := applyPackageFilter(t, pkgPath, rawErr) + assert.NoError(t, filteredErr, "package %q should be valid in source mode (after applying validation.yml filter)", pkgName) + }) + } +} + +// ----------------------------------------------------------------------- +// TestSourceMode_BadEmbeddedEcs +// +// Asserts that a package containing _embedded_ecs keys in dynamic_templates: +// - passes legacy validation (_embedded_ecs is not checked in legacy mode) +// - fails source-mode validation with a clear reference to the offending key +// - passes build-mode validation (rule is source-only, not build-only) +// ----------------------------------------------------------------------- + +func TestSourceMode_BadEmbeddedEcs(t *testing.T) { + packagePath := filepath.Join(testPackagesDir, "bad_embedded_ecs") + + // Legacy mode: ValidateNoEmbeddedEcsInDynamicTemplates does not run → passes. + legacyV, err := NewFromPath(ModeLegacy, packagePath) + require.NoError(t, err) + assert.NoError(t, legacyV.Validate(), "bad_embedded_ecs should pass legacy validation") + + // Source mode: _embedded_ecs is rejected → fails with a clear error. + sourceV, err := NewFromPath(ModeSource, packagePath) + require.NoError(t, err) + sourceErr := sourceV.Validate() + require.Error(t, sourceErr, "bad_embedded_ecs should fail source-mode validation") + assert.Contains(t, sourceErr.Error(), "_embedded_ecs") + + // Build mode: rule is source-only (modes:[Source]), not build-only → passes. + buildV, err := NewFromPath(ModeBuild, packagePath) + require.NoError(t, err) + assert.NoError(t, buildV.Validate(), "bad_embedded_ecs should pass build-mode validation (_embedded_ecs is a build artifact)") +} + +// ----------------------------------------------------------------------- +// TestBuildMode_SkipsBuildExcludedRules +// +// Verifies that rules tagged modes:[Legacy, Source] (i.e. excluded from build +// mode) do not fire when validating in ModeBuild. +// +// ValidateExternalFieldsWithDevFolder is the canary: good_integration_with_dev_tools +// has external fields and a _dev/build/build.yml; it passes in source mode via +// that rule — in build mode the rule must be absent (no "external key defined" error). +// ----------------------------------------------------------------------- + +func TestBuildMode_SkipsBuildExcludedRules(t *testing.T) { + // good_integration_with_dev_tools has external fields + _dev/build/build.yml + // and is the canonical fixture for ValidateExternalFieldsWithDevFolder. + packagePath := filepath.Join(testPackagesDir, "good_integration_with_dev_tools") + + buildV, err := NewFromPath(ModeBuild, packagePath) + require.NoError(t, err) + + buildErr := buildV.Validate() + if buildErr != nil { + // The only acceptable errors here are from build-mode rejection rules + // (Tasks 04-07, not yet implemented). ValidateExternalFieldsWithDevFolder + // must NOT have contributed an error. + assert.NotContains(t, buildErr.Error(), "external key defined", + "ValidateExternalFieldsWithDevFolder must not run in build mode") + } +} + // ----------------------------------------------------------------------- // TestLegacyPreservation_FromZip (golden test) // diff --git a/spec/integration/data_stream/manifest.spec.yml b/spec/integration/data_stream/manifest.spec.yml index 640ca5cf6..9da578a3f 100644 --- a/spec/integration/data_stream/manifest.spec.yml +++ b/spec/integration/data_stream/manifest.spec.yml @@ -348,7 +348,8 @@ spec: $ref: "./fields/fields.spec.yml#/items/properties/index" patternProperties: # Exception for fields imported by elastic-package when import_mappings is used. - # TODO: Allow this only on built packages. + # Source packages: rejected by ValidateNoEmbeddedEcsInDynamicTemplates (issue #549). + # Built packages: permitted (the key is auto-injected at build time). "^_embedded_ecs": type: object additionalProperties: false diff --git a/test/packages/bad_embedded_ecs/changelog.yml b/test/packages/bad_embedded_ecs/changelog.yml new file mode 100644 index 000000000..27e62eecd --- /dev/null +++ b/test/packages/bad_embedded_ecs/changelog.yml @@ -0,0 +1,5 @@ +- version: 0.0.1 + changes: + - description: Initial release + type: enhancement + link: https://github.com/elastic/package-spec/pull/1 diff --git a/test/packages/bad_embedded_ecs/data_stream/logs/agent/stream/stream.yml.hbs b/test/packages/bad_embedded_ecs/data_stream/logs/agent/stream/stream.yml.hbs new file mode 100644 index 000000000..5845510de --- /dev/null +++ b/test/packages/bad_embedded_ecs/data_stream/logs/agent/stream/stream.yml.hbs @@ -0,0 +1,7 @@ +paths: +{{#each paths as |path i|}} + - {{path}} +{{/each}} +exclude_files: [".gz$"] +processors: + - add_locale: ~ diff --git a/test/packages/bad_embedded_ecs/data_stream/logs/fields/base-fields.yml b/test/packages/bad_embedded_ecs/data_stream/logs/fields/base-fields.yml new file mode 100644 index 000000000..7c798f453 --- /dev/null +++ b/test/packages/bad_embedded_ecs/data_stream/logs/fields/base-fields.yml @@ -0,0 +1,12 @@ +- name: data_stream.type + type: constant_keyword + description: Data stream type. +- name: data_stream.dataset + type: constant_keyword + description: Data stream dataset. +- name: data_stream.namespace + type: constant_keyword + description: Data stream namespace. +- name: '@timestamp' + type: date + description: Event timestamp. diff --git a/test/packages/bad_embedded_ecs/data_stream/logs/manifest.yml b/test/packages/bad_embedded_ecs/data_stream/logs/manifest.yml new file mode 100644 index 000000000..f4a360472 --- /dev/null +++ b/test/packages/bad_embedded_ecs/data_stream/logs/manifest.yml @@ -0,0 +1,31 @@ +# This data stream intentionally contains _embedded_ecs-* dynamic template keys. +# It serves two purposes: +# 1. Schema coverage: verifies the spec's patternProperties "^_embedded_ecs" block +# accepts these keys (exercised by TestLegacyPreservation_FromPath). +# 2. Semantic rejection: verifies ValidateNoEmbeddedEcsInDynamicTemplates rejects +# them in source mode (exercised by TestSourceMode_BadEmbeddedEcs). +title: Logs +type: logs +streams: + - input: logfile + title: Sample logs + description: Collect sample logs + vars: + - name: paths + type: text + title: Paths + multi: true + default: + - /var/log/*.log +elasticsearch: + index_template: + mappings: + dynamic_templates: + - _embedded_ecs-ip_to_ip: + mapping: + type: ip + match: ip + - _embedded_ecs-port_to_long: + mapping: + type: long + match: port diff --git a/test/packages/bad_embedded_ecs/docs/README.md b/test/packages/bad_embedded_ecs/docs/README.md new file mode 100644 index 000000000..05f822286 --- /dev/null +++ b/test/packages/bad_embedded_ecs/docs/README.md @@ -0,0 +1,7 @@ +# Bad ECS Embedded in Source + +Test fixture: integration package containing `_embedded_ecs` keys in `dynamic_templates`. + +These keys are auto-injected by `elastic-package` at build time when `import_mappings` is +enabled. In source packages they must not appear. This package is valid in legacy mode but +rejected in source mode by `ValidateNoEmbeddedEcsInDynamicTemplates`. diff --git a/test/packages/bad_embedded_ecs/manifest.yml b/test/packages/bad_embedded_ecs/manifest.yml new file mode 100644 index 000000000..178c7c751 --- /dev/null +++ b/test/packages/bad_embedded_ecs/manifest.yml @@ -0,0 +1,24 @@ +format_version: 3.6.0 +name: bad_embedded_ecs +title: Bad ECS embedded in source +description: > + Integration with _embedded_ecs keys in dynamic_templates. + Valid in legacy mode; rejected in source mode by ValidateNoEmbeddedEcsInDynamicTemplates. +version: 0.0.1 +type: integration +source: + license: "Apache-2.0" +conditions: + kibana: + version: '^8.0.0' +policy_templates: + - name: logs + title: Logs + description: Collect logs + inputs: + - type: logfile + title: Collect logfile logs + description: Collect logs using logfile input +owner: + github: elastic/foobar + type: community diff --git a/test/packages/good_v3/data_stream/ecs_import_mappings/manifest.yml b/test/packages/good_v3/data_stream/ecs_import_mappings/manifest.yml index e7dab8a94..38d6f4102 100644 --- a/test/packages/good_v3/data_stream/ecs_import_mappings/manifest.yml +++ b/test/packages/good_v3/data_stream/ecs_import_mappings/manifest.yml @@ -1,4 +1,9 @@ -title: "Includes ECS imported mappings in manifest" +# Previously contained _embedded_ecs-* entries to test JSON schema acceptance of +# patternProperties "^_embedded_ecs". Those entries were moved to +# test/packages/bad_embedded_ecs/data_stream/logs/manifest.yml (issue #549): +# _embedded_ecs keys are build artifacts and must not appear in source packages. +# This data stream now exercises regular dynamic_templates entries only. +title: "Includes custom dynamic mappings in manifest" type: logs streams: - input: logfile @@ -15,462 +20,11 @@ elasticsearch: index_template: mappings: dynamic_templates: - - _embedded_ecs-ecs_timestamp: - mapping: - ignore_malformed: false - type: date - path_match: '@timestamp' - - _embedded_ecs-data_stream_to_constant: - mapping: - type: constant_keyword - path_match: data_stream.* - - _embedded_ecs-resolved_ip_to_ip: - mapping: - type: ip - match: resolved_ip - - _embedded_ecs-forwarded_ip_to_ip: - mapping: - type: ip - match: forwarded_ip - match_mapping_type: string - - _embedded_ecs-ip_to_ip: - mapping: - type: ip - match: ip + - string_as_keyword: match_mapping_type: string - - _embedded_ecs-port_to_long: - mapping: - type: long - match: port - - _embedded_ecs-thread_id_to_long: - mapping: - type: long - path_match: '*.thread.id' - - _embedded_ecs-status_code_to_long: - mapping: - type: long - match: status_code - - _embedded_ecs-line_to_long: - mapping: - type: long - path_match: '*.file.line' - - _embedded_ecs-priority_to_long: - mapping: - type: long - path_match: log.syslog.priority - - _embedded_ecs-code_to_long: - mapping: - type: long - path_match: '*.facility.code' - - _embedded_ecs-code_to_long: - mapping: - type: long - path_match: '*.severity.code' - - _embedded_ecs-bytes_to_long: - mapping: - type: long - match: bytes - path_unmatch: '*.data.bytes' - - _embedded_ecs-packets_to_long: - mapping: - type: long - match: packets - - _embedded_ecs-public_key_exponent_to_long: - mapping: - type: long - match: public_key_exponent - - _embedded_ecs-severity_to_long: - mapping: - type: long - path_match: event.severity - - _embedded_ecs-duration_to_long: - mapping: - type: long - path_match: event.duration - - _embedded_ecs-pid_to_long: - mapping: - type: long - match: pid - - _embedded_ecs-uptime_to_long: - mapping: - type: long - match: uptime - - _embedded_ecs-sequence_to_long: - mapping: - type: long - match: sequence - - _embedded_ecs-entropy_to_long: - mapping: - type: long - match: '*entropy' - - _embedded_ecs-size_to_long: - mapping: - type: long - match: '*size' - - _embedded_ecs-entrypoint_to_long: - mapping: - type: long - match: entrypoint - - _embedded_ecs-ttl_to_long: - mapping: - type: long - match: ttl - - _embedded_ecs-major_to_long: - mapping: - type: long - match: major - - _embedded_ecs-minor_to_long: - mapping: - type: long - match: minor - - _embedded_ecs-as_number_to_long: - mapping: - type: long - path_match: '*.as.number' - - _embedded_ecs-pgid_to_long: - mapping: - type: long - match: pgid - - _embedded_ecs-exit_code_to_long: - mapping: - type: long - match: exit_code - - _embedded_ecs-chi_to_long: - mapping: - type: long - match: chi2 - - _embedded_ecs-args_count_to_long: - mapping: - type: long - match: args_count - - _embedded_ecs-virtual_address_to_long: - mapping: - type: long - match: virtual_address - - _embedded_ecs-io_text_to_wildcard: - mapping: - type: wildcard - path_match: '*.io.text' - - _embedded_ecs-strings_to_wildcard: - mapping: - type: wildcard - path_match: registry.data.strings - - _embedded_ecs-path_to_wildcard: - mapping: - type: wildcard - path_match: '*url.path' - - _embedded_ecs-message_id_to_wildcard: - mapping: - type: wildcard - match: message_id - - _embedded_ecs-command_line_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: wildcard - match: command_line - - _embedded_ecs-error_stack_trace_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: wildcard - match: stack_trace - - _embedded_ecs-http_content_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: wildcard - path_match: '*.body.content' - - _embedded_ecs-url_full_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: wildcard - path_match: '*.url.full' - - _embedded_ecs-url_original_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: wildcard - path_match: '*.url.original' - - _embedded_ecs-user_agent_original_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: wildcard - path_match: user_agent.original - - _embedded_ecs-error_message_to_match_only: - mapping: - type: match_only_text - path_match: error.message - - _embedded_ecs-message_match_only_text: - mapping: - type: match_only_text - path_match: message - - _embedded_ecs-agent_name_to_keyword: - mapping: - type: keyword - path_match: agent.name - - _embedded_ecs-event_original_non_indexed_keyword: mapping: type: keyword - index: false - doc_values: false - path_match: 'event.original' - - _embedded_ecs-x509_public_key_exponent_non_indexed_keyword: + - long_as_long: + match_mapping_type: long mapping: - type: keyword - index: false - doc_values: false - path_match: '*.x509.public_key_exponent' - - _embedded_ecs-service_name_to_keyword: - mapping: - type: keyword - path_match: '*.service.name' - - _embedded_ecs-sections_name_to_keyword: - mapping: - type: keyword - path_match: '*.sections.name' - - _embedded_ecs-resource_name_to_keyword: - mapping: - type: keyword - path_match: '*.resource.name' - - _embedded_ecs-observer_name_to_keyword: - mapping: - type: keyword - path_match: observer.name - - _embedded_ecs-question_name_to_keyword: - mapping: - type: keyword - path_match: '*.question.name' - - _embedded_ecs-group_name_to_keyword: - mapping: - type: keyword - path_match: '*.group.name' - - _embedded_ecs-geo_name_to_keyword: - mapping: - type: keyword - path_match: '*.geo.name' - - _embedded_ecs-host_name_to_keyword: - mapping: - type: keyword - path_match: host.name - - _embedded_ecs-severity_name_to_keyword: - mapping: - type: keyword - path_match: '*.severity.name' - - _embedded_ecs-title_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: keyword - match: title - - _embedded_ecs-executable_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: keyword - match: executable - - _embedded_ecs-file_path_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: keyword - path_match: '*.file.path' - - _embedded_ecs-file_target_path_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: keyword - path_match: '*.file.target_path' - - _embedded_ecs-name_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: keyword - match: name - - _embedded_ecs-full_name_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: keyword - match: full_name - - _embedded_ecs-os_full_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: keyword - path_match: '*.os.full' - - _embedded_ecs-working_directory_to_multifield: - mapping: - fields: - text: - type: match_only_text - type: keyword - match: working_directory - - _embedded_ecs-timestamp_to_date: - mapping: - type: date - match: timestamp - - _embedded_ecs-delivery_timestamp_to_date: - mapping: - type: date - match: delivery_timestamp - - _embedded_ecs-not_after_to_date: - mapping: - type: date - match: not_after - - _embedded_ecs-not_before_to_date: - mapping: - type: date - match: not_before - - _embedded_ecs-accessed_to_date: - mapping: - type: date - match: accessed - - _embedded_ecs-origination_timestamp_to_date: - mapping: - type: date - match: origination_timestamp - - _embedded_ecs-created_to_date: - mapping: - type: date - match: created - - _embedded_ecs-installed_to_date: - mapping: - type: date - match: installed - - _embedded_ecs-creation_date_to_date: - mapping: - type: date - match: creation_date - - _embedded_ecs-ctime_to_date: - mapping: - type: date - match: ctime - - _embedded_ecs-mtime_to_date: - mapping: - type: date - match: mtime - - _embedded_ecs-ingested_to_date: - mapping: - type: date - match: ingested - - _embedded_ecs-start_to_date: - mapping: - type: date - match: start - - _embedded_ecs-end_to_date: - mapping: - type: date - match: end - - _embedded_ecs-score_base_to_float: - mapping: - type: float - path_match: '*.score.base' - - _embedded_ecs-score_temporal_to_float: - mapping: - type: float - path_match: '*.score.temporal' - - _embedded_ecs-score_to_float: - mapping: - type: float - match: '*_score' - - _embedded_ecs-score_norm_to_float: - mapping: - type: float - match: '*_score_norm' - - _embedded_ecs-usage_to_float: - mapping: - scaling_factor: 1000 - type: scaled_float - match: usage - - _embedded_ecs-location_to_geo_point: - mapping: - type: geo_point - match: location - - _embedded_ecs-same_as_process_to_boolean: - mapping: - type: boolean - match: same_as_process - - _embedded_ecs-established_to_boolean: - mapping: - type: boolean - match: established - - _embedded_ecs-resumed_to_boolean: - mapping: - type: boolean - match: resumed - - _embedded_ecs-max_bytes_per_process_exceeded_to_boolean: - mapping: - type: boolean - match: max_bytes_per_process_exceeded - - _embedded_ecs-interactive_to_boolean: - mapping: - type: boolean - match: interactive - - _embedded_ecs-exists_to_boolean: - mapping: - type: boolean - match: exists - - _embedded_ecs-trusted_to_boolean: - mapping: - type: boolean - match: trusted - - _embedded_ecs-valid_to_boolean: - mapping: - type: boolean - match: valid - - _embedded_ecs-go_stripped_to_boolean: - mapping: - type: boolean - match: go_stripped - - _embedded_ecs-coldstart_to_boolean: - mapping: - type: boolean - match: coldstart - - _embedded_ecs-exports_to_flattened: - mapping: - type: flattened - match: exports - - _embedded_ecs-structured_data_to_flattened: - mapping: - type: flattened - match: structured_data - - _embedded_ecs-imports_to_flattened: - mapping: - type: flattened - match: '*imports' - - _embedded_ecs-attachments_to_nested: - mapping: - type: nested - match: attachments - - _embedded_ecs-segments_to_nested: - mapping: - type: nested - match: segments - - _embedded_ecs-elf_sections_to_nested: - mapping: - type: nested - path_match: '*.elf.sections' - - _embedded_ecs-pe_sections_to_nested: - mapping: - type: nested - path_match: '*.pe.sections' - - _embedded_ecs-macho_sections_to_nested: - mapping: - type: nested - path_match: '*.macho.sections' + type: long From 50cc6708e01e1efaa867763a40e77cbf150e7e7c Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 14:06:42 +0200 Subject: [PATCH 05/23] Add build mode: _dev/ rejection + good_built fixture (task 04) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add ValidateNoDevFolder semantic validator (build-mode only) Walks the package FS and errors on any _dev/ directory, skipping the subtree after the first hit to avoid duplicate child errors. - Register {fn: ValidateNoDevFolder, modes: [Build]} in spec.go rulesDef - Add test/packages/build_mode/good_built/ — canonical minimal built package fixture (no _dev/, no .link files, no external: ecs) - Add test/packages/build_mode/bad_built_with_dev/ — copy of good_built with an empty _dev/build/build.yml planted to trigger the rule - Add TestBuildMode_NoDevFolder to api_test.go covering: good_built passes ModeBuild bad_built_with_dev fails ModeBuild with clear error message bad_built_with_dev passes ModeSource (_dev/ is allowed in source) Co-Authored-By: Claude Sonnet 4.6 --- .../semantic/validate_no_dev_folder.go | 39 +++++++++ .../semantic/validate_no_dev_folder_test.go | 87 +++++++++++++++++++ code/go/internal/validator/spec.go | 1 + code/go/pkg/validator/api_test.go | 38 ++++++++ .../build_mode/bad_built_with_dev/LICENSE.txt | 3 + .../bad_built_with_dev/_dev/build/build.yml | 0 .../bad_built_with_dev/changelog.yml | 6 ++ .../events/agent/stream/stream.yml.hbs | 4 + .../data_stream/events/fields/base-fields.yml | 12 +++ .../data_stream/events/fields/fields.yml | 3 + .../data_stream/events/manifest.yml | 15 ++++ .../bad_built_with_dev/docs/README.md | 5 ++ .../bad_built_with_dev/manifest.yml | 23 +++++ .../build_mode/good_built/LICENSE.txt | 3 + .../build_mode/good_built/changelog.yml | 6 ++ .../events/agent/stream/stream.yml.hbs | 4 + .../data_stream/events/fields/base-fields.yml | 12 +++ .../data_stream/events/fields/fields.yml | 3 + .../data_stream/events/manifest.yml | 15 ++++ .../build_mode/good_built/docs/README.md | 5 ++ .../build_mode/good_built/manifest.yml | 23 +++++ 21 files changed, 307 insertions(+) create mode 100644 code/go/internal/validator/semantic/validate_no_dev_folder.go create mode 100644 code/go/internal/validator/semantic/validate_no_dev_folder_test.go create mode 100644 test/packages/build_mode/bad_built_with_dev/LICENSE.txt create mode 100644 test/packages/build_mode/bad_built_with_dev/_dev/build/build.yml create mode 100644 test/packages/build_mode/bad_built_with_dev/changelog.yml create mode 100644 test/packages/build_mode/bad_built_with_dev/data_stream/events/agent/stream/stream.yml.hbs create mode 100644 test/packages/build_mode/bad_built_with_dev/data_stream/events/fields/base-fields.yml create mode 100644 test/packages/build_mode/bad_built_with_dev/data_stream/events/fields/fields.yml create mode 100644 test/packages/build_mode/bad_built_with_dev/data_stream/events/manifest.yml create mode 100644 test/packages/build_mode/bad_built_with_dev/docs/README.md create mode 100644 test/packages/build_mode/bad_built_with_dev/manifest.yml create mode 100644 test/packages/build_mode/good_built/LICENSE.txt create mode 100644 test/packages/build_mode/good_built/changelog.yml create mode 100644 test/packages/build_mode/good_built/data_stream/events/agent/stream/stream.yml.hbs create mode 100644 test/packages/build_mode/good_built/data_stream/events/fields/base-fields.yml create mode 100644 test/packages/build_mode/good_built/data_stream/events/fields/fields.yml create mode 100644 test/packages/build_mode/good_built/data_stream/events/manifest.yml create mode 100644 test/packages/build_mode/good_built/docs/README.md create mode 100644 test/packages/build_mode/good_built/manifest.yml diff --git a/code/go/internal/validator/semantic/validate_no_dev_folder.go b/code/go/internal/validator/semantic/validate_no_dev_folder.go new file mode 100644 index 000000000..1e75b0055 --- /dev/null +++ b/code/go/internal/validator/semantic/validate_no_dev_folder.go @@ -0,0 +1,39 @@ +// 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 semantic + +import ( + "io/fs" + + "github.com/elastic/package-spec/v3/code/go/internal/fspath" + "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" +) + +// ValidateNoDevFolder errors for any _dev/ directory found in the package. +// _dev/ is a source-only artifact used during development (tests, deploy +// configs, build manifests). It must not appear in a built package that is +// validated with ModeBuild. +func ValidateNoDevFolder(fsys fspath.FS) specerrors.ValidationErrors { + var errs specerrors.ValidationErrors + walkErr := fs.WalkDir(fsys, ".", func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() && d.Name() == "_dev" { + errs = append(errs, specerrors.NewStructuredErrorf( + "file %q: _dev directory is not allowed in built packages", + fsys.Path(p), + )) + // Skip the subtree to avoid generating child errors for each + // file inside the _dev directory. + return fs.SkipDir + } + return nil + }) + if walkErr != nil { + errs = append(errs, specerrors.NewStructuredError(walkErr, specerrors.UnassignedCode)) + } + return errs +} diff --git a/code/go/internal/validator/semantic/validate_no_dev_folder_test.go b/code/go/internal/validator/semantic/validate_no_dev_folder_test.go new file mode 100644 index 000000000..14d1cf1fe --- /dev/null +++ b/code/go/internal/validator/semantic/validate_no_dev_folder_test.go @@ -0,0 +1,87 @@ +// 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 semantic + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/elastic/package-spec/v3/code/go/internal/fspath" +) + +func TestValidateNoDevFolder(t *testing.T) { + tests := []struct { + name string + dirs []string // directories to create relative to the temp root + expectErrors bool + errorContains []string + }{ + { + name: "no _dev directory", + dirs: []string{"data_stream/foo/fields"}, + expectErrors: false, + }, + { + name: "_dev at package root", + dirs: []string{"_dev/build"}, + expectErrors: true, + errorContains: []string{"_dev", "_dev directory is not allowed in built packages"}, + }, + { + name: "_dev inside data_stream", + dirs: []string{"data_stream/foo/_dev/test"}, + expectErrors: true, + errorContains: []string{"_dev", "_dev directory is not allowed in built packages"}, + }, + { + name: "multiple _dev directories", + dirs: []string{ + "_dev/build", + "data_stream/foo/_dev/test", + }, + expectErrors: true, + errorContains: []string{"_dev directory is not allowed in built packages"}, + }, + { + name: "directory named _devtools is not rejected", + dirs: []string{"_devtools"}, + expectErrors: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tempDir := t.TempDir() + + for _, d := range tc.dirs { + require.NoError(t, os.MkdirAll(filepath.Join(tempDir, d), 0o755)) + } + + fsys := fspath.DirFS(tempDir) + errs := ValidateNoDevFolder(fsys) + + if !tc.expectErrors { + assert.Nil(t, errs, "expected no errors but got: %v", errs) + return + } + + require.NotNil(t, errs, "expected validation errors but got none") + var sb strings.Builder + for _, e := range errs { + sb.WriteString(e.Error()) + sb.WriteString("\n") + } + combined := sb.String() + for _, substr := range tc.errorContains { + assert.Contains(t, combined, substr) + } + }) + } +} diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index 3deb518d6..b6b67a464 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -259,6 +259,7 @@ func (s Spec) rules(pkgType string, rootSpec spectypes.ItemSpec) validationRules modes: []modes.Mode{modes.Legacy, modes.Source}}, {fn: semantic.ValidateNoEmbeddedEcsInDynamicTemplates, types: []string{"integration"}, modes: []modes.Mode{modes.Source}}, + {fn: semantic.ValidateNoDevFolder, modes: []modes.Mode{modes.Build}}, } var validationRules validationRules diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go index 2ead972ca..e30709450 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -285,6 +285,44 @@ func TestBuildMode_SkipsBuildExcludedRules(t *testing.T) { } } +// ----------------------------------------------------------------------- +// TestBuildMode_NoDevFolder +// +// Verifies that ModeBuild: +// - Accepts the canonical good_built fixture (no _dev/ present). +// - Rejects a package containing a _dev/ directory with a clear error. +// - Does NOT reject _dev/ when validating in ModeSource (source allows _dev/). +// ----------------------------------------------------------------------- + +func TestBuildMode_NoDevFolder(t *testing.T) { + goodBuiltPath := filepath.Join(testPackagesDir, "build_mode", "good_built") + badBuiltPath := filepath.Join(testPackagesDir, "build_mode", "bad_built_with_dev") + + t.Run("good_built passes ModeBuild", func(t *testing.T) { + v, err := NewFromPath(ModeBuild, goodBuiltPath) + require.NoError(t, err) + assert.NoError(t, v.Validate(), "good_built should pass build-mode validation") + }) + + t.Run("bad_built_with_dev fails ModeBuild", func(t *testing.T) { + v, err := NewFromPath(ModeBuild, badBuiltPath) + require.NoError(t, err) + buildErr := v.Validate() + require.Error(t, buildErr, "bad_built_with_dev should fail build-mode validation") + assert.Contains(t, buildErr.Error(), "_dev directory is not allowed in built packages") + }) + + t.Run("bad_built_with_dev passes ModeSource (source allows _dev/)", func(t *testing.T) { + v, err := NewFromPath(ModeSource, badBuiltPath) + require.NoError(t, err) + sourceErr := v.Validate() + if sourceErr != nil { + assert.NotContains(t, sourceErr.Error(), "_dev directory is not allowed", + "ValidateNoDevFolder must not run in source mode") + } + }) +} + // ----------------------------------------------------------------------- // TestLegacyPreservation_FromZip (golden test) // diff --git a/test/packages/build_mode/bad_built_with_dev/LICENSE.txt b/test/packages/build_mode/bad_built_with_dev/LICENSE.txt new file mode 100644 index 000000000..f6f788a89 --- /dev/null +++ b/test/packages/build_mode/bad_built_with_dev/LICENSE.txt @@ -0,0 +1,3 @@ +Elastic License 2.0 + +URL: https://www.elastic.co/licensing/elastic-license diff --git a/test/packages/build_mode/bad_built_with_dev/_dev/build/build.yml b/test/packages/build_mode/bad_built_with_dev/_dev/build/build.yml new file mode 100644 index 000000000..e69de29bb diff --git a/test/packages/build_mode/bad_built_with_dev/changelog.yml b/test/packages/build_mode/bad_built_with_dev/changelog.yml new file mode 100644 index 000000000..e00f88133 --- /dev/null +++ b/test/packages/build_mode/bad_built_with_dev/changelog.yml @@ -0,0 +1,6 @@ +# newer versions go on top +- version: "0.0.1" + changes: + - description: Initial draft of the package + type: enhancement + link: https://github.com/elastic/integrations/pull/1 diff --git a/test/packages/build_mode/bad_built_with_dev/data_stream/events/agent/stream/stream.yml.hbs b/test/packages/build_mode/bad_built_with_dev/data_stream/events/agent/stream/stream.yml.hbs new file mode 100644 index 000000000..9390bc05c --- /dev/null +++ b/test/packages/build_mode/bad_built_with_dev/data_stream/events/agent/stream/stream.yml.hbs @@ -0,0 +1,4 @@ +paths: +{{#each paths}} + - {{this}} +{{/each}} diff --git a/test/packages/build_mode/bad_built_with_dev/data_stream/events/fields/base-fields.yml b/test/packages/build_mode/bad_built_with_dev/data_stream/events/fields/base-fields.yml new file mode 100644 index 000000000..0d1791ffe --- /dev/null +++ b/test/packages/build_mode/bad_built_with_dev/data_stream/events/fields/base-fields.yml @@ -0,0 +1,12 @@ +- name: data_stream.type + type: constant_keyword + description: Data stream type. +- name: data_stream.dataset + type: constant_keyword + description: Data stream dataset. +- name: data_stream.namespace + type: constant_keyword + description: Data stream namespace. +- name: "@timestamp" + type: date + description: Event timestamp. diff --git a/test/packages/build_mode/bad_built_with_dev/data_stream/events/fields/fields.yml b/test/packages/build_mode/bad_built_with_dev/data_stream/events/fields/fields.yml new file mode 100644 index 000000000..f21b45be3 --- /dev/null +++ b/test/packages/build_mode/bad_built_with_dev/data_stream/events/fields/fields.yml @@ -0,0 +1,3 @@ +- name: message + type: keyword + description: Log message. diff --git a/test/packages/build_mode/bad_built_with_dev/data_stream/events/manifest.yml b/test/packages/build_mode/bad_built_with_dev/data_stream/events/manifest.yml new file mode 100644 index 000000000..d51b2bd1c --- /dev/null +++ b/test/packages/build_mode/bad_built_with_dev/data_stream/events/manifest.yml @@ -0,0 +1,15 @@ +title: Events +type: logs +streams: + - input: logfile + title: Events logs + description: Collect events log data + vars: + - name: paths + type: text + title: Paths + multi: true + required: true + show_user: true + default: + - /var/log/*.log diff --git a/test/packages/build_mode/bad_built_with_dev/docs/README.md b/test/packages/build_mode/bad_built_with_dev/docs/README.md new file mode 100644 index 000000000..c0b01c5ec --- /dev/null +++ b/test/packages/build_mode/bad_built_with_dev/docs/README.md @@ -0,0 +1,5 @@ +# Good Built Package + +This is the canonical minimal built-package fixture used for build-mode validation +tests (issue #549). It has no `_dev/` directories, no `.link` files, and no +`external: ecs` field references — making it valid under `ModeBuild`. diff --git a/test/packages/build_mode/bad_built_with_dev/manifest.yml b/test/packages/build_mode/bad_built_with_dev/manifest.yml new file mode 100644 index 000000000..0dc0d4080 --- /dev/null +++ b/test/packages/build_mode/bad_built_with_dev/manifest.yml @@ -0,0 +1,23 @@ +format_version: 3.6.0 +name: good_built +title: Good Built Package +description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +version: 0.0.1 +type: integration +source: + license: "Apache-2.0" +conditions: + kibana: + version: '^8.0.0 || ^9.0.0' +policy_templates: + - name: events + title: Events logs + description: Collect events data + inputs: + - type: logfile + title: Collect events logs + description: Collecting events log data + multi: false +owner: + github: elastic/foobar + type: community diff --git a/test/packages/build_mode/good_built/LICENSE.txt b/test/packages/build_mode/good_built/LICENSE.txt new file mode 100644 index 000000000..f6f788a89 --- /dev/null +++ b/test/packages/build_mode/good_built/LICENSE.txt @@ -0,0 +1,3 @@ +Elastic License 2.0 + +URL: https://www.elastic.co/licensing/elastic-license diff --git a/test/packages/build_mode/good_built/changelog.yml b/test/packages/build_mode/good_built/changelog.yml new file mode 100644 index 000000000..e00f88133 --- /dev/null +++ b/test/packages/build_mode/good_built/changelog.yml @@ -0,0 +1,6 @@ +# newer versions go on top +- version: "0.0.1" + changes: + - description: Initial draft of the package + type: enhancement + link: https://github.com/elastic/integrations/pull/1 diff --git a/test/packages/build_mode/good_built/data_stream/events/agent/stream/stream.yml.hbs b/test/packages/build_mode/good_built/data_stream/events/agent/stream/stream.yml.hbs new file mode 100644 index 000000000..9390bc05c --- /dev/null +++ b/test/packages/build_mode/good_built/data_stream/events/agent/stream/stream.yml.hbs @@ -0,0 +1,4 @@ +paths: +{{#each paths}} + - {{this}} +{{/each}} diff --git a/test/packages/build_mode/good_built/data_stream/events/fields/base-fields.yml b/test/packages/build_mode/good_built/data_stream/events/fields/base-fields.yml new file mode 100644 index 000000000..0d1791ffe --- /dev/null +++ b/test/packages/build_mode/good_built/data_stream/events/fields/base-fields.yml @@ -0,0 +1,12 @@ +- name: data_stream.type + type: constant_keyword + description: Data stream type. +- name: data_stream.dataset + type: constant_keyword + description: Data stream dataset. +- name: data_stream.namespace + type: constant_keyword + description: Data stream namespace. +- name: "@timestamp" + type: date + description: Event timestamp. diff --git a/test/packages/build_mode/good_built/data_stream/events/fields/fields.yml b/test/packages/build_mode/good_built/data_stream/events/fields/fields.yml new file mode 100644 index 000000000..f21b45be3 --- /dev/null +++ b/test/packages/build_mode/good_built/data_stream/events/fields/fields.yml @@ -0,0 +1,3 @@ +- name: message + type: keyword + description: Log message. diff --git a/test/packages/build_mode/good_built/data_stream/events/manifest.yml b/test/packages/build_mode/good_built/data_stream/events/manifest.yml new file mode 100644 index 000000000..d51b2bd1c --- /dev/null +++ b/test/packages/build_mode/good_built/data_stream/events/manifest.yml @@ -0,0 +1,15 @@ +title: Events +type: logs +streams: + - input: logfile + title: Events logs + description: Collect events log data + vars: + - name: paths + type: text + title: Paths + multi: true + required: true + show_user: true + default: + - /var/log/*.log diff --git a/test/packages/build_mode/good_built/docs/README.md b/test/packages/build_mode/good_built/docs/README.md new file mode 100644 index 000000000..c0b01c5ec --- /dev/null +++ b/test/packages/build_mode/good_built/docs/README.md @@ -0,0 +1,5 @@ +# Good Built Package + +This is the canonical minimal built-package fixture used for build-mode validation +tests (issue #549). It has no `_dev/` directories, no `.link` files, and no +`external: ecs` field references — making it valid under `ModeBuild`. diff --git a/test/packages/build_mode/good_built/manifest.yml b/test/packages/build_mode/good_built/manifest.yml new file mode 100644 index 000000000..0dc0d4080 --- /dev/null +++ b/test/packages/build_mode/good_built/manifest.yml @@ -0,0 +1,23 @@ +format_version: 3.6.0 +name: good_built +title: Good Built Package +description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +version: 0.0.1 +type: integration +source: + license: "Apache-2.0" +conditions: + kibana: + version: '^8.0.0 || ^9.0.0' +policy_templates: + - name: events + title: Events logs + description: Collect events data + inputs: + - type: logfile + title: Collect events logs + description: Collecting events log data + multi: false +owner: + github: elastic/foobar + type: community From d8a368f0b1301984d0d0b87de3d0ab69629721b4 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 14:13:55 +0200 Subject: [PATCH 06/23] Add build mode: .link file rejection (task 05) - Add ValidateNoLinkFiles semantic validator that errors on any .link file found in the package tree - Register rule in spec.go with modes: [Build] (build-only) - Add bad_built_with_link fixture: copy of good_built with a .link file planted under data_stream/events/fields/ - Add unit tests for ValidateNoLinkFiles (5 cases) - Add TestBuildMode_NoLinkFiles integration test: build mode rejects .link files; source mode (with_links fixture) is unaffected Co-Authored-By: Claude Sonnet 4.6 --- .../semantic/validate_no_link_files.go | 38 ++++++++ .../semantic/validate_no_link_files_test.go | 89 +++++++++++++++++++ code/go/internal/validator/spec.go | 1 + code/go/pkg/validator/api_test.go | 32 +++++++ .../bad_built_with_link/LICENSE.txt | 3 + .../bad_built_with_link/changelog.yml | 6 ++ .../events/agent/stream/stream.yml.hbs | 4 + .../data_stream/events/fields/base-fields.yml | 12 +++ .../events/fields/base-fields.yml.link | 0 .../data_stream/events/fields/fields.yml | 3 + .../data_stream/events/manifest.yml | 15 ++++ .../bad_built_with_link/docs/README.md | 5 ++ .../bad_built_with_link/manifest.yml | 23 +++++ 13 files changed, 231 insertions(+) create mode 100644 code/go/internal/validator/semantic/validate_no_link_files.go create mode 100644 code/go/internal/validator/semantic/validate_no_link_files_test.go create mode 100644 test/packages/build_mode/bad_built_with_link/LICENSE.txt create mode 100644 test/packages/build_mode/bad_built_with_link/changelog.yml create mode 100644 test/packages/build_mode/bad_built_with_link/data_stream/events/agent/stream/stream.yml.hbs create mode 100644 test/packages/build_mode/bad_built_with_link/data_stream/events/fields/base-fields.yml create mode 100644 test/packages/build_mode/bad_built_with_link/data_stream/events/fields/base-fields.yml.link create mode 100644 test/packages/build_mode/bad_built_with_link/data_stream/events/fields/fields.yml create mode 100644 test/packages/build_mode/bad_built_with_link/data_stream/events/manifest.yml create mode 100644 test/packages/build_mode/bad_built_with_link/docs/README.md create mode 100644 test/packages/build_mode/bad_built_with_link/manifest.yml diff --git a/code/go/internal/validator/semantic/validate_no_link_files.go b/code/go/internal/validator/semantic/validate_no_link_files.go new file mode 100644 index 000000000..f8d80c7fa --- /dev/null +++ b/code/go/internal/validator/semantic/validate_no_link_files.go @@ -0,0 +1,38 @@ +// 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 semantic + +import ( + "io/fs" + "path/filepath" + + "github.com/elastic/package-spec/v3/code/go/internal/fspath" + "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" +) + +// ValidateNoLinkFiles errors for any .link file found in the package. +// .link files are a source-only artifact used during development to share +// files across packages. They must be resolved and inlined by the build +// step before a package is distributed. A built package must not contain +// any .link files when validated with ModeBuild. +func ValidateNoLinkFiles(fsys fspath.FS) specerrors.ValidationErrors { + var errs specerrors.ValidationErrors + walkErr := fs.WalkDir(fsys, ".", func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() && filepath.Ext(d.Name()) == ".link" { + errs = append(errs, specerrors.NewStructuredErrorf( + "file %q: .link files are not allowed in built packages", + fsys.Path(p), + )) + } + return nil + }) + if walkErr != nil { + errs = append(errs, specerrors.NewStructuredError(walkErr, specerrors.UnassignedCode)) + } + return errs +} diff --git a/code/go/internal/validator/semantic/validate_no_link_files_test.go b/code/go/internal/validator/semantic/validate_no_link_files_test.go new file mode 100644 index 000000000..2b9494543 --- /dev/null +++ b/code/go/internal/validator/semantic/validate_no_link_files_test.go @@ -0,0 +1,89 @@ +// 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 semantic + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/elastic/package-spec/v3/code/go/internal/fspath" +) + +func TestValidateNoLinkFiles(t *testing.T) { + tests := []struct { + name string + files []string // files to create relative to the temp root + expectErrors bool + errorContains []string + }{ + { + name: "no .link files", + files: []string{"data_stream/foo/fields/base-fields.yml"}, + expectErrors: false, + }, + { + name: ".link file in fields directory", + files: []string{"data_stream/foo/fields/some-fields.yml.link"}, + expectErrors: true, + errorContains: []string{"some-fields.yml.link", ".link files are not allowed in built packages"}, + }, + { + name: ".link file at package root", + files: []string{"elasticsearch/ingest_pipeline/default.yml.link"}, + expectErrors: true, + errorContains: []string{"default.yml.link", ".link files are not allowed in built packages"}, + }, + { + name: "multiple .link files", + files: []string{ + "data_stream/foo/fields/some-fields.yml.link", + "data_stream/foo/agent/stream/stream.yml.hbs.link", + }, + expectErrors: true, + errorContains: []string{".link files are not allowed in built packages"}, + }, + { + name: "file with .link in the middle of the name is not rejected", + files: []string{"data_stream/foo/fields/base-fields.yml"}, + expectErrors: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tempDir := t.TempDir() + + for _, f := range tc.files { + fullPath := filepath.Join(tempDir, f) + require.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + require.NoError(t, os.WriteFile(fullPath, []byte(""), 0o644)) + } + + fsys := fspath.DirFS(tempDir) + errs := ValidateNoLinkFiles(fsys) + + if !tc.expectErrors { + assert.Nil(t, errs, "expected no errors but got: %v", errs) + return + } + + require.NotNil(t, errs, "expected validation errors but got none") + var sb strings.Builder + for _, e := range errs { + sb.WriteString(e.Error()) + sb.WriteString("\n") + } + combined := sb.String() + for _, substr := range tc.errorContains { + assert.Contains(t, combined, substr) + } + }) + } +} diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index b6b67a464..991feca89 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -260,6 +260,7 @@ func (s Spec) rules(pkgType string, rootSpec spectypes.ItemSpec) validationRules {fn: semantic.ValidateNoEmbeddedEcsInDynamicTemplates, types: []string{"integration"}, modes: []modes.Mode{modes.Source}}, {fn: semantic.ValidateNoDevFolder, modes: []modes.Mode{modes.Build}}, + {fn: semantic.ValidateNoLinkFiles, modes: []modes.Mode{modes.Build}}, } var validationRules validationRules diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go index e30709450..209b4ea8c 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -323,6 +323,38 @@ func TestBuildMode_NoDevFolder(t *testing.T) { }) } +// ----------------------------------------------------------------------- +// TestBuildMode_NoLinkFiles +// +// Verifies that ModeBuild: +// - Rejects a package containing a .link file with a clean error. +// - Does NOT raise a .link error when validating in ModeSource (source +// resolves .link files transparently via linkedfiles.NewFS). +// ----------------------------------------------------------------------- + +func TestBuildMode_NoLinkFiles(t *testing.T) { + badBuiltPath := filepath.Join(testPackagesDir, "build_mode", "bad_built_with_link") + withLinksPath := filepath.Join(testPackagesDir, "with_links") + + t.Run("bad_built_with_link fails ModeBuild", func(t *testing.T) { + v, err := NewFromPath(ModeBuild, badBuiltPath) + require.NoError(t, err) + buildErr := v.Validate() + require.Error(t, buildErr, "bad_built_with_link should fail build-mode validation") + assert.Contains(t, buildErr.Error(), ".link files are not allowed in built packages") + }) + + t.Run("with_links passes ModeSource (.link files allowed in source mode)", func(t *testing.T) { + v, err := NewFromPath(ModeSource, withLinksPath) + require.NoError(t, err) + sourceErr := v.Validate() + if sourceErr != nil { + assert.NotContains(t, sourceErr.Error(), ".link files are not allowed", + "ValidateNoLinkFiles must not run in source mode") + } + }) +} + // ----------------------------------------------------------------------- // TestLegacyPreservation_FromZip (golden test) // diff --git a/test/packages/build_mode/bad_built_with_link/LICENSE.txt b/test/packages/build_mode/bad_built_with_link/LICENSE.txt new file mode 100644 index 000000000..f6f788a89 --- /dev/null +++ b/test/packages/build_mode/bad_built_with_link/LICENSE.txt @@ -0,0 +1,3 @@ +Elastic License 2.0 + +URL: https://www.elastic.co/licensing/elastic-license diff --git a/test/packages/build_mode/bad_built_with_link/changelog.yml b/test/packages/build_mode/bad_built_with_link/changelog.yml new file mode 100644 index 000000000..e00f88133 --- /dev/null +++ b/test/packages/build_mode/bad_built_with_link/changelog.yml @@ -0,0 +1,6 @@ +# newer versions go on top +- version: "0.0.1" + changes: + - description: Initial draft of the package + type: enhancement + link: https://github.com/elastic/integrations/pull/1 diff --git a/test/packages/build_mode/bad_built_with_link/data_stream/events/agent/stream/stream.yml.hbs b/test/packages/build_mode/bad_built_with_link/data_stream/events/agent/stream/stream.yml.hbs new file mode 100644 index 000000000..9390bc05c --- /dev/null +++ b/test/packages/build_mode/bad_built_with_link/data_stream/events/agent/stream/stream.yml.hbs @@ -0,0 +1,4 @@ +paths: +{{#each paths}} + - {{this}} +{{/each}} diff --git a/test/packages/build_mode/bad_built_with_link/data_stream/events/fields/base-fields.yml b/test/packages/build_mode/bad_built_with_link/data_stream/events/fields/base-fields.yml new file mode 100644 index 000000000..0d1791ffe --- /dev/null +++ b/test/packages/build_mode/bad_built_with_link/data_stream/events/fields/base-fields.yml @@ -0,0 +1,12 @@ +- name: data_stream.type + type: constant_keyword + description: Data stream type. +- name: data_stream.dataset + type: constant_keyword + description: Data stream dataset. +- name: data_stream.namespace + type: constant_keyword + description: Data stream namespace. +- name: "@timestamp" + type: date + description: Event timestamp. diff --git a/test/packages/build_mode/bad_built_with_link/data_stream/events/fields/base-fields.yml.link b/test/packages/build_mode/bad_built_with_link/data_stream/events/fields/base-fields.yml.link new file mode 100644 index 000000000..e69de29bb diff --git a/test/packages/build_mode/bad_built_with_link/data_stream/events/fields/fields.yml b/test/packages/build_mode/bad_built_with_link/data_stream/events/fields/fields.yml new file mode 100644 index 000000000..f21b45be3 --- /dev/null +++ b/test/packages/build_mode/bad_built_with_link/data_stream/events/fields/fields.yml @@ -0,0 +1,3 @@ +- name: message + type: keyword + description: Log message. diff --git a/test/packages/build_mode/bad_built_with_link/data_stream/events/manifest.yml b/test/packages/build_mode/bad_built_with_link/data_stream/events/manifest.yml new file mode 100644 index 000000000..d51b2bd1c --- /dev/null +++ b/test/packages/build_mode/bad_built_with_link/data_stream/events/manifest.yml @@ -0,0 +1,15 @@ +title: Events +type: logs +streams: + - input: logfile + title: Events logs + description: Collect events log data + vars: + - name: paths + type: text + title: Paths + multi: true + required: true + show_user: true + default: + - /var/log/*.log diff --git a/test/packages/build_mode/bad_built_with_link/docs/README.md b/test/packages/build_mode/bad_built_with_link/docs/README.md new file mode 100644 index 000000000..c0b01c5ec --- /dev/null +++ b/test/packages/build_mode/bad_built_with_link/docs/README.md @@ -0,0 +1,5 @@ +# Good Built Package + +This is the canonical minimal built-package fixture used for build-mode validation +tests (issue #549). It has no `_dev/` directories, no `.link` files, and no +`external: ecs` field references — making it valid under `ModeBuild`. diff --git a/test/packages/build_mode/bad_built_with_link/manifest.yml b/test/packages/build_mode/bad_built_with_link/manifest.yml new file mode 100644 index 000000000..0dc0d4080 --- /dev/null +++ b/test/packages/build_mode/bad_built_with_link/manifest.yml @@ -0,0 +1,23 @@ +format_version: 3.6.0 +name: good_built +title: Good Built Package +description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +version: 0.0.1 +type: integration +source: + license: "Apache-2.0" +conditions: + kibana: + version: '^8.0.0 || ^9.0.0' +policy_templates: + - name: events + title: Events logs + description: Collect events data + inputs: + - type: logfile + title: Collect events logs + description: Collecting events log data + multi: false +owner: + github: elastic/foobar + type: community From 138fa63700572d207541dfc569a98accf79cab04 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 14:19:47 +0200 Subject: [PATCH 07/23] Add build mode: external: ecs field rejection (task 06) - ValidateNoExternalEcs errors on any field with external: ecs in build mode - Registered in spec.go scoped to modes:[Build] (no types guard, consistent with ValidateNoDevFolder / ValidateNoLinkFiles) - Fixture: test/packages/build_mode/bad_built_external_ecs/ (good_built copy with one host.name field carrying external: ecs) - Unit tests cover: no externals, materialized fields, single/multiple ecs externals, non-ecs external (not rejected) - Integration test: bad fixture fails ModeBuild; good_v3 passes ModeSource Co-Authored-By: Claude Sonnet 4.6 --- .../semantic/validate_no_external_ecs.go | 29 ++++++ .../semantic/validate_no_external_ecs_test.go | 95 +++++++++++++++++++ code/go/internal/validator/spec.go | 1 + code/go/pkg/validator/api_test.go | 33 +++++++ .../bad_built_external_ecs/LICENSE.txt | 3 + .../bad_built_external_ecs/changelog.yml | 6 ++ .../events/agent/stream/stream.yml.hbs | 4 + .../data_stream/events/fields/base-fields.yml | 12 +++ .../data_stream/events/fields/fields.yml | 5 + .../data_stream/events/manifest.yml | 15 +++ .../bad_built_external_ecs/docs/README.md | 5 + .../bad_built_external_ecs/manifest.yml | 23 +++++ 12 files changed, 231 insertions(+) create mode 100644 code/go/internal/validator/semantic/validate_no_external_ecs.go create mode 100644 code/go/internal/validator/semantic/validate_no_external_ecs_test.go create mode 100644 test/packages/build_mode/bad_built_external_ecs/LICENSE.txt create mode 100644 test/packages/build_mode/bad_built_external_ecs/changelog.yml create mode 100644 test/packages/build_mode/bad_built_external_ecs/data_stream/events/agent/stream/stream.yml.hbs create mode 100644 test/packages/build_mode/bad_built_external_ecs/data_stream/events/fields/base-fields.yml create mode 100644 test/packages/build_mode/bad_built_external_ecs/data_stream/events/fields/fields.yml create mode 100644 test/packages/build_mode/bad_built_external_ecs/data_stream/events/manifest.yml create mode 100644 test/packages/build_mode/bad_built_external_ecs/docs/README.md create mode 100644 test/packages/build_mode/bad_built_external_ecs/manifest.yml diff --git a/code/go/internal/validator/semantic/validate_no_external_ecs.go b/code/go/internal/validator/semantic/validate_no_external_ecs.go new file mode 100644 index 000000000..f43e8c74c --- /dev/null +++ b/code/go/internal/validator/semantic/validate_no_external_ecs.go @@ -0,0 +1,29 @@ +// 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 semantic + +import ( + "github.com/elastic/package-spec/v3/code/go/internal/fspath" + "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" +) + +// ValidateNoExternalEcs errors for any field referencing external: ecs. +// The build process materializes ECS field references — once built, fields +// should carry full definitions, not external pointers. A built package must +// not contain any fields with external: ecs when validated with ModeBuild. +func ValidateNoExternalEcs(fsys fspath.FS) specerrors.ValidationErrors { + validateFunc := func(metadata fieldFileMetadata, f field) specerrors.ValidationErrors { + if f.External != "ecs" { + return nil + } + return specerrors.ValidationErrors{ + specerrors.NewStructuredErrorf( + "file \"%s\" is invalid: field %s has external: ecs reference; ECS fields must be materialized before packaging", + metadata.fullFilePath, f.Name, + ), + } + } + return validateFields(fsys, validateFunc) +} diff --git a/code/go/internal/validator/semantic/validate_no_external_ecs_test.go b/code/go/internal/validator/semantic/validate_no_external_ecs_test.go new file mode 100644 index 000000000..4f66fc226 --- /dev/null +++ b/code/go/internal/validator/semantic/validate_no_external_ecs_test.go @@ -0,0 +1,95 @@ +// 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 semantic + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/elastic/package-spec/v3/code/go/internal/fspath" +) + +func TestValidateNoExternalEcs(t *testing.T) { + tests := []struct { + name string + files map[string]string // path → content + expectErrors bool + errorContains []string + }{ + { + name: "no external fields — no errors", + files: map[string]string{ + "data_stream/foo/fields/fields.yml": "- name: message\n type: keyword\n", + }, + expectErrors: false, + }, + { + name: "materialized ECS field (no external key) — no errors", + files: map[string]string{ + "data_stream/foo/fields/base-fields.yml": "- name: data_stream.type\n type: constant_keyword\n description: Data stream type.\n", + }, + expectErrors: false, + }, + { + name: "field with external: ecs — rejected", + files: map[string]string{ + "data_stream/foo/fields/ecs.yml": "- name: host.name\n external: ecs\n", + }, + expectErrors: true, + errorContains: []string{"host.name", "external: ecs", "ECS fields must be materialized"}, + }, + { + name: "multiple fields with external: ecs — all rejected", + files: map[string]string{ + "data_stream/foo/fields/ecs.yml": "- name: host.name\n external: ecs\n- name: agent.version\n external: ecs\n", + }, + expectErrors: true, + errorContains: []string{"external: ecs", "ECS fields must be materialized"}, + }, + { + name: "field with non-ecs external — not rejected by this rule", + files: map[string]string{ + "data_stream/foo/fields/custom.yml": "- name: myfield\n external: custom_dep\n", + }, + expectErrors: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tempDir := t.TempDir() + + for relPath, content := range tc.files { + fullPath := filepath.Join(tempDir, relPath) + require.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + require.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644)) + } + + fsys := fspath.DirFS(tempDir) + errs := ValidateNoExternalEcs(fsys) + + if !tc.expectErrors { + assert.Nil(t, errs, "expected no errors but got: %v", errs) + return + } + + require.NotNil(t, errs, "expected validation errors but got none") + var sb strings.Builder + for _, e := range errs { + sb.WriteString(e.Error()) + sb.WriteString("\n") + } + combined := sb.String() + for _, substr := range tc.errorContains { + assert.Contains(t, combined, substr) + } + }) + } +} diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index 991feca89..11f33af88 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -261,6 +261,7 @@ func (s Spec) rules(pkgType string, rootSpec spectypes.ItemSpec) validationRules modes: []modes.Mode{modes.Source}}, {fn: semantic.ValidateNoDevFolder, modes: []modes.Mode{modes.Build}}, {fn: semantic.ValidateNoLinkFiles, modes: []modes.Mode{modes.Build}}, + {fn: semantic.ValidateNoExternalEcs, modes: []modes.Mode{modes.Build}}, } var validationRules validationRules diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go index 209b4ea8c..921696a94 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -355,6 +355,39 @@ func TestBuildMode_NoLinkFiles(t *testing.T) { }) } +// ----------------------------------------------------------------------- +// TestBuildMode_NoExternalEcs +// +// Verifies that ModeBuild: +// - Rejects a package containing a field with external: ecs. +// - Does NOT raise an external-ecs error in ModeSource (source allows +// external: ecs references via the build dependency mechanism). +// ----------------------------------------------------------------------- + +func TestBuildMode_NoExternalEcs(t *testing.T) { + badBuiltPath := filepath.Join(testPackagesDir, "build_mode", "bad_built_external_ecs") + goodV3Path := filepath.Join(testPackagesDir, "good_v3") + + t.Run("bad_built_external_ecs fails ModeBuild", func(t *testing.T) { + v, err := NewFromPath(ModeBuild, badBuiltPath) + require.NoError(t, err) + buildErr := v.Validate() + require.Error(t, buildErr, "bad_built_external_ecs should fail build-mode validation") + assert.Contains(t, buildErr.Error(), "external: ecs") + assert.Contains(t, buildErr.Error(), "ECS fields must be materialized") + }) + + t.Run("good_v3 with external: ecs passes ModeSource", func(t *testing.T) { + v, err := NewFromPath(ModeSource, goodV3Path) + require.NoError(t, err) + sourceErr := v.Validate() + if sourceErr != nil { + assert.NotContains(t, sourceErr.Error(), "ECS fields must be materialized", + "ValidateNoExternalEcs must not run in source mode") + } + }) +} + // ----------------------------------------------------------------------- // TestLegacyPreservation_FromZip (golden test) // diff --git a/test/packages/build_mode/bad_built_external_ecs/LICENSE.txt b/test/packages/build_mode/bad_built_external_ecs/LICENSE.txt new file mode 100644 index 000000000..f6f788a89 --- /dev/null +++ b/test/packages/build_mode/bad_built_external_ecs/LICENSE.txt @@ -0,0 +1,3 @@ +Elastic License 2.0 + +URL: https://www.elastic.co/licensing/elastic-license diff --git a/test/packages/build_mode/bad_built_external_ecs/changelog.yml b/test/packages/build_mode/bad_built_external_ecs/changelog.yml new file mode 100644 index 000000000..e00f88133 --- /dev/null +++ b/test/packages/build_mode/bad_built_external_ecs/changelog.yml @@ -0,0 +1,6 @@ +# newer versions go on top +- version: "0.0.1" + changes: + - description: Initial draft of the package + type: enhancement + link: https://github.com/elastic/integrations/pull/1 diff --git a/test/packages/build_mode/bad_built_external_ecs/data_stream/events/agent/stream/stream.yml.hbs b/test/packages/build_mode/bad_built_external_ecs/data_stream/events/agent/stream/stream.yml.hbs new file mode 100644 index 000000000..9390bc05c --- /dev/null +++ b/test/packages/build_mode/bad_built_external_ecs/data_stream/events/agent/stream/stream.yml.hbs @@ -0,0 +1,4 @@ +paths: +{{#each paths}} + - {{this}} +{{/each}} diff --git a/test/packages/build_mode/bad_built_external_ecs/data_stream/events/fields/base-fields.yml b/test/packages/build_mode/bad_built_external_ecs/data_stream/events/fields/base-fields.yml new file mode 100644 index 000000000..0d1791ffe --- /dev/null +++ b/test/packages/build_mode/bad_built_external_ecs/data_stream/events/fields/base-fields.yml @@ -0,0 +1,12 @@ +- name: data_stream.type + type: constant_keyword + description: Data stream type. +- name: data_stream.dataset + type: constant_keyword + description: Data stream dataset. +- name: data_stream.namespace + type: constant_keyword + description: Data stream namespace. +- name: "@timestamp" + type: date + description: Event timestamp. diff --git a/test/packages/build_mode/bad_built_external_ecs/data_stream/events/fields/fields.yml b/test/packages/build_mode/bad_built_external_ecs/data_stream/events/fields/fields.yml new file mode 100644 index 000000000..e09745251 --- /dev/null +++ b/test/packages/build_mode/bad_built_external_ecs/data_stream/events/fields/fields.yml @@ -0,0 +1,5 @@ +- name: message + type: keyword + description: Log message. +- name: host.name + external: ecs diff --git a/test/packages/build_mode/bad_built_external_ecs/data_stream/events/manifest.yml b/test/packages/build_mode/bad_built_external_ecs/data_stream/events/manifest.yml new file mode 100644 index 000000000..d51b2bd1c --- /dev/null +++ b/test/packages/build_mode/bad_built_external_ecs/data_stream/events/manifest.yml @@ -0,0 +1,15 @@ +title: Events +type: logs +streams: + - input: logfile + title: Events logs + description: Collect events log data + vars: + - name: paths + type: text + title: Paths + multi: true + required: true + show_user: true + default: + - /var/log/*.log diff --git a/test/packages/build_mode/bad_built_external_ecs/docs/README.md b/test/packages/build_mode/bad_built_external_ecs/docs/README.md new file mode 100644 index 000000000..c0b01c5ec --- /dev/null +++ b/test/packages/build_mode/bad_built_external_ecs/docs/README.md @@ -0,0 +1,5 @@ +# Good Built Package + +This is the canonical minimal built-package fixture used for build-mode validation +tests (issue #549). It has no `_dev/` directories, no `.link` files, and no +`external: ecs` field references — making it valid under `ModeBuild`. diff --git a/test/packages/build_mode/bad_built_external_ecs/manifest.yml b/test/packages/build_mode/bad_built_external_ecs/manifest.yml new file mode 100644 index 000000000..0dc0d4080 --- /dev/null +++ b/test/packages/build_mode/bad_built_external_ecs/manifest.yml @@ -0,0 +1,23 @@ +format_version: 3.6.0 +name: good_built +title: Good Built Package +description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +version: 0.0.1 +type: integration +source: + license: "Apache-2.0" +conditions: + kibana: + version: '^8.0.0 || ^9.0.0' +policy_templates: + - name: events + title: Events logs + description: Collect events data + inputs: + - type: logfile + title: Collect events logs + description: Collecting events log data + multi: false +owner: + github: elastic/foobar + type: community From d827f09b3a5caeaca8b9b6e9f328a1ccb4519be5 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 14:29:54 +0200 Subject: [PATCH 08/23] Add build mode: stream input materialization rejection (task 07) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ValidateStreamInputMaterialized rejects built packages that still carry source-only 'package:' fields in two locations: * data_stream/*/manifest.yml streams[].package — composable-input pattern * manifest.yml policy_templates[].inputs[].package — package-reference pattern Both must be materialized (input:/type:) before packaging. - Also rejects data stream streams missing the required 'input:' field. - Registered for ModeBuild + integration packages only. - Unit tests: 9 cases covering both branches. - Integration tests: 6 cases (3 bad fixtures × fail in build / pass in source). - New fixtures: bad_built_stream_package, bad_built_missing_input, bad_built_policy_template_package. Co-Authored-By: Claude Sonnet 4.6 --- .../validate_stream_input_materialized.go | 156 +++++++++++++++++ ...validate_stream_input_materialized_test.go | 157 ++++++++++++++++++ code/go/internal/validator/spec.go | 2 + code/go/pkg/validator/api_test.go | 71 ++++++++ .../bad_built_missing_input/LICENSE.txt | 3 + .../bad_built_missing_input/changelog.yml | 6 + .../events/agent/stream/stream.yml.hbs | 4 + .../data_stream/events/fields/base-fields.yml | 12 ++ .../data_stream/events/fields/fields.yml | 5 + .../data_stream/events/manifest.yml | 5 + .../bad_built_missing_input/docs/README.md | 5 + .../bad_built_missing_input/manifest.yml | 23 +++ .../LICENSE.txt | 3 + .../changelog.yml | 6 + .../events/agent/stream/stream.yml.hbs | 4 + .../data_stream/events/fields/base-fields.yml | 12 ++ .../data_stream/events/fields/fields.yml | 3 + .../data_stream/events/manifest.yml | 15 ++ .../docs/README.md | 5 + .../manifest.yml | 22 +++ .../bad_built_stream_package/LICENSE.txt | 3 + .../bad_built_stream_package/changelog.yml | 6 + .../events/agent/stream/stream.yml.hbs | 4 + .../data_stream/events/fields/base-fields.yml | 12 ++ .../data_stream/events/fields/fields.yml | 5 + .../data_stream/events/manifest.yml | 6 + .../bad_built_stream_package/docs/README.md | 5 + .../bad_built_stream_package/manifest.yml | 23 +++ 28 files changed, 583 insertions(+) create mode 100644 code/go/internal/validator/semantic/validate_stream_input_materialized.go create mode 100644 code/go/internal/validator/semantic/validate_stream_input_materialized_test.go create mode 100644 test/packages/build_mode/bad_built_missing_input/LICENSE.txt create mode 100644 test/packages/build_mode/bad_built_missing_input/changelog.yml create mode 100644 test/packages/build_mode/bad_built_missing_input/data_stream/events/agent/stream/stream.yml.hbs create mode 100644 test/packages/build_mode/bad_built_missing_input/data_stream/events/fields/base-fields.yml create mode 100644 test/packages/build_mode/bad_built_missing_input/data_stream/events/fields/fields.yml create mode 100644 test/packages/build_mode/bad_built_missing_input/data_stream/events/manifest.yml create mode 100644 test/packages/build_mode/bad_built_missing_input/docs/README.md create mode 100644 test/packages/build_mode/bad_built_missing_input/manifest.yml create mode 100644 test/packages/build_mode/bad_built_policy_template_package/LICENSE.txt create mode 100644 test/packages/build_mode/bad_built_policy_template_package/changelog.yml create mode 100644 test/packages/build_mode/bad_built_policy_template_package/data_stream/events/agent/stream/stream.yml.hbs create mode 100644 test/packages/build_mode/bad_built_policy_template_package/data_stream/events/fields/base-fields.yml create mode 100644 test/packages/build_mode/bad_built_policy_template_package/data_stream/events/fields/fields.yml create mode 100644 test/packages/build_mode/bad_built_policy_template_package/data_stream/events/manifest.yml create mode 100644 test/packages/build_mode/bad_built_policy_template_package/docs/README.md create mode 100644 test/packages/build_mode/bad_built_policy_template_package/manifest.yml create mode 100644 test/packages/build_mode/bad_built_stream_package/LICENSE.txt create mode 100644 test/packages/build_mode/bad_built_stream_package/changelog.yml create mode 100644 test/packages/build_mode/bad_built_stream_package/data_stream/events/agent/stream/stream.yml.hbs create mode 100644 test/packages/build_mode/bad_built_stream_package/data_stream/events/fields/base-fields.yml create mode 100644 test/packages/build_mode/bad_built_stream_package/data_stream/events/fields/fields.yml create mode 100644 test/packages/build_mode/bad_built_stream_package/data_stream/events/manifest.yml create mode 100644 test/packages/build_mode/bad_built_stream_package/docs/README.md create mode 100644 test/packages/build_mode/bad_built_stream_package/manifest.yml diff --git a/code/go/internal/validator/semantic/validate_stream_input_materialized.go b/code/go/internal/validator/semantic/validate_stream_input_materialized.go new file mode 100644 index 000000000..64d3d8251 --- /dev/null +++ b/code/go/internal/validator/semantic/validate_stream_input_materialized.go @@ -0,0 +1,156 @@ +// 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 semantic + +import ( + "io/fs" + "path" + "slices" + + "gopkg.in/yaml.v3" + + "github.com/elastic/package-spec/v3/code/go/internal/fspath" + "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" +) + +// streamMaterializationEntry captures the fields needed to check input materialization +// in a data stream manifest's streams[] array. +type streamMaterializationEntry struct { + Input string `yaml:"input"` + Package string `yaml:"package"` +} + +type dataStreamMaterializationManifest struct { + Streams []streamMaterializationEntry `yaml:"streams"` +} + +// policyTemplateInputMaterialization captures only the fields needed from a policy template input +// to check whether materialization has taken place. +type policyTemplateInputMaterialization struct { + Type string `yaml:"type"` + Package string `yaml:"package"` +} + +type policyTemplateMaterialization struct { + Name string `yaml:"name"` + Inputs []policyTemplateInputMaterialization `yaml:"inputs"` +} + +type packageMaterializationManifest struct { + Type string `yaml:"type"` + PolicyTemplates []policyTemplateMaterialization `yaml:"policy_templates"` +} + +// ValidateStreamInputMaterialized errors when build-mode manifests carry +// source-only 'package:' fields that the build process should have materialized: +// +// - data_stream/*/manifest.yml: each stream must have 'input:' set and must NOT +// have 'package:' (composable-input pattern, source-only). +// - manifest.yml: each policy_template input must have 'type:' set and must NOT +// have 'package:' (package-reference pattern, source-only). +func ValidateStreamInputMaterialized(fsys fspath.FS) specerrors.ValidationErrors { + var errs specerrors.ValidationErrors + + errs = append(errs, validateDataStreamStreamsMaterialized(fsys)...) + errs = append(errs, validatePolicyTemplateInputsMaterialized(fsys)...) + + return errs +} + +// validateDataStreamStreamsMaterialized checks every data_stream/*/manifest.yml for +// stream entries that carry a source-only 'package:' field or are missing 'input:'. +func validateDataStreamStreamsMaterialized(fsys fspath.FS) specerrors.ValidationErrors { + dataStreams, err := listDataStreams(fsys) + if err != nil { + return specerrors.ValidationErrors{ + specerrors.NewStructuredErrorf("can't list data streams: %w", err), + } + } + + // Sort for deterministic error ordering. + slices.Sort(dataStreams) + + var errs specerrors.ValidationErrors + for _, dsName := range dataStreams { + manifestRelPath := path.Join(dataStreamDir, dsName, "manifest.yml") + data, err := fs.ReadFile(fsys, manifestRelPath) + if err != nil { + errs = append(errs, specerrors.NewStructuredErrorf( + "file %q: failed to read data stream manifest: %w", + fsys.Path(manifestRelPath), err, + )) + continue + } + + var manifest dataStreamMaterializationManifest + if err := yaml.Unmarshal(data, &manifest); err != nil { + errs = append(errs, specerrors.NewStructuredErrorf( + "file %q: failed to parse data stream manifest: %w", + fsys.Path(manifestRelPath), err, + )) + continue + } + + fullPath := fsys.Path(manifestRelPath) + for i, s := range manifest.Streams { + if s.Package != "" { + errs = append(errs, specerrors.NewStructuredErrorf( + "file %q: stream[%d] has 'package:' which is source-only; build packages must use 'input:' + 'template_paths:'", + fullPath, i, + )) + } + if s.Input == "" { + errs = append(errs, specerrors.NewStructuredErrorf( + "file %q: stream[%d] missing required 'input:' field", + fullPath, i, + )) + } + } + } + + return errs +} + +// validatePolicyTemplateInputsMaterialized checks the package-level manifest.yml for +// policy_template inputs that carry a source-only 'package:' field instead of 'type:'. +func validatePolicyTemplateInputsMaterialized(fsys fspath.FS) specerrors.ValidationErrors { + manifestRelPath := "manifest.yml" + data, err := fs.ReadFile(fsys, manifestRelPath) + if err != nil { + // If there is no manifest we have nothing to check; other validators handle + // the missing-manifest case. + return nil + } + + var manifest packageMaterializationManifest + if err := yaml.Unmarshal(data, &manifest); err != nil { + return specerrors.ValidationErrors{ + specerrors.NewStructuredErrorf( + "file %q: failed to parse manifest: %w", + fsys.Path(manifestRelPath), err, + ), + } + } + + // Only integration packages have policy_templates with composable inputs. + if manifest.Type != packageTypeIntegration { + return nil + } + + fullPath := fsys.Path(manifestRelPath) + var errs specerrors.ValidationErrors + for _, pt := range manifest.PolicyTemplates { + for i, input := range pt.Inputs { + if input.Package != "" { + errs = append(errs, specerrors.NewStructuredErrorf( + "file %q: policy_template %q input[%d] has 'package:' which is source-only; build packages must use 'type:'", + fullPath, pt.Name, i, + )) + } + } + } + + return errs +} diff --git a/code/go/internal/validator/semantic/validate_stream_input_materialized_test.go b/code/go/internal/validator/semantic/validate_stream_input_materialized_test.go new file mode 100644 index 000000000..1b4e4e03a --- /dev/null +++ b/code/go/internal/validator/semantic/validate_stream_input_materialized_test.go @@ -0,0 +1,157 @@ +// 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 semantic + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/elastic/package-spec/v3/code/go/internal/fspath" +) + +func TestValidateStreamInputMaterialized(t *testing.T) { + tests := []struct { + name string + // files maps relative path → YAML content written under a temp dir. + files map[string]string + expectErrors bool + errorContains []string + }{ + // --------------------------------------------------------------- + // Data stream manifest: happy paths + // --------------------------------------------------------------- + { + name: "data stream with input: set — no errors", + files: map[string]string{ + "data_stream/logs/manifest.yml": "title: Logs\ntype: logs\nstreams:\n - input: logfile\n title: Logs\n description: Collect logs\n", + }, + expectErrors: false, + }, + { + name: "no data_stream directory — no errors", + files: map[string]string{ + "manifest.yml": "type: integration\n", + }, + expectErrors: false, + }, + { + name: "data stream manifest with no streams array — no errors", + files: map[string]string{ + "data_stream/logs/manifest.yml": "title: Logs\ntype: logs\n", + }, + expectErrors: false, + }, + // --------------------------------------------------------------- + // Data stream manifest: error cases + // --------------------------------------------------------------- + { + name: "data stream stream has package: — rejected", + files: map[string]string{ + "data_stream/logs/manifest.yml": "title: Logs\ntype: logs\nstreams:\n - package: filelog_otel\n title: Logs\n description: Collect logs\n", + }, + expectErrors: true, + errorContains: []string{ + "stream[0]", + "'package:'", + "source-only", + "build packages must use 'input:'", + }, + }, + { + name: "data stream stream missing input: — rejected", + files: map[string]string{ + "data_stream/logs/manifest.yml": "title: Logs\ntype: logs\nstreams:\n - title: Logs\n description: Collect logs\n", + }, + expectErrors: true, + errorContains: []string{ + "stream[0]", + "missing required 'input:'", + }, + }, + { + name: "multiple data streams, one bad — rejected", + files: map[string]string{ + "data_stream/good/manifest.yml": "title: Good\ntype: logs\nstreams:\n - input: logfile\n title: Good\n description: Good\n", + "data_stream/bad/manifest.yml": "title: Bad\ntype: logs\nstreams:\n - package: some_package\n title: Bad\n description: Bad\n", + }, + expectErrors: true, + errorContains: []string{ + "'package:'", + "source-only", + }, + }, + // --------------------------------------------------------------- + // Package manifest policy_templates: happy paths + // --------------------------------------------------------------- + { + name: "policy_template input with type: set — no errors", + files: map[string]string{ + "manifest.yml": "type: integration\npolicy_templates:\n - name: logs\n title: Logs\n description: Logs\n inputs:\n - type: logfile\n title: Logs\n description: Logs\n", + }, + expectErrors: false, + }, + { + name: "non-integration package manifest — no errors", + files: map[string]string{ + "manifest.yml": "type: input\npolicy_templates:\n - name: logs\n title: Logs\n description: Logs\n inputs:\n - package: some_package\n title: Logs\n description: Logs\n", + }, + expectErrors: false, + }, + // --------------------------------------------------------------- + // Package manifest policy_templates: error cases + // --------------------------------------------------------------- + { + name: "policy_template input has package: — rejected", + files: map[string]string{ + "manifest.yml": "type: integration\npolicy_templates:\n - name: events\n title: Events\n description: Events\n inputs:\n - package: filelog_otel\n title: Collect events\n description: Collecting events\n", + }, + expectErrors: true, + errorContains: []string{ + "policy_template", + "events", + "input[0]", + "'package:'", + "source-only", + "build packages must use 'type:'", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tempDir := t.TempDir() + + for relPath, content := range tc.files { + fullPath := filepath.Join(tempDir, relPath) + require.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + require.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644)) + } + + fsys := fspath.DirFS(tempDir) + errs := ValidateStreamInputMaterialized(fsys) + + if !tc.expectErrors { + assert.Nil(t, errs, "expected no errors but got: %v", errs) + return + } + + require.NotNil(t, errs, "expected validation errors but got none") + var sb strings.Builder + for _, e := range errs { + sb.WriteString(e.Error()) + sb.WriteString("\n") + } + combined := sb.String() + for _, substr := range tc.errorContains { + assert.Contains(t, combined, substr) + } + }) + } +} diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index 11f33af88..4741413ef 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -262,6 +262,8 @@ func (s Spec) rules(pkgType string, rootSpec spectypes.ItemSpec) validationRules {fn: semantic.ValidateNoDevFolder, modes: []modes.Mode{modes.Build}}, {fn: semantic.ValidateNoLinkFiles, modes: []modes.Mode{modes.Build}}, {fn: semantic.ValidateNoExternalEcs, modes: []modes.Mode{modes.Build}}, + {fn: semantic.ValidateStreamInputMaterialized, modes: []modes.Mode{modes.Build}, + types: []string{"integration"}}, } var validationRules validationRules diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go index 921696a94..90a0f6e93 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -388,6 +388,77 @@ func TestBuildMode_NoExternalEcs(t *testing.T) { }) } +// ----------------------------------------------------------------------- +// TestBuildMode_StreamInputMaterialized +// +// Verifies that ModeBuild: +// - Rejects a data stream manifest stream that carries a source-only 'package:' field. +// - Rejects a data stream manifest stream that is missing the required 'input:' field. +// - Rejects a package manifest policy_template input that carries a source-only 'package:' field. +// - Does NOT raise these errors in ModeSource (source allows composable-input pattern). +// - good_built still passes. +// ----------------------------------------------------------------------- + +func TestBuildMode_StreamInputMaterialized(t *testing.T) { + goodBuiltPath := filepath.Join(testPackagesDir, "build_mode", "good_built") + badStreamPackagePath := filepath.Join(testPackagesDir, "build_mode", "bad_built_stream_package") + badMissingInputPath := filepath.Join(testPackagesDir, "build_mode", "bad_built_missing_input") + badPolicyTemplatePkgPath := filepath.Join(testPackagesDir, "build_mode", "bad_built_policy_template_package") + + t.Run("good_built passes ModeBuild (stream materialization)", func(t *testing.T) { + v, err := NewFromPath(ModeBuild, goodBuiltPath) + require.NoError(t, err) + assert.NoError(t, v.Validate(), "good_built should pass build-mode validation") + }) + + t.Run("bad_built_stream_package fails ModeBuild (package: in data stream)", func(t *testing.T) { + v, err := NewFromPath(ModeBuild, badStreamPackagePath) + require.NoError(t, err) + buildErr := v.Validate() + require.Error(t, buildErr, "bad_built_stream_package should fail build-mode validation") + assert.Contains(t, buildErr.Error(), "'package:'") + assert.Contains(t, buildErr.Error(), "source-only") + }) + + t.Run("bad_built_stream_package passes ModeSource (package: allowed in source)", func(t *testing.T) { + v, err := NewFromPath(ModeSource, badStreamPackagePath) + require.NoError(t, err) + sourceErr := v.Validate() + if sourceErr != nil { + assert.NotContains(t, sourceErr.Error(), "source-only", + "ValidateStreamInputMaterialized must not run in source mode") + } + }) + + t.Run("bad_built_missing_input fails ModeBuild (missing input: in data stream)", func(t *testing.T) { + v, err := NewFromPath(ModeBuild, badMissingInputPath) + require.NoError(t, err) + buildErr := v.Validate() + require.Error(t, buildErr, "bad_built_missing_input should fail build-mode validation") + assert.Contains(t, buildErr.Error(), "missing required 'input:'") + }) + + t.Run("bad_built_policy_template_package fails ModeBuild (package: in policy_template input)", func(t *testing.T) { + v, err := NewFromPath(ModeBuild, badPolicyTemplatePkgPath) + require.NoError(t, err) + buildErr := v.Validate() + require.Error(t, buildErr, "bad_built_policy_template_package should fail build-mode validation") + assert.Contains(t, buildErr.Error(), "'package:'") + assert.Contains(t, buildErr.Error(), "source-only") + assert.Contains(t, buildErr.Error(), "build packages must use 'type:'") + }) + + t.Run("bad_built_policy_template_package passes ModeSource (package: allowed in source)", func(t *testing.T) { + v, err := NewFromPath(ModeSource, badPolicyTemplatePkgPath) + require.NoError(t, err) + sourceErr := v.Validate() + if sourceErr != nil { + assert.NotContains(t, sourceErr.Error(), "build packages must use 'type:'", + "ValidateStreamInputMaterialized must not run in source mode") + } + }) +} + // ----------------------------------------------------------------------- // TestLegacyPreservation_FromZip (golden test) // diff --git a/test/packages/build_mode/bad_built_missing_input/LICENSE.txt b/test/packages/build_mode/bad_built_missing_input/LICENSE.txt new file mode 100644 index 000000000..f6f788a89 --- /dev/null +++ b/test/packages/build_mode/bad_built_missing_input/LICENSE.txt @@ -0,0 +1,3 @@ +Elastic License 2.0 + +URL: https://www.elastic.co/licensing/elastic-license diff --git a/test/packages/build_mode/bad_built_missing_input/changelog.yml b/test/packages/build_mode/bad_built_missing_input/changelog.yml new file mode 100644 index 000000000..e00f88133 --- /dev/null +++ b/test/packages/build_mode/bad_built_missing_input/changelog.yml @@ -0,0 +1,6 @@ +# newer versions go on top +- version: "0.0.1" + changes: + - description: Initial draft of the package + type: enhancement + link: https://github.com/elastic/integrations/pull/1 diff --git a/test/packages/build_mode/bad_built_missing_input/data_stream/events/agent/stream/stream.yml.hbs b/test/packages/build_mode/bad_built_missing_input/data_stream/events/agent/stream/stream.yml.hbs new file mode 100644 index 000000000..9390bc05c --- /dev/null +++ b/test/packages/build_mode/bad_built_missing_input/data_stream/events/agent/stream/stream.yml.hbs @@ -0,0 +1,4 @@ +paths: +{{#each paths}} + - {{this}} +{{/each}} diff --git a/test/packages/build_mode/bad_built_missing_input/data_stream/events/fields/base-fields.yml b/test/packages/build_mode/bad_built_missing_input/data_stream/events/fields/base-fields.yml new file mode 100644 index 000000000..0d1791ffe --- /dev/null +++ b/test/packages/build_mode/bad_built_missing_input/data_stream/events/fields/base-fields.yml @@ -0,0 +1,12 @@ +- name: data_stream.type + type: constant_keyword + description: Data stream type. +- name: data_stream.dataset + type: constant_keyword + description: Data stream dataset. +- name: data_stream.namespace + type: constant_keyword + description: Data stream namespace. +- name: "@timestamp" + type: date + description: Event timestamp. diff --git a/test/packages/build_mode/bad_built_missing_input/data_stream/events/fields/fields.yml b/test/packages/build_mode/bad_built_missing_input/data_stream/events/fields/fields.yml new file mode 100644 index 000000000..e09745251 --- /dev/null +++ b/test/packages/build_mode/bad_built_missing_input/data_stream/events/fields/fields.yml @@ -0,0 +1,5 @@ +- name: message + type: keyword + description: Log message. +- name: host.name + external: ecs diff --git a/test/packages/build_mode/bad_built_missing_input/data_stream/events/manifest.yml b/test/packages/build_mode/bad_built_missing_input/data_stream/events/manifest.yml new file mode 100644 index 000000000..ca8722e90 --- /dev/null +++ b/test/packages/build_mode/bad_built_missing_input/data_stream/events/manifest.yml @@ -0,0 +1,5 @@ +title: Events +type: logs +streams: + - title: Events logs + description: Collect events log data (missing input — invalid in built packages) diff --git a/test/packages/build_mode/bad_built_missing_input/docs/README.md b/test/packages/build_mode/bad_built_missing_input/docs/README.md new file mode 100644 index 000000000..c0b01c5ec --- /dev/null +++ b/test/packages/build_mode/bad_built_missing_input/docs/README.md @@ -0,0 +1,5 @@ +# Good Built Package + +This is the canonical minimal built-package fixture used for build-mode validation +tests (issue #549). It has no `_dev/` directories, no `.link` files, and no +`external: ecs` field references — making it valid under `ModeBuild`. diff --git a/test/packages/build_mode/bad_built_missing_input/manifest.yml b/test/packages/build_mode/bad_built_missing_input/manifest.yml new file mode 100644 index 000000000..0dc0d4080 --- /dev/null +++ b/test/packages/build_mode/bad_built_missing_input/manifest.yml @@ -0,0 +1,23 @@ +format_version: 3.6.0 +name: good_built +title: Good Built Package +description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +version: 0.0.1 +type: integration +source: + license: "Apache-2.0" +conditions: + kibana: + version: '^8.0.0 || ^9.0.0' +policy_templates: + - name: events + title: Events logs + description: Collect events data + inputs: + - type: logfile + title: Collect events logs + description: Collecting events log data + multi: false +owner: + github: elastic/foobar + type: community diff --git a/test/packages/build_mode/bad_built_policy_template_package/LICENSE.txt b/test/packages/build_mode/bad_built_policy_template_package/LICENSE.txt new file mode 100644 index 000000000..f6f788a89 --- /dev/null +++ b/test/packages/build_mode/bad_built_policy_template_package/LICENSE.txt @@ -0,0 +1,3 @@ +Elastic License 2.0 + +URL: https://www.elastic.co/licensing/elastic-license diff --git a/test/packages/build_mode/bad_built_policy_template_package/changelog.yml b/test/packages/build_mode/bad_built_policy_template_package/changelog.yml new file mode 100644 index 000000000..e00f88133 --- /dev/null +++ b/test/packages/build_mode/bad_built_policy_template_package/changelog.yml @@ -0,0 +1,6 @@ +# newer versions go on top +- version: "0.0.1" + changes: + - description: Initial draft of the package + type: enhancement + link: https://github.com/elastic/integrations/pull/1 diff --git a/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/agent/stream/stream.yml.hbs b/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/agent/stream/stream.yml.hbs new file mode 100644 index 000000000..9390bc05c --- /dev/null +++ b/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/agent/stream/stream.yml.hbs @@ -0,0 +1,4 @@ +paths: +{{#each paths}} + - {{this}} +{{/each}} diff --git a/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/fields/base-fields.yml b/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/fields/base-fields.yml new file mode 100644 index 000000000..0d1791ffe --- /dev/null +++ b/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/fields/base-fields.yml @@ -0,0 +1,12 @@ +- name: data_stream.type + type: constant_keyword + description: Data stream type. +- name: data_stream.dataset + type: constant_keyword + description: Data stream dataset. +- name: data_stream.namespace + type: constant_keyword + description: Data stream namespace. +- name: "@timestamp" + type: date + description: Event timestamp. diff --git a/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/fields/fields.yml b/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/fields/fields.yml new file mode 100644 index 000000000..f21b45be3 --- /dev/null +++ b/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/fields/fields.yml @@ -0,0 +1,3 @@ +- name: message + type: keyword + description: Log message. diff --git a/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/manifest.yml b/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/manifest.yml new file mode 100644 index 000000000..d51b2bd1c --- /dev/null +++ b/test/packages/build_mode/bad_built_policy_template_package/data_stream/events/manifest.yml @@ -0,0 +1,15 @@ +title: Events +type: logs +streams: + - input: logfile + title: Events logs + description: Collect events log data + vars: + - name: paths + type: text + title: Paths + multi: true + required: true + show_user: true + default: + - /var/log/*.log diff --git a/test/packages/build_mode/bad_built_policy_template_package/docs/README.md b/test/packages/build_mode/bad_built_policy_template_package/docs/README.md new file mode 100644 index 000000000..c0b01c5ec --- /dev/null +++ b/test/packages/build_mode/bad_built_policy_template_package/docs/README.md @@ -0,0 +1,5 @@ +# Good Built Package + +This is the canonical minimal built-package fixture used for build-mode validation +tests (issue #549). It has no `_dev/` directories, no `.link` files, and no +`external: ecs` field references — making it valid under `ModeBuild`. diff --git a/test/packages/build_mode/bad_built_policy_template_package/manifest.yml b/test/packages/build_mode/bad_built_policy_template_package/manifest.yml new file mode 100644 index 000000000..50b69410f --- /dev/null +++ b/test/packages/build_mode/bad_built_policy_template_package/manifest.yml @@ -0,0 +1,22 @@ +format_version: 3.6.0 +name: good_built +title: Good Built Package +description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +version: 0.0.1 +type: integration +source: + license: "Apache-2.0" +conditions: + kibana: + version: '^8.0.0 || ^9.0.0' +policy_templates: + - name: events + title: Events logs + description: Collect events data + inputs: + - package: filelog_otel + title: Collect events logs + description: Collecting events log data (source-only package reference — invalid in built packages) +owner: + github: elastic/foobar + type: community diff --git a/test/packages/build_mode/bad_built_stream_package/LICENSE.txt b/test/packages/build_mode/bad_built_stream_package/LICENSE.txt new file mode 100644 index 000000000..f6f788a89 --- /dev/null +++ b/test/packages/build_mode/bad_built_stream_package/LICENSE.txt @@ -0,0 +1,3 @@ +Elastic License 2.0 + +URL: https://www.elastic.co/licensing/elastic-license diff --git a/test/packages/build_mode/bad_built_stream_package/changelog.yml b/test/packages/build_mode/bad_built_stream_package/changelog.yml new file mode 100644 index 000000000..e00f88133 --- /dev/null +++ b/test/packages/build_mode/bad_built_stream_package/changelog.yml @@ -0,0 +1,6 @@ +# newer versions go on top +- version: "0.0.1" + changes: + - description: Initial draft of the package + type: enhancement + link: https://github.com/elastic/integrations/pull/1 diff --git a/test/packages/build_mode/bad_built_stream_package/data_stream/events/agent/stream/stream.yml.hbs b/test/packages/build_mode/bad_built_stream_package/data_stream/events/agent/stream/stream.yml.hbs new file mode 100644 index 000000000..9390bc05c --- /dev/null +++ b/test/packages/build_mode/bad_built_stream_package/data_stream/events/agent/stream/stream.yml.hbs @@ -0,0 +1,4 @@ +paths: +{{#each paths}} + - {{this}} +{{/each}} diff --git a/test/packages/build_mode/bad_built_stream_package/data_stream/events/fields/base-fields.yml b/test/packages/build_mode/bad_built_stream_package/data_stream/events/fields/base-fields.yml new file mode 100644 index 000000000..0d1791ffe --- /dev/null +++ b/test/packages/build_mode/bad_built_stream_package/data_stream/events/fields/base-fields.yml @@ -0,0 +1,12 @@ +- name: data_stream.type + type: constant_keyword + description: Data stream type. +- name: data_stream.dataset + type: constant_keyword + description: Data stream dataset. +- name: data_stream.namespace + type: constant_keyword + description: Data stream namespace. +- name: "@timestamp" + type: date + description: Event timestamp. diff --git a/test/packages/build_mode/bad_built_stream_package/data_stream/events/fields/fields.yml b/test/packages/build_mode/bad_built_stream_package/data_stream/events/fields/fields.yml new file mode 100644 index 000000000..e09745251 --- /dev/null +++ b/test/packages/build_mode/bad_built_stream_package/data_stream/events/fields/fields.yml @@ -0,0 +1,5 @@ +- name: message + type: keyword + description: Log message. +- name: host.name + external: ecs diff --git a/test/packages/build_mode/bad_built_stream_package/data_stream/events/manifest.yml b/test/packages/build_mode/bad_built_stream_package/data_stream/events/manifest.yml new file mode 100644 index 000000000..16f358447 --- /dev/null +++ b/test/packages/build_mode/bad_built_stream_package/data_stream/events/manifest.yml @@ -0,0 +1,6 @@ +title: Events +type: logs +streams: + - package: filelog_otel + title: Events logs + description: Collect events log data (source-only package reference — invalid in built packages) diff --git a/test/packages/build_mode/bad_built_stream_package/docs/README.md b/test/packages/build_mode/bad_built_stream_package/docs/README.md new file mode 100644 index 000000000..c0b01c5ec --- /dev/null +++ b/test/packages/build_mode/bad_built_stream_package/docs/README.md @@ -0,0 +1,5 @@ +# Good Built Package + +This is the canonical minimal built-package fixture used for build-mode validation +tests (issue #549). It has no `_dev/` directories, no `.link` files, and no +`external: ecs` field references — making it valid under `ModeBuild`. diff --git a/test/packages/build_mode/bad_built_stream_package/manifest.yml b/test/packages/build_mode/bad_built_stream_package/manifest.yml new file mode 100644 index 000000000..0dc0d4080 --- /dev/null +++ b/test/packages/build_mode/bad_built_stream_package/manifest.yml @@ -0,0 +1,23 @@ +format_version: 3.6.0 +name: good_built +title: Good Built Package +description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +version: 0.0.1 +type: integration +source: + license: "Apache-2.0" +conditions: + kibana: + version: '^8.0.0 || ^9.0.0' +policy_templates: + - name: events + title: Events logs + description: Collect events data + inputs: + - type: logfile + title: Collect events logs + description: Collecting events log data + multi: false +owner: + github: elastic/foobar + type: community From 2eda0edc0776940f3f40c6b9f80733f2ede84799 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 14:33:50 +0200 Subject: [PATCH 09/23] Add changelog entry for validation modes (task 08) Co-Authored-By: Claude Sonnet 4.6 --- spec/changelog.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/changelog.yml b/spec/changelog.yml index 0f846785f..bee3bf753 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 `source` and `build` validation modes via a new constructor API; existing functions are deprecated in favour of `NewFromPath/Zip/FS(mode, ...)`. + type: enhancement + link: https://github.com/elastic/package-spec/issues/549 - version: 3.6.3 changes: - description: Add optional `release` field to agentless deployment mode to explicitly declare its release stage. From 6ac4740cadacd732c64f08aa5deb85c428f16fe0 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 14:37:26 +0200 Subject: [PATCH 10/23] Fix listDataStreams to skip non-directory entries under data_stream/ Stray files (.gitkeep, .DS_Store, etc.) sitting directly under data_stream/ were included in the list returned by listDataStreams, causing every caller to attempt opening /manifest.yml and surface a confusing internal error instead of a clean validation message. Filter to directories only using IsDir(). The relative order of entries is unchanged since fs.ReadDir already returns them sorted by filename. Add TestListDataStreamsIgnoresNonDirectories to validate_no_embedded_ecs_test.go to cover the stray-file case end-to-end. Co-Authored-By: Claude Sonnet 4.6 --- code/go/internal/validator/semantic/types.go | 8 +++++--- .../semantic/validate_no_embedded_ecs_test.go | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/code/go/internal/validator/semantic/types.go b/code/go/internal/validator/semantic/types.go index 51a1fc5b7..c557ae600 100644 --- a/code/go/internal/validator/semantic/types.go +++ b/code/go/internal/validator/semantic/types.go @@ -352,9 +352,11 @@ func listDataStreams(fsys fspath.FS) ([]string, error) { return nil, fmt.Errorf("can't list data streams directory: %w", err) } - list := make([]string, len(dataStreams)) - for i, dataStream := range dataStreams { - list[i] = dataStream.Name() + var list []string + for _, dataStream := range dataStreams { + if dataStream.IsDir() { + list = append(list, dataStream.Name()) + } } return list, nil } diff --git a/code/go/internal/validator/semantic/validate_no_embedded_ecs_test.go b/code/go/internal/validator/semantic/validate_no_embedded_ecs_test.go index 5b545a1f7..f286f130f 100644 --- a/code/go/internal/validator/semantic/validate_no_embedded_ecs_test.go +++ b/code/go/internal/validator/semantic/validate_no_embedded_ecs_test.go @@ -170,3 +170,20 @@ elasticsearch: }) } } + +// TestListDataStreamsIgnoresNonDirectories verifies that stray files directly +// under data_stream/ (e.g. .gitkeep, .DS_Store) are silently skipped and do +// not cause spurious validation errors. +func TestListDataStreamsIgnoresNonDirectories(t *testing.T) { + tempDir := t.TempDir() + + // Create data_stream/ with only a stray file — no subdirectories. + dataStreamDir := filepath.Join(tempDir, "data_stream") + require.NoError(t, os.MkdirAll(dataStreamDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(dataStreamDir, ".gitkeep"), []byte(""), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(dataStreamDir, ".DS_Store"), []byte(""), 0644)) + + fsys := fspath.DirFS(tempDir) + errs := ValidateNoEmbeddedEcsInDynamicTemplates(fsys) + assert.Nil(t, errs, "stray files under data_stream/ should produce no errors, got: %v", errs) +} From 3c232718dd94b2cbf45d1098225cd29aa0ab7d3f Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 15:15:35 +0200 Subject: [PATCH 11/23] Fix bad_built fixture names and mid-name .link test case (review B1, B2) - Update all bad_built_* manifests: name/title/description now match their directory instead of copying good_built's values - Fix TestValidateNoLinkFiles: mid-name .link case now creates base-fields.link.yml so filepath.Ext returning .yml is exercised Co-Authored-By: Claude Sonnet 4.6 --- .../validator/semantic/validate_no_link_files_test.go | 2 +- .../packages/build_mode/bad_built_external_ecs/manifest.yml | 6 +++--- .../build_mode/bad_built_missing_input/manifest.yml | 6 +++--- .../bad_built_policy_template_package/manifest.yml | 6 +++--- .../build_mode/bad_built_stream_package/manifest.yml | 6 +++--- test/packages/build_mode/bad_built_with_dev/manifest.yml | 6 +++--- test/packages/build_mode/bad_built_with_link/manifest.yml | 6 +++--- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/code/go/internal/validator/semantic/validate_no_link_files_test.go b/code/go/internal/validator/semantic/validate_no_link_files_test.go index 2b9494543..443fd238b 100644 --- a/code/go/internal/validator/semantic/validate_no_link_files_test.go +++ b/code/go/internal/validator/semantic/validate_no_link_files_test.go @@ -51,7 +51,7 @@ func TestValidateNoLinkFiles(t *testing.T) { }, { name: "file with .link in the middle of the name is not rejected", - files: []string{"data_stream/foo/fields/base-fields.yml"}, + files: []string{"data_stream/foo/fields/base-fields.link.yml"}, expectErrors: false, }, } diff --git a/test/packages/build_mode/bad_built_external_ecs/manifest.yml b/test/packages/build_mode/bad_built_external_ecs/manifest.yml index 0dc0d4080..9b10b6ed1 100644 --- a/test/packages/build_mode/bad_built_external_ecs/manifest.yml +++ b/test/packages/build_mode/bad_built_external_ecs/manifest.yml @@ -1,7 +1,7 @@ format_version: 3.6.0 -name: good_built -title: Good Built Package -description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +name: bad_built_external_ecs +title: Bad Built Package - External ECS Fields +description: Built-package fixture with external ECS fields - invalid in build mode (issue #549). version: 0.0.1 type: integration source: diff --git a/test/packages/build_mode/bad_built_missing_input/manifest.yml b/test/packages/build_mode/bad_built_missing_input/manifest.yml index 0dc0d4080..a598b06c2 100644 --- a/test/packages/build_mode/bad_built_missing_input/manifest.yml +++ b/test/packages/build_mode/bad_built_missing_input/manifest.yml @@ -1,7 +1,7 @@ format_version: 3.6.0 -name: good_built -title: Good Built Package -description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +name: bad_built_missing_input +title: Bad Built Package - Missing Input +description: Built-package fixture with a stream missing required input field - invalid in build mode (issue #549). version: 0.0.1 type: integration source: diff --git a/test/packages/build_mode/bad_built_policy_template_package/manifest.yml b/test/packages/build_mode/bad_built_policy_template_package/manifest.yml index 50b69410f..c81e0e6e8 100644 --- a/test/packages/build_mode/bad_built_policy_template_package/manifest.yml +++ b/test/packages/build_mode/bad_built_policy_template_package/manifest.yml @@ -1,7 +1,7 @@ format_version: 3.6.0 -name: good_built -title: Good Built Package -description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +name: bad_built_policy_template_package +title: Bad Built Package - Policy Template Package Reference +description: Built-package fixture with a source-only package reference in policy_templates - invalid in build mode (issue #549). version: 0.0.1 type: integration source: diff --git a/test/packages/build_mode/bad_built_stream_package/manifest.yml b/test/packages/build_mode/bad_built_stream_package/manifest.yml index 0dc0d4080..f04a853ad 100644 --- a/test/packages/build_mode/bad_built_stream_package/manifest.yml +++ b/test/packages/build_mode/bad_built_stream_package/manifest.yml @@ -1,7 +1,7 @@ format_version: 3.6.0 -name: good_built -title: Good Built Package -description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +name: bad_built_stream_package +title: Bad Built Package - Stream Package Reference +description: Built-package fixture with a source-only package reference in a data stream - invalid in build mode (issue #549). version: 0.0.1 type: integration source: diff --git a/test/packages/build_mode/bad_built_with_dev/manifest.yml b/test/packages/build_mode/bad_built_with_dev/manifest.yml index 0dc0d4080..a4b04ba08 100644 --- a/test/packages/build_mode/bad_built_with_dev/manifest.yml +++ b/test/packages/build_mode/bad_built_with_dev/manifest.yml @@ -1,7 +1,7 @@ format_version: 3.6.0 -name: good_built -title: Good Built Package -description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +name: bad_built_with_dev +title: Bad Built Package - Dev Folder Present +description: Built-package fixture containing a _dev folder - invalid in build mode (issue #549). version: 0.0.1 type: integration source: diff --git a/test/packages/build_mode/bad_built_with_link/manifest.yml b/test/packages/build_mode/bad_built_with_link/manifest.yml index 0dc0d4080..58e357e55 100644 --- a/test/packages/build_mode/bad_built_with_link/manifest.yml +++ b/test/packages/build_mode/bad_built_with_link/manifest.yml @@ -1,7 +1,7 @@ format_version: 3.6.0 -name: good_built -title: Good Built Package -description: Canonical minimal built-package fixture for build-mode validation tests (issue #549). +name: bad_built_with_link +title: Bad Built Package - Link Files Present +description: Built-package fixture containing .link files - invalid in build mode (issue #549). version: 0.0.1 type: integration source: From 9827ff2eb340fd244e7499a39a7b4d2bfbe339f7 Mon Sep 17 00:00:00 2001 From: Tere Date: Wed, 27 May 2026 15:19:15 +0200 Subject: [PATCH 12/23] Fix medium-priority review items (S2, T1, T2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit S2: skip 'missing input:' check when 'package:' already flagged on the same stream entry. The root cause is 'package:', not a forgotten 'input:'; emitting both errors was misleading for the composable-input pattern. T1: strengthen good_v3 source-mode sub-test to require.NoError (it is a known-good source fixture). Add explanatory comments to each bad_built_* source-mode sub-test explaining why they cannot use hard assertions — those fixtures have unrelated source-validation errors that are not under test. T2: remove stale '(Tasks 04-07, not yet implemented)' parenthetical from TestBuildMode_SkipsBuildExcludedRules — those rules are now implemented. Co-Authored-By: Claude Sonnet 4.6 --- .../validate_stream_input_materialized.go | 5 ++++ code/go/pkg/validator/api_test.go | 24 ++++++++++++------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/code/go/internal/validator/semantic/validate_stream_input_materialized.go b/code/go/internal/validator/semantic/validate_stream_input_materialized.go index 64d3d8251..b2eb6d829 100644 --- a/code/go/internal/validator/semantic/validate_stream_input_materialized.go +++ b/code/go/internal/validator/semantic/validate_stream_input_materialized.go @@ -100,6 +100,11 @@ func validateDataStreamStreamsMaterialized(fsys fspath.FS) specerrors.Validation "file %q: stream[%d] has 'package:' which is source-only; build packages must use 'input:' + 'template_paths:'", fullPath, i, )) + // Skip the 'input:' check for this stream: the root cause is the + // presence of 'package:', not a forgotten 'input:'. Emitting both + // errors would be misleading — the user intended the composable-input + // pattern and needs to materialise it, not simply add 'input:'. + continue } if s.Input == "" { errs = append(errs, specerrors.NewStructuredErrorf( diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go index 90a0f6e93..1c455459f 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -277,9 +277,8 @@ func TestBuildMode_SkipsBuildExcludedRules(t *testing.T) { buildErr := buildV.Validate() if buildErr != nil { - // The only acceptable errors here are from build-mode rejection rules - // (Tasks 04-07, not yet implemented). ValidateExternalFieldsWithDevFolder - // must NOT have contributed an error. + // The only acceptable errors here are from build-mode rejection rules. + // ValidateExternalFieldsWithDevFolder must NOT have contributed an error. assert.NotContains(t, buildErr.Error(), "external key defined", "ValidateExternalFieldsWithDevFolder must not run in build mode") } @@ -313,6 +312,9 @@ func TestBuildMode_NoDevFolder(t *testing.T) { }) t.Run("bad_built_with_dev passes ModeSource (source allows _dev/)", func(t *testing.T) { + // bad_built_with_dev is not a valid source package (its _dev/build/build.yml + // is empty and other source rules fire). We can only assert that the + // build-only rule — ValidateNoDevFolder — did not contribute an error. v, err := NewFromPath(ModeSource, badBuiltPath) require.NoError(t, err) sourceErr := v.Validate() @@ -345,6 +347,9 @@ func TestBuildMode_NoLinkFiles(t *testing.T) { }) t.Run("with_links passes ModeSource (.link files allowed in source mode)", func(t *testing.T) { + // with_links resolves .link files transparently in source mode; other + // source rules may still fire on that fixture. We can only assert that + // ValidateNoLinkFiles did not contribute an error. v, err := NewFromPath(ModeSource, withLinksPath) require.NoError(t, err) sourceErr := v.Validate() @@ -378,13 +383,10 @@ func TestBuildMode_NoExternalEcs(t *testing.T) { }) t.Run("good_v3 with external: ecs passes ModeSource", func(t *testing.T) { + // good_v3 is a known-good source fixture; it must pass source-mode cleanly. v, err := NewFromPath(ModeSource, goodV3Path) require.NoError(t, err) - sourceErr := v.Validate() - if sourceErr != nil { - assert.NotContains(t, sourceErr.Error(), "ECS fields must be materialized", - "ValidateNoExternalEcs must not run in source mode") - } + require.NoError(t, v.Validate(), "good_v3 should pass source-mode validation") }) } @@ -421,6 +423,9 @@ func TestBuildMode_StreamInputMaterialized(t *testing.T) { }) t.Run("bad_built_stream_package passes ModeSource (package: allowed in source)", func(t *testing.T) { + // bad_built_stream_package is not a valid source package (it lacks a + // _dev/build/build.yml for its external ECS field, among other issues). + // We can only assert that ValidateStreamInputMaterialized did not fire. v, err := NewFromPath(ModeSource, badStreamPackagePath) require.NoError(t, err) sourceErr := v.Validate() @@ -449,6 +454,9 @@ func TestBuildMode_StreamInputMaterialized(t *testing.T) { }) t.Run("bad_built_policy_template_package passes ModeSource (package: allowed in source)", func(t *testing.T) { + // bad_built_policy_template_package has an unlisted package reference in its + // requires section, so other source rules fire. We can only assert that + // ValidateStreamInputMaterialized did not fire. v, err := NewFromPath(ModeSource, badPolicyTemplatePkgPath) require.NoError(t, err) sourceErr := v.Validate() From d6573c2d9dcb0fec1d21d973aaf70cda89663ded Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 28 May 2026 09:38:41 +0200 Subject: [PATCH 13/23] Fix correctness and cleanup issues found in code review - NewFromFS(ModeSource): wrap with linkedfiles.NewFS instead of BlockFS so .link files are resolved transparently, matching the documented mode contract - NewFromZip: drop the mode parameter and always use ModeBuild; zip files are by definition built packages (elastic-package build output / package registry format). Deprecated ValidateFromZip keeps ModeLegacy via internal newFromZip. - folder_item_spec: skip syntactic validation for .link files that BlockFS cannot open, preventing cryptic ErrUnsupportedLinkFile errors before the clean semantic ValidateNoLinkFiles error fires in build mode - Consolidate duplicate packageTypeIntegration constant into integrationPackageType (types.go) across all five files in the semantic package Co-Authored-By: Claude Sonnet 4.6 --- .../go/internal/validator/folder_item_spec.go | 7 +++++ .../validate_datastream_package_categories.go | 2 +- .../validate_integration_inputs_deprecated.go | 2 +- ...lidate_integration_policy_template_path.go | 3 +- ...e_policy_template_datastream_categories.go | 2 +- .../validate_stream_input_materialized.go | 7 ++--- code/go/pkg/validator/api.go | 29 ++++++++++++++----- code/go/pkg/validator/api_test.go | 23 +++++---------- code/go/pkg/validator/mode.go | 8 +++-- code/go/pkg/validator/validator.go | 4 +-- 10 files changed, 52 insertions(+), 35 deletions(-) diff --git a/code/go/internal/validator/folder_item_spec.go b/code/go/internal/validator/folder_item_spec.go index bca7cb808..9321d9785 100644 --- a/code/go/internal/validator/folder_item_spec.go +++ b/code/go/internal/validator/folder_item_spec.go @@ -5,10 +5,12 @@ package validator import ( + "errors" "fmt" "io/fs" "regexp" + "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" "github.com/elastic/package-spec/v3/code/go/internal/spectypes" "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" ) @@ -40,6 +42,11 @@ func matchingFileExists(spec spectypes.ItemSpec, files []fs.DirEntry) (bool, err func validateFile(spec spectypes.ItemSpec, fsys fs.FS, itemPath string) specerrors.ValidationErrors { err := validateMaxSize(fsys, itemPath, spec) if err != nil { + if errors.Is(err, linkedfiles.ErrUnsupportedLinkFile) { + // .link files in build mode: skip syntactic checks here; the semantic + // ValidateNoLinkFiles rule emits a clean validation error for them. + return nil + } return specerrors.ValidationErrors{specerrors.NewStructuredError(err, specerrors.UnassignedCode)} } if mediaType := spec.ContentMediaType(); mediaType != nil { diff --git a/code/go/internal/validator/semantic/validate_datastream_package_categories.go b/code/go/internal/validator/semantic/validate_datastream_package_categories.go index 0a4de7638..7e538b383 100644 --- a/code/go/internal/validator/semantic/validate_datastream_package_categories.go +++ b/code/go/internal/validator/semantic/validate_datastream_package_categories.go @@ -105,7 +105,7 @@ func ValidateDatastreamPackageCategories(fsys fspath.FS) specerrors.ValidationEr specerrors.NewStructuredErrorf("file \"%s\" is invalid: %w", fsys.Path(manifestPath), err)} } - if pkgType != packageTypeIntegration { + if pkgType != integrationPackageType { return nil } diff --git a/code/go/internal/validator/semantic/validate_integration_inputs_deprecated.go b/code/go/internal/validator/semantic/validate_integration_inputs_deprecated.go index b393da6a4..09ca8eb34 100644 --- a/code/go/internal/validator/semantic/validate_integration_inputs_deprecated.go +++ b/code/go/internal/validator/semantic/validate_integration_inputs_deprecated.go @@ -47,7 +47,7 @@ func ValidateIntegrationInputsDeprecation(fsys fspath.FS) specerrors.ValidationE specerrors.NewStructuredErrorf("file \"%s\" is invalid: %w", fsys.Path(manifestPath), err)} } // skip if not an integration package - if m.Type != packageTypeIntegration { + if m.Type != integrationPackageType { return nil } diff --git a/code/go/internal/validator/semantic/validate_integration_policy_template_path.go b/code/go/internal/validator/semantic/validate_integration_policy_template_path.go index b7a93cc18..971e48c3c 100644 --- a/code/go/internal/validator/semantic/validate_integration_policy_template_path.go +++ b/code/go/internal/validator/semantic/validate_integration_policy_template_path.go @@ -20,7 +20,6 @@ import ( const ( defaultStreamTemplatePath = "stream.yml.hbs" - packageTypeIntegration = "integration" ) type policyTemplateInput struct { @@ -78,7 +77,7 @@ func ValidateIntegrationPolicyTemplates(fsys fspath.FS) specerrors.ValidationErr specerrors.NewStructuredErrorf("file \"%s\" is invalid: %w", fsys.Path(manifestPath), errFailedToParseManifest)} } - if manifest.Type != packageTypeIntegration { + if manifest.Type != integrationPackageType { return nil } diff --git a/code/go/internal/validator/semantic/validate_policy_template_datastream_categories.go b/code/go/internal/validator/semantic/validate_policy_template_datastream_categories.go index dec6f3643..5d9088f34 100644 --- a/code/go/internal/validator/semantic/validate_policy_template_datastream_categories.go +++ b/code/go/internal/validator/semantic/validate_policy_template_datastream_categories.go @@ -70,7 +70,7 @@ func ValidatePolicyTemplateDatastreamCategories(fsys fspath.FS) specerrors.Valid } // only validate integration type packages - if pkgType != packageTypeIntegration { + if pkgType != integrationPackageType { return nil } diff --git a/code/go/internal/validator/semantic/validate_stream_input_materialized.go b/code/go/internal/validator/semantic/validate_stream_input_materialized.go index b2eb6d829..1752cc161 100644 --- a/code/go/internal/validator/semantic/validate_stream_input_materialized.go +++ b/code/go/internal/validator/semantic/validate_stream_input_materialized.go @@ -39,8 +39,8 @@ type policyTemplateMaterialization struct { } type packageMaterializationManifest struct { - Type string `yaml:"type"` - PolicyTemplates []policyTemplateMaterialization `yaml:"policy_templates"` + Type string `yaml:"type"` + PolicyTemplates []policyTemplateMaterialization `yaml:"policy_templates"` } // ValidateStreamInputMaterialized errors when build-mode manifests carry @@ -139,8 +139,7 @@ func validatePolicyTemplateInputsMaterialized(fsys fspath.FS) specerrors.Validat } } - // Only integration packages have policy_templates with composable inputs. - if manifest.Type != packageTypeIntegration { + if manifest.Type != integrationPackageType { return nil } diff --git a/code/go/pkg/validator/api.go b/code/go/pkg/validator/api.go index b564eb9d9..ed2d73116 100644 --- a/code/go/pkg/validator/api.go +++ b/code/go/pkg/validator/api.go @@ -54,12 +54,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) @@ -84,18 +93,24 @@ 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) { 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/api_test.go b/code/go/pkg/validator/api_test.go index 1c455459f..c2c221156 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -468,24 +468,17 @@ func TestBuildMode_StreamInputMaterialized(t *testing.T) { } // ----------------------------------------------------------------------- -// TestLegacyPreservation_FromZip (golden test) +// TestNewFromZip_BuildMode // -// 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. +// NewFromZip always validates in ModeBuild — zip files are built packages. +// This test zips the canonical good_built fixture and confirms no errors. // ----------------------------------------------------------------------- -func TestLegacyPreservation_FromZip(t *testing.T) { - packagePath := filepath.Join(testPackagesDir, "good") - zipPath := createPackageZip(t, packagePath) +func TestNewFromZip_BuildMode(t *testing.T) { + builtPath := filepath.Join(testPackagesDir, "build_mode", "good_built") + zipPath := createPackageZip(t, builtPath) - 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) + v, err := NewFromZip(zipPath) require.NoError(t, err) - newErr := newV.Validate() - - assertErrorsEqual(t, legacyErr, newErr) + assert.NoError(t, v.Validate(), "good_built should pass ModeBuild validation via zip") } 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 2a2f388fab2afb1f2a38ce0cd543c5b3a1366212 Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 28 May 2026 10:26:48 +0200 Subject: [PATCH 14/23] Update changelog link to PR #1175 Co-Authored-By: Claude Sonnet 4.6 --- spec/changelog.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/changelog.yml b/spec/changelog.yml index bee3bf753..10f47aa66 100644 --- a/spec/changelog.yml +++ b/spec/changelog.yml @@ -10,7 +10,7 @@ link: https://github.com/elastic/package-spec/pull/807 - description: Add `source` and `build` validation modes via a new constructor API; existing functions are deprecated in favour of `NewFromPath/Zip/FS(mode, ...)`. type: enhancement - link: https://github.com/elastic/package-spec/issues/549 + link: https://github.com/elastic/package-spec/pull/1175 - version: 3.6.3 changes: - description: Add optional `release` field to agentless deployment mode to explicitly declare its release stage. From 14cc75ce4c2d2f38a4d60fec686d84a2ccb24c09 Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 28 May 2026 10:30:15 +0200 Subject: [PATCH 15/23] Fix lint: add godoc comments to exported modes package symbols Also apply gofmt/goimports formatting to spec.go import order and test struct field alignment caught by make check. Co-Authored-By: Claude Sonnet 4.6 --- code/go/internal/validator/modes/modes.go | 3 +++ .../semantic/validate_stream_input_materialized_test.go | 2 +- code/go/internal/validator/spec.go | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) 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/semantic/validate_stream_input_materialized_test.go b/code/go/internal/validator/semantic/validate_stream_input_materialized_test.go index 1b4e4e03a..71d07b5db 100644 --- a/code/go/internal/validator/semantic/validate_stream_input_materialized_test.go +++ b/code/go/internal/validator/semantic/validate_stream_input_materialized_test.go @@ -18,7 +18,7 @@ import ( func TestValidateStreamInputMaterialized(t *testing.T) { tests := []struct { - name string + name string // files maps relative path → YAML content written under a temp dir. files map[string]string expectErrors bool diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index 4741413ef..ffa3c9843 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 9c3e7024ed973008d5e0290bf3056d15e917f507 Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 28 May 2026 11:11:12 +0200 Subject: [PATCH 16/23] 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 ed2d73116..24c18c47f 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)) @@ -105,6 +113,14 @@ func newFromZip(mode Mode, zipPath string, opts ...Option) (_ *Validator, err er // 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 { + 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 { if mode == ModeSource { fsys = linkedfiles.NewFS(location, fsys) From 460165a95c861db56b5361ba6138fcda490271cb Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 28 May 2026 14:50:42 +0200 Subject: [PATCH 17/23] Skip template validation for composable streams with no local templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Composable integration streams reference an input package via 'package:'. When no explicit template_path/template_paths is set, all templates come from the referenced input package and only exist after build. Template validation for those streams is skipped — ValidateStreamInputMaterialized already enforces that 'package:' is replaced by 'input:' in build mode. If a composable stream does define its own template_path or template_paths (overlay templates that live in the source package), those files are still validated as before. Co-Authored-By: Claude Sonnet 4.6 --- ...lidate_integration_policy_template_path.go | 12 ++++ ...e_integration_policy_template_path_test.go | 59 +++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/code/go/internal/validator/semantic/validate_integration_policy_template_path.go b/code/go/internal/validator/semantic/validate_integration_policy_template_path.go index 971e48c3c..41586b38a 100644 --- a/code/go/internal/validator/semantic/validate_integration_policy_template_path.go +++ b/code/go/internal/validator/semantic/validate_integration_policy_template_path.go @@ -40,6 +40,7 @@ type integrationPackageManifest struct { // package manifest type stream struct { Input string `yaml:"input"` + Package string `yaml:"package"` TemplatePath string `yaml:"template_path"` TemplatePaths []string `yaml:"template_paths"` } @@ -140,6 +141,17 @@ func validateAllDataStreamStreamTemplates(fsys fspath.FS, dsMap map[string]dataS dsManifestPath := path.Join(dsDir, "manifest.yml") manifest := dsMap[dsDir] for _, s := range manifest.Streams { + // Composable streams reference an input package via 'package:'. When + // no explicit template_path/template_paths is set on the stream, all + // templates come from the dependency and are only present after build. + // Skip those — ValidateStreamInputMaterialized enforces that 'package:' + // is replaced by 'input:' in build mode. + // However, if the composable stream defines its own template_path or + // template_paths, those files must exist in the source package and are + // validated here. + if s.Package != "" && s.TemplatePath == "" && len(s.TemplatePaths) == 0 { + continue + } if err := validateSingleDataStreamStreamTemplates(fsys, dsDir, s); err != nil { errs = append(errs, specerrors.NewStructuredErrorf( "file \"%s\" is invalid: data stream \"%s\" stream input %q: %w", diff --git a/code/go/internal/validator/semantic/validate_integration_policy_template_path_test.go b/code/go/internal/validator/semantic/validate_integration_policy_template_path_test.go index 3c5fd7080..1c1dce459 100644 --- a/code/go/internal/validator/semantic/validate_integration_policy_template_path_test.go +++ b/code/go/internal/validator/semantic/validate_integration_policy_template_path_test.go @@ -254,6 +254,65 @@ streams: errs := ValidateIntegrationPolicyTemplates(fspath.DirFS(d)) require.Empty(t, errs) }) + + t.Run("composable stream with no explicit templates skips validation", func(t *testing.T) { + d := t.TempDir() + writeMinimalIntegrationManifest(t, d) + // No agent/stream directory — templates come entirely from the input package. + err := os.MkdirAll(filepath.Join(d, "data_stream", "logs"), 0o755) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(d, "data_stream", "logs", "manifest.yml"), []byte(` +streams: + - package: some_input_package + title: Composable + description: d +`), 0o644) + require.NoError(t, err) + + errs := ValidateIntegrationPolicyTemplates(fspath.DirFS(d)) + require.Empty(t, errs) + }) + + t.Run("composable stream with explicit template_paths validates those files", func(t *testing.T) { + d := t.TempDir() + writeMinimalIntegrationManifest(t, d) + err := os.MkdirAll(filepath.Join(d, "data_stream", "logs", "agent", "stream"), 0o755) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(d, "data_stream", "logs", "agent", "stream", "overlay.yml.hbs"), []byte(`x`), 0o644) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(d, "data_stream", "logs", "manifest.yml"), []byte(` +streams: + - package: some_input_package + title: Composable with overlay + description: d + template_paths: + - overlay.yml.hbs +`), 0o644) + require.NoError(t, err) + + errs := ValidateIntegrationPolicyTemplates(fspath.DirFS(d)) + require.Empty(t, errs) + }) + + t.Run("composable stream with explicit template_paths fails when file missing", func(t *testing.T) { + d := t.TempDir() + writeMinimalIntegrationManifest(t, d) + err := os.MkdirAll(filepath.Join(d, "data_stream", "logs", "agent", "stream"), 0o755) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(d, "data_stream", "logs", "manifest.yml"), []byte(` +streams: + - package: some_input_package + title: Composable with overlay + description: d + template_paths: + - missing.yml.hbs +`), 0o644) + require.NoError(t, err) + + errs := ValidateIntegrationPolicyTemplates(fspath.DirFS(d)) + require.Len(t, errs, 1) + require.Contains(t, errs[0].Error(), "template file not found") + }) } func TestValidateIntegrationPolicyTemplates_NonIntegrationType(t *testing.T) { d := t.TempDir() From 7242f168cc68e33553e845fe90e0cee574ff0960 Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 28 May 2026 14:56:21 +0200 Subject: [PATCH 18/23] Skip template validation for composable policy template inputs with no local templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the stream-level fix: policy template inputs referencing an input package via 'package:' do not carry agent/input templates in source — those come from the dependency and are only present after build. Add Package field to policyTemplateInput so the composable case is explicitly recognised. When 'package:' is set and no template_path/ template_paths is declared, validation is skipped. When the composable input defines its own overlay templates, those files are still validated. Co-Authored-By: Claude Sonnet 4.6 --- ...lidate_integration_policy_template_path.go | 9 +++ ...e_integration_policy_template_path_test.go | 64 +++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/code/go/internal/validator/semantic/validate_integration_policy_template_path.go b/code/go/internal/validator/semantic/validate_integration_policy_template_path.go index 41586b38a..79f28043d 100644 --- a/code/go/internal/validator/semantic/validate_integration_policy_template_path.go +++ b/code/go/internal/validator/semantic/validate_integration_policy_template_path.go @@ -24,6 +24,7 @@ const ( type policyTemplateInput struct { Type string `yaml:"type"` + Package string `yaml:"package"` TemplatePath string `yaml:"template_path"` TemplatePaths []string `yaml:"template_paths"` } @@ -110,6 +111,14 @@ func ValidateIntegrationPolicyTemplates(fsys fspath.FS) specerrors.ValidationErr // under agent/input when template_paths or template_path is set (Fleet: template_paths first). func validateIntegrationPolicyTemplateInputs(fsys fspath.FS, policyTemplate integrationPolicyTemplate) error { for _, input := range policyTemplate.Inputs { + // Composable inputs reference an input package via 'package:'. When no + // explicit template_path/template_paths is set, all templates come from + // the dependency and are only present after build. Skip those. + // If the composable input defines its own template_path or template_paths + // (overlay templates that live in the source package), those are validated. + if input.Package != "" && input.TemplatePath == "" && len(input.TemplatePaths) == 0 { + continue + } if len(input.TemplatePaths) > 0 { for _, tp := range input.TemplatePaths { if err := validateAgentInputTemplatePath(fsys, tp); err != nil { diff --git a/code/go/internal/validator/semantic/validate_integration_policy_template_path_test.go b/code/go/internal/validator/semantic/validate_integration_policy_template_path_test.go index 1c1dce459..618c3aeae 100644 --- a/code/go/internal/validator/semantic/validate_integration_policy_template_path_test.go +++ b/code/go/internal/validator/semantic/validate_integration_policy_template_path_test.go @@ -382,6 +382,70 @@ streams: errs := ValidateIntegrationPolicyTemplates(fspath.DirFS(d)) require.Empty(t, errs) } +func TestValidateIntegrationPolicyTemplates_ComposableInputs(t *testing.T) { + t.Run("composable input with no templates skips validation", func(t *testing.T) { + d := t.TempDir() + err := os.WriteFile(filepath.Join(d, "manifest.yml"), []byte(` +type: integration +policy_templates: + - name: pt + inputs: + - package: some_input_package + title: Composable + description: d +`), 0o644) + require.NoError(t, err) + + errs := ValidateIntegrationPolicyTemplates(fspath.DirFS(d)) + require.Empty(t, errs) + }) + + t.Run("composable input with explicit template_paths validates those files", func(t *testing.T) { + d := t.TempDir() + err := os.MkdirAll(filepath.Join(d, "agent", "input"), 0o755) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(d, "agent", "input", "overlay.yml.hbs"), []byte(`x`), 0o644) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(d, "manifest.yml"), []byte(` +type: integration +policy_templates: + - name: pt + inputs: + - package: some_input_package + title: Composable with overlay + description: d + template_paths: + - overlay.yml.hbs +`), 0o644) + require.NoError(t, err) + + errs := ValidateIntegrationPolicyTemplates(fspath.DirFS(d)) + require.Empty(t, errs) + }) + + t.Run("composable input with explicit template_paths fails when file missing", func(t *testing.T) { + d := t.TempDir() + err := os.MkdirAll(filepath.Join(d, "agent", "input"), 0o755) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(d, "manifest.yml"), []byte(` +type: integration +policy_templates: + - name: pt + inputs: + - package: some_input_package + title: Composable with overlay + description: d + template_paths: + - missing.yml.hbs +`), 0o644) + require.NoError(t, err) + + errs := ValidateIntegrationPolicyTemplates(fspath.DirFS(d)) + require.Len(t, errs, 1) + require.Contains(t, errs[0].Error(), "template file not found") + }) +} + func TestFindPathAtDirectory(t *testing.T) { d := t.TempDir() From 411b8f8e8f086384fea3cfd4efa405d52730db93 Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 28 May 2026 15:15:50 +0200 Subject: [PATCH 19/23] Allow partial var definitions on composable streams and policy template inputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In composable packages, streams and policy template inputs reference an input package via 'package:'. Vars listed there are partial overrides of the dependency's vars: name and type come from the dependency and are only present in the built package. Requiring name and type in source mode causes false validation errors. Fix: change the vars property on stream items (data_stream/manifest.spec.yml) and on policy template input items (integration/manifest.spec.yml) from a direct $ref to the full vars definition to a permissive base schema ({type: array, items: {type: object}}). Full validation (name + type required, all allOf conditions) is applied via an if/then only when the stream or input is non-composable (required: [input] / required: [type]). Also fix four allOf if-conditions in #/definitions/vars to guard on required: [type] before applying type-specific constraints (select, password, duration, not-duration). Without this guard, absent type causes all conditions to fire vacuously in JSON Schema, generating spurious errors. The #/definitions/vars definition itself is unchanged: it still requires [name, type] and is used as-is for input packages, package-level vars, and policy template-level vars — all non-composable contexts. Spec version compat: before: 3.6.0 patches remove 'package:' and add input/type to required, so the if: required: [input/type] condition is always true for older specs and full validation continues to apply. Co-Authored-By: Claude Sonnet 4.6 --- .../integration/data_stream/manifest.spec.yml | 33 +++++++++++++++---- spec/integration/manifest.spec.yml | 15 ++++++++- .../data_stream/logs/manifest.yml | 4 +++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/spec/integration/data_stream/manifest.spec.yml b/spec/integration/data_stream/manifest.spec.yml index 9da578a3f..b79ac83f7 100644 --- a/spec/integration/data_stream/manifest.spec.yml +++ b/spec/integration/data_stream/manifest.spec.yml @@ -175,6 +175,7 @@ spec: $ref: "../../integration/manifest.spec.yml#/definitions/deprecated" allOf: - if: + required: [type] properties: type: const: select @@ -217,13 +218,16 @@ spec: properties: name: pattern: "(_file|_url)$" - - properties: - type: - const: password + - allOf: + - required: [type] + - properties: + type: + const: password then: required: - secret - if: + required: [type] properties: type: const: duration @@ -234,9 +238,11 @@ spec: pattern: '^(\d+[smh]|\d+ms)+$' - if: not: - properties: - type: - const: duration + allOf: + - required: [type] + - properties: + type: + const: duration then: not: anyOf: @@ -641,7 +647,14 @@ spec: required_vars: $ref: "#/definitions/required_vars" vars: - $ref: "#/definitions/vars" + # Base schema is permissive: composable streams ('package:') carry + # partial var overrides where name and type come from the dependency + # and are only present in the built package. + # Full validation ($ref: "#/definitions/vars") is applied below via + # if/then for non-composable streams ('input:'). + type: array + items: + type: object var_groups: $ref: "../../integration/manifest.spec.yml#/definitions/var_groups" sections: @@ -659,6 +672,12 @@ spec: oneOf: - required: [input] - required: [package] + if: + required: [input] + then: + properties: + vars: + $ref: "#/definitions/vars" agent: $ref: "../../integration/manifest.spec.yml#/definitions/agent" elasticsearch: diff --git a/spec/integration/manifest.spec.yml b/spec/integration/manifest.spec.yml index 9578e3655..349f68d19 100644 --- a/spec/integration/manifest.spec.yml +++ b/spec/integration/manifest.spec.yml @@ -881,7 +881,14 @@ spec: required_vars: $ref: "./data_stream/manifest.spec.yml#/definitions/required_vars" vars: - $ref: "./data_stream/manifest.spec.yml#/definitions/vars" + # Base schema is permissive: composable inputs ('package:') carry + # partial var overrides; name and type come from the dependency + # and are only materialised in the built package. + # Full validation is applied below via if/then for non-composable + # inputs ('type:'). + type: array + items: + type: object var_groups: $ref: "#/definitions/var_groups" sections: @@ -909,6 +916,12 @@ spec: oneOf: - required: [type] - required: [package] + if: + required: [type] + then: + properties: + vars: + $ref: "./data_stream/manifest.spec.yml#/definitions/vars" multiple: type: boolean icons: diff --git a/test/packages/good_requires/data_stream/logs/manifest.yml b/test/packages/good_requires/data_stream/logs/manifest.yml index 0ab41203c..6c5e2f0c3 100644 --- a/test/packages/good_requires/data_stream/logs/manifest.yml +++ b/test/packages/good_requires/data_stream/logs/manifest.yml @@ -4,3 +4,7 @@ streams: - package: sql_input title: Apache logs via SQL input description: Collect Apache logs using the SQL input package + vars: + - name: hosts + default: localhost:5432 + - default: 30s From 498dbf21b3fa30dbb650888185b428cc0ee62052 Mon Sep 17 00:00:00 2001 From: Tere Date: Thu, 28 May 2026 15:20:50 +0200 Subject: [PATCH 20/23] Restrict SVR00010 (input qualifier) to build mode only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In composable source packages, policy template inputs use 'package:' instead of 'type:'. This leaves input.Type as "" for all composable inputs. When a policy template has multiple such inputs, SVR00010 fires spuriously because they all share the same empty type — even though the name is assigned by elastic-package at build time, not in source. Move ValidateIntegrationInputQualifier to modes: [Build]. In build mode, ValidateStreamInputMaterialized has already ensured every 'package:' was replaced by a real 'type:', so the name-uniqueness check is meaningful. Update tests: remove bad_input_qualifier_ambiguous from TestValidateFile (legacy mode) and add TestBuildMode_InputQualifierAmbiguous to verify the rule fires in build mode and is silent in source mode. Co-Authored-By: Claude Sonnet 4.6 --- code/go/internal/validator/spec.go | 3 ++- code/go/pkg/validator/api_test.go | 33 +++++++++++++++++++++++++ code/go/pkg/validator/validator_test.go | 9 +++---- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index ffa3c9843..a0ea88589 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -252,7 +252,8 @@ func (s Spec) rules(pkgType string, rootSpec spectypes.ItemSpec) validationRules {fn: semantic.ValidateKibanaTagDuplicates}, {fn: semantic.ValidatePipelineOnFailure, types: []string{"integration"}, since: semver.MustParse("3.6.0")}, {fn: semantic.ValidateIntegrationInputsDeprecation, types: []string{"integration"}, since: semver.MustParse("3.6.0")}, - {fn: semantic.ValidateIntegrationInputQualifier, types: []string{"integration"}, since: semver.MustParse("3.6.0")}, + {fn: semantic.ValidateIntegrationInputQualifier, types: []string{"integration"}, since: semver.MustParse("3.6.0"), + modes: []modes.Mode{modes.Build}}, {fn: semantic.ValidateDeprecatedReplacedBy, since: semver.MustParse("3.6.0")}, {fn: semantic.ValidatePackageReferences, types: []string{"integration"}, since: semver.MustParse("3.6.0")}, {fn: semantic.ValidateTestPackageRequirements, types: []string{"integration"}, since: semver.MustParse("3.6.0"), diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go index c2c221156..f9f7fafc2 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -467,6 +467,39 @@ func TestBuildMode_StreamInputMaterialized(t *testing.T) { }) } +// ----------------------------------------------------------------------- +// TestBuildMode_InputQualifierAmbiguous +// +// SVR00010 (ValidateIntegrationInputQualifier) is build-mode only. +// In source mode, composable inputs use 'package:' so their type is "", +// and the input name is assigned by elastic-package at build time. +// Verifies that: +// - ModeBuild raises SVR00010 for a package with duplicate-type inputs that lack names. +// - ModeSource does NOT raise SVR00010 for the same package. +// ----------------------------------------------------------------------- + +func TestBuildMode_InputQualifierAmbiguous(t *testing.T) { + ambiguousPath := filepath.Join(testPackagesDir, "bad_input_qualifier_ambiguous") + + t.Run("bad_input_qualifier_ambiguous fails ModeBuild (SVR00010)", func(t *testing.T) { + v, err := NewFromPath(ModeBuild, ambiguousPath) + require.NoError(t, err) + buildErr := v.Validate() + require.Error(t, buildErr, "bad_input_qualifier_ambiguous should fail build-mode validation") + assert.Contains(t, buildErr.Error(), "must have a name when multiple inputs of the same type are present") + }) + + t.Run("bad_input_qualifier_ambiguous does not raise SVR00010 in ModeSource", func(t *testing.T) { + v, err := NewFromPath(ModeSource, ambiguousPath) + require.NoError(t, err) + sourceErr := v.Validate() + if sourceErr != nil { + assert.NotContains(t, sourceErr.Error(), "must have a name when multiple inputs of the same type are present", + "ValidateIntegrationInputQualifier must not run in source mode") + } + }) +} + // ----------------------------------------------------------------------- // TestNewFromZip_BuildMode // diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index 704ce1acd..dfd8cf52f 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -458,12 +458,9 @@ func TestValidateFile(t *testing.T) { "field policy_templates.0.inputs.0.type: Must not be present", }, }, - "bad_input_qualifier_ambiguous": { - "manifest.yml", - []string{ - `policy template "nginx": input with type "otelcol" must have a name when multiple inputs of the same type are present (SVR00010)`, - }, - }, + // bad_input_qualifier_ambiguous is tested in TestBuildMode_InputQualifierAmbiguous: + // SVR00010 is a build-mode-only rule — composable source inputs have type "" + // (package: instead of type:) and names are assigned by elastic-package at build time. "bad_input_qualifier_old_version": { "manifest.yml", []string{ From 440ecd0e51724dbabaa02819b9bc9d02083e758d Mon Sep 17 00:00:00 2001 From: Tere Date: Fri, 29 May 2026 11:09:12 +0200 Subject: [PATCH 21/23] Enhance validation error handling and mode checks - Added validation for the mode in NewSpec to ensure it is valid, returning an error for invalid modes. - Improved error handling in validatePolicyTemplateInputsMaterialized to return structured errors when reading the manifest fails, while still allowing for the case where the manifest does not exist. - Updated the error handling in the Validator's Validate method to join multiple errors when closing the closer fails. Co-Authored-By: Claude Sonnet 4.6 --- .../semantic/validate_stream_input_materialized.go | 10 +++++++--- code/go/internal/validator/spec.go | 3 +++ code/go/pkg/validator/api.go | 12 +++++++++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/code/go/internal/validator/semantic/validate_stream_input_materialized.go b/code/go/internal/validator/semantic/validate_stream_input_materialized.go index 1752cc161..b523b8506 100644 --- a/code/go/internal/validator/semantic/validate_stream_input_materialized.go +++ b/code/go/internal/validator/semantic/validate_stream_input_materialized.go @@ -5,6 +5,7 @@ package semantic import ( + "errors" "io/fs" "path" "slices" @@ -124,9 +125,12 @@ func validatePolicyTemplateInputsMaterialized(fsys fspath.FS) specerrors.Validat manifestRelPath := "manifest.yml" data, err := fs.ReadFile(fsys, manifestRelPath) if err != nil { - // If there is no manifest we have nothing to check; other validators handle - // the missing-manifest case. - return nil + if errors.Is(err, fs.ErrNotExist) { + return nil + } + return specerrors.ValidationErrors{ + specerrors.NewStructuredErrorf("reading %q: %w", manifestRelPath, err), + } } var manifest packageMaterializationManifest diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index a0ea88589..faa442cb5 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 703e0885e0b7a3b177400a4ee3996260ee2e909b Mon Sep 17 00:00:00 2001 From: Tere Date: Fri, 29 May 2026 11:26:09 +0200 Subject: [PATCH 22/23] Address PR review comments - Rename abbreviated loop variables (dsName -> dataStreamName, pt -> policyTemplate, tc -> testCase) - Use errors.As instead of direct type assertion for wrapped ValidationErrors - Check Close() errors for zip writer and output file in tests - Fix bad_built_* fixture READMEs that incorrectly described themselves as valid packages Co-Authored-By: Claude Sonnet 4.6 --- .../semantic/validate_stream_input_materialized.go | 10 +++++----- .../validate_stream_input_materialized_test.go | 10 +++++----- code/go/pkg/validator/api_test.go | 7 ++++--- .../build_mode/bad_built_external_ecs/docs/README.md | 8 ++++---- .../bad_built_policy_template_package/docs/README.md | 8 ++++---- .../build_mode/bad_built_stream_package/docs/README.md | 8 ++++---- .../build_mode/bad_built_with_dev/docs/README.md | 8 ++++---- .../build_mode/bad_built_with_link/docs/README.md | 8 ++++---- 8 files changed, 34 insertions(+), 33 deletions(-) diff --git a/code/go/internal/validator/semantic/validate_stream_input_materialized.go b/code/go/internal/validator/semantic/validate_stream_input_materialized.go index b523b8506..1f08eede7 100644 --- a/code/go/internal/validator/semantic/validate_stream_input_materialized.go +++ b/code/go/internal/validator/semantic/validate_stream_input_materialized.go @@ -74,8 +74,8 @@ func validateDataStreamStreamsMaterialized(fsys fspath.FS) specerrors.Validation slices.Sort(dataStreams) var errs specerrors.ValidationErrors - for _, dsName := range dataStreams { - manifestRelPath := path.Join(dataStreamDir, dsName, "manifest.yml") + for _, dataStreamName := range dataStreams { + manifestRelPath := path.Join(dataStreamDir, dataStreamName, "manifest.yml") data, err := fs.ReadFile(fsys, manifestRelPath) if err != nil { errs = append(errs, specerrors.NewStructuredErrorf( @@ -149,12 +149,12 @@ func validatePolicyTemplateInputsMaterialized(fsys fspath.FS) specerrors.Validat fullPath := fsys.Path(manifestRelPath) var errs specerrors.ValidationErrors - for _, pt := range manifest.PolicyTemplates { - for i, input := range pt.Inputs { + for _, policyTemplate := range manifest.PolicyTemplates { + for i, input := range policyTemplate.Inputs { if input.Package != "" { errs = append(errs, specerrors.NewStructuredErrorf( "file %q: policy_template %q input[%d] has 'package:' which is source-only; build packages must use 'type:'", - fullPath, pt.Name, i, + fullPath, policyTemplate.Name, i, )) } } diff --git a/code/go/internal/validator/semantic/validate_stream_input_materialized_test.go b/code/go/internal/validator/semantic/validate_stream_input_materialized_test.go index 71d07b5db..70b1d3b85 100644 --- a/code/go/internal/validator/semantic/validate_stream_input_materialized_test.go +++ b/code/go/internal/validator/semantic/validate_stream_input_materialized_test.go @@ -124,11 +124,11 @@ func TestValidateStreamInputMaterialized(t *testing.T) { }, } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { tempDir := t.TempDir() - for relPath, content := range tc.files { + for relPath, content := range testCase.files { fullPath := filepath.Join(tempDir, relPath) require.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) require.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644)) @@ -137,7 +137,7 @@ func TestValidateStreamInputMaterialized(t *testing.T) { fsys := fspath.DirFS(tempDir) errs := ValidateStreamInputMaterialized(fsys) - if !tc.expectErrors { + if !testCase.expectErrors { assert.Nil(t, errs, "expected no errors but got: %v", errs) return } @@ -149,7 +149,7 @@ func TestValidateStreamInputMaterialized(t *testing.T) { sb.WriteString("\n") } combined := sb.String() - for _, substr := range tc.errorContains { + for _, substr := range testCase.errorContains { assert.Contains(t, combined, substr) } }) diff --git a/code/go/pkg/validator/api_test.go b/code/go/pkg/validator/api_test.go index f9f7fafc2..0259510e7 100644 --- a/code/go/pkg/validator/api_test.go +++ b/code/go/pkg/validator/api_test.go @@ -51,7 +51,8 @@ func assertErrorsEqual(t *testing.T, legacy, newAPI error) { // 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 { + var verrs specerrors.ValidationErrors + if errors.As(err, &verrs) { msgs := make([]string, len(verrs)) for i, ve := range verrs { msgs[i] = ve.Error() @@ -113,10 +114,10 @@ func createPackageZip(t *testing.T, packagePath string) string { f, err := os.Create(zipPath) require.NoError(t, err) - defer f.Close() + defer func() { require.NoError(t, f.Close()) }() zw := zip.NewWriter(f) - defer zw.Close() + defer func() { require.NoError(t, zw.Close()) }() err = filepath.WalkDir(packagePath, func(path string, d fs.DirEntry, walkErr error) error { if walkErr != nil { diff --git a/test/packages/build_mode/bad_built_external_ecs/docs/README.md b/test/packages/build_mode/bad_built_external_ecs/docs/README.md index c0b01c5ec..3c649908a 100644 --- a/test/packages/build_mode/bad_built_external_ecs/docs/README.md +++ b/test/packages/build_mode/bad_built_external_ecs/docs/README.md @@ -1,5 +1,5 @@ -# Good Built Package +# Bad Built Package - External ECS -This is the canonical minimal built-package fixture used for build-mode validation -tests (issue #549). It has no `_dev/` directories, no `.link` files, and no -`external: ecs` field references — making it valid under `ModeBuild`. +This test fixture contains `external: ecs` field references that must be rejected +in build mode (issue `#549`). Built packages must materialize all ECS fields rather +than reference them via `external: ecs`. diff --git a/test/packages/build_mode/bad_built_policy_template_package/docs/README.md b/test/packages/build_mode/bad_built_policy_template_package/docs/README.md index c0b01c5ec..bedaff3a1 100644 --- a/test/packages/build_mode/bad_built_policy_template_package/docs/README.md +++ b/test/packages/build_mode/bad_built_policy_template_package/docs/README.md @@ -1,5 +1,5 @@ -# Good Built Package +# Bad Built Package - Policy Template Uses `package:` -This is the canonical minimal built-package fixture used for build-mode validation -tests (issue #549). It has no `_dev/` directories, no `.link` files, and no -`external: ecs` field references — making it valid under `ModeBuild`. +This fixture is intentionally invalid for build-mode validation tests (issue `#549`). +It models a built package where policy template inputs use `package:` instead of +the required materialized `type:` field. diff --git a/test/packages/build_mode/bad_built_stream_package/docs/README.md b/test/packages/build_mode/bad_built_stream_package/docs/README.md index c0b01c5ec..957b72908 100644 --- a/test/packages/build_mode/bad_built_stream_package/docs/README.md +++ b/test/packages/build_mode/bad_built_stream_package/docs/README.md @@ -1,5 +1,5 @@ -# Good Built Package +# Bad Built Package - Stream Package Reference -This is the canonical minimal built-package fixture used for build-mode validation -tests (issue #549). It has no `_dev/` directories, no `.link` files, and no -`external: ecs` field references — making it valid under `ModeBuild`. +This fixture is intentionally invalid for build-mode validation tests (issue `#549`). +It uses source-only `package:` references in stream definitions, which must be +rejected under `ModeBuild`. diff --git a/test/packages/build_mode/bad_built_with_dev/docs/README.md b/test/packages/build_mode/bad_built_with_dev/docs/README.md index c0b01c5ec..293603b4c 100644 --- a/test/packages/build_mode/bad_built_with_dev/docs/README.md +++ b/test/packages/build_mode/bad_built_with_dev/docs/README.md @@ -1,5 +1,5 @@ -# Good Built Package +# Bad Built Package - With Dev Directory -This is the canonical minimal built-package fixture used for build-mode validation -tests (issue #549). It has no `_dev/` directories, no `.link` files, and no -`external: ecs` field references — making it valid under `ModeBuild`. +This fixture contains a `_dev/` directory, which is a source-only artifact that +must be rejected in build mode (issue `#549`). Used to verify that `ModeBuild` +correctly flags packages containing `_dev/` directories. diff --git a/test/packages/build_mode/bad_built_with_link/docs/README.md b/test/packages/build_mode/bad_built_with_link/docs/README.md index c0b01c5ec..b57a2fe61 100644 --- a/test/packages/build_mode/bad_built_with_link/docs/README.md +++ b/test/packages/build_mode/bad_built_with_link/docs/README.md @@ -1,5 +1,5 @@ -# Good Built Package +# Bad Built Package - Link File Present -This is the canonical minimal built-package fixture used for build-mode validation -tests (issue #549). It has no `_dev/` directories, no `.link` files, and no -`external: ecs` field references — making it valid under `ModeBuild`. +This built-package fixture contains `.link` files and is invalid in build mode +(issue `#549`). Build mode rejects packages with `.link` files, as they are +source-only artifacts that must be resolved during the build process. From a960b91dde5525f3e46dd50d6746795afdd1771f Mon Sep 17 00:00:00 2001 From: Tere Date: Fri, 29 May 2026 11:38:31 +0200 Subject: [PATCH 23/23] Use package-relative path in manifest read error Replace bare manifestRelPath with fsys.Path(manifestRelPath) in the read-error branch so structured errors report the full package-relative path, consistent with the parse-error branch below it. Co-Authored-By: Claude Sonnet 4.6 --- .../validator/semantic/validate_stream_input_materialized.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/go/internal/validator/semantic/validate_stream_input_materialized.go b/code/go/internal/validator/semantic/validate_stream_input_materialized.go index 1f08eede7..885db88a8 100644 --- a/code/go/internal/validator/semantic/validate_stream_input_materialized.go +++ b/code/go/internal/validator/semantic/validate_stream_input_materialized.go @@ -129,7 +129,7 @@ func validatePolicyTemplateInputsMaterialized(fsys fspath.FS) specerrors.Validat return nil } return specerrors.ValidationErrors{ - specerrors.NewStructuredErrorf("reading %q: %w", manifestRelPath, err), + specerrors.NewStructuredErrorf("reading %q: %w", fsys.Path(manifestRelPath), err), } }