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: dockerfile ENTRYPOINT wrapping #396

Merged
merged 2 commits into from
Aug 5, 2024
Merged

fix: dockerfile ENTRYPOINT wrapping #396

merged 2 commits into from
Aug 5, 2024

Conversation

miki725
Copy link
Contributor

@miki725 miki725 commented Aug 5, 2024

Issue

segfaults in some cases and invalid wrapping in others

Description

docker has a few edge cases how ENTRYPOINT can be provided which broke chalk wrapping.

  • simply ENTRYPOINT was not handled correctly and was resulting in segfault
  • ENTRYPOINT [""] was attempting to wrap "" which is invalid

All these variants are handled now:

  • ENTRYPOINT
  • ENTRYPOINT []
  • ENTRYPOINT [""]

In addition explicitly invalid commands are ignored and wrapping is disabled. This way chalk does not yield and error but the runtime is bound to fail.

  • ENTRYPOINT [" "]
  • ENTRYPOINT ["", ""]

Testing

➜ make tests args="test_docker.py::test_wrap_entrypoint --logs --pdb -x"

docker has a few edge cases how ENTRYPOINT can be provided which broke
chalk wrapping.

* simply `ENTRYPOINT` was not handled correctly and was resulting in
  segfault
* `ENTRYPOINT [""]` was attempting to wrap `""` which is invalid

All these variants are handled now:

* `ENTRYPOINT`
* `ENTRYPOINT []`
* `ENTRYPOINT [""]`

In addition explicitly invalid commands are ignored and wrapping is
disabled. This way chalk does not yield and error but the runtime
is bound to fail.

* `ENTRYPOINT [" "]`
* `ENTRYPOINT ["", ""]
@ee7 ee7 self-requested a review August 5, 2024 17:21
Copy link
Contributor

@ee7 ee7 left a comment

Choose a reason for hiding this comment

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

LGTM. And looks like a good combination of new tests.

@miki725 miki725 merged commit 96eabf5 into main Aug 5, 2024
4 checks passed
@miki725 miki725 deleted the entrypoint branch August 5, 2024 18:02
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