Skip to content

fix: security-secretstore-setup volume init semantics#4092

Merged
vli11 merged 3 commits intoedgexfoundry:mainfrom
vli11:fix_3852
Jul 14, 2022
Merged

fix: security-secretstore-setup volume init semantics#4092
vli11 merged 3 commits intoedgexfoundry:mainfrom
vli11:fix_3852

Conversation

@vli11
Copy link
Copy Markdown
Contributor

@vli11 vli11 commented Jul 14, 2022

fixes: #3852
Signed-off-by: Valina Li valina.li@intel.com

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?) NA; because it is a fix to a workaround
  • 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?) NA; because it is a fix to a workaround

Testing Instructions

New Dependency Instructions (If applicable)

fixes: edgexfoundry#3852
Signed-off-by: Valina Li <valina.li@intel.com>
@vli11 vli11 self-assigned this Jul 14, 2022
@vli11 vli11 added bug Something isn't working 1-low priority denoting isolated changes labels Jul 14, 2022
Copy link
Copy Markdown
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.

Both of these commands should have been MOVED from the Dockerfile to the entrypoint script

&& mkdir -p /vault/config/assets \
&& chown -Rh 100:1000 /vault/    

Signed-off-by: Valina Li <valina.li@intel.com>
@vli11 vli11 requested a review from bnevis-i July 14, 2022 03:01
bnevis-i
bnevis-i previously approved these changes Jul 14, 2022
Copy link
Copy Markdown
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.

LGTM

lenny-goodell
lenny-goodell previously approved these changes Jul 14, 2022
Copy link
Copy Markdown
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

Comment on lines 52 to +55
# Setup the entry point script, create token dir, and assign perms
COPY --from=builder /edgex-go/cmd/security-secretstore-setup/entrypoint.sh /usr/local/bin/
RUN chmod +x /usr/local/bin/entrypoint.sh \
&& ln -s /usr/local/bin/entrypoint.sh / \
&& mkdir -p /vault/config/assets \
&& chown -Rh 100:1000 /vault/
&& ln -s /usr/local/bin/entrypoint.sh /
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please update the comment in line 52 to fit the real implementation

Signed-off-by: Valina Li <valina.li@intel.com>
@vli11 vli11 dismissed stale reviews from lenny-goodell and bnevis-i via 5b96f28 July 14, 2022 16:25
@vli11 vli11 requested a review from jim-wang-yutsung July 14, 2022 16:25
@sonarqubecloud
Copy link
Copy Markdown

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

Copy link
Copy Markdown

@jim-wang-yutsung jim-wang-yutsung left a comment

Choose a reason for hiding this comment

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

LGTM

@vli11 vli11 merged commit 66f7195 into edgexfoundry:main Jul 14, 2022
@vli11 vli11 deleted the fix_3852 branch July 14, 2022 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1-low priority denoting isolated changes bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security-secretstore-setup relies on undocument Docker volume init semantics

4 participants