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

Handle non string value #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

johnraz
Copy link

@johnraz johnraz commented Mar 17, 2021

This PR contains 2 main changes:

  1. Give more context when a secret is not found, here the JSON unmarshalling error was swallowed and we were only seeing "secret no found". It was really hard to diagnose and I actually had to do this change and recompile to find out what the issue was.
  2. While amazon secrets manager usually holds only string as secret value we ended up with integers in some places... automatically created by amazon RDS... I changed the code so that any type would be unmarshalled properly and then converted to a string. It's more flexible than failing completely to load the all secret map.

I'm not a Gopher so I apologize in advance for any wrong doing ;-)

@CLAassistant
Copy link

CLAassistant commented Mar 17, 2021

CLA assistant check
All committers have signed the CLA.

@tboerger
Copy link

Using interface everywhere feels at least for me wrong, IMHO the json paarser should make sure everything is a string.

@johnraz
Copy link
Author

johnraz commented Mar 26, 2021

Hello @tboerger,

I'm not sure to understand what you are proposing?

Do you mean it should keep erroring if aws-secrets sends back non string values?
Or do you mean the conversion should happen earlier and a string should be passed down the stack after the parser has done its job?

Thanks for you time and feedback.

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

3 participants