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

Merged
merged 4 commits into from
Jul 14, 2023

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented Jul 12, 2023

Fixes #1051

This PR adds a new filter parameter capabilities for the search endpoint.

Current behavior when capabilities is defined:

  • If package has no capabilities defined
    • package can be added to the response.
  • If package has capabilities defined:
    • package can be added to the response if all defined capabilities in the package manifest are present in the capabilities query parameter.

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

How to test

  1. Build elastic-package using the latest changeset from package-spec
    cd /path/to/elastic-package
    go mod edit -replace github.com/elastic/package-spec/v2=github.com/elastic/package-spec/v2@72f19e9
    go mod tidy
    make build
  2. Update some test packages to include capabilities in its manifest
    • It requires to be format_version >= 2.10.0
    • Update also version in manifest and changelog (e.g. 0.0.9), so it is easier to check when querying the Elastic Package Registry.
    • As an example, elastic_package_registry could be modified as:
     $ git diff
    diff --git packages/elastic_package_registry/manifest.yml packages/elastic_package_registry/manifest.yml
    index b8a41f9b5..0d819ff0e 100644
    --- packages/elastic_package_registry/manifest.yml
    +++ packages/elastic_package_registry/manifest.yml
    @@ -1,4 +1,4 @@
    -format_version: 1.0.0
    +format_version: 2.10.0
     name: elastic_package_registry
     title: "Elastic Package Registry"
     version: 0.0.7
    @@ -9,6 +9,9 @@ categories:
     conditions:
       kibana.version: "^8.0.0"
       elastic.subscription: basic
    +  elastic.capabilities:
    +    - observability
    +    - security
     icons:
       - src: /img/elastic_package_registry-logo.svg
         title: Elastic Package Registry logo
  3. Build the package
    elastic-package build
  4. Build a new local docker image for package-registry
    cd /path/to/package-registry   
    docker build -t docker.elastic.co/package-registry/package-registry:v1.21.0-rc1 .
  5. Run local package-registry using the packages directory with the built packages as a volume mount point and using the Proxy feature:
    docker run --rm -p 8080:8080 -e EPR_FEATURE_PROXY_MODE=true -e EPR_PROXY_TO=https://epr.elastic.co -v /home/mariorodriguez/Coding/work/integrations/build/packages/:/packages/package-registry -it docker.elastic.co/package-registry/package-registry:v1.21.0-rc1
  6. Execute some queries against this local package-registry:
    # elastic_package_registry package should not be in the response
    # - no capabilities matching
    curl -s "http://localhost:8080/search?capabilities=uptime&prerelease=true" | jq -r '.[]|"\(.name) - \(.version) \(.conditions.elastic.capabilities)"'
    # - missing `security` capability
    curl -s "http://localhost:8080/search?capabilities=observability&prerelease=true" | jq -r '.[]|"\(.name) - \(.version) \(.conditions.elastic.capabilities)"'
    
    # elastic_package_registry package should be in the response
    # - all the capabilities defined in the package are in the `capabilities` query parameter
    curl -s "http://localhost:8080/search?capabilities=observability,security,uptime&prerelease=true"  | jq -r '.[]|"\(.name) - \(.version) \(.conditions.elastic.capabilities)"'
    curl -s "http://localhost:8080/search?capabilities=observability,security&prerelease=true"  | jq -r '.[]|"\(.name) - \(.version) \(.conditions.elastic.capabilities)"'

@mrodm mrodm self-assigned this Jul 12, 2023
@elasticmachine
Copy link

elasticmachine commented Jul 12, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-07-12T13:21:19.106+0000

  • Duration: 10 min 28 sec

Test stats 🧪

Test Results
Failed 0
Passed 502
Skipped 0
Total 502

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

Comment on lines 397 to 398
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

packages/packages_test.go Outdated Show resolved Hide resolved
packages/package.go Outdated Show resolved Hide resolved
@@ -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.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support search based on capabilities
3 participants