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

Support package signatures #760

Merged
merged 7 commits into from
Oct 25, 2021

Conversation

mtojek
Copy link
Contributor

@mtojek mtojek commented Oct 21, 2021

Issue: #728

This PR introduces support for package signatures. A signature file will be a simple hash and will be exposed through the API.

@mtojek mtojek self-assigned this Oct 21, 2021
@elasticmachine
Copy link

elasticmachine commented Oct 21, 2021

💚 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: 2021-10-25T12:26:55.659+0000

  • Duration: 5 min 35 sec

  • Commit: 6fa5ee7

Test stats 🧪

Test Results
Failed 0
Passed 128
Skipped 0
Total 128

🤖 GitHub comments

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

  • /test : Re-trigger the build.

@mtojek mtojek requested a review from jsoriano October 21, 2021 09:39
@@ -19,6 +19,7 @@
"crm",
"azure"
],
"signature": "e16ddaf4f91df524b27bf4f2e4b1ac09",
Copy link
Member

Choose a reason for hiding this comment

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

Not sure about the best option, but have you considered providing a download path instead?

Suggested change
"signature": "e16ddaf4f91df524b27bf4f2e4b1ac09",
"signature": "/epr/example/example-1.0.1.zip.sig",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about this, but it means that Kibana will have to pull another file (another GET call). Not sure which approach is preferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, used signature_path.

@@ -0,0 +1 @@
e16ddaf4f91df524b27bf4f2e4b1ac09
Copy link
Member

Choose a reason for hiding this comment

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

Is this the final format of the signature? What kind of hash is this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

md5(elastic)

It's just a mock as the EPR doesn't enforce any hash form, it will depend on the internal logic on the CI side, unless we want to document and define it also here.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, no need to enter into details here, but I am wondering about the more convenient form to distribute different signatures (also related to my other question about providing a download path). For example gpg signatures are quite longer and usually distributed as files. Would we still want to include them in the package index?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a similar problem we had with template vs template_path in data stream's manifest and we ended up with template_path as these files are long.

Definitely a signature_path would be more human readable than JSON index with signature blobs.

I will adjust the implementation then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, used signature_path.

packages/signature.go Outdated Show resolved Hide resolved
@mtojek mtojek marked this pull request as draft October 25, 2021 11:58
@mtojek
Copy link
Contributor Author

mtojek commented Oct 25, 2021

Back to draft as it didn't expose signature files as static resources.

@mtojek mtojek marked this pull request as ready for review October 25, 2021 12:26
@mtojek mtojek requested a review from jsoriano October 25, 2021 12:59
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.

3 participants