feat: SecretProvider for storing/retrieving secrets #707

lenny-goodell · 2020-12-29T22:46:37Z

PR Checklist

Please check if your PR fulfills the following requirements:

Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/device-sdk-c/blob/master/.github/CONTRIBUTING.md

What is the current behavior?

Issue Number: #653

What is the new behavior?

Added creation and initialization SecretProvider during bootstrap for both secure and non-secure modes
Added InsecureSecrets and SecretStore config to device-simple as samples and used to test initializing SecretProvider in secure mode.
APIs to take advantage of the SecretProvider are TBD in future PRs

Does this PR introduce a breaking change?

Yes
No

New Imports

Yes
No

Specific Instructions

This PR is about just about creating and initializing the Secret Provider. Using it come in following PR.
Do the following to test this PR:

clone latest developer scripts repo
Modify developer-scripts/releases/nightly-build/compose-files/docker-compose-nexus.yml as follows:
- Add ADD_SECRETSTORE_TOKENS: "device-simple" to environment section for vault-worker

    environment:
      SECRETSTORE_SETUP_DONE_FLAG: /tmp/edgex/secrets/edgex-consul/.secretstore-setup-done
      ADD_SECRETSTORE_TOKENS: "device-simple"
    hostname: edgex-vault-worker

Run compose from developer-scripts/releases/nightly-build/compose-files
- make run
build Device SDK from branch for this PR (i.e. clone the branch)

Run Device Simple in secure mode

cd to example/cmd/device-simple/
sudo EDGEX_SECURITY_SECRET_STORE=true ./device-simple
- Need to run as root to access vault token file
Verify service starts successfully with the following in the log messages:

level=INFO ts=2021-01-08T21:44:12.7857283Z app=device-simple source=secret.go:51 msg="Creating SecretClient"
level=INFO ts=2021-01-08T21:44:12.7872455Z app=device-simple source=secret.go:58 msg="Reading secret store configuration and authentication token"
level=INFO ts=2021-01-08T21:44:12.7873587Z app=device-simple source=secret.go:70 msg="Attempting to create secret client"
level=INFO ts=2021-01-08T21:44:12.805743Z app=device-simple source=secret.go:75 msg="Created SecretClient"

Run Device Simple in non-secure mode

EDGEX_SECURITY_SECRET_STORE=false ./device-simple
Verify service starts successfully without any log message about SecretClient

Other information

internal/v2/application/command.go

internal/autoevent/executor_test.go

chr1shung

LGTM

Add the ability to create SecretProvider and properly initilize it when in secure mode, Use of SecretProvied will be added in future PRs. close #653 Signed-off-by: lenny <[email protected]>

chr1shung

LGTM

tonyespy

In general this looks pretty good, although I had some questions about what appear to be some unrelated changes.

Also I wasn't sure how to test this PR. Could you provide some basic instructions on how to do so?

tonyespy · 2021-01-07T20:18:24Z

internal/common/config.go

