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

[Fleet] Allow uploading zip packages in integrations UI #148599

Closed
7 tasks done
jen-huang opened this issue Jan 9, 2023 · 22 comments
Closed
7 tasks done

[Fleet] Allow uploading zip packages in integrations UI #148599

jen-huang opened this issue Jan 9, 2023 · 22 comments
Assignees
Labels
enhancement New value added to drive a business result Team:Fleet Team label for Observability Data Collection Fleet team

Comments

@jen-huang
Copy link
Contributor

jen-huang commented Jan 9, 2023

Supersedes #70582

Today we have the endpoint POST /api/fleet/epm/packages which accepts a zip package archive and uploads it to Fleet in the same way that a regular package install would. This feature is important to reduce the total delivery time for package developers. While much of the functionality is complete through enabling bundled packages with Kibana, there are still some outstanding tasks before this API is ready for documentation and wider use:

  • The package policy editor doesn't use the streams from the uploaded package if another version of the package also exists in the registry (original report)
  • Ensure uploaded package can be fully uninstalled
  • Ensure uploaded package can be fully overwritten (similar to a re-install)
  • Ensure uploaded package does not conflict with bundled or registry packages (whether they are installed or not)
  • Robust unit tests around the code path (installPackageByUpload, parseAndVerifyArchive)
  • Improved integration tests
  • OpenAPI documentation

Sample test instructions can be found at elastic/integrations#4597:

  1. Download elasticsearch-1.1.0-preview1.zip
  2. Upload the package to Kibana:
curl -XPOST -H 'content-type: application/zip' -H 'kbn-xsrf: true' http://my-kibana:5601/api/fleet/epm/packages -u elastic:changeme --data-binary @elasticsearch-1.1.0-preview1.zip

Related enhancements that can be done once the API is finished:

@jen-huang jen-huang added enhancement New value added to drive a business result Team:Fleet Team label for Observability Data Collection Fleet team labels Jan 9, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@mukeshelastic mukeshelastic changed the title [Fleet] Finish upload package API [Fleet] Allow uploading zip packages in integrations UI Jan 24, 2023
@jlind23 jlind23 assigned juliaElastic and hop-dev and unassigned juliaElastic and hop-dev Jan 31, 2023
@juliaElastic
Copy link
Contributor

juliaElastic commented Feb 6, 2023

After reviewing the issue, the linked improvements in #70582 are still needed. Started working on them.

Can't reproduce this issue:

This one I'll hold off on, as it is a change in package-spec repo.

@juliaElastic
Copy link
Contributor

@jen-huang @kpollich @jsoriano Do we have an example package that is not yet in the registry? The elasticsearch package mentioned in the description is already there. I would need one to test some scenarios where the package is not in registry.

@hop-dev
Copy link
Contributor

hop-dev commented Feb 6, 2023

@juliaElastic you could use elastic-package build --zip -v to build one of our test packages in x-pack/test/fleet_api_integration/apis/fixtures if that works? I know alot of them are not 100% valid per the package spec but they should be good enough

@juliaElastic
Copy link
Contributor

@hop-dev it worked, thanks.

@juliaElastic
Copy link
Contributor

juliaElastic commented Feb 6, 2023

@jen-huang @kpollich I am wondering if this issue is still relevant: #82010 Not sure how to reproduce a cache miss. Do any of you know? I'm looking at the uploaded packages after kibana restart in local, and don't see any issues.
I think this code was refactored since the issue was raised, and we fetch the info from source if missing in cache. If you agree, we can close this one.

image

@kpollich
Copy link
Member

kpollich commented Feb 6, 2023

I think the above is likely no longer relevant. From what I remember, all installation paths add packages to the in-memory cache as expected. I'm going to close that issue as I don't think it's relevant to the current state of the EPM codebase.

juliaElastic added a commit that referenced this issue Feb 7, 2023
## Summary

Related to #148599
Fixes #81995

Changed `GET /epm/packages` API to list uploaded packages as well that
are not in registry.

