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

File delivery #2666

Merged
merged 21 commits into from Jun 9, 2023
Merged

File delivery #2666

merged 21 commits into from Jun 9, 2023

Conversation

pzl
Copy link
Member

@pzl pzl commented Jun 2, 2023

What is the problem this PR solves?

Delivering user-uploaded files (via kibana) to integrations (specifically, endpoint use-case)

How does this PR solve the problem?

an API route was added, /api/fleet/file/[ID]. Authenticated calls to this will locate file metadata documents in elasticsearch for the presence of a file, and that the authenticated agent is present in the file's file.Meta.target_agents field (i.e. only allowed client IDs can download). Fleet server iterates through the file contents to send on the response pipe

How to test this PR locally

Alternatively, a simpler setup would be to check out this PR, and modify the authentication section in internal/pkg/api/handleFileDelivery.go line 48. Comment out the authAgent call (or otherwise mock it), and put a different, perhaps static ID as the last parameter to the FindFileForAgent call a few lines down (e.g. ... fileID, "mockID")). As long as a file is uploaded to the right indices (.fleet-filedelivery-meta-endpoint and .fleet-filedelivery-data-endpoint) with that matching agent_id in the target_agents field, then a command-line curl, wget or similar to https://localhost:8221/api/fleet/file/[ID] should retrieve the file like any other file-based HTTP url.

Design Checklist

  • I have ensured my design is stateless and will work when multiple fleet-server instances are behind a load balancer.
  • I have or intend to scale test my changes, ensuring it will work reliably with 100K+ agents connected.
  • I have included fail safe mechanisms to limit the load on fleet-server: rate limiting, circuit breakers, caching, load shedding, etc.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in ./changelog/fragments using the changelog tool

Related issues

@pzl pzl added the enhancement New feature or request label Jun 2, 2023
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 2, 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-06-09T19:23:13.075+0000

  • Duration: 42 min 53 sec

Test stats 🧪

Test Results
Failed 0
Passed 706
Skipped 1
Total 707

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@pzl pzl marked this pull request as ready for review June 5, 2023 17:13
@pzl pzl requested a review from a team as a code owner June 5, 2023 17:13
@mergify
Copy link
Contributor

mergify bot commented Jun 6, 2023

This pull request is now in conflicts. Could you fix it @pzl? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b file-delivery upstream/file-delivery
git merge upstream/main
git push upstream file-delivery

Copy link
Contributor

@michel-laterman michel-laterman left a comment

Choose a reason for hiding this comment

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

looks good so far.

Can you also add a test case for the e2e tests? (or a follow up item to do so)

model/openapi.yml Outdated Show resolved Hide resolved
internal/pkg/api/api.go Show resolved Hide resolved
internal/pkg/file/delivery/delivery.go Outdated Show resolved Hide resolved
internal/pkg/file/delivery/delivery.go Outdated Show resolved Hide resolved
internal/pkg/file/es.go Outdated Show resolved Hide resolved
@michel-laterman michel-laterman added the Team:Fleet Label for the Fleet team label Jun 7, 2023
@kevinlog
Copy link

kevinlog commented Jun 8, 2023

I built this and ran Fleet Server locally with a Kibana stack to test everything e2e and it worked!

I can upload multi-chunk files and Endpoint will be able to download them from ES through this API.

In use in Response Console:
image

The file ends up on the host in the expected folder from Endpoint and is executable by root:
image

from a functionality POV, LGTM

Copy link
Contributor

@michel-laterman michel-laterman left a comment

Choose a reason for hiding this comment

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

LGTM.

Please follow it up with e2e tests. you can ping me if you need help writing them

@pzl
Copy link
Member Author

pzl commented Jun 9, 2023

Thanks, adding plenty of tests in a followup. Going to merge this to get builds going over the weekend, for other teams to start integrating

@pzl pzl enabled auto-merge (squash) June 9, 2023 19:31
@pzl pzl merged commit 3701864 into elastic:main Jun 9, 2023
17 of 18 checks passed
@pzl pzl deleted the file-delivery branch June 9, 2023 20:23
@pzl pzl mentioned this pull request Jun 13, 2023
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Team:Fleet Label for the Fleet team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants