-
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 all 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,30 @@ | ||
| // 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 | ||
|
|
||
| // Mode represents the validation context: which semantic rules run and how | ||
| // linked (.link) files are handled during package validation. | ||
| type Mode string | ||
|
|
||
| const ( | ||
| // LegacyMode preserves the original validation behavior: linked files are | ||
| // resolved transparently and no rules are mode-gated. | ||
| LegacyMode Mode = "legacy" | ||
| // SourceMode validates a package as a checked-out source tree: linked files | ||
| // are resolved transparently and source-only rules are enforced. | ||
| SourceMode Mode = "source" | ||
| // BuildMode validates a package as a built artifact: linked files are | ||
| // unconditionally blocked and build-only rules are enforced. | ||
| BuildMode Mode = "build" | ||
| ) | ||
|
|
||
| // Valid reports whether m is a recognised validation mode. | ||
| func (m Mode) Valid() bool { | ||
| switch m { | ||
| case LegacyMode, SourceMode, BuildMode: | ||
| 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 validator | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestValid(t *testing.T) { | ||
| tests := map[string]struct { | ||
| mode Mode | ||
| valid bool | ||
| }{ | ||
| "valid": { | ||
| mode: LegacyMode, | ||
| valid: true, | ||
| }, | ||
| "invalid": { | ||
| mode: Mode("invalid"), | ||
| valid: false, | ||
| }, | ||
| "source": { | ||
| mode: SourceMode, | ||
| valid: true, | ||
| }, | ||
| "build": { | ||
| mode: BuildMode, | ||
| 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 | ||
|---|---|---|---|---|
|
|
@@ -9,6 +9,7 @@ import ( | |||
| "errors" | ||||
| "fmt" | ||||
| "io/fs" | ||||
| "log" | ||||
| "os" | ||||
|
|
||||
| "github.com/elastic/package-spec/v3/code/go/internal/linkedfiles" | ||||
|
|
@@ -17,25 +18,80 @@ import ( | |||
| "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. | ||||
| 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) | ||||
| // Mode is the validation context that controls semantic rules and linked-file handling. | ||||
| type Mode = validator.Mode | ||||
|
|
||||
| var ( | ||||
| // LegacyMode preserves the original validation behavior. | ||||
| LegacyMode Mode = validator.LegacyMode | ||||
| // SourceMode validates a checked-out source tree. | ||||
| SourceMode Mode = validator.SourceMode | ||||
| // BuildMode validates a built package artifact. | ||||
| BuildMode Mode = validator.BuildMode | ||||
| ) | ||||
|
|
||||
| // Validator holds the configuration for a package validation run. | ||||
| // Create one with NewValidator, then call ValidateFromPath, ValidateFromZip, or ValidateFromFS. | ||||
| type Validator struct { | ||||
| mode Mode | ||||
| warningsAsErrors bool | ||||
| } | ||||
|
|
||||
| // Option configures a Validator. | ||||
| type Option func(*Validator) | ||||
|
|
||||
| // WithWarningsAsErrors controls whether validation warnings are promoted to errors. | ||||
| // When enabled is true, warnings are reported as errors regardless of the | ||||
| // PACKAGE_SPEC_WARNINGS_AS_ERRORS environment variable. When enabled is false, | ||||
| // warnings remain warnings even if the environment variable is set. | ||||
| func WithWarningsAsErrors(enabled bool) Option { | ||||
| return func(v *Validator) { v.warningsAsErrors = enabled } | ||||
| } | ||||
|
|
||||
| // New creates a Validator for the given mode and options. | ||||
| func New(mode Mode, opts ...Option) (*Validator, error) { | ||||
| if !mode.Valid() { | ||||
| return nil, fmt.Errorf("invalid validation mode %q", mode) | ||||
| } | ||||
| v := &Validator{ | ||||
| mode: mode, | ||||
| warningsAsErrors: common.IsDefinedWarningsAsErrors(), | ||||
| } | ||||
| for _, opt := range opts { | ||||
| opt(v) | ||||
| } | ||||
|
|
||||
| return v, nil | ||||
| } | ||||
|
|
||||
| // ValidateFromPath validates the package at path on disk. | ||||
| func (v *Validator) ValidateFromPath(path string) error { | ||||
| fsys := os.DirFS(path) | ||||
| if v.mode == BuildMode { | ||||
| fsys = linkedfiles.NewBlockFS(fsys) | ||||
| } else { | ||||
| fsys = linkedfiles.NewFS(path, fsys) | ||||
| } | ||||
|
|
||||
| return v.validate(path, fsys) | ||||
| } | ||||
|
|
||||
| // ValidateFromZip validates a package on its zip format. | ||||
| func ValidateFromZip(packagePath string) error { | ||||
| r, err := zip.OpenReader(packagePath) | ||||
| // ValidateFromZip validates the package stored in a zip file. | ||||
| // Zip files are supported in LegacyMode and BuildMode only. | ||||
| func (v *Validator) ValidateFromZip(zipPath string) error { | ||||
| if v.mode != LegacyMode && v.mode != BuildMode { | ||||
| return errors.New("zip files are only supported in LegacyMode or BuildMode") | ||||
| } | ||||
|
|
||||
| r, err := zip.OpenReader(zipPath) | ||||
| if err != nil { | ||||
| return fmt.Errorf("failed to open zip file (%s): %w", packagePath, err) | ||||
| return fmt.Errorf("failed to open zip file (%s): %w", zipPath, 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) | ||||
| return fmt.Errorf("failed to read root directory in zip file (%s): %w", zipPath, err) | ||||
| } | ||||
| if len(dirs) != 1 { | ||||
| return fmt.Errorf("a single directory is expected in zip file, %d found", len(dirs)) | ||||
|
|
@@ -46,39 +102,78 @@ func ValidateFromZip(packagePath string) error { | |||
| return err | ||||
| } | ||||
|
|
||||
| return ValidateFromFS(packagePath, subDir) | ||||
| fsys := linkedfiles.NewBlockFS(subDir) | ||||
| return v.validate(zipPath, fsys) | ||||
| } | ||||
|
|
||||
| // ValidateFromFS validates a package against the appropiate specification and returns any errors. | ||||
| // Package files are obtained through the given filesystem. | ||||
| func ValidateFromFS(location string, fsys fs.FS) 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()) | ||||
| // ValidateFromFS validates the package accessible through fsys at location. | ||||
| func (v *Validator) ValidateFromFS(location string, fsys fs.FS) error { | ||||
| if v.mode == LegacyMode { | ||||
| // If we are not explicitly using the linkedfiles.FS, we wrap fsys with | ||||
| // a linkedfiles.BlockFS to block the use of linked files. | ||||
| if _, ok := fsys.(*linkedfiles.FS); !ok { | ||||
| fsys = linkedfiles.NewBlockFS(fsys) | ||||
| } | ||||
| } else if _, ok := fsys.(*linkedfiles.FS); ok && v.mode == BuildMode { | ||||
| return errors.New("linked files are not supported in BuildMode") | ||||
| } else if _, ok := fsys.(*linkedfiles.BlockFS); ok && v.mode == SourceMode { | ||||
| return errors.New("block linked files are not supported in SourceMode") | ||||
| } | ||||
|
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. |
||||
|
|
||||
| return v.validate(location, fsys) | ||||
| } | ||||
|
|
||||
| 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) | ||||
| } | ||||
| func (v *Validator) validate(location string, fsys fs.FS) error { | ||||
| pkg, err := packages.NewPackageFromFS(location, fsys) | ||||
| if err != nil { | ||||
| return err | ||||
| } | ||||
|
|
||||
| if pkg.SpecVersion == nil { | ||||
| return errors.New("could not determine specification version for package") | ||||
| } | ||||
|
|
||||
| spec, err := validator.NewSpec(*pkg.SpecVersion) | ||||
| spec, err := validator.NewSpec(*pkg.SpecVersion, v.mode) | ||||
| if err != nil { | ||||
| return err | ||||
| } | ||||
| spec.WarningsAsErrors = warningsAsErrors | ||||
| spec.WarningsAsErrors = v.warningsAsErrors | ||||
|
|
||||
|
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 v.mode != LegacyMode { | ||||
| log.Printf("Warning: validation mode '%s' is in technical preview", v.mode) | ||||
| } | ||||
|
|
||||
| if errs := spec.ValidatePackage(*pkg); len(errs) > 0 { | ||||
| return errs | ||||
| } | ||||
|
|
||||
| return nil | ||||
| } | ||||
|
|
||||
| // ValidateFromPath is a convenience function that creates a new Validator in LegacyMode and calls ValidateFromPath. | ||||
| // Deprecated: Use NewValidator and ValidateFromPath instead. | ||||
| func ValidateFromPath(path string) error { | ||||
| v, err := New(LegacyMode) | ||||
| if err != nil { | ||||
| return err | ||||
| } | ||||
| return v.ValidateFromPath(path) | ||||
| } | ||||
|
|
||||
| // ValidateFromZip is a convenience function that creates a new Validator in LegacyMode and calls ValidateFromZip. | ||||
| // Deprecated: Use NewValidator and ValidateFromZip instead. | ||||
| func ValidateFromZip(zipPath string) error { | ||||
| v, err := New(LegacyMode) | ||||
| if err != nil { | ||||
| return err | ||||
| } | ||||
| return v.ValidateFromZip(zipPath) | ||||
| } | ||||
|
|
||||
| // ValidateFromFS is a convenience function that creates a new Validator in LegacyMode and calls ValidateFromFS. | ||||
| // Deprecated: Use NewValidator and ValidateFromFS instead. | ||||
| func ValidateFromFS(location string, fsys fs.FS) error { | ||||
| v, err := New(LegacyMode) | ||||
| if err != nil { | ||||
| return err | ||||
| } | ||||
| return v.ValidateFromFS(location, fsys) | ||||
| } | ||||
Uh oh!
There was an error while loading. Please reload this page.