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

Add prerelease search parameter and deprecate experimental #785

Merged
merged 9 commits into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from 8 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
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Breaking changes

* Packages with major version 0 or with prerelease labels are only returned by search requests when they include `prerelease=true` or `experimental=true`. [#785](https://github.com/elastic/package-registry/pull/785)
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, do you think we should perform an exercise and check how many packages are affected because of this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Packages with prerelease tags: 1, apm-server 8.0.0-rc1
Packages with major version 0: 48, they are all marked as beta or experimental, except a couple of them. We will have to think what to do about them. The couple of ga ones should be probably versioned as 1.x.

Btw, I have also found that kibana, logstash and microsoft packages are marked as experimental, but have 1.x versions, we will have to check this too.

I can delay the merge of this till we clarify the status of these packages.

* Release level of a package without release tag is based on its semantic versioning now, previously it was experimental. [#785](https://github.com/elastic/package-registry/pull/785)
* Release level of a data stream without release tag is the same as the package that contains it, previously it was experimental. [#785](https://github.com/elastic/package-registry/pull/785)

### Bugfixes

### Added

* Add the `prerelease` parameter in search requests to include in-development versions of packages. [#785](https://github.com/elastic/package-registry/pull/785)

### Deprecated

* `experimental` parameter in search requests is deprecated. [#785](https://github.com/elastic/package-registry/pull/785)

### Known Issues

## [v1.6.0](https://github.com/elastic/package-registry/compare/v1.5.1...v1.6.0)
Expand All @@ -37,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Deprecated


### Known Issues


Expand Down
14 changes: 14 additions & 0 deletions categories.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,25 @@ func newCategoriesFilterFromQuery(query url.Values) (*packages.Filter, error) {
}
}

// Deprecated: release tags to be removed.
if v := query.Get("experimental"); v != "" {
filter.Experimental, err = strconv.ParseBool(v)
if err != nil {
return nil, fmt.Errorf("invalid 'experimental' query param: '%s'", v)
}

// For compatibility with older versions of Kibana.
if filter.Experimental {
jsoriano marked this conversation as resolved.
Show resolved Hide resolved
filter.Prerelease = true
}
}

if v := query.Get("prerelease"); v != "" {
// In case of error, keep it false
filter.Prerelease, err = strconv.ParseBool(v)
if err != nil {
return nil, fmt.Errorf("invalid 'prerelease' query param: '%s'", v)
}
}

return &filter, nil
Expand Down
6 changes: 6 additions & 0 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ func TestEndpoints(t *testing.T) {
{"/categories?experimental=true", "/categories", "categories-experimental.json", categoriesHandler(indexer, testCacheTime)},
{"/categories?experimental=foo", "/categories", "categories-experimental-error.json", categoriesHandler(indexer, testCacheTime)},
{"/categories?experimental=true&kibana.version=6.5.2", "/categories", "categories-kibana652.json", categoriesHandler(indexer, testCacheTime)},
{"/categories?prerelease=true", "/categories", "categories-prerelease.json", categoriesHandler(indexer, testCacheTime)},
{"/categories?prerelease=foo", "/categories", "categories-prerelease-error.json", categoriesHandler(indexer, testCacheTime)},
{"/categories?prerelease=true&kibana.version=6.5.2", "/categories", "categories-prerelease-kibana652.json", categoriesHandler(indexer, testCacheTime)},
{"/categories?include_policy_templates=true", "/categories", "categories-include-policy-templates.json", categoriesHandler(indexer, testCacheTime)},
{"/categories?include_policy_templates=foo", "/categories", "categories-include-policy-templates-error.json", categoriesHandler(indexer, testCacheTime)},
{"/search?kibana.version=6.5.2", "/search", "search-kibana652.json", searchHandler(indexer, testCacheTime)},
Expand All @@ -75,6 +78,9 @@ func TestEndpoints(t *testing.T) {
{"/search?experimental=true", "/search", "search-package-experimental.json", searchHandler(indexer, testCacheTime)},
{"/search?experimental=foo", "/search", "search-package-experimental-error.json", searchHandler(indexer, testCacheTime)},
{"/search?category=datastore&experimental=true", "/search", "search-category-datastore.json", searchHandler(indexer, testCacheTime)},
{"/search?prerelease=true", "/search", "search-package-prerelease.json", searchHandler(indexer, testCacheTime)},
{"/search?prerelease=foo", "/search", "search-package-prerelease-error.json", searchHandler(indexer, testCacheTime)},
{"/search?category=datastore&prerelease=true", "/search", "search-category-datastore-prerelease.json", searchHandler(indexer, testCacheTime)},
{"/favicon.ico", "", "favicon.ico", faviconHandleFunc},

// Removed flags, kept to ensure that they don't break requests from old versions.
Expand Down
2 changes: 1 addition & 1 deletion packages/datastream.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func NewDataStream(basePath string, p *Package) (*DataStream, error) {
}

if d.Release == "" {
d.Release = DefaultRelease
d.Release = p.Release
}

// Default for the enabled flags is true.
Expand Down
13 changes: 12 additions & 1 deletion packages/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ func NewPackage(basePath string, fsBuilder FileSystemBuilder) (*Package, error)
}

if p.Release == "" {
p.Release = DefaultRelease
p.Release = releaseForSemVerCompat(p.versionSemVer)
}

if !IsValidRelease(p.Release) {
Expand Down Expand Up @@ -348,6 +348,17 @@ func (p *Package) IsNewerOrEqual(pp *Package) bool {
return !p.versionSemVer.LessThan(pp.versionSemVer)
}

func (p *Package) IsPrerelease() bool {
return isPrerelease(p.versionSemVer)
}

func isPrerelease(version *semver.Version) bool {
if version.Major() < 1 {
return true
}
return version.Prerelease() != ""
}

// LoadAssets (re)loads all the assets of the package
// Based on the time when this is called, it might be that not all assets for a package exist yet, so it is reset every time.
func (p *Package) LoadAssets() (err error) {
Expand Down
21 changes: 21 additions & 0 deletions packages/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,27 @@ func TestNewPackageFromPath(t *testing.T) {
}
}

func TestIsPrerelease(t *testing.T) {
cases := []struct {
version string
prerelease bool
}{
{"0.1.0-rc1", true},
{"0.1.0", true}, // Major version 0 shouldn't be considered stable.
{"1.0.0-beta1", true},
{"1.0.0-rc.1", true},
{"1.0.0-SNAPSHOT", true},
jsoriano marked this conversation as resolved.
Show resolved Hide resolved
{"1.0.0", false},
}

for _, c := range cases {
t.Run(c.version, func(t *testing.T) {
semver := semver.MustParse(c.version)
assert.Equal(t, c.prerelease, isPrerelease(semver))
})
}
}

func BenchmarkNewPackage(b *testing.B) {
fsBuilder := func(p *Package) (PackageFileSystem, error) {
return NewExtractedPackageFileSystem(p)
Expand Down
11 changes: 10 additions & 1 deletion packages/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,13 @@ func (i *FileSystemIndexer) getPackagePaths(packagesPath string) ([]string, erro
type Filter struct {
AllVersions bool
Category string
Experimental bool
Prerelease bool
KibanaVersion *semver.Version
PackageName string
PackageVersion string

// Deprecated, release tags to be removed.
Experimental bool
}

// Apply applies the filter to the list of packages, if the filter is nil, no filtering is done.
Expand All @@ -259,6 +262,11 @@ func (f *Filter) Apply(ctx context.Context, packages Packages) Packages {
continue
}

// Skip prerelease packages by default
if p.IsPrerelease() && !f.Prerelease {
continue
}

if f.KibanaVersion != nil {
if valid := p.HasKibanaVersion(f.KibanaVersion); !valid {
continue
Expand Down Expand Up @@ -342,6 +350,7 @@ func NameVersionFilter(name, version string) GetOptions {
return GetOptions{
Filter: &Filter{
Experimental: true,
Prerelease: true,
PackageName: name,
PackageVersion: version,
},
Expand Down
18 changes: 16 additions & 2 deletions packages/releases.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,19 @@

package packages

import (
"github.com/Masterminds/semver/v3"
)

const (
ReleaseExperimental = "experimental"
ReleaseBeta = "beta"
ReleaseGa = "ga"

// Default release if no release is configured
DefaultRelease = ReleaseExperimental
DefaultLicense = "basic"
DefaultRelease = ReleaseGa
DefaultPrerelease = ReleaseBeta
DefaultLicense = "basic"
)

var ReleaseTypes = map[string]interface{}{
Expand All @@ -24,3 +29,12 @@ func IsValidRelease(release string) bool {
_, exists := ReleaseTypes[release]
return exists
}

// releaseForSemVerCompat is a compatibility function that returns a release
// for a given version.
func releaseForSemVerCompat(version *semver.Version) string {
if isPrerelease(version) {
return DefaultPrerelease
}
return DefaultRelease
}
14 changes: 14 additions & 0 deletions search.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,26 @@ func newSearchFilterFromQuery(query url.Values) (*packages.Filter, error) {
}
}

// Deprecated: release tags to be removed.
if v := query.Get("experimental"); v != "" {
// In case of error, keep it false
filter.Experimental, err = strconv.ParseBool(v)
if err != nil {
return nil, fmt.Errorf("invalid 'experimental' query param: '%s'", v)
}

// For compatibility with older versions of Kibana.
if filter.Experimental {
filter.Prerelease = true
}
}

if v := query.Get("prerelease"); v != "" {
// In case of error, keep it false
filter.Prerelease, err = strconv.ParseBool(v)
if err != nil {
return nil, fmt.Errorf("invalid 'prerelease' query param: '%s'", v)
}
}

return &filter, nil
Expand Down
2 changes: 1 addition & 1 deletion testdata/generated/categories-experimental.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
{
"id": "cloud",
"title": "Cloud",
"count": 1
"count": 2
},
{
"id": "containers",
Expand Down
32 changes: 1 addition & 31 deletions testdata/generated/categories-include-policy-templates.json
Original file line number Diff line number Diff line change
@@ -1,29 +1,9 @@
[
{
"id": "aws",
"title": "AWS",
"count": 2
},
{
"id": "azure",
"title": "Azure",
"count": 2
},
{
"id": "cloud",
"title": "Cloud",
"count": 2
},
{
"id": "compute",
"title": "compute",
"count": 2
},
{
"id": "containers",
"title": "Containers",
"count": 1
},
{
"id": "crm",
"title": "CRM",
Expand All @@ -32,23 +12,13 @@
{
"id": "custom",
"title": "Custom",
"count": 14
"count": 12
},
{
"id": "datastore",
"title": "Datastore",
"count": 2
},
{
"id": "message_queue",
"title": "Message Queue",
"count": 1
},
{
"id": "monitoring",
"title": "Monitoring",
"count": 1
},
{
"id": "web",
"title": "Web",
Expand Down
1 change: 1 addition & 0 deletions testdata/generated/categories-prerelease-error.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
invalid 'prerelease' query param: 'foo'
jsoriano marked this conversation as resolved.
Show resolved Hide resolved
32 changes: 32 additions & 0 deletions testdata/generated/categories-prerelease-kibana652.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
[
{
"id": "aws",
"title": "AWS",
"count": 1
},
{
"id": "containers",
"title": "Containers",
"count": 1
},
{
"id": "custom",
"title": "Custom",
"count": 6
},
{
"id": "message_queue",
"title": "Message Queue",
"count": 1
},
{
"id": "monitoring",
"title": "Monitoring",
"count": 1
},
{
"id": "web",
"title": "Web",
"count": 1
}
]
47 changes: 47 additions & 0 deletions testdata/generated/categories-prerelease.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
[
{
"id": "aws",
"title": "AWS",
"count": 2
},
{
"id": "azure",
"title": "Azure",
"count": 1
},
{
"id": "cloud",
"title": "Cloud",
"count": 2
},
{
"id": "containers",
"title": "Containers",
"count": 1
},
{
"id": "crm",
"title": "CRM",
"count": 1
},
{
"id": "custom",
"title": "Custom",
"count": 14
},
{
"id": "message_queue",
"title": "Message Queue",
"count": 1
},
{
"id": "monitoring",
"title": "Monitoring",
"count": 1
},
{
"id": "web",
"title": "Web",
"count": 3
}
]
Loading