diff --git a/cmd/lint.go b/cmd/lint.go index 35a0e38344..b5cd2c1b49 100644 --- a/cmd/lint.go +++ b/cmd/lint.go @@ -95,7 +95,7 @@ func validateSourceCommandAction(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("locating package root failed: %w", err) } - errs, skipped := validation.ValidateAndFilterFromPath(packageRoot) + errs, skipped := validation.ValidateSourceFromPath(packageRoot) if skipped != nil { logger.Infof("Skipped errors: %v", skipped) } diff --git a/internal/builder/packages.go b/internal/builder/packages.go index f05b355cb1..23766e418b 100644 --- a/internal/builder/packages.go +++ b/internal/builder/packages.go @@ -272,7 +272,7 @@ func BuildPackage(options BuildOptions) (string, error) { } logger.Debugf("Validating built package (path: %s)", buildPackageRoot) - errs, skipped := validation.ValidateAndFilterFromPath(buildPackageRoot) + errs, skipped := validation.ValidateBuiltFromPath(buildPackageRoot) if skipped != nil { logger.Infof("Skipped errors: %v", skipped) } @@ -300,7 +300,7 @@ func buildZippedPackage(options BuildOptions, buildPackageRoot string) (string, logger.Debug("Skip validation of the built .zip package") } else { logger.Debugf("Validating built .zip package (path: %s)", zippedPackagePath) - errs, skipped := validation.ValidateAndFilterFromZip(zippedPackagePath) + errs, skipped := validation.ValidateBuiltFromZip(zippedPackagePath) if skipped != nil { logger.Infof("Skipped errors: %v", skipped) } diff --git a/internal/packages/installer/factory.go b/internal/packages/installer/factory.go index d7a3a7bbec..8b4357ca02 100644 --- a/internal/packages/installer/factory.go +++ b/internal/packages/installer/factory.go @@ -80,7 +80,7 @@ func NewForPackage(options Options) (Installer, error) { logger.Debug("Skip validation of the built .zip package") } else { logger.Debugf("Validating built .zip package (path: %s)", options.ZipPath) - errs, skipped := validation.ValidateAndFilterFromZip(options.ZipPath) + errs, skipped := validation.ValidateBuiltFromZip(options.ZipPath) if skipped != nil { logger.Infof("Skipped errors: %v", skipped) } diff --git a/internal/validation/testdata/built_package/LICENSE.txt b/internal/validation/testdata/built_package/LICENSE.txt new file mode 100644 index 0000000000..f6f788a89a --- /dev/null +++ b/internal/validation/testdata/built_package/LICENSE.txt @@ -0,0 +1,3 @@ +Elastic License 2.0 + +URL: https://www.elastic.co/licensing/elastic-license diff --git a/internal/validation/testdata/built_package/changelog.yml b/internal/validation/testdata/built_package/changelog.yml new file mode 100644 index 0000000000..e00f881335 --- /dev/null +++ b/internal/validation/testdata/built_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/internal/validation/testdata/built_package/data_stream/events/agent/stream/stream.yml.hbs b/internal/validation/testdata/built_package/data_stream/events/agent/stream/stream.yml.hbs new file mode 100644 index 0000000000..9390bc05cb --- /dev/null +++ b/internal/validation/testdata/built_package/data_stream/events/agent/stream/stream.yml.hbs @@ -0,0 +1,4 @@ +paths: +{{#each paths}} + - {{this}} +{{/each}} diff --git a/internal/validation/testdata/built_package/data_stream/events/fields/base-fields.yml b/internal/validation/testdata/built_package/data_stream/events/fields/base-fields.yml new file mode 100644 index 0000000000..0d1791ffed --- /dev/null +++ b/internal/validation/testdata/built_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/internal/validation/testdata/built_package/data_stream/events/fields/fields.yml b/internal/validation/testdata/built_package/data_stream/events/fields/fields.yml new file mode 100644 index 0000000000..f21b45be3e --- /dev/null +++ b/internal/validation/testdata/built_package/data_stream/events/fields/fields.yml @@ -0,0 +1,3 @@ +- name: message + type: keyword + description: Log message. diff --git a/internal/validation/testdata/built_package/data_stream/events/manifest.yml b/internal/validation/testdata/built_package/data_stream/events/manifest.yml new file mode 100644 index 0000000000..c84e225c8c --- /dev/null +++ b/internal/validation/testdata/built_package/data_stream/events/manifest.yml @@ -0,0 +1,23 @@ +title: Events +type: logs +elasticsearch: + index_template: + mappings: + dynamic_templates: + - _embedded_ecs: + match_mapping_type: string + mapping: + type: keyword +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/internal/validation/testdata/built_package/docs/README.md b/internal/validation/testdata/built_package/docs/README.md new file mode 100644 index 0000000000..9083a3a45a --- /dev/null +++ b/internal/validation/testdata/built_package/docs/README.md @@ -0,0 +1,3 @@ +# Built Package + +Minimal built-package fixture for ValidateBuiltFromPath/Zip tests. diff --git a/internal/validation/testdata/built_package/manifest.yml b/internal/validation/testdata/built_package/manifest.yml new file mode 100644 index 0000000000..b7db2af490 --- /dev/null +++ b/internal/validation/testdata/built_package/manifest.yml @@ -0,0 +1,23 @@ +format_version: 3.6.0 +name: built_package +title: Built Package +description: Minimal built-package fixture for ValidateBuiltFromPath/Zip tests. +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/internal/validation/testdata/composable_source_package/LICENSE.txt b/internal/validation/testdata/composable_source_package/LICENSE.txt new file mode 100644 index 0000000000..f6f788a89a --- /dev/null +++ b/internal/validation/testdata/composable_source_package/LICENSE.txt @@ -0,0 +1,3 @@ +Elastic License 2.0 + +URL: https://www.elastic.co/licensing/elastic-license diff --git a/internal/validation/testdata/composable_source_package/_dev/build/docs/README.md b/internal/validation/testdata/composable_source_package/_dev/build/docs/README.md new file mode 100644 index 0000000000..62b8323314 --- /dev/null +++ b/internal/validation/testdata/composable_source_package/_dev/build/docs/README.md @@ -0,0 +1,3 @@ +# {{package.title}} + +{{package.description}} diff --git a/internal/validation/testdata/composable_source_package/changelog.yml b/internal/validation/testdata/composable_source_package/changelog.yml new file mode 100644 index 0000000000..e00f881335 --- /dev/null +++ b/internal/validation/testdata/composable_source_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/internal/validation/testdata/composable_source_package/data_stream/events/agent/stream/stream.yml.hbs b/internal/validation/testdata/composable_source_package/data_stream/events/agent/stream/stream.yml.hbs new file mode 100644 index 0000000000..9390bc05cb --- /dev/null +++ b/internal/validation/testdata/composable_source_package/data_stream/events/agent/stream/stream.yml.hbs @@ -0,0 +1,4 @@ +paths: +{{#each paths}} + - {{this}} +{{/each}} diff --git a/internal/validation/testdata/composable_source_package/data_stream/events/fields/base-fields.yml b/internal/validation/testdata/composable_source_package/data_stream/events/fields/base-fields.yml new file mode 100644 index 0000000000..0d1791ffed --- /dev/null +++ b/internal/validation/testdata/composable_source_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/internal/validation/testdata/composable_source_package/data_stream/events/manifest.yml b/internal/validation/testdata/composable_source_package/data_stream/events/manifest.yml new file mode 100644 index 0000000000..b5ada69211 --- /dev/null +++ b/internal/validation/testdata/composable_source_package/data_stream/events/manifest.yml @@ -0,0 +1,6 @@ +title: Events +type: logs +streams: + - package: test_input + title: Events logs + description: Collect events log data via composable input diff --git a/internal/validation/testdata/composable_source_package/docs/README.md b/internal/validation/testdata/composable_source_package/docs/README.md new file mode 100644 index 0000000000..1aeb0b49f1 --- /dev/null +++ b/internal/validation/testdata/composable_source_package/docs/README.md @@ -0,0 +1,3 @@ +# Composable Source Package + +Minimal composable source-package fixture for ValidateSourceFromPath tests. diff --git a/internal/validation/testdata/composable_source_package/manifest.yml b/internal/validation/testdata/composable_source_package/manifest.yml new file mode 100644 index 0000000000..5d2336d5bd --- /dev/null +++ b/internal/validation/testdata/composable_source_package/manifest.yml @@ -0,0 +1,26 @@ +format_version: 3.6.0 +name: composable_source_package +title: Composable Source Package +description: "Minimal composable source-package fixture; uses package: references that are source-only." +version: 0.0.1 +type: integration +source: + license: "Apache-2.0" +conditions: + kibana: + version: '^8.0.0 || ^9.0.0' +requires: + input: + - package: test_input + version: "1.0.0" +policy_templates: + - name: events + title: Events logs + description: Collect events data + inputs: + - package: test_input + title: Collect events logs + description: Collecting events log data via composable input +owner: + github: elastic/foobar + type: community diff --git a/internal/validation/testdata/source_package/LICENSE.txt b/internal/validation/testdata/source_package/LICENSE.txt new file mode 100644 index 0000000000..f6f788a89a --- /dev/null +++ b/internal/validation/testdata/source_package/LICENSE.txt @@ -0,0 +1,3 @@ +Elastic License 2.0 + +URL: https://www.elastic.co/licensing/elastic-license diff --git a/internal/validation/testdata/source_package/_dev/build/docs/README.md b/internal/validation/testdata/source_package/_dev/build/docs/README.md new file mode 100644 index 0000000000..62b8323314 --- /dev/null +++ b/internal/validation/testdata/source_package/_dev/build/docs/README.md @@ -0,0 +1,3 @@ +# {{package.title}} + +{{package.description}} diff --git a/internal/validation/testdata/source_package/changelog.yml b/internal/validation/testdata/source_package/changelog.yml new file mode 100644 index 0000000000..e00f881335 --- /dev/null +++ b/internal/validation/testdata/source_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/internal/validation/testdata/source_package/data_stream/events/agent/stream/stream.yml.hbs b/internal/validation/testdata/source_package/data_stream/events/agent/stream/stream.yml.hbs new file mode 100644 index 0000000000..9390bc05cb --- /dev/null +++ b/internal/validation/testdata/source_package/data_stream/events/agent/stream/stream.yml.hbs @@ -0,0 +1,4 @@ +paths: +{{#each paths}} + - {{this}} +{{/each}} diff --git a/internal/validation/testdata/source_package/data_stream/events/fields/base-fields.yml b/internal/validation/testdata/source_package/data_stream/events/fields/base-fields.yml new file mode 100644 index 0000000000..0d1791ffed --- /dev/null +++ b/internal/validation/testdata/source_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/internal/validation/testdata/source_package/data_stream/events/fields/fields.yml b/internal/validation/testdata/source_package/data_stream/events/fields/fields.yml new file mode 100644 index 0000000000..f21b45be3e --- /dev/null +++ b/internal/validation/testdata/source_package/data_stream/events/fields/fields.yml @@ -0,0 +1,3 @@ +- name: message + type: keyword + description: Log message. diff --git a/internal/validation/testdata/source_package/data_stream/events/manifest.yml b/internal/validation/testdata/source_package/data_stream/events/manifest.yml new file mode 100644 index 0000000000..d51b2bd1c1 --- /dev/null +++ b/internal/validation/testdata/source_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/internal/validation/testdata/source_package/docs/README.md b/internal/validation/testdata/source_package/docs/README.md new file mode 100644 index 0000000000..abb02732f1 --- /dev/null +++ b/internal/validation/testdata/source_package/docs/README.md @@ -0,0 +1,3 @@ +# Source Package + +Minimal source-package fixture for ValidateSourceFromPath tests. diff --git a/internal/validation/testdata/source_package/manifest.yml b/internal/validation/testdata/source_package/manifest.yml new file mode 100644 index 0000000000..e7ebddf326 --- /dev/null +++ b/internal/validation/testdata/source_package/manifest.yml @@ -0,0 +1,23 @@ +format_version: 3.6.0 +name: source_package +title: Source Package +description: Minimal source-package fixture for ValidateSourceFromPath tests. +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/internal/validation/validation.go b/internal/validation/validation.go index 267e490c13..50ccae1048 100644 --- a/internal/validation/validation.go +++ b/internal/validation/validation.go @@ -13,59 +13,94 @@ import ( "github.com/elastic/package-spec/v3/code/go/pkg/specerrors" "github.com/elastic/package-spec/v3/code/go/pkg/validator" + + "github.com/elastic/elastic-package/internal/logger" ) -func ValidateFromPath(rootPath string) error { - return validator.ValidateFromPath(rootPath) +// ValidateSourceFromPath validates a package source tree — checked out from version +// control, not yet built. Source-only artifacts (_dev/, .link files, external: ecs +// references) are permitted. Validation errors are filtered against the package's +// validation.yml config; the first return value is the remaining errors after filtering, +// the second is the errors that were filtered out. +func ValidateSourceFromPath(packageRoot string) (error, error) { + return validateFromPath(validator.SourceMode, packageRoot) } -func ValidateFromZip(packagePath string) error { - return validator.ValidateFromZip(packagePath) +// ValidateBuiltFromPath validates a built (unzipped) package directory. Source-only +// artifacts (_dev/, .link files, external: ecs references) are rejected. Validation +// errors are filtered against the package's validation.yml config; the first return +// value is the remaining errors after filtering, the second is the errors that were +// filtered out. +func ValidateBuiltFromPath(packageRoot string) (error, error) { + return validateFromPath(validator.BuildMode, packageRoot) } -func ValidateAndFilterFromPath(packageRoot string) (error, error) { - allErrors := validator.ValidateFromPath(packageRoot) +func validateFromPath(mode validator.Mode, packageRoot string) (error, error) { + v, err := validator.New(mode) + if err != nil { + return fmt.Errorf("failed to create validator: %w", err), nil + } + allErrors := v.ValidateFromPath(packageRoot) if allErrors == nil { return nil, nil } - fsys := os.DirFS(packageRoot) result, err := filterErrors(allErrors, fsys) if err != nil { - return err, nil + logger.Debugf("failed to filter validation errors: %v", err) + return allErrors, nil } return result.Processed, result.Removed } -func ValidateAndFilterFromZip(zipPackagePath string) (error, error) { - allErrors := validator.ValidateFromZip(zipPackagePath) +// ValidateBuiltFromZip validates a built package zip archive. Zip files are always +// treated as built artifacts; source-only artifacts are rejected. Validation errors +// are filtered against the package's validation.yml config; the first return value +// is the remaining errors after filtering, the second is the errors that were +// filtered out. +func ValidateBuiltFromZip(zipPackagePath string) (error, error) { + v, err := validator.New(validator.BuildMode) + if err != nil { + return fmt.Errorf("failed to create validator: %w", err), nil + } + + allErrors := v.ValidateFromZip(zipPackagePath) if allErrors == nil { return nil, nil } - + // Open a separate, independent zip reader for filterErrors. fsys, err := zip.OpenReader(zipPackagePath) if err != nil { - return fmt.Errorf("failed to open zip file (%s): %w", zipPackagePath, err), nil + logger.Debugf("failed to open zip for filter: %v", err) + return allErrors, nil } defer fsys.Close() - + // fsFromPackageZip navigates into the single package subdirectory so that + // filterErrors can locate validation.yml at the package root, not the zip root. fsZip, err := fsFromPackageZip(fsys) if err != nil { - return fmt.Errorf("failed to extract filesystem from zip file (%s): %w", zipPackagePath, err), nil + logger.Debugf("failed to extract filesystem from zip for filter: %v", err) + return allErrors, nil } - result, err := filterErrors(allErrors, fsZip) if err != nil { - return err, nil + logger.Debugf("failed to filter validation errors: %v", err) + return allErrors, nil } return result.Processed, result.Removed } func fsFromPackageZip(fsys fs.FS) (fs.FS, error) { - dirs, err := fs.ReadDir(fsys, ".") + entries, err := fs.ReadDir(fsys, ".") if err != nil { return nil, fmt.Errorf("failed to read root directory in zip file fs: %w", err) } + var dirs []fs.DirEntry + for _, e := range entries { + if e.IsDir() { + dirs = append(dirs, e) + } + } if len(dirs) != 1 { return nil, fmt.Errorf("a single directory is expected in zip file, %d found", len(dirs)) } @@ -77,6 +112,7 @@ func fsFromPackageZip(fsys fs.FS) (fs.FS, error) { return subDir, nil } +// TODO: follow-up issue — move this logic into specerrors as an exported function so consumers don't reimplement it. func filterErrors(allErrors error, fsys fs.FS) (specerrors.FilterResult, error) { var errs specerrors.ValidationErrors if !errors.As(allErrors, &errs) { diff --git a/internal/validation/validation_test.go b/internal/validation/validation_test.go new file mode 100644 index 0000000000..dd98c01a3e --- /dev/null +++ b/internal/validation/validation_test.go @@ -0,0 +1,142 @@ +// 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 validation + +import ( + "archive/zip" + "io/fs" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + builtPackageDir = "testdata/built_package" + sourcePackageDir = "testdata/source_package" + composableSourcePackageDir = "testdata/composable_source_package" +) + +func TestValidateSourceFromPath(t *testing.T) { + tests := []struct { + name string + dir string + expectError bool + }{ + // source_package has _dev/; ModeSource allows it. + {name: "valid source package", dir: sourcePackageDir}, + // built_package has _embedded_ecs dynamic templates auto-injected at build time; + // ModeSource rejects them via ValidateNoEmbeddedEcsInDynamicTemplates, confirming + // the built fixture is meaningfully distinct from the source fixture. + {name: "rejects _embedded_ecs dynamic templates", dir: builtPackageDir, expectError: true}, + // composable_source_package uses package: in stream definitions; ModeSource allows it. + {name: "valid composable source package", dir: composableSourcePackageDir}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validationErr, filteredErr := ValidateSourceFromPath(tt.dir) + if tt.expectError { + assert.Error(t, validationErr) + return + } + assert.NoError(t, validationErr) + assert.NoError(t, filteredErr) + }) + } +} + +func TestValidateBuiltFromPath(t *testing.T) { + tests := []struct { + name string + dir string + expectError bool + }{ + // built_package has no _dev/ or source-only artifacts; ModeBuild accepts it. + {name: "valid built package", dir: builtPackageDir}, + // source_package has _dev/; ModeBuild rejects it via ValidateNoDevFolder. + // Guards against regression if ValidateBuiltFromPath is ever switched back to ModeLegacy. + {name: "rejects _dev/ directory", dir: sourcePackageDir, expectError: true}, + // composable_source_package has package: in stream definitions and policy template inputs; + // ModeBuild rejects these via ValidateStreamInputMaterialized. + {name: "rejects un-materialized package: references", dir: composableSourcePackageDir, expectError: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validationErr, filteredErr := ValidateBuiltFromPath(tt.dir) + if tt.expectError { + assert.Error(t, validationErr) + return + } + assert.NoError(t, validationErr) + assert.NoError(t, filteredErr) + }) + } +} + +func TestValidateBuiltFromZip(t *testing.T) { + tests := []struct { + name string + dir string + expectError bool + }{ + // built_package has no _dev/ or source-only artifacts; ModeBuild accepts it. + {name: "valid built package", dir: builtPackageDir}, + // source_package has _dev/; ModeBuild rejects it via ValidateNoDevFolder. + // Guards the zip path the same way TestValidateBuiltFromPath guards the directory path. + {name: "rejects _dev/ directory", dir: sourcePackageDir, expectError: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + zipPath := makeZipFromDir(t, tt.dir) + validationErr, filteredErr := ValidateBuiltFromZip(zipPath) + if tt.expectError { + assert.Error(t, validationErr) + return + } + assert.NoError(t, validationErr) + assert.NoError(t, filteredErr) + }) + } +} + +// makeZipFromDir zips dir into a temp file with the package directory as the single +// top-level entry, matching the layout that fsFromPackageZip expects. +func makeZipFromDir(t *testing.T, dir string) string { + t.Helper() + tmp := filepath.Join(t.TempDir(), filepath.Base(dir)+".zip") + f, err := os.Create(tmp) + require.NoError(t, err) + defer f.Close() + + w := zip.NewWriter(f) + base := filepath.Base(dir) + err = filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + return nil + } + rel, err := filepath.Rel(dir, path) + if err != nil { + return err + } + fw, err := w.Create(base + "/" + filepath.ToSlash(rel)) + if err != nil { + return err + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + _, err = fw.Write(data) + return err + }) + require.NoError(t, err) + require.NoError(t, w.Close()) + return tmp +}