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

Dockerfile does not conform to entrypoint best practice #8 #9

Closed
wants to merge 3 commits into from

Conversation

ozbillwang
Copy link
Contributor

  1. Add bash for beginning user who should be able to docker run image with bash (or sh)
  2. Add docker-entrypoint.sh to conform to entrypoint best practice
  3. Sample in https://github.com/docker-library/official-images#consistency doesn't really work, adjust with simpler solution.

@ozbillwang
Copy link
Contributor Author

@muppet3000

Could you review this PR?

Copy link

@muppet3000 muppet3000 left a comment

Choose a reason for hiding this comment

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

Whilst this works for the best part. It doesn't fix the specific case that causes the problems with things like Jenkins/Automation jobs running it.
For example, Jenkins starts a container by running a "cat" command, leaving the container to run in the background and then the actual commands it wants to run it runs using "exec" commands.
In the case of the code that you have added it will execute "cat" as an argument to "git" and subsequently fail.

There are a couple of options here, either, take the easy route and add "cat" to the list of commands to run in isolation e.g. "sh, bash" but that'll then break when someone wants to run "ls". The more complex, but more complete solution is to use the output of "compgen -c" (prints all executables on the path) and check if $1 is in the list, if it is execute it, otherwise, pass everything as an argument to "git".

The downside of the latter solution is that you have to add compgen to your image (which, if you're trying to keep it minimal is more bloat).

Another note - since "sh" is available in Alpine it is not mandatory to provide bash, which no doubt will cause your image to grow more than you'd like, seeing as you started with alpine I'm guessing you wanted it as small as possible.

Sorry I haven't been able to do the PR myself, been manic outside of work and I'm unable to work on it during work hours!

@ozbillwang
Copy link
Contributor Author

let's follow #10 for the same request.

@ozbillwang ozbillwang deleted the #8 branch October 3, 2024 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants