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

Added secrets management examples with the workflow #653

Merged
merged 8 commits into from
Apr 29, 2022

Conversation

amulyavarote
Copy link
Contributor

@amulyavarote amulyavarote commented Apr 18, 2022

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:

  • The quickstart code compiles correctly
  • You've tested new builds of the quickstart if you changed quickstart code
  • You've updated the quickstart's README if necessary
  • If you have changed the steps for a quickstart be sure that you have updated the automated validation accordingly. All of our quickstarts have annotations that allow them to be executed automatically as code. For more information see mechanical-markdown. For user guide with examples see Examples.

Copy link
Contributor

@paulyuk paulyuk left a 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
Copy link
Contributor

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
Copy link
Contributor

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.

Copy link
Member

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.

Copy link
Contributor

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?

secrets_management/csharp/sdk/order-processor/Program.cs Outdated Show resolved Hide resolved
secrets_management/csharp/sdk/order-processor/secrets.json Outdated Show resolved Hide resolved
secrets_management/javascript/sdk/order-processor/index.js Outdated Show resolved Hide resolved
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>
Signed-off-by: Amulya Varote <amulyavarote@microsoft.com>
Signed-off-by: Amulya Varote <amulyavarote@microsoft.com>
Signed-off-by: Amulya Varote <amulyavarote@microsoft.com>
Copy link
Member

@msfussell msfussell left a 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.

@msfussell msfussell merged commit cf3054f into dapr:master Apr 29, 2022
@amulyavarote amulyavarote deleted the secrets_management_eg branch April 29, 2022 21:06
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.

Quickstarts examples for secrets management
4 participants