Define beta features in spec (#341)

* Add Version field to Package

* Fix tests

* Fix: error message

* Fix changelog

* Update code/go/internal/validator/folder_spec.go

Co-authored-by: Josh Dover <[email protected]>

* Log warning

* Spec testing

* Fix: beta and ga as valid levels

* Use switch

Co-authored-by: Josh Dover <[email protected]>

code/go/internal/validator/common_spec.go

@@ -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"`
 }
 
 type commonSpecLimits struct {

code/go/internal/validator/folder_spec.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io/fs"
 	"io/ioutil"
+	"log"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -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"
 )
 
@@ -71,44 +71,57 @@ 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.
+	switch s.Release {
+		case "", "ga": // do nothing
+		case "beta":
+			if 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 {
+				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))
+			}
+		default:
+			errs = append(errs, errors.Errorf("unsupport release level, supported values: beta, ga"))
+	}
+
 	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
 		}
 
@@ -151,7 +164,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...)
 			}
@@ -163,21 +176,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())
@@ -186,7 +199,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
@@ -204,9 +217,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)
 		}

code/go/internal/validator/package.go

@@ -19,6 +19,7 @@ import (
 type Package struct {
 	Name        string
 	Type        string
+	Version     *semver.Version
 	SpecVersion *semver.Version
 
 	fs       fs.FS
@@ -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 {
@@ -76,15 +78,25 @@ 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)
+		return nil, errors.Wrapf(err, "could not read specification version from package manifest file [%v]", manifest.SpecVersion)
 	}
 
 	// Instantiate Package object and return it
 	p := Package{
 		Name:        manifest.Name,
 		Type:        manifest.Type,
+		Version:     packageVersion,
 		SpecVersion: specVersion,
 		fs:          fsys,
 

code/go/internal/validator/package_test.go

@@ -47,3 +47,4 @@ func TestNewPackage(t *testing.T) {
 		})
 	}
 }
+

code/go/internal/validator/spec.go

@@ -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
 	}

code/go/internal/validator/spec_test.go

@@ -5,6 +5,7 @@
 package validator
 
 import (
+	"github.com/elastic/package-spec/code/go/internal/fspath"
 	"testing"
 
 	"github.com/Masterminds/semver/v3"
@@ -33,3 +34,32 @@ func TestNewSpec(t *testing.T) {
 		}
 	}
 }
+
+func TestNoBetaFeatures_Package_GA(t *testing.T) {
+	// given
+	s := Spec{
+		*semver.MustParse("1.0.0"),
+		fspath.DirFS("testdata/fakespec"),
+		"1",
+	}
+	pkg, err := NewPackage("testdata/packages/features_ga")
+	require.NoError(t, err)
+
+	err = s.ValidatePackage(*pkg)
+	require.Empty(t, err)
+}
+
+func TestBetaFeatures_Package_GA(t *testing.T) {
+	// given
+	s := Spec{
+		*semver.MustParse("1.0.0"),
+		fspath.DirFS("testdata/fakespec"),
+		"1",
+	}
+	pkg, err := NewPackage("testdata/packages/features_beta")
+	require.NoError(t, err)
+
+	errs := s.ValidatePackage(*pkg)
+	require.Len(t,errs, 1)
+	require.Equal(t, errs[0].Error(), "spec for [testdata/packages/features_beta/beta] defines beta features which can't be enabled for packages with a stable semantic version")
+}

code/go/internal/validator/testdata/fakespec/1/fake/beta/spec.yml

@@ -0,0 +1,3 @@
+spec:
+  additionalContents: true
+  release: beta

code/go/internal/validator/testdata/fakespec/1/fake/ga/spec.yml

@@ -0,0 +1,2 @@
+spec:
+  additionalContents: true

code/go/internal/validator/testdata/fakespec/1/fake/spec.yml

@@ -0,0 +1,12 @@
+spec:
+  additionalContents: true
+  contents:
+  - description: Folder containing beta features
+    type: folder
+    name: beta
+    $ref: "./beta/spec.yml"
+  - description: Folder containing GA features
+    type: folder
+    name: ga
+    required: true
+    $ref: "./ga/spec.yml"

code/go/internal/validator/testdata/packages/features_beta/changelog.yml

@@ -0,0 +1,6 @@
+# newer versions go on top
+- version: "2.3.4"
+  changes:
+    - description: Initial draft of the package
+      type: enhancement
+      link: https://github.com/elastic/package-spec/pull/341

code/go/internal/validator/testdata/packages/features_beta/manifest.yml

@@ -0,0 +1,4 @@
+format_version: 1.0.0
+name: features_beta
+version: 2.3.4
+type: fake

code/go/internal/validator/testdata/packages/features_ga/changelog.yml

@@ -0,0 +1,6 @@
+# newer versions go on top
+- version: "1.2.3"
+  changes:
+    - description: Initial draft of the package
+      type: enhancement
+      link: https://github.com/elastic/package-spec/pull/341

code/go/internal/validator/testdata/packages/features_ga/manifest.yml

@@ -0,0 +1,4 @@
+format_version: 1.0.0
+name: features_ga
+version: 1.2.3
+type: fake

code/go/pkg/validator/validator_test.go

@@ -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": {
@@ -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])
 			}
 		})
 	}

test/packages/no_spec_version/manifest.yml

@@ -1,3 +1,4 @@
 # no format_version in this file
 name: mypackage
-type: integration
+type: integration
+version: 1.0.0

versions/1/changelog.yml

@@ -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