-
Notifications
You must be signed in to change notification settings - Fork 126
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
Docker / build refactor & update Nginx #69
Conversation
Image builder now multi-stage Image builder uses debian:bullseye-slim to build module (same as official nginx image's base) Use nginx official image as final image base Now using sed to modify OOTB nginx.conf to load the module. Moved test-runner customisations into separate compose test stack Updated Makefile to use docker compose for tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! Looks great.
I pulled the branch and rebuilt the images, and ran make
, which resulted in this:
make[1]: Entering directory '/home/josh/Projects/teslagov/ngx-http-auth-jwt-module-bin'
docker run --rm --name "jwt-nginx-cont" -d -p 8000:8000 teslagov/jwt-nginx
9fb46434e2f04880c6557f0279e0c08cc155df404e4b6e2db77d196506cbc103
docker cp jwt-nginx-cont:/usr/lib64/nginx/modules/ngx_http_auth_jwt_module.so .
docker cp jwt-nginx-cont:/usr/local/lib/libjansson.so.4.13.0 .
Error: No such container:path: jwt-nginx-cont:/usr/local/lib/libjansson.so.4.13.0
make[1]: *** [Makefile:42: start-nginx] Error 1
make[1]: Leaving directory '/home/josh/Projects/teslagov/ngx-http-auth-jwt-module-bin'
make: *** [Makefile:15: all] Error 2
Can you take a look?
@KensingtonTech note that I updated the NGINX_VERSION=1.16.1 make rebuild-nginx This is important because we don't always run on the latest NGINX. So you'll want to rebase your branch. Commit: 148987d BTW I think we can consolidate the |
…le via env vars. Clean up dangling stage images during build.
@JoshMcCullough I pushed another commit that makes DOCKER_ORG_NAME, DOCKER_IMAGE_NAME, and NGINX_VERSION configurable via env vars. |
I'm seeing some errors when running:
|
Fix for v1.16.1. Test runner runs correct nginx version.
@JoshMcCullough apologies for the lull. The issue with 1.16.1 has been addressed, as well as an issue where the test runner was always using the latest version. |
Looks good, thanks! |
@JoshMcCullough I hope you don’t mind, but I’ve taken the liberty of publishing the image to Docker hub (https://hub.docker.com/r/kensingtontech/nginx-jwt). I’ve made sure to add your 1.16.1 in there, as well. |
Nice! We really should be better about this sort of thing, and should publish boundaries, images, etc. Thanks for contributing. |
It's been a while, TeslaGov! Glad to see this project is still kicking. I had a need to revisit it and found that the build / Docker end of things was in need of a serious facelift. Please see the description below.