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

feat: add awsv4 presign and rds util #44

Merged
merged 27 commits into from
Mar 1, 2023
Merged

Conversation

windmgc
Copy link
Member

@windmgc windmgc commented Jan 29, 2023

This PR adds two features:

Please note that the PR is still working in progress.

TODO:

  • lots of redundant code between v4 and presign, refactor the signing functions to common utils
  • fix the comments and annotations of the functions
  • tests

@windmgc windmgc changed the title [WIP]feat: add awsv4 presign and rds util feat: add awsv4 presign and rds util Feb 6, 2023
@windmgc windmgc marked this pull request as ready for review February 6, 2023 07:07
@windmgc
Copy link
Member Author

windmgc commented Feb 6, 2023

@Tieske Could you please review this? Thanks!

I will continue adding more test cases for presign during the review process

Copy link
Member

@Tieske Tieske left a comment

Choose a reason for hiding this comment

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

had a quick look, but running out of time, will have to check again.

Comment on lines 5 to 7
ngx.time = function()
return 1667543171
end
Copy link
Member

Choose a reason for hiding this comment

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

This changes the global environment, and hence will spill over to other tests.

This should probably be in a setup block, with a matching teardown block to undo the change

Copy link
Member Author

@windmgc windmgc Feb 20, 2023

Choose a reason for hiding this comment

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

fixed in before_each and after_each
fixed in setup and teardown

@Tieske
Copy link
Member

Tieske commented Feb 28, 2023

@windmgc I added a commit to update the doc comments to render properly.

please check;

make docs
open ./docs/index.html

@windmgc
Copy link
Member Author

windmgc commented Feb 28, 2023

@Tieske thanks a lot!

@Tieske
Copy link
Member

Tieske commented Feb 28, 2023

@windmgc here's my update of the interface, please review; windmgc#1

@windmgc
Copy link
Member Author

windmgc commented Mar 1, 2023

@windmgc here's my update of the interface, please review; windmgc#1

rebase merged to this dev branch to reserve commit msgs

Copy link
Member

@Tieske Tieske left a comment

Choose a reason for hiding this comment

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

lgtm

@Tieske Tieske merged commit 625e0ea into Kong:main Mar 1, 2023
@windmgc windmgc deleted the add-presign branch March 1, 2023 10:26
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.

None yet

2 participants