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

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

Merged
merged 3 commits into from
Jul 14, 2022

Conversation

vli11
Copy link
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
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
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
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
Contributor

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
@sonarcloud
Copy link

sonarcloud bot commented Jul 14, 2022

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
Contributor

@jim-wang-intel jim-wang-intel 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