Skip to content

Support package signatures#760

Merged
mtojek merged 7 commits intoelastic:masterfrom
mtojek:728-support-package-signatures
Oct 25, 2021
Merged

Support package signatures#760
mtojek merged 7 commits intoelastic:masterfrom
mtojek:728-support-package-signatures

Conversation

@mtojek
Copy link
Copy Markdown
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
Copy Markdown

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
"crm",
"azure"
],
"signature": "e16ddaf4f91df524b27bf4f2e4b1ac09",
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

@mtojek mtojek marked this pull request as draft October 25, 2021 11:58
@mtojek
Copy link
Copy Markdown
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