-
Notifications
You must be signed in to change notification settings - Fork 91
validator: introduce mode-aware constructors and validation API #1177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 19 commits
c2b6ed7
1166296
892c661
a5c8cfc
30cfd76
d482f31
2769edc
324c4a0
d999602
fe83a0a
491658c
15750f5
fcaf8c2
f4e266e
e9d0013
cbd3b7c
0f275c0
d1efd03
4233331
da2a325
8539209
b0049ff
d4508b1
8d441f1
15cc91e
cc5e273
2a3fef8
d8a2a6c
ebac0c7
d895e49
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| // 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 | ||
|
|
||
| // Mode represents the validation mode used when validating a package. | ||
| type Mode string | ||
|
|
||
| // Validation modes for package validation: Legacy preserves existing behavior, | ||
| // Source validates checked-out source trees, and Build validates built artifacts. | ||
| const ( | ||
| Legacy 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: | ||
| return true | ||
| } | ||
| return false | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| // Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| // or more contributor license agreements. Licensed under the Elastic License; | ||
| // you may not use this file except in compliance with the Elastic License. | ||
|
|
||
| package modes | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestValid(t *testing.T) { | ||
| tests := map[string]struct { | ||
| mode Mode | ||
| valid bool | ||
| }{ | ||
| "valid": { | ||
| mode: Legacy, | ||
| valid: true, | ||
| }, | ||
| "invalid": { | ||
| mode: Mode("invalid"), | ||
| valid: false, | ||
| }, | ||
| "source": { | ||
| mode: Source, | ||
| valid: true, | ||
| }, | ||
| "build": { | ||
| mode: Build, | ||
| valid: true, | ||
| }, | ||
| "": { | ||
| mode: Mode(""), | ||
| valid: false, | ||
| }, | ||
| } | ||
|
|
||
| for name, test := range tests { | ||
| t.Run(name, func(t *testing.T) { | ||
| assert.Equal(t, test.valid, test.mode.Valid(), "mode %s should be %v", test.mode, test.valid) | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -11,21 +11,61 @@ import ( | |||
| "io/fs" | ||||
| "os" | ||||
|
|
||||
| "github.com/Masterminds/semver/v3" | ||||
|
|
||||
| "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" | ||||
| "github.com/elastic/package-spec/v3/code/go/internal/packages" | ||||
| "github.com/elastic/package-spec/v3/code/go/internal/validator" | ||||
| "github.com/elastic/package-spec/v3/code/go/internal/validator/common" | ||||
| ) | ||||
|
|
||||
| // ValidateFromPath validates a package located at the given path against the | ||||
| // appropriate specification and returns any errors. | ||||
| type specFn func(semver.Version) (*validator.Spec, error) | ||||
|
|
||||
| // ValidateFromPath validates a package located at the given path using the legacy specification. | ||||
| // This function preserves byte-for-byte identical behavior with existing validation workflows. | ||||
| // Linked (.link) files are resolved transparently. | ||||
| // Deprecated: Use ValidateFromSourcePath or ValidateFromBuildPath depending on the package type. | ||||
| func ValidateFromPath(packageRootPath string) error { | ||||
| // We wrap the fs.FS with a linkedfiles.LinksFS to handle linked files. | ||||
| linksFS := linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) | ||||
| return ValidateFromFS(packageRootPath, linksFS) | ||||
|
|
||||
| legacySpec := func(version semver.Version) (*validator.Spec, error) { | ||||
| return validator.NewLegacySpec(version) | ||||
| } | ||||
|
|
||||
| return validateFromFS(packageRootPath, linksFS, legacySpec) | ||||
|
teresaromero marked this conversation as resolved.
Outdated
|
||||
| } | ||||
|
|
||||
| // ValidateFromBuildPath validates a built package located at the given path. | ||||
| // This function uses the build specification, appropriate for packages produced by | ||||
| // elastic-package build, distributed as zip files, or served by the package registry. | ||||
| // Linked files (.link) are blocked; source-only artifacts are rejected. | ||||
| func ValidateFromBuildPath(packageRootPath string) error { | ||||
| fs := os.DirFS(packageRootPath) | ||||
|
|
||||
| buildSpec := func(version semver.Version) (*validator.Spec, error) { | ||||
| return validator.NewBuildSpec(version) | ||||
| } | ||||
| return validateFromFS(packageRootPath, fs, buildSpec) | ||||
|
teresaromero marked this conversation as resolved.
Outdated
|
||||
| } | ||||
|
|
||||
| // ValidateFromSourcePath validates a package source tree located at the given path. | ||||
| // This function uses the source specification for checked-out source trees. | ||||
| // Linked (.link) files are resolved transparently. | ||||
| func ValidateFromSourcePath(packageRootPath string) error { | ||||
| // We wrap the fs.FS with a linkedfiles.LinksFS to handle linked files. | ||||
| linksFS := linkedfiles.NewFS(packageRootPath, os.DirFS(packageRootPath)) | ||||
|
|
||||
| sourceSpec := func(version semver.Version) (*validator.Spec, error) { | ||||
| return validator.NewSourceSpec(version) | ||||
| } | ||||
|
|
||||
| return validateFromFS(packageRootPath, linksFS, sourceSpec) | ||||
|
teresaromero marked this conversation as resolved.
Outdated
|
||||
| } | ||||
|
|
||||
| // ValidateFromZip validates a package on its zip format. | ||||
| // ValidateFromZip validates a package in zip file format. | ||||
| // This function uses the build specification since zip files are by definition built packages. | ||||
| // Linked files (.link) are blocked; source-only artifacts are rejected. | ||||
| func ValidateFromZip(packagePath string) error { | ||||
| r, err := zip.OpenReader(packagePath) | ||||
| if err != nil { | ||||
|
|
@@ -46,16 +86,16 @@ func ValidateFromZip(packagePath string) error { | |||
| return err | ||||
| } | ||||
|
|
||||
| return ValidateFromFS(packagePath, subDir) | ||||
| } | ||||
| buildSpec := func(version semver.Version) (*validator.Spec, error) { | ||||
| return validator.NewBuildSpec(version) | ||||
| } | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still don't like the handling of linked file systems, but this is internal and we can revisit later, I don't have much better ideas now. |
||||
|
|
||||
| // 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 { | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method is removed, right? This would be a breaking change in the public API.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should deprecate all |
||||
| return validateFromFS(location, fsys, common.IsDefinedWarningsAsErrors()) | ||||
| return validateFromFS(packagePath, subDir, buildSpec) | ||||
|
teresaromero marked this conversation as resolved.
Outdated
|
||||
| } | ||||
|
|
||||
| func validateFromFS(location string, fsys fs.FS, warningsAsErrors bool) error { | ||||
| // validateFromFS validates a package against the appropriate specification and returns any errors. | ||||
| // Package files are obtained through the given filesystem. | ||||
| func validateFromFS(location string, fsys fs.FS, specFn specFn) error { | ||||
| // If we are not explicitly using the linkedfiles.FS, we wrap fsys with | ||||
| // a linkedfiles.BlockFS to block the use of linked files. | ||||
| if _, ok := fsys.(*linkedfiles.FS); !ok { | ||||
|
|
@@ -70,11 +110,11 @@ 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 := specFn(*pkg.SpecVersion) | ||||
| if err != nil { | ||||
| return err | ||||
| } | ||||
| spec.WarningsAsErrors = warningsAsErrors | ||||
| spec.WarningsAsErrors = common.IsDefinedWarningsAsErrors() | ||||
|
|
||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should append a technical preview warning when Source or Build modes are used, at least till we complete the validators.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added just using the log pkg as i havent seen any logger implemented on the package d895e49 i followed the same pattern as in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we don't have an abstraction for warnings, maybe we should add it. We could leverage errs := spec.ValidatePackage(*pkg)
if v.mode != LegacyMode {
err := specerrors.NewStructuredErrorf("validation mode '%s' is in technical preview", v.mode)
if v.warningsAsErrors() {
errs = append(errs, err)
} else {
log.Printf("Warning: %s", err.Message())
}
}
if len(errs) > 0 {
return errs
}
return nil |
||||
| if errs := spec.ValidatePackage(*pkg); len(errs) > 0 { | ||||
| return errs | ||||
|
|
||||
Uh oh!
There was an error while loading. Please reload this page.