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

Move the git commit parsing out of the start.sh and into the build #1462

Closed
jonespm opened this issue Dec 8, 2022 · 2 comments · Fixed by #1463
Closed

Move the git commit parsing out of the start.sh and into the build #1462

jonespm opened this issue Dec 8, 2022 · 2 comments · Fixed by #1463
Assignees
Labels
dependencies Pull requests that update a dependency file

Comments

@jonespm
Copy link
Member

jonespm commented Dec 8, 2022

Thank you for contributing to this project!

Describe your problem or feature you'd like added

Currently, git values are parsed in the start.sh script at runtime. This requires git be available at runtime. Because of occasional security issues with git and libcurl it would be beneficial to just remove these packages. I believe we could just do it in the build and then remove them after.

Describe the solution you'd like

Add something like this to the Dockerfile.

RUN bash -l -c 'GIT_REPO="$(git config --local remote.origin.url)"; \
                GIT_COMMIT="$(git rev-parse HEAD)"; \ 
                GIT_BRANCH="$(git name-rev "$GIT_COMMIT" --name-only)"; \
                echo export GIT_REPO=$GIT_REPO > /etc/git.version; \
                echo export GIT_COMMIT=$GIT_COMMIT >> /etc/git.version; \
                echo export GIT_BRANCH=$GIT_BRANCH >> /etc/git.version'

Remove git and read it in the start script.

Describe any possible alternatives you've considered

Some hacky solutions for pasing the .git have been considered like

GIT_COMMIT="$(cat .git/HEAD | cut -c 1-7)
GIT_BRANCH="$(grep $GIT_COMMIT .git/packed-refs | sort | tail -n 1 | cut -d/ -f2- | rev | cut -d/ -f1 | rev)

But I don't really like those a lot

Additional context

It looks like you have to use a file and can't set these directly since every RUN only knows about it's own context.
https://stackoverflow.com/questions/34911622/dockerfile-set-env-to-result-of-command

jonespm added a commit to jonespm/student-dashboard-django that referenced this issue Dec 8, 2022
…-its-umich-edu#1462

THIS IS A draft: It was suggested to have this command to parse git in a
separate file and also need to add to Dockerfile.openshift.

Also this is going to need to be rebased.
@ssciolla ssciolla added the dependencies Pull requests that update a dependency file label Dec 8, 2022
@jonespm jonespm added this to To do in MyLA-2023.01.01 via automation Jan 5, 2023
@jonespm jonespm self-assigned this Jan 5, 2023
@jonespm jonespm moved this from To do to In progress in MyLA-2023.01.01 Jan 12, 2023
jonespm added a commit to jonespm/student-dashboard-django that referenced this issue Jan 12, 2023
…-its-umich-edu#1462

THIS IS A draft: It was suggested to have this command to parse git in a
separate file and also need to add to Dockerfile.openshift.

Also this is going to need to be rebased.
jonespm added a commit to jonespm/student-dashboard-django that referenced this issue Jan 23, 2023
…-its-umich-edu#1462

THIS IS A draft: It was suggested to have this command to parse git in a
separate file and also need to add to Dockerfile.openshift.

Also this is going to need to be rebased.
@jonespm
Copy link
Member Author

jonespm commented Jan 23, 2023

Test Plan:

  1. Verify the footer still shows the Git version when logging in
    For instance something likeGit version: 8ef3a65 (commit) 2022.02.01 (branch)
    In the container there will also be a file with new information in /etc/git.version
  2. Verify that the security issue around libssl is no longer present. (Either with docker scan my-learning-analytics-web or on Docker Hub)

MyLA-2023.01.01 automation moved this from In progress to Review/QA Jan 26, 2023
jonespm added a commit that referenced this issue Jan 26, 2023
…the build (#1463)

Co-authored-by: Sam Sciolla <35741256+ssciolla@users.noreply.github.com>
@jennlove-um jennlove-um moved this from Review/QA to Review/QA - DEV in MyLA-2023.01.01 Jan 26, 2023
@pushyamig
Copy link
Contributor

test Pass

  1. The Footer is showing current Git Version. I tested this from From Canvas test and Local myla instanse
  2. I did a docker scan my-learning-analytics-web Locally myla build, I did not see the severity on libssl,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
No open projects
3 participants