Testing steps:
- Built a test package that is not in registry from kibana repo:
```
x-pack/test/fleet_api_integration/apis/fixtures/package_verification/packages/src/verified-1.0.0
elastic-package build --zip -v
```
- Uploaded the resulting zip
[verified-1.0.0.zip](https://github.com/elastic/kibana/files/10664094/verified-1.0.0.zip)
```
curl -XPOST -H 'content-type: application/zip' -H 'kbn-xsrf: true' http://localhost:5601/julia/api/fleet/epm/packages -u elastic:changeme --data-binary @verified-1.0.0.zip
```
- Navigate to `Integrations` page, verify that the `Verified` package is
displayed under `Browse` / `Installed` integrations, and the details are
visible when clicking on the package.

<img width="639" alt="image"
src="https://user-images.githubusercontent.com/90178898/217000305-90c28293-bdfa-42d5-b30b-50f7a63aad11.png">
<img width="757" alt="image"
src="https://user-images.githubusercontent.com/90178898/217000420-208d3b90-5341-4f1f-8ab8-554f78821bbf.png">
<img width="1147" alt="image"
src="https://user-images.githubusercontent.com/90178898/217000541-06324345-4204-43cc-8411-6c961ec37d17.png">


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
darnautov pushed a commit to darnautov/kibana that referenced this issue Feb 7, 2023
…0332)

## Summary

Related to elastic#148599
Fixes elastic#81995

Changed `GET /epm/packages` API to list uploaded packages as well that
are not in registry.

Testing steps:
- Built a test package that is not in registry from kibana repo:
```
x-pack/test/fleet_api_integration/apis/fixtures/package_verification/packages/src/verified-1.0.0
elastic-package build --zip -v
```
- Uploaded the resulting zip
[verified-1.0.0.zip](https://github.com/elastic/kibana/files/10664094/verified-1.0.0.zip)
```
curl -XPOST -H 'content-type: application/zip' -H 'kbn-xsrf: true' http://localhost:5601/julia/api/fleet/epm/packages -u elastic:changeme --data-binary @verified-1.0.0.zip
```
- Navigate to `Integrations` page, verify that the `Verified` package is
displayed under `Browse` / `Installed` integrations, and the details are
visible when clicking on the package.

<img width="639" alt="image"
src="https://user-images.githubusercontent.com/90178898/217000305-90c28293-bdfa-42d5-b30b-50f7a63aad11.png">
<img width="757" alt="image"
src="https://user-images.githubusercontent.com/90178898/217000420-208d3b90-5341-4f1f-8ab8-554f78821bbf.png">
<img width="1147" alt="image"
src="https://user-images.githubusercontent.com/90178898/217000541-06324345-4204-43cc-8411-6c961ec37d17.png">


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
@juliaElastic
Copy link
Contributor

juliaElastic commented Feb 7, 2023

Tested the Uninstall of uploaded packages and seems to work fine.

@kpollich What is the expected behavior of the Reinstall button on the UI for uploaded packages?
Now the button calls the POST /epm/packages API that calls the installPackageFromRegistry function.

We could potentially add a drag and drop feature to upload the zip file again on the UI, though this is out of scope for this issue I think. Should we make the button disabled with a tooltip for uploaded packages?

image

@kpollich
Copy link
Member

kpollich commented Feb 7, 2023

We could potentially add a drag and drop feature to upload the zip file again on the UI, though this is out of scope for this issue I think. Should we make the button disabled with a tooltip for uploaded packages?

Agreed on a both. A dropzone implementation could be nice, but seems needless for the corner case of reinstalling an uploaded package. Let's go forward with the disabled button + tooltip stating

This integration was installed by upload and cannot be automatically reinstalled. Please upload it again to reinstall.

benakansara pushed a commit to benakansara/kibana that referenced this issue Feb 7, 2023
…0332)

## Summary

Related to elastic#148599
Fixes elastic#81995

Changed `GET /epm/packages` API to list uploaded packages as well that
are not in registry.

Testing steps:
- Built a test package that is not in registry from kibana repo:
```
x-pack/test/fleet_api_integration/apis/fixtures/package_verification/packages/src/verified-1.0.0
elastic-package build --zip -v
```
- Uploaded the resulting zip
[verified-1.0.0.zip](https://github.com/elastic/kibana/files/10664094/verified-1.0.0.zip)
```
curl -XPOST -H 'content-type: application/zip' -H 'kbn-xsrf: true' http://localhost:5601/julia/api/fleet/epm/packages -u elastic:changeme --data-binary @verified-1.0.0.zip
```
- Navigate to `Integrations` page, verify that the `Verified` package is
displayed under `Browse` / `Installed` integrations, and the details are
visible when clicking on the package.

<img width="639" alt="image"
src="https://user-images.githubusercontent.com/90178898/217000305-90c28293-bdfa-42d5-b30b-50f7a63aad11.png">
<img width="757" alt="image"
src="https://user-images.githubusercontent.com/90178898/217000420-208d3b90-5341-4f1f-8ab8-554f78821bbf.png">
<img width="1147" alt="image"
src="https://user-images.githubusercontent.com/90178898/217000541-06324345-4204-43cc-8411-6c961ec37d17.png">


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
juliaElastic added a commit that referenced this issue Feb 8, 2023
…150444)

