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

feat: Add ability to customize spiffe services #4533

Merged
merged 2 commits into from
Apr 27, 2023

Conversation

ejlee3
Copy link
Contributor

@ejlee3 ejlee3 commented Apr 26, 2023

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/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?)
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?)
  • I have opened a PR for the related docs change (if not, why?)
    docs PR

Testing Instructions

Testing without setting SPIFFE_CUSTOM_SERVICES

  1. pull the current PR branch
  2. build all the edgex-go services make docker in edgex-go
  3. pull the latest images from edgex-compose compose-builder directory make pull delayed-start ds-virtual ds-modbus
  4. in edgex-compose compose-builder directory, run make run dev delayed-start ds-virtual ds-modbus
  5. Wait for all security services to start up. Ensure that edgex-security-spire-config container has run the seed_builtin_entries script properly. Last log chunk should be:
...
+ echo -n app-external-mqtt-trigger
+ sed -e s/app-service-/app-/
+ service=app-external-mqtt-trigger
+ spire-server entry create -socketPath /tmp/edgex/secrets/spiffe/private/api.sock -parentID spiffe://edgexfoundry.org/spire/agent/x509pop/cn/agent0 -dns edgex-app-external-mqtt-trigger -spiffeID spiffe://edgexfoundry.org/service/app-external-mqtt-trigger -selector docker:label:com.docker.compose.service:app-external-mqtt-trigger
Entry ID         : cfee5346-947e-4a2c-aba4-912d5f7a205f
SPIFFE ID        : spiffe://edgexfoundry.org/service/app-external-mqtt-trigger
Parent ID        : spiffe://edgexfoundry.org/spire/agent/x509pop/cn/agent0
Revision         : 0
X509-SVID TTL    : default
JWT-SVID TTL     : default
Selector         : docker:label:com.docker.compose.service:app-external-mqtt-trigger
DNS name         : edgex-app-external-mqtt-trigger
+ exit 0
+ exec tail -f /dev/null

Testing with setting SPIFFE_CUSTOM_SERVICES

  1. pull the current PR branch
  2. Modify the cmd/security-spire-config/seed_builtin_entries.sh so the service list is:
    SPIFFE_SERVICES='security-spiffe-token-provider support-notifications support-scheduler \
                 device-bacnet device-camera device-grove device-modbus device-mqtt device-rest device-snmp \
                 device-virtual device-rfid-llrp device-coap device-gpio \
                 app-http-export app-mqtt-export app-sample'
    
  3. build all the edgex-go services make docker in edgex-go
  4. pull the latest images from edgex-compose compose-builder directory make pull delayed-start ds-virtual ds-modbus
  5. In edgex-compose change compose-builder/common-sec-stage-gate.env to add the following env variable:
    SPIFFE_CUSTOM_SERVICES='app-external-mqtt-trigger app-rfid-llrp-inventory'
    
  6. in edgex-compose compose-builder directory, run make run dev delayed-start ds-virtual ds-modbus
  7. Wait for all security services to start up. Ensure that edgex-security-spire-config container has run the seed_builtin_entries script properly. Last log chunk should look something like this:
...
+ echo -n app-rfid-llrp-inventory
+ sed -e s/app-service-/app-/
+ service=app-rfid-llrp-inventory
+ spire-server entry create -socketPath /tmp/edgex/secrets/spiffe/private/api.sock -parentID spiffe://edgexfoundry.org/spire/agent/x509pop/cn/agent0 -dns edgex-app-external-mqtt-trigger -spiffeID spiffe://edgexfoundry.org/service/app-rfid-llrp-inventory -selector docker:label:com.docker.compose.service:app-rfid-llrp-inventory
Entry ID         : cfee5346-947e-4a2c-aba4-912d5f7a205f
SPIFFE ID        : spiffe://edgexfoundry.org/service/app-rfid-llrp-inventory
Parent ID        : spiffe://edgexfoundry.org/spire/agent/x509pop/cn/agent0
Revision         : 0
X509-SVID TTL    : default
JWT-SVID TTL     : default
Selector         : docker:label:com.docker.compose.service:app-rfid-llrp-inventory
DNS name         : edgex-app-rfid-llrp-inventory
+ exit 0
+ exec tail -f /dev/null

New Dependency Instructions (If applicable)

lenny-goodell
lenny-goodell previously approved these changes Apr 26, 2023
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

@ejlee3 ejlee3 marked this pull request as ready for review April 26, 2023 20:17
@codecov-commenter
Copy link

Codecov Report

Merging #4533 (a03a97e) into main (5776811) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main    #4533   +/-   ##
=======================================
  Coverage   41.54%   41.54%           
=======================================
  Files         106      106           
  Lines        9764     9764           
=======================================
  Hits         4056     4056           
  Misses       5362     5362           
  Partials      346      346           

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.

Looks good, but please prefix the new variable with EDGEX_ as per direction from Lenny to indicate EdgeX specific env vars.

cmd/security-spire-config/seed_builtin_entries.sh Outdated Show resolved Hide resolved
Signed-off-by: Elizabeth J Lee <[email protected]>
@lenny-goodell
Copy link
Member

prefix the new variable with EDGEX_

Yes, good catch @bnevis-i

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.

Still waiting for EDGEX_ prefix.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    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
No Duplication information No Duplication information

@ejlee3 ejlee3 merged commit 824c097 into edgexfoundry:main Apr 27, 2023
@ejlee3 ejlee3 deleted the customize-security-proxy branch April 27, 2023 16:32
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.

5 participants