Skip to content
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

Define beta features in spec #341

Merged
merged 9 commits into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions code/go/internal/validator/common_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ type commonSpec struct {
DevelopmentFolder bool `yaml:"developmentFolder"`

Limits commonSpecLimits `yaml:",inline"`

// Release type of the spec: beta, ga.
// Packages using beta features won't be able to go GA.
// Default release: ga
Release string `yaml:"release"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check somewhere that we only use beta or ga here? Or even only beta?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a condition to check for "beta" and "ga" and report an error whenever there is a different level used.

}

type commonSpecLimits struct {
Expand Down
43 changes: 25 additions & 18 deletions code/go/internal/validator/folder_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"io/fs"
"io/ioutil"
"log"
"path/filepath"
"regexp"
"strings"
Expand All @@ -16,7 +17,6 @@ import (
"gopkg.in/yaml.v3"

ve "github.com/elastic/package-spec/code/go/internal/errors"
"github.com/elastic/package-spec/code/go/internal/fspath"
"github.com/elastic/package-spec/code/go/internal/spectypes"
)

Expand Down Expand Up @@ -71,44 +71,51 @@ func (s *folderSpec) load(fs fs.FS, specPath string) error {
return nil
}

func (s *folderSpec) validate(packageName string, fsys fspath.FS, path string) ve.ValidationErrors {
func (s *folderSpec) validate(pkg *Package, path string) ve.ValidationErrors {
var errs ve.ValidationErrors
files, err := fs.ReadDir(fsys, path)
files, err := fs.ReadDir(pkg, path)
if err != nil {
errs = append(errs, errors.Wrapf(err, "could not read folder [%s]", fsys.Path(path)))
errs = append(errs, errors.Wrapf(err, "could not read folder [%s]", pkg.Path(path)))
return errs
}

// This is not taking into account if the folder is for development. Enforce
// this limit in all cases to avoid having to read too many files.
if contentsLimit := s.Limits.TotalContentsLimit; contentsLimit > 0 && len(files) > contentsLimit {
errs = append(errs, errors.Errorf("folder [%s] exceeds the limit of %d files", fsys.Path(path), contentsLimit))
errs = append(errs, errors.Errorf("folder [%s] exceeds the limit of %d files", pkg.Path(path), contentsLimit))
return errs
}

// Don't enable beta features for packages marked as GA.
if s.Release == "beta" && pkg.Version.Prerelease() == "" {
errs = append(errs, errors.Errorf("spec for [%s] defines beta features which can't be enabled for packages with a stable semantic version", pkg.Path(path)))
} else if s.Release == "beta" && pkg.Version.Prerelease() != "" {
log.Printf("Warning: package with non-stable semantic version and active beta features (enabled in [%s]) can't be released as stable version.q", pkg.Path(path))
mtojek marked this conversation as resolved.
Show resolved Hide resolved
}

for _, file := range files {
fileName := file.Name()
itemSpec, err := s.findItemSpec(packageName, fileName)
itemSpec, err := s.findItemSpec(pkg.Name, fileName)
if err != nil {
errs = append(errs, err)
continue
}

if itemSpec == nil && s.AdditionalContents {
// No spec found for current folder item but we do allow additional contents in folder.
// No spec found for current folder item, but we do allow additional contents in folder.
if file.IsDir() {
if !s.DevelopmentFolder && strings.Contains(fileName, "-") {
errs = append(errs,
fmt.Errorf(`file "%s" is invalid: directory name inside package %s contains -: %s`,
fsys.Path(path, fileName), packageName, fileName))
pkg.Path(path, fileName), pkg.Name, fileName))
}
}
continue
}

if itemSpec == nil && !s.AdditionalContents {
// No spec found for current folder item and we do not allow additional contents in folder.
errs = append(errs, fmt.Errorf("item [%s] is not allowed in folder [%s]", fileName, fsys.Path(path)))
errs = append(errs, fmt.Errorf("item [%s] is not allowed in folder [%s]", fileName, pkg.Path(path)))
continue
}

Expand Down Expand Up @@ -151,7 +158,7 @@ func (s *folderSpec) validate(packageName string, fsys fspath.FS, path string) v
}

subFolderPath := filepath.Join(path, fileName)
subErrs := subFolderSpec.validate(packageName, fsys, subFolderPath)
subErrs := subFolderSpec.validate(pkg, subFolderPath)
if len(subErrs) > 0 {
errs = append(errs, subErrs...)
}
Expand All @@ -163,21 +170,21 @@ func (s *folderSpec) validate(packageName string, fsys fspath.FS, path string) v
}
} else {
if !itemSpec.isSameType(file) {
errs = append(errs, fmt.Errorf("[%s] is a file but is expected to be a folder", fsys.Path(fileName)))
errs = append(errs, fmt.Errorf("[%s] is a file but is expected to be a folder", pkg.Path(fileName)))
continue
}

itemPath := filepath.Join(path, file.Name())
itemValidationErrs := itemSpec.validate(s.fs, fsys, s.specPath, itemPath)
itemValidationErrs := itemSpec.validate(s.fs, pkg, s.specPath, itemPath)
if itemValidationErrs != nil {
for _, ive := range itemValidationErrs {
errs = append(errs, errors.Wrapf(ive, "file \"%s\" is invalid", fsys.Path(itemPath)))
errs = append(errs, errors.Wrapf(ive, "file \"%s\" is invalid", pkg.Path(itemPath)))
}
}

info, err := fs.Stat(fsys, itemPath)
info, err := fs.Stat(pkg, itemPath)
if err != nil {
errs = append(errs, errors.Wrapf(err, "failed to obtain file size for \"%s\"", fsys.Path(itemPath)))
errs = append(errs, errors.Wrapf(err, "failed to obtain file size for \"%s\"", pkg.Path(itemPath)))
} else {
s.totalContents++
s.totalSize += spectypes.FileSize(info.Size())
Expand All @@ -186,7 +193,7 @@ func (s *folderSpec) validate(packageName string, fsys fspath.FS, path string) v
}

if sizeLimit := s.Limits.TotalSizeLimit; sizeLimit > 0 && s.totalSize > sizeLimit {
errs = append(errs, errors.Errorf("folder [%s] exceeds the total size limit of %s", fsys.Path(path), sizeLimit))
errs = append(errs, errors.Errorf("folder [%s] exceeds the total size limit of %s", pkg.Path(path), sizeLimit))
}

// validate that required items in spec are all accounted for
Expand All @@ -204,9 +211,9 @@ func (s *folderSpec) validate(packageName string, fsys fspath.FS, path string) v
if !fileFound {
var err error
if itemSpec.Name != "" {
err = fmt.Errorf("expecting to find [%s] %s in folder [%s]", itemSpec.Name, itemSpec.ItemType, fsys.Path(path))
err = fmt.Errorf("expecting to find [%s] %s in folder [%s]", itemSpec.Name, itemSpec.ItemType, pkg.Path(path))
} else if itemSpec.Pattern != "" {
err = fmt.Errorf("expecting to find %s matching pattern [%s] in folder [%s]", itemSpec.ItemType, itemSpec.Pattern, fsys.Path(path))
err = fmt.Errorf("expecting to find %s matching pattern [%s] in folder [%s]", itemSpec.ItemType, itemSpec.Pattern, pkg.Path(path))
}
errs = append(errs, err)
}
Expand Down
12 changes: 12 additions & 0 deletions code/go/internal/validator/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
type Package struct {
Name string
Type string
Version *semver.Version
SpecVersion *semver.Version

fs fs.FS
Expand Down Expand Up @@ -66,6 +67,7 @@ func NewPackageFromFS(location string, fsys fs.FS) (*Package, error) {
var manifest struct {
Name string `yaml:"name"`
Type string `yaml:"type"`
Version string `yaml:"version"`
SpecVersion string `yaml:"format_version"`
}
if err := yaml.Unmarshal(data, &manifest); err != nil {
Expand All @@ -76,6 +78,15 @@ func NewPackageFromFS(location string, fsys fs.FS) (*Package, error) {
return nil, errors.New("package type undefined in the package manifest file")
}

if manifest.Version == "" {
return nil, errors.New("package version undefined in the package manifest file")
}

packageVersion, err := semver.NewVersion(manifest.Version)
if err != nil {
return nil, errors.Wrapf(err, "could not read package version from package manifest file [%v]", pkgManifestPath)
}

specVersion, err := semver.NewVersion(manifest.SpecVersion)
if err != nil {
return nil, errors.Wrapf(err, "could not read specification version from package manifest file [%v]", pkgManifestPath)
Expand All @@ -85,6 +96,7 @@ func NewPackageFromFS(location string, fsys fs.FS) (*Package, error) {
p := Package{
Name: manifest.Name,
Type: manifest.Type,
Version: packageVersion,
SpecVersion: specVersion,
fs: fsys,

Expand Down
2 changes: 1 addition & 1 deletion code/go/internal/validator/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (s Spec) ValidatePackage(pkg Package) ve.ValidationErrors {
}

// Syntactic validations
errs = rootSpec.validate(pkg.Name, &pkg, ".")
errs = rootSpec.validate(&pkg, ".")
if len(errs) != 0 {
return errs
}
Expand Down
24 changes: 13 additions & 11 deletions code/go/pkg/validator/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestValidateFile(t *testing.T) {
"missing_version": {
"manifest.yml",
[]string{
"field version: Invalid type. Expected: string, given: null",
"package version undefined in the package manifest file",
},
},
"bad_time_series": {
Expand All @@ -97,19 +97,21 @@ func TestValidateFile(t *testing.T) {
require.NoError(t, errs)
} else {
require.Error(t, errs)
require.Len(t, errs, len(test.expectedErrContains))
vErrs, ok := errs.(errors.ValidationErrors)
require.True(t, ok)

var errMessages []string
for _, vErr := range vErrs {
errMessages = append(errMessages, vErr.Error())
}
if ok {
require.Len(t, errs, len(test.expectedErrContains))
var errMessages []string
for _, vErr := range vErrs {
errMessages = append(errMessages, vErr.Error())
}

for _, expectedErrMessage := range test.expectedErrContains {
expectedErr := errPrefix + expectedErrMessage
require.Contains(t, errMessages, expectedErr)
for _, expectedErrMessage := range test.expectedErrContains {
expectedErr := errPrefix + expectedErrMessage
require.Contains(t, errMessages, expectedErr)
}
return
}
require.Equal(t, errs.Error(), test.expectedErrContains[0])
}
})
}
Expand Down
3 changes: 2 additions & 1 deletion test/packages/no_spec_version/manifest.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# no format_version in this file
name: mypackage
type: integration
type: integration
version: 1.0.0
5 changes: 4 additions & 1 deletion versions/1/changelog.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
changes:
- description: Prepare for next version
type: enhancement
link: https://github.com/elastic/package-spec/pull/?
link: https://github.com/elastic/package-spec/pull/337
- description: Define beta features in spec
type: enhancement
link: https://github.com/elastic/package-spec/pull/341
- version: 1.9.0
changes:
- description: Prepare for next version
Expand Down