Skip to content

Comments

feat: add AWS Secret Manager feature to store DSN#273

Merged
burningalchemist merged 1 commit intoburningalchemist:masterfrom
elturkym:aws_secret_manager_support
Aug 4, 2023
Merged

feat: add AWS Secret Manager feature to store DSN#273
burningalchemist merged 1 commit intoburningalchemist:masterfrom
elturkym:aws_secret_manager_support

Conversation

@elturkym
Copy link
Contributor

This change allow AWS users, to store the database credentials in AWS Secret manager instead of adding the credentials in the configuration file directly.

Changes:

  • Adding a new config aws_secret_name under target to store the data source name (DSN) which has the database connection info.
  • Update the config.go to read the DSN from the given secret in the configuration file.
  • Update the config.go to override the DSN from AWS secret manager.
  • Update README file with the steps to use AWS Secret manager.

@burningalchemist
Copy link
Owner

Hey @elturkym, thank you for your contribution!

Could you please fix the comment style (coming from gofmt in the build log)? Apart from that, looks good to me! 👍

@burningalchemist burningalchemist self-requested a review July 14, 2023 16:02
config/config.go Outdated
Comment on lines 253 to 255
//
//AWS Secret
//
Copy link
Owner

@burningalchemist burningalchemist Jul 14, 2023

Choose a reason for hiding this comment

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

As per the gofmt -d output:

Suggested change
//
//AWS Secret
//
// AWS Secret

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I have address this formatting issue.

Copy link
Owner

Choose a reason for hiding this comment

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

I think it's still not enough. We also need to remove heading and trailing slashes. Just like in the suggestion I made above. 🙂👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, will address that

@burningalchemist burningalchemist changed the title Add AWS secret manager feature to store DSN feat: add AWS Secret Manager feature to store DSN Jul 15, 2023
@elturkym elturkym force-pushed the aws_secret_manager_support branch from a3cd747 to 0432c0b Compare July 17, 2023 16:30
@elturkym
Copy link
Contributor Author

My apologies, looks like there are new formatting issues, I will look at them.

@burningalchemist
Copy link
Owner

Sure, no worries. Check my last comment. 🙂👍

@burningalchemist
Copy link
Owner

Hey @elturkym, do you need any help or assistance from my end to address the remaining issues? :)

@elturkym
Copy link
Contributor Author

I was trying to run the workflow locally before submitting a new PR, but let me raise a new PR and check if there is still an error

@elturkym elturkym force-pushed the aws_secret_manager_support branch from 2af02da to ca5f02d Compare August 4, 2023 18:11
Copy link
Owner

@burningalchemist burningalchemist left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks! 🚀

@burningalchemist burningalchemist merged commit f7fb730 into burningalchemist:master Aug 4, 2023
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.

2 participants