## Summary

Related to #148599

Closes #82007

Factored out common logic in `installPackageFromRegistry` and
`installPackageByUpload` to a new function. This improves the upload
flow as well to handle install failure.

We might want to introduce a `force` flag for the upload API to
reinstall a package, now the logic does not do anything if the installed
version package is uploaded again.
EDIT: found [here](#82007) that
upload should work with an implicit `force` flag, so will update that.

More manual and automated tests needed.


### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
@juliaElastic
Copy link
Contributor

juliaElastic commented Feb 8, 2023

@joshdover I tried to reproduce the issue reported here on 8.7 but couldn't:

The package policy editor doesn't use the streams from the uploaded package if another version of the package also exists in the registry

I uploaded the elasticsearch-1.3.0-preview1 package from the linked pr and I'm seeing the new data stream on the Package Policy Create UI. Tested the Edit and Upgrade package policy forms as well, looks good.
Did you see the issue in a different scenario?

image

@joshdover
Copy link
Contributor

@juliaElastic it must be fixed, I was testing this against 8.5 at the time which I think was before some of the changes we did for input packages were in place that may have fixed this.

juliaElastic added a commit that referenced this issue Feb 8, 2023
## Summary

Disable Reinstall button and added tooltip to prevent clicking Reinstall
for uploaded packages.

Related to
#148599 (comment)

<img width="731" alt="image"
src="https://user-images.githubusercontent.com/90178898/217497440-e36bad4d-d4dc-4d1c-b434-b17f0a4ead0a.png">



### Checklist


- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
juliaElastic added a commit that referenced this issue Feb 10, 2023
## Summary

Related to #148599

Added more integration tests.

Added openapi spec.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Feb 10, 2023
## Summary

Related to elastic#148599

Added more integration tests.

Added openapi spec.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 486d56b)
kibanamachine added a commit that referenced this issue Feb 10, 2023
# Backport

This will backport the following commits from `main` to `8.7`:
- [[Fleet] added more upload tests and openapi
(#150743)](#150743)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Julia
Bardi","email":"90178898+juliaElastic@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-02-10T12:07:23Z","message":"[Fleet]
added more upload tests and openapi (#150743)\n\n##
Summary\r\n\r\nRelated to
#148599 more
integration tests.\r\n\r\nAdded openapi spec.\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"486d56b33fcb137f6f2dc62a13acd58ea19a1e00","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","Team:Fleet","v8.7.0","v8.8.0"],"number":150743,"url":"#150743
added more upload tests and openapi (#150743)\n\n##
Summary\r\n\r\nRelated to
#148599 more
integration tests.\r\n\r\nAdded openapi spec.\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"486d56b33fcb137f6f2dc62a13acd58ea19a1e00"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"#150743
added more upload tests and openapi (#150743)\n\n##
Summary\r\n\r\nRelated to
#148599 more
integration tests.\r\n\r\nAdded openapi spec.\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"486d56b33fcb137f6f2dc62a13acd58ea19a1e00"}}]}]
BACKPORT-->

Co-authored-by: Julia Bardi <90178898+juliaElastic@users.noreply.github.com>
juliaElastic added a commit that referenced this issue Feb 10, 2023
## Summary

Related to #148599
Added unit tests on parse package archive logic

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Feb 10, 2023
## Summary

Related to elastic#148599
Added unit tests on parse package archive logic

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 3aca0c2)
kibanamachine added a commit that referenced this issue Feb 10, 2023
… (#150908)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[Fleet] added unit tests on parse package archive logic
(#150888)](#150888)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Julia
Bardi","email":"90178898+juliaElastic@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-02-10T15:54:27Z","message":"[Fleet]
added unit tests on parse package archive logic (#150888)\n\n##
Summary\r\n\r\nRelated to
#148599 unit tests on
parse package archive logic\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"3aca0c2212d4caa16d8539e934210849d813aad1","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","backport:prev-minor","v8.8.0"],"number":150888,"url":"#150888
added unit tests on parse package archive logic (#150888)\n\n##
Summary\r\n\r\nRelated to
#148599 unit tests on
parse package archive logic\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"3aca0c2212d4caa16d8539e934210849d813aad1"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"#150888
added unit tests on parse package archive logic (#150888)\n\n##
Summary\r\n\r\nRelated to
#148599 unit tests on
parse package archive logic\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"3aca0c2212d4caa16d8539e934210849d813aad1"}}]}]
BACKPORT-->

Co-authored-by: Julia Bardi <90178898+juliaElastic@users.noreply.github.com>
@juliaElastic
Copy link
Contributor

