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

Secret key value pair selection #9

Closed
wants to merge 4 commits into from
Closed

Secret key value pair selection #9

wants to merge 4 commits into from

Conversation

stefanhenseler
Copy link
Contributor

Hi.

thank you for implementing this provider, I think summon is an awesome tool. I've added some functionality because we use key-value pairs and one secret per environment application. I thought it would be neat if we could declare the key directly via summon. Would be cool if we could integrate this.

Thanks Stefan

@stefanhenseler
Copy link
Contributor Author

Ups, I just noticed that this is a duplicate. :) Looking forward to the next release then. sorry

@sgnn7
Copy link
Contributor

sgnn7 commented Feb 20, 2019

Hey @synax, thanks for your contribution and I think you might have closed this PR a bit prematurely! :)

Your PR has the golang modules change which is really nice and has a bit of a cleaner approach than the original PR. If you could take the same syntax from there (# vs :) and whatever else is on it (e.g. docs), I think this PR might take precedence. Let me know what you think!

@doodlesbykumbi
Copy link
Contributor

@synax I wrote the original PR and I second the comments above. Also, I think having accessing to arbitrarily nested values would be nice but I'm no longer sure if gjson is the way to go.

@stefanhenseler stefanhenseler reopened this Mar 1, 2019
@stefanhenseler
Copy link
Contributor Author

stefanhenseler commented Mar 1, 2019

Hey, sorry for the delayed response. Would be great if we could integrate it in to the next release. I think I should have time next week to finish this off. I'll change the : char to # and merge the documentation added by @doodlesbykumbi with the content I added. @sgnn7 Anything else you would like me to change/add in this PR to get it merged?

@sgnn7
Copy link
Contributor

sgnn7 commented Mar 1, 2019

@synax Awesome! I think that's all that's needed and if something changes on our end, we will update the comments here. 👍

@stefanhenseler
Copy link
Contributor Author

So, this should now implement #6 and should be ready to be merged.

@sgnn7
Copy link
Contributor

sgnn7 commented Mar 6, 2019

@synax Great work! It seems like we will need a few more changes on the pipeline to make this work so we will use this branch as a base for another PR that includes the full changes needed (I'll update this thread with the new PR).

Thanks again!

@sgnn7
Copy link
Contributor

sgnn7 commented Mar 6, 2019

Closing this PR since #10 supersedes it

@sgnn7 sgnn7 closed this Mar 6, 2019
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.

3 participants