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 quoting of exec argument in the entrypoint script #492

Merged
merged 1 commit into from
Apr 16, 2021
Merged

Fix quoting of exec argument in the entrypoint script #492

merged 1 commit into from
Apr 16, 2021

Conversation

jhujhiti
Copy link
Contributor

New Behavior

This fixes improper quoting in the exec at the end of the Docker entrypoint. "$@" is equivalent to "$1" "$2" ... in bash. Fixing this allows Docker commands (CMD) with spaces in them to be passed through this entrypoint script.

Contrast to Current Behavior

There is no change in behavior of the container if launched with the default arguments, but we're now able to override the Docker command with something like sh -c "echo hello ; /opt/netbox/launch-netbox.sh" to do something before the application starts. I'm using this to test quick, trivial fixes to the container rather than rebuilding the container with a new entrypoint or launch script.

Discussion: Benefits and Drawbacks

There's no downside here. All existing behavior is maintained.

There's a relevant shellcheck test disabled here that was added, seemingly unrelatedly, in #163 . The shellcheck error tells us to use these quotes.

Changes to the Wiki

None

Proposed Release Note Entry

This probably isn't worth mentioning in the release notes, but:

The container can now be started properly with an overridden Docker command.

Double Check

  • I have read the comments and followed the PR template.
  • I have explained my PR according to the information in the comments.
  • My PR targets the develop branch.

@cimnine cimnine added the bug This issue describes a confirmed bug. label Apr 16, 2021
@cimnine cimnine added this to the next milestone Apr 16, 2021
@cimnine cimnine merged commit eee07f7 into netbox-community:develop Apr 16, 2021
@jhujhiti jhujhiti deleted the upstreaming branch April 16, 2021 15:56
@cimnine cimnine mentioned this pull request Apr 23, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants