-
Notifications
You must be signed in to change notification settings - Fork 523
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
Added secrets management examples with the workflow #653
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi just have a few style things. Let's please check on the docker login question too.
Awesome contribution, thank you!
echo "DAPR_CLI_VERSION=$CLI_VERSION" >> $GITHUB_ENV | ||
echo "Found $CLI_VERSION" | ||
shell: bash | ||
- name: Set up Dapr CLI - Mac/Linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this pull from Docker hub or GHCR by default now? If Docker hub we need a docker login action as seen in other workflows.
version: v1 | ||
metadata: | ||
- name: secretsFile | ||
value: secrets.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this means secrets.json is colocated in the source code / repo folder, and that is an anti pattern. We really need this file in a User or Home subfolder. I understand that is a pain for different operating systems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a Quickstart, having the file versioned and ready is a good experience IMO. Generating the file on demand can be more of a pain to make the Quickstart reproducible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get your point, just it's still very bad practice. Really we should do your suggestion here: dapr/components-contrib#1037
That will take a while, what if we had a script that did a quick version for linux/mac/windows?
Signed-off-by: Amulya Varote <amulyavarote@microsoft.com>
Signed-off-by: Amulya Varote <amulyavarote@microsoft.com>
Signed-off-by: Amulya Varote <amulyavarote@microsoft.com>
Signed-off-by: Amulya Varote <amulyavarote@microsoft.com>
Signed-off-by: Amulya Varote <amulyavarote@microsoft.com>
092452b
to
d04806b
Compare
Signed-off-by: Amulya Varote <amulyavarote@microsoft.com>
Signed-off-by: Amulya Varote <amulyavarote@microsoft.com>
Signed-off-by: Amulya Varote <amulyavarote@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. We may choose to revisit using Env Var secret store in future.
Signed-off-by: Amulya Varote amulyavarote@microsoft.com
Description
Please explain the changes you've made
Added secrets management examples with the workflow.
Examples included for Csharp, Python and Javascript
Issue reference
We strive to have all PRs being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #652
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: