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.

Sign up for GitHub

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 capabilities parameter in search endpoint #1054

Merged
merged 4 commits into from
Jul 14, 2023
Merged

Add capabilities parameter in search endpoint #1054

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c290aed
Add capabilities parameter in search endpoint
mrodm Jul 12, 2023
8b75ff8
Add changelog entry
mrodm Jul 12, 2023
8854475
Add package if all its capabilities are included in the query value
mrodm Jul 13, 2023
2916cd0
Merge remote-tracking branch 'upstream/main' into add_capabilities_se…
mrodm Jul 13, 2023
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

* Add new query parameter "capabilities" in search endpoint [#1054](https://github.com/elastic/package-registry/pull/1054)

### Deprecated

### Known Issues
Expand Down
17 changes: 16 additions & 1 deletion packages/package.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ type KibanaConditions struct {

// ElasticConditions defines conditions related to Elastic subscriptions or partnerships.
type ElasticConditions struct {
Subscription string `config:"subscription" json:"subscription" yaml:"subscription"`
Subscription string `config:"subscription" json:"subscription" yaml:"subscription"`
Capabilities []string `config:"capabilities,omitempty" json:"capabilities,omitempty" yaml:"capabilities,omitempty"`
}

type Version struct {
Expand Down Expand Up @@ -387,6 +388,20 @@ func (p *Package) HasKibanaVersion(version *semver.Version) bool {
return p.Conditions.Kibana.constraint.Check(version)
}

func (p *Package) HasCapabilities(capabilities []string) bool {
mrodm marked this conversation as resolved.
Show resolved Hide resolved
if p.Conditions == nil || p.Conditions.Elastic == nil || p.Conditions.Elastic.Capabilities == nil || capabilities == nil {
return true
}

// TODO if there are several capabilities in the query, should all of them match with the ones defined in the package ?
for _, c := range capabilities {
if !util.StringsContains(p.Conditions.Elastic.Capabilities, c) {
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be the other way around. For each capability in the package, there should be a matching one in the declared capabilities.

Suggested change
for _, c := range capabilities {
if !util.StringsContains(p.Conditions.Elastic.Capabilities, c) {
for _, requiredCapability := range p.Conditions.Elastic.Capabilities {
if !util.StringsContains(capabilities, requiredCapabilities)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So all the capabilities defined in the package must be present in the query in order to be added to the response ? @jsoriano

Copy link
Member

Choose a reason for hiding this comment

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

Yes. Clients (aka Kibana) will indicate all the capabilities they support. We should only return packages whose required capabilities are satisfied in this list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated logic to do it like this.
Example in the description has also been updated taken into account this logic.

For the latter scenario, let's consider the following capabilities for a given package:

conditions:
  elastic.capabilities:
    - observability
    - uptime

here it is shown if the package could be added to the response or not (let's consider that there are no other filters):

Query parameter Package can be added
No capability in the query yes
capabilities=observability no
capabilities=observability,security no
capabilities=observability,uptime yes
capabilities=observability,uptime,security yes

return false
}
}
return true
}

func (p *Package) IsNewerOrEqual(pp *Package) bool {
return !p.versionSemVer.LessThan(pp.versionSemVer)
}
Expand Down
13 changes: 13 additions & 0 deletions packages/packages.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ type Filter struct {
PackageName string
PackageVersion string
PackageType string
Capabilities []string

// Deprecated, release tags to be removed.
Experimental bool
Expand Down Expand Up @@ -342,6 +343,12 @@ func (f *Filter) Apply(ctx context.Context, packages Packages) Packages {
continue
}

if f.Capabilities != nil {
if valid := p.HasCapabilities(f.Capabilities); !valid {
continue
}
}

addPackage := true
if !f.AllVersions {
// Check if the version exists and if it should be added or not.
Expand Down Expand Up @@ -405,6 +412,12 @@ func (f *Filter) legacyApply(ctx context.Context, packages Packages) Packages {
continue
}

if f.Capabilities != nil {
if valid := p.HasCapabilities(f.Capabilities); !valid {
continue
}
}

addPackage := true
if !f.AllVersions {
// Check if the version exists and if it should be added or not.
Expand Down
119 changes: 111 additions & 8 deletions packages/packages_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,36 @@ func TestPackagesFilter(t *testing.T) {
Type: "integration",
KibanaVersion: "^8.0.0",
},
{
Name: "obs_package",
Version: "1.1.0",
Type: "integration",
Capabilities: []string{"observability"},
},
{
Name: "obs_sec_package",
Version: "1.0.0",
Type: "integration",
Capabilities: []string{"observability", "security"},
},
{
Name: "obs_sec_package",
Version: "2.0.0-rc1",
Type: "integration",
Capabilities: []string{"observability", "security"},
},
{
Name: "obs_sec_package",
Version: "2.0.0",
Type: "integration",
Capabilities: []string{"observability", "security"},
},
{
Name: "obs_sec_uptime_package",
Version: "2.0.0",
Type: "integration",
Capabilities: []string{"observability", "security", "uptime"},
},
}
packages := buildFilterTestPackages(filterTestPackages)

Expand Down Expand Up @@ -164,6 +194,9 @@ func TestPackagesFilter(t *testing.T) {
{Name: "apache", Version: "1.0.0"},
{Name: "nginx", Version: "2.0.0"},
{Name: "redisenterprise", Version: "1.0.0"},
{Name: "obs_package", Version: "1.1.0"},
{Name: "obs_sec_package", Version: "2.0.0"},
{Name: "obs_sec_uptime_package", Version: "2.0.0"},
},
},
{
Expand Down Expand Up @@ -231,6 +264,7 @@ func TestPackagesFilter(t *testing.T) {
filterTestPackage{Name: "apache", Version: "1.0.0-rc1"},
filterTestPackage{Name: "apache", Version: "2.0.0-rc2"},
filterTestPackage{Name: "redisenterprise", Version: "0.1.1"},
filterTestPackage{Name: "obs_sec_package", Version: "2.0.0-rc1"},
),
},
{
Expand Down Expand Up @@ -350,6 +384,60 @@ func TestPackagesFilter(t *testing.T) {
{Name: "etcd", Version: "1.0.0-rc2"},
},
},
{
Title: "non existing capabilities search",
Copy link
Member

Choose a reason for hiding this comment

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

👍

Filter: Filter{
Capabilities: []string{"no_match"},
},
Expected: []filterTestPackage{
{Name: "apache", Version: "1.0.0"},
{Name: "nginx", Version: "2.0.0"},
{Name: "redisenterprise", Version: "1.0.0"},
},
},
{
Title: "observability capabilities search",
Filter: Filter{
Capabilities: []string{"observability"},
},
Expected: []filterTestPackage{
{Name: "apache", Version: "1.0.0"},
{Name: "nginx", Version: "2.0.0"},
{Name: "redisenterprise", Version: "1.0.0"},
{Name: "obs_package", Version: "1.1.0"},
{Name: "obs_sec_package", Version: "2.0.0"},
{Name: "obs_sec_uptime_package", Version: "2.0.0"},
mrodm marked this conversation as resolved.
Show resolved Hide resolved
},
},
{
Title: "observability and security capabilities search",
Filter: Filter{
Capabilities: []string{"observability", "security"},
},
Expected: []filterTestPackage{
{Name: "apache", Version: "1.0.0"},
{Name: "nginx", Version: "2.0.0"},
{Name: "redisenterprise", Version: "1.0.0"},
{Name: "obs_sec_package", Version: "2.0.0"},
{Name: "obs_sec_uptime_package", Version: "2.0.0"},
},
},
{
Title: "observability, security and uptime capabilities search - legacy kibana",
Filter: Filter{
Experimental: true,
Capabilities: []string{"observability", "security", "uptime"},
},
Expected: []filterTestPackage{
{Name: "apache", Version: "1.0.0"},
{Name: "nginx", Version: "2.0.0"},
{Name: "mysql", Version: "0.9.0"},
{Name: "logstash", Version: "1.1.0"},
{Name: "etcd", Version: "1.0.0-rc2"},
{Name: "redisenterprise", Version: "1.0.0"},
{Name: "obs_sec_uptime_package", Version: "2.0.0"},
},
},
}

for _, c := range cases {
Expand All @@ -366,6 +454,7 @@ type filterTestPackage struct {
Release string
Type string
KibanaVersion string
Capabilities []string
}

func (p filterTestPackage) Build() *Package {
Expand All @@ -377,15 +466,29 @@ func (p filterTestPackage) Build() *Package {
build.Release = p.Release
build.Type = p.Type

constraints, err := semver.NewConstraint(p.KibanaVersion)
if err != nil {
panic(err)
if p.KibanaVersion != "" {
constraints, err := semver.NewConstraint(p.KibanaVersion)
if err != nil {
panic(err)
}
build.Conditions = &Conditions{
Kibana: &KibanaConditions{
Version: p.KibanaVersion,
constraint: constraints,
},
}
}
build.Conditions = &Conditions{
Kibana: &KibanaConditions{
Version: p.KibanaVersion,
constraint: constraints,
},
if p.Capabilities != nil {
elasticConditions := ElasticConditions{
Capabilities: p.Capabilities,
}
if build.Conditions != nil {
build.Conditions.Elastic = &elasticConditions
} else {
build.Conditions = &Conditions{
Elastic: &elasticConditions,
}
}
}
return &build
}
Expand Down
5 changes: 5 additions & 0 deletions search.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/url"
"sort"
"strconv"
"strings"
"time"

"github.com/Masterminds/semver/v3"
Expand Down Expand Up @@ -97,6 +98,10 @@ func newSearchFilterFromQuery(query url.Values) (*packages.Filter, error) {
filter.PackageType = v
}

if v := query.Get("capabilities"); v != "" {
filter.Capabilities = strings.Split(v, ",")
}

if v := query.Get("all"); v != "" {
// Default is false, also on error
filter.AllVersions, err = strconv.ParseBool(v)
Expand Down