@@ -26,6 +26,8 @@ type ConfigurationStruct struct {
 	DeviceList []DeviceConfig `consul:"-"`
 	// Driver is a string map contains customized configuration for the protocol driver implemented based on Device SDK
 	Driver map[string]string
+	// SecretStore contain information for connecting to the secure SecretStore (Vault) to retrieve or store secrets


sp: contain --> contains

internal/common/types.go

internal/v2/application/command.go

pkg/service/main.go

go.mod

Signed-off-by: lenny <[email protected]>

codecov-io · 2021-01-08T18:39:58Z

Codecov Report

Merging #707 (c4cbbed) into master (3720988) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #707      +/-   ##
==========================================
- Coverage   45.37%   45.31%   -0.06%     
==========================================
  Files          32       32              
  Lines        3121     3125       +4     
==========================================
  Hits         1416     1416              
- Misses       1592     1596       +4     
  Partials      113      113

Impacted Files	Coverage Δ
internal/common/config.go	`0.00% <0.00%> (ø)`
internal/common/types.go	`0.00% <ø> (ø)`
pkg/service/main.go	`0.00% <0.00%> (ø)`
pkg/service/service.go	`0.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3720988...c4cbbed. Read the comment docs.

cloudxxx8

LGTM

tonyespy · 2021-01-09T17:08:01Z

@cloudxxx8 why did you merge this? Per my last review I still had some testing to do and hadn't yet approved this.

tonyespy · 2021-01-09T17:13:47Z

@lenny-intel I re-tested today using the edgexfoundry snap. I added the following override to security-secretstore-setup (which in turn execs security-file-token-provider):

ADD_SECRETSTORE_TOKENS: device-simple

...and the token is generated for device-simple once I bring everything up. I then manually copied the file from the $SNAP_DATA/secrets/device-simple to the /res directory of device-simple, updated the configuration.toml path for the token and everything worked. I also verified that device-simple works with EDGEX_SECURITY_SECRET_STORE=false as well.

Note, when I tried testing by adding ADD_SECRETSTORE_TOKENS to vault-worked in docker-compose-nexus.yml per your instructions, the device-simple token wasn't generated for me when I brought everything up.

cloudxxx8 · 2021-01-10T14:37:39Z

sorry, @tonyespy , I should have read through your comments thoroughly. I merged this PR because I saw you put comments not "change request" and Lenny had resolved all the comments. Also, I would like to unblock other two PRs early.

lenny-goodell requested review from cloudxxx8 and chr1shung December 29, 2020 22:46

lenny-goodell linked an issue Dec 29, 2020 that may be closed by this pull request

Update Device SDK Go to use SecretProvider #653

Closed

lenny-goodell force-pushed the SecretProvider branch from 9d682a4 to 50a8ddc Compare December 30, 2020 15:21

lenny-goodell requested a review from iain-anderson December 30, 2020 15:24

lenny-goodell commented Dec 30, 2020

View reviewed changes

internal/v2/application/command.go Show resolved Hide resolved

lenny-goodell mentioned this pull request Dec 31, 2020

feat: Enable use of secrets via SecretProvider for MQTT broker credentials edgexfoundry/device-mqtt-go#197

Merged

13 tasks

lenny-goodell requested review from tonyespy and weichou1229 December 31, 2020 21:43

lenny-goodell mentioned this pull request Dec 31, 2020

feat: Add /api/v2/secret endpoint to store a secret #713

Merged

6 tasks

lenny-goodell force-pushed the SecretProvider branch from 50a8ddc to 5ab4d46 Compare December 31, 2020 22:14

lenny-goodell commented Dec 31, 2020

View reviewed changes

internal/autoevent/executor_test.go Show resolved Hide resolved

chr1shung previously approved these changes Jan 4, 2021

View reviewed changes

lenny-goodell dismissed chr1shung’s stale review via 592832d January 4, 2021 17:59

lenny-goodell force-pushed the SecretProvider branch 2 times, most recently from 592832d to d287bb3 Compare January 4, 2021 20:21

feat: SecretProvider for storing/retrieving secrets

bcd0eef

Add the ability to create SecretProvider and properly initilize it when in secure mode, Use of SecretProvied will be added in future PRs. close #653 Signed-off-by: lenny <[email protected]>

lenny-goodell force-pushed the SecretProvider branch from d287bb3 to bcd0eef Compare January 4, 2021 22:45

chr1shung previously approved these changes Jan 5, 2021

View reviewed changes

lenny-goodell linked an issue Jan 6, 2021 that may be closed by this pull request

Go Device SDK Provide internal API to retrieve credentials from a secret store #564

Closed

tonyespy reviewed Jan 7, 2021

View reviewed changes

chr1shung mentioned this pull request Jan 8, 2021

feat(v2): add v2 ProvisionWatcher cache and callback API #718

Merged

6 tasks

feat: Updates from PR review.

c4cbbed

Signed-off-by: lenny <[email protected]>

lenny-goodell dismissed chr1shung’s stale review via c4cbbed January 8, 2021 18:30

cloudxxx8 approved these changes Jan 9, 2021

View reviewed changes

cloudxxx8 merged commit 3c425af into edgexfoundry:master Jan 9, 2021

tonyespy mentioned this pull request Jan 9, 2021

Use of blank identifiers in public and private functions #721

Closed

lenny-goodell deleted the SecretProvider branch February 23, 2021 22:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: SecretProvider for storing/retrieving secrets #707

feat: SecretProvider for storing/retrieving secrets #707

lenny-goodell commented Dec 29, 2020 •

edited

Loading

chr1shung left a comment

chr1shung left a comment

tonyespy left a comment

tonyespy Jan 7, 2021

codecov-io commented Jan 8, 2021

cloudxxx8 left a comment

tonyespy commented Jan 9, 2021

tonyespy commented Jan 9, 2021 •

edited

Loading

cloudxxx8 commented Jan 10, 2021

feat: SecretProvider for storing/retrieving secrets #707

feat: SecretProvider for storing/retrieving secrets #707

Conversation

lenny-goodell commented Dec 29, 2020 • edited Loading

PR Checklist

What is the current behavior?

Issue Number: #653

What is the new behavior?

Does this PR introduce a breaking change?

New Imports

Specific Instructions

Other information

chr1shung left a comment

Choose a reason for hiding this comment

chr1shung left a comment

Choose a reason for hiding this comment

tonyespy left a comment

Choose a reason for hiding this comment

tonyespy Jan 7, 2021

Choose a reason for hiding this comment

codecov-io commented Jan 8, 2021

Codecov Report

cloudxxx8 left a comment

Choose a reason for hiding this comment

tonyespy commented Jan 9, 2021

tonyespy commented Jan 9, 2021 • edited Loading

cloudxxx8 commented Jan 10, 2021

lenny-goodell commented Dec 29, 2020 •

edited

Loading

tonyespy commented Jan 9, 2021 •

edited

Loading