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

Fix unsigned headers error #29

Merged
merged 11 commits into from
Jan 4, 2023

Conversation

immavalls
Copy link
Contributor

Proposal to fix #28

@CLAassistant
Copy link

CLAassistant commented Jan 2, 2023

CLA assistant check
All committers have signed the CLA.

@immavalls immavalls requested a review from oleiade January 2, 2023 17:52
@immavalls
Copy link
Contributor Author

Can we create a teat that covers this error? All the tests pass, maybe because we are not testing against aws?

@oleiade oleiade mentioned this pull request Jan 3, 2023
Copy link
Member

@oleiade oleiade left a comment

Choose a reason for hiding this comment

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

Great changes. I believe this shall fix #28, indeed 👏🏻

Regarding testing this change, I believe we need to add a step to verify that the the host header has indeed been added by the signing process in this one: https://github.com/grafana/k6-jslib-aws/blob/main/tests/internal/new_signature.js#LL80C4-L91C15

PS: also while we're at it, we could rename the tests/internal/new_signature.js file to signature.js. The new_signature naming is a leftover of the signature code's rewrite.

@immavalls immavalls merged commit ea90373 into grafana:main Jan 4, 2023
@immavalls immavalls deleted the fix/unsigned-headers-error branch January 12, 2023 12:28
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.

Error some headers are not signed with version 0.7.0
3 participants