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.

Sign up for GitHub

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

Jump to bottom

feat(security): Enable Vault's Consul secrets engine #3179

Merged
merged 3 commits into from
Feb 22, 2021
Merged

feat(security): Enable Vault's Consul secrets engine #3179

merged 3 commits into from
Feb 22, 2021

Conversation

jim-wang-intel
Copy link
Contributor

  • Add Secret Engine Enabler so that it can be re-used for both of Consul and KV secrets engines
  • Hookup the code in secretstore-setup so that both KV and Consul secret engines are enabled

Closes: #3154

Signed-off-by: Jim Wang yutsung.jim.wang@intel.com

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x ] 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/edgex-go/blob/master/.github/Contributing.md.

What is the current behavior?

No Consul secret engine for Vault yet

Issue Number: #3154

What is the new behavior?

Enable Vault's Consul secret engine

Does this PR introduce a breaking change?

  • Yes
  • [x ] No

New Imports

  • Yes
  • [x ] No

Specific Instructions

Are there any specific instructions or things that should be known prior to reviewing?

Other information

To verify the Consul secret engine is enabled locally, one can observe the log message of docker container edgex-secretstore-setup for the first time enabling:
docker logs edgex-secretstore-setup | grep Consul


jim@jim-NUC7i5DNHE:~/go/src/github.com/edgexfoundry/developer-scripts/compose-builder$ docker logs edgex-secretstore-setup | grep Consul
level=INFO ts=2021-02-18T20:46:21.475937091Z app=edgex-security-secretstore-setup source=common.go:97 msg="successfully made request to update mounts for Consul"
level=INFO ts=2021-02-18T20:46:21.475958918Z app=edgex-security-secretstore-setup source=enabler.go:84 msg="Consul secrets engine enabled"

@jim-wang-intel jim-wang-intel added enhancement New feature or request security-services 3-high priority denoting release-blocking issues ireland labels Feb 18, 2021
@jim-wang-intel jim-wang-intel added this to the Ireland milestone Feb 18, 2021
@jim-wang-intel jim-wang-intel self-assigned this Feb 18, 2021
bnevis-i
bnevis-i previously approved these changes Feb 18, 2021
View reviewed changes
Copy link
Collaborator

@bnevis-i bnevis-i left a comment

Choose a reason for hiding this comment

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

Works as expected.

lenny-goodell
lenny-goodell requested changes Feb 18, 2021
View reviewed changes
Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

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

Most of these changes are rendered obsolete by by PRs. The rebase will be nasty. ;-)

@jim-wang-intel
Copy link
Contributor Author

Most of these changes are rendered obsolete by by PRs. The rebase will be nasty. ;-)

not too bad I think; just 20 files... :-)

@jim-wang-intel
Copy link
Contributor Author

@lenny-intel Rebased to the lastest secretstore client from go-mod-secret. Please review. Thanks.

@codecov-io
Copy link

Codecov Report

Merging #3179 (17b3f08) into master (61638ca) will increase coverage by 0.09%.
The diff coverage is 67.74%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3179      +/-   ##
==========================================
+ Coverage   40.76%   40.86%   +0.09%     
==========================================
  Files         170      171       +1     
  Lines       14507    14523      +16     
==========================================
+ Hits         5914     5935      +21     
+ Misses       8264     8257       -7     
- Partials      329      331       +2
Impacted Files Coverage Δ
internal/security/secretstore/init.go 3.82% <0.00%> (+0.10%) ⬆️
...rnal/security/secretstore/secretsengine/enabler.go 84.00% <84.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 61638ca...17b3f08. Read the comment docs.

lenny-goodell
lenny-goodell requested changes Feb 22, 2021
View reviewed changes
Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

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

Nice Job, just a few minor items to address.

internal/security/secretstore/secretsengine/enabler.go Outdated Show resolved Hide resolved
internal/security/secretstore/secretsengine/enabler.go Outdated Show resolved Hide resolved
internal/security/secretstore/secretsengine/enabler.go Outdated Show resolved Hide resolved
internal/security/secretstore/secretsengine/enabler.go Outdated Show resolved Hide resolved
@jim-wang-intel
feat(security): Enable Vault's consul secrets engine
5adc20e 
- Add Secret Engine Enabler so that it can be re-used for both of Consul and KV secrets engines
- Hookup the code in secretstore-setup so that both KV and Consul secret engines are enabled

Closes: edgexfoundry#3154

Signed-off-by: Jim Wang <yutsung.jim.wang@intel.com>
@jim-wang-intel
refactor: move secretsengine's enabler to secretstore-setup and resov…
4cb289a 
…led merging conflicts

Refator to move enabler to secretstore-setup in edgex-go
Rebased and resolved the merging conflicts
Use the secret client from go-mod-secret

Signed-off-by: Jim Wang <yutsung.jim.wang@intel.com>
@jim-wang-intel
fix: address Lenny's PR feedback
e54cfd9 
Address PR comments

Signed-off-by: Jim Wang <yutsung.jim.wang@intel.com>
@sonarcloud
Copy link

sonarcloud bot commented Feb 22, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jim-wang-intel
Copy link
Contributor Author

@lenny-intel I've addressed your comments, please re-review it. Thanks!

lenny-goodell
lenny-goodell approved these changes Feb 22, 2021
View reviewed changes
Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

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

LGTM

@jim-wang-intel jim-wang-intel merged commit 13b869e into edgexfoundry:master Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lenny-goodell lenny-goodell lenny-goodell approved these changes

@bnevis-i bnevis-i bnevis-i left review comments

@beaufrusetta beaufrusetta Awaiting requested review from beaufrusetta

@hutchic hutchic Awaiting requested review from hutchic

Assignees

@jim-wang-intel jim-wang-intel

Labels
3-high priority denoting release-blocking issues enhancement New feature or request ireland security-services
Projects
None yet
Milestone
Ireland
Development

Successfully merging this pull request may close these issues.

[Secure Consul Ph. 1] Install Vault Consul secrets engine and implementation
4 participants
@jim-wang-intel @codecov-io @bnevis-i @lenny-goodell