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: do not wrap --health-cmd in quotes #25

Merged
merged 1 commit into from
Jun 29, 2024
Merged

fix: do not wrap --health-cmd in quotes #25

merged 1 commit into from
Jun 29, 2024

Conversation

aksiksi
Copy link
Owner

@aksiksi aksiksi commented Jun 29, 2024

We cannot wrap the health command with single quotes because Nix calls escapeShellArg under the hood on each entry in the ExtraOptions array.

This results in us passing in the single-quote wrapped command as-is - i.e., including single quotes - to the runtime, which causes the entire quote-enclosed string to be interpreted as an executable rather than a shell command.

I have also added a simple health check to the integration test.

Fixes #22.

@codecov-commenter
Copy link

codecov-commenter commented Jun 29, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.31%. Comparing base (3cc5f14) to head (08eefbc).
Report is 1 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #25   +/-   ##
=======================================
  Coverage   73.31%   73.31%           
=======================================
  Files           6        6           
  Lines         873      873           
=======================================
  Hits          640      640           
  Misses        197      197           
  Partials       36       36           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aksiksi aksiksi force-pushed the health-cmd branch 4 times, most recently from a5e01b0 to 66b0f2d Compare June 29, 2024 16:42
@aksiksi aksiksi merged commit e84cf34 into main Jun 29, 2024
3 checks passed
@aksiksi aksiksi deleted the health-cmd branch June 29, 2024 17:07
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.

Quoting of health-cmd leads to health check failures in generated code
2 participants