-
Notifications
You must be signed in to change notification settings - Fork 485
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): Implementation to set up Consul ACL #3215
feat(security): Implementation to set up Consul ACL #3215
Conversation
internal/security/bootstrapper/command/setupacl/aclbootstrap.go
Outdated
Show resolved
Hide resolved
internal/security/bootstrapper/command/setupacl/aclbootstrap.go
Outdated
Show resolved
Hide resolved
0b77750
to
2c33bc0
Compare
no need to clean up even when restarting all services since Consul's ACL is in a good bootstrapped ACL state so no need to re-run the setup registry ACL again. Unless you want to clean out all the volumes. |
Right, so I see an issue here when volumes are cleared, but sentinel file is left behind. Is there a Consul API that will tell us that it is setup already or not? |
the sentinel file is on the volume. I am NOT aware of any Consul API can tell us the ACL bootstrap status. |
ohhh! Didn't realize that. ;-) Then I am good! |
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, I'll concede on the 5 sec wait... ;-)
Thanks, it rarely happens and I've put the issue link notes. Since last time I saw, I've never seen it happened again on my box afterwards. So I can not even re-produce, and my guess is probably last time i've had some left-over state for some reason in consul as Consul is stateful machine. |
recheck |
internal/security/bootstrapper/command/setupacl/aclbootstrap.go
Outdated
Show resolved
Hide resolved
Hi @ernestojeda , I am starting seeing the snap pulling core18 from internet error in the CI pipeline and it happens 2-in-a-row now, could you please look into that? |
Not much I can do other than re-run the job. I will open a ticket with the LF to see if they can diagnose any network issue. |
Ok, thanks for the information. |
Mainly feature added is to enable Consul's ACL and bootstrap ACL. The following are detailed changes: - Add Consul's ACL configuration file with "allow" policy - Disable Consul's DNS port - Add ACL related configuration toml on security-bootstrapper - Add setupRegistryACL subcommand to security-bootstrapper - Add checking and retry logic for "non-empty" Consul leader - Add implementation for setting up Consul's ACL, including bootstrap ACL and configure Consul secrets access for Vault Closes: edgexfoundry#3156 Signed-off-by: Jim Wang <yutsung.jim.wang@intel.com>
Added the acl done sentinel file once Consul's registry had been setup successfully once This is to prevent the error if it is re-run second time or later. Like 2nd time compose-up or re-run the consul service in the snap. Signed-off-by: Jim Wang <yutsung.jim.wang@intel.com>
Address Lenny's PR comments and feedback Signed-off-by: Jim Wang <yutsung.jim.wang@intel.com>
Remove unused waitGroup Signed-off-by: Jim Wang <yutsung.jim.wang@intel.com>
- add Sentinel filepath configuration in toml Address Bryon's PR feedback/comments Signed-off-by: Jim Wang <yutsung.jim.wang@intel.com>
88047cb
to
191f7d1
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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
This PR is part of Phase 1 to secure Consul. Main feature added is to enable Consul's ACL and bootstrap ACL. The following are detailed changes:
Closes: #3156
Signed-off-by: Jim Wang yutsung.jim.wang@intel.com
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?
No ACL of Consul as of today.
Issue Number: #3156
What is the new behavior?
Start up Consul with ACL enabled with "allow" policy and also bootstrap its ACL for the first time use. Once bootstrapping is successfully, we will use Vault mgmt token to configure consul secret access.
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?
This implements the majority of consul bootstrap process in golang code and can be running in docker-compose. The implementation for snap would be in a separate PR.
Other information
To verify this locally, one needs to git clone this PR and then add environment override
ENABLE_REGISTRY_ACL=true
into bothsecretstore-setup
andconsul
service of docker-compose file:Also make sure to make the volume of secrets to have written permission in the Consul container:
as the generated Consul will be written into that secrets volume.