Tested uploads with elasticsearch package, that has latest 1.2.0 GA version in registry, initially the package was not installed.

  • uploaded version 1.3.0-preview1
  • uploaded an older 1.0.0 version (downgrade)
  • upgraded uploaded package to latest 1.2.0 registry version
  • uploaded again version 1.3.0-preview1

image

image

image

@juliaElastic
Copy link
Contributor

The API work is done, so closing the issue.
Planning to investigate gzip support separately.

@juliaElastic
Copy link
Contributor

juliaElastic commented Feb 13, 2023

I tried uploading a tar.gz file (compressed an extracted zip package) and it was successful. There is also an integration test with application/gzip content type.

tar -czf verified-1.0.0.tar.gz verified-1.0.0/
curl -XPOST -H 'content-type: application/gzip' -H 'kbn-xsrf: true' http://localhost:5601/julia/api/fleet/epm/packages -u elastic:changeme --data-binary @verified-1.0.0.tar.gz

// response:
{"items":[{"id":"logs-verified.log-1.0.0","type":"ingest_pipeline"},{"id":"logs-verified.log","type":"index_template"},{"id":"logs-verified.log@package","type":"component_template"},{"id":"logs-verified.log@custom","type":"component_template"}],"response":[{"id":"logs-verified.log-1.0.0","type":"ingest_pipeline"},{"id":"logs-verified.log","type":"index_template"},{"id":"logs-verified.log@package","type":"component_template"},{"id":"logs-verified.log@custom","type":"component_template"}],"_meta":{"install_source":"upload"}}%  

Support gzip in addition to zip (as of last testing, seems gzip has some issues)

@kpollich Could you let me know how to reproduce the issues mentioned in the description?

Another thing, I noticed that the upload package logic skips verification. Is this something we want to support in the future? If so, might be worth creating a follow up issue.

@kpollich
Copy link
Member

@kpollich Could you let me know how to reproduce the issues mentioned in the description?

It's been a very long time, so I'm not sure. If gzip functionality is working in recent tests, then I'd expect work we've done to archive parsing logic in recent sprints/releases has resolved whatever issues we had initially.

Another thing, I noticed that the upload package logic skips verification. Is this something we want to support in the future? If so, might be worth creating a follow up issue.

Yes, it'd be great to support verification of uploaded packages if possible. I agree that a follow up issue would be appropriate here. I suppose we'll have to think about how to provide a signature file along with an uploaded package, right?

@juliaElastic
Copy link
Contributor

Yes, it'd be great to support verification of uploaded packages if possible. I agree that a follow up issue would be appropriate here. I suppose we'll have to think about how to provide a signature file along with an uploaded package, right?

Yes, we might have to change the API to support multiple files or create another API endpoint to verify an uploaded package. If we eventually add the elastic-package support for this, it would be easier for users to send the signature as well, even if there are more than one underlying API calls.

@kpollich
Copy link
Member

I think having the upload API support multiple files would be ideal, but not sure on how much work that'd actually be. A separate API for uploading signatures would be a fallback.

Another option could be to allow for a different archive directory structure that includes signature files at the root of the archive. We'd have to have some conditional logic that determines whether this is the case, though, because the current validation logic expects only a single top-level directory I believe.

@joshdover
Copy link
Contributor

Another option could be to allow for a different archive directory structure that includes signature files at the root of the archive. We'd have to have some conditional logic that determines whether this is the case, though, because the current validation logic expects only a single top-level directory I believe.

I don't think this would be feasible since the content of the signature itself depends on the content of the zip.

+1 to supporting multiple files on the same API call. This is supported by multipart content-type I believe

@nimarezainia
Copy link
Contributor

@kpollich & @juliaElastic is there a docs issue for this feature?

@juliaElastic
Copy link
Contributor

@nimarezainia Not yet, perhaps we could add a section in the docs about how to upload packages around here: https://www.elastic.co/guide/en/fleet/master/air-gapped.html#air-gapped-diy-epr
@kpollich WDYT?

@nimarezainia
Copy link
Contributor

Thanks @juliaElastic. if there was a docs issue with enough details for the documentation to follow that would be great. Regarding the location where that content resides I would leave it to their expertise.

(fyi @dedemorton & @karenzone )

@dedemorton
Copy link
Contributor

@juliaElastic If this is an alternative to users hosting their own package registry, then, yes, it should be documented in the topic about air gapped environments. To get this in quickly, I would suggest opening a PR with the doc changes and tagging @elastic/ingest-docs for review and technical edits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

No branches or pull requests

8 participants