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

Deploy on Alpine and add a non-root user #13

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

kjsanger
Copy link
Member

Fix setting version string in the built application

Fix setting version string in the built application
@kjsanger kjsanger requested a review from jmtcsngr March 28, 2024 14:33
Copy link
Member

@jmtcsngr jmtcsngr left a comment

Choose a reason for hiding this comment

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

Using make to call git inside the container feels strange. Passing things through variables may be better. As it is now, it works. So we could create an issue to improve later if you think it is reasonable.

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
@kjsanger
Copy link
Member Author

kjsanger commented Apr 2, 2024

Using make to call git inside the container feels strange. Passing things through variables may be better. As it is now, it works. So we could create an issue to improve later if you think it is reasonable.

Then we'd have two different build commands, one building in the container and one for outside (the Makefile). We don't worry about copying bash scripts into containers to run them (it's an accepted way of avoiding hairy RUN commands), so I don't think we should worry about Makefiles (especially as make is provided by the container).

@jmtcsngr
Copy link
Member

jmtcsngr commented Apr 2, 2024

I found it unusual to mount .git in container build to resolve the version. Also had issues executing tests with make so I think there will be more work on the Makefile anyway. I had to check how things were working in actions to figure out how to do things locally.

@jmtcsngr jmtcsngr merged commit 404291a into wtsi-npg:devel Apr 2, 2024
5 checks passed
@kjsanger
Copy link
Member Author

kjsanger commented Apr 2, 2024

The container doesn't install the test dependencies - it's purely for building after the tests have been run by the test workflow. The test framework is installed for the tests here

go install -mod=mod github.com/onsi/ginkgo/v2/ginkgo

@kjsanger kjsanger deleted the feature/improve-dockerfile branch April 30, 2024 16:21
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.

2 participants