Repository navigation
fix(templates): pass update-parameter to kubectl without eval - #67
Merged
Merged
Conversation
retry_kubectl ran its command through eval inside a double-quoted string, so the double quotes in update-parameter were stripped. Run the command from its arguments instead, and replace the bash-only ((attempt++)) with POSIX arithmetic so the retry loop terminates under dash. Fixes crossplane#66 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
jcogilvie
force-pushed
the
fix-eval-arg
branch
from
October 20, 2026 18:43
7da3499 to
54bbea3
Compare
jcogilvie
marked this pull request as ready for review
October 20, 2026 18:55
ulucinar
approved these changes
Oct 22, 2026
ulucinar
left a comment
ulucinar
left a comment
Collaborator
There was a problem hiding this comment.
Thank you @jcogilvie for fixing the eval issue and the arithmetic command issue. While we are there, for consistency, we may also consider using the similar attempt=$((attempt+1)) in the files under hack:
- hack/check_endpoints.sh
- hack/patch-ns.sh
- hack/patch.sh
Use POSIX arithmetic in the hack scripts and printf for the attempt message in the update template so escape sequences in the command are not interpreted. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of your changes
Fixes #66.
Since v2.2.0 the update step passed its kubectl command to
retry_kubectl "...", which ran it witheval.{{ $resource.UpdateParameter }}was inserted raw inside that double-quoted string, so the"characters in theupdate-parameterannotation ended the string and kubectl received invalid JSON ({description:x}). Values with spaces failed earlier withUnterminated quoted string.01-update.yaml.tmpl:retry_kubectlnow takes the command as arguments and runs"$@"instead ofeval; the patch lines no longer wrap the command in double quotes.((attempt++))withattempt=$((attempt+1)). Chainsaw runs scripts withsh(dash), where((attempt++))fails, so the counter never advanced and the retry loop never reachedmax_attempts.TestUpdateScriptPreservesParameter, which renders the update step, runs the script undershwith a stub kubectl, and asserts the parameter arrives unchanged (including spaces).Known limitation (unchanged): an
update-parametercontaining a single quote still breaks the surrounding-p '...'.I have:
go test ./internal/templates.How has this code been tested
Unit tests, including the new script-level test above. Not run against a real cluster.