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: secretAccessKey has no length constraints #98

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

egor-romanov
Copy link
Contributor

secretAccessKey has no length constraints which currently may break things with non AWS S3 compatible services

Check for more info:
https://docs.aws.amazon.com/IAM/latest/APIReference/API_AccessKey.html

  • AccessKeyId has: Minimum length of 16. Maximum length of 128.
  • SecretAccessKey has none

https://docs.aws.amazon.com/IAM/latest/APIReference/API_AccessKey.html

AccessKeyId has: Minimum length of 16. Maximum length of 128.
SecretAccessKey has none
@egor-romanov egor-romanov requested a review from a team as a code owner April 2, 2024 08:53
@egor-romanov egor-romanov requested review from codebien and olegbespalov and removed request for a team April 2, 2024 08:53
@CLAassistant
Copy link

CLAassistant commented Apr 2, 2024

CLA assistant check
All committers have signed the CLA.

@oleiade
Copy link
Member

oleiade commented Apr 2, 2024

Hey, @egor-romanov 👋🏻 Nice to see you here 🙇🏻

I think this check is something we mimicked from the node SDK at the time of the implementation, but after doing a bit more research about it, it looks like it's not indeed a strict requirement from the specification perspective 👍🏻

Could you also make a pass on the tests and confirm we do not have a test that tries to match that in one way or another? Finally, could you also sign the CLA to make merging possible?

@egor-romanov
Copy link
Contributor Author

heyy!

sure thing, will do 👍🙌

@andrewslotin andrewslotin merged commit 5805899 into grafana:main Apr 3, 2024
3 checks passed
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.

5 participants