-
Notifications
You must be signed in to change notification settings - Fork 484
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
feat(security): Remove Vault dependency on Consul by using file backend #2886
feat(security): Remove Vault dependency on Consul by using file backend #2886
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2886 +/- ##
==========================================
+ Coverage 40.25% 40.66% +0.40%
==========================================
Files 170 170
Lines 13873 14156 +283
==========================================
+ Hits 5585 5756 +171
- Misses 7924 8023 +99
- Partials 364 377 +13
Continue to review full report at Codecov.
|
3a79bd6
to
7192dc2
Compare
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, moved to draft until ADR is Approved.
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
66e22be
to
c0e29ae
Compare
2e600a3
c0e29ae
to
2e600a3
Compare
@jim-wang-intel Merge conflicts. |
2e600a3
to
8de1748
Compare
Conflicts resolved. Please check. |
…nd instead of consul Change the Vault backend storage configuration from Consul to Filesystem so that we can remove the Vault dependency on Consul. The docker-compose file will also need to change to remove the -consul dependency from repo developer-script. The current edgex-consul healthy check on Vault also need to be remove as that check becomes superfluous. Closes: edgexfoundry#2882 Signed-off-by: Jim Wang <[email protected]>
Resolve conflicts with master Signed-off-by: Jim Wang <[email protected]>
8de1748
to
565a8fe
Compare
Kudos, SonarCloud Quality Gate passed! |
redirect_addr = "http://edgex-vault:8200" | ||
cluster_addr = "http://edgex-vault:8201" | ||
} | ||
backend "file" { |
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.
Where does this go once this service is removed?
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.
it will go to "bootstrapper" i guess and this one is taking the baby step. so in terms of removing is beyond the scope of this related issue.
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.
issue #2882
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.
It goes straight into the docker-compose file as static configuration via env var (TLS was generating the variable parameters)
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.
Should also check to see if the snap depends on this script to generate the config file, or uses something else.
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.
@lenny-intel please suggest a change. i do not know what you want me to do.
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
…nd (edgexfoundry#2886) * feat(security): Remove Vault dependency on Consul by using file backend instead of consul Change the Vault backend storage configuration from Consul to Filesystem so that we can remove the Vault dependency on Consul. The docker-compose file will also need to change to remove the -consul dependency from repo developer-script. The current edgex-consul healthy check on Vault also need to be remove as that check becomes superfluous. Closes: edgexfoundry#2882
Change the Vault backend storage configuration from Consul to filesystem so that we can remove Vault dependency on Consul.
The docker-compose file will also need to change to remove the -consul dependency from repo developer-script.
The current edgex-consul healthy check on Vault also need to be remove as that check becomes superfluous.
Closes: #2882
Signed-off-by: Jim Wang [email protected]
PR Checklist
Please check if your PR fulfills the following requirements:
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?
Currently the Vault's backend storage is
Consul
and thus has the dependency onConsul
.Issue Number: #2882
What is the new behavior?
To break and remove the direct dependency on
Consul
. After this PR is merged, Vault won't directly depend on Consul.Does this PR introduce a breaking change?
New Imports
Specific Instructions
Are there any specific instructions or things that should be known prior to reviewing?
Pending on the related ADR to be approved
Other information
To test it, one can use docker-compose file generated from developer-script and remove the dependency on consul for
vault
service like:Note that the services list under the
depends_on
now does not haveconsul
any more. In other words, it is changed fromto
Run
docker-compose up vault
to test it and see there is no error in the docker logs regarding the Vault itself.