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

[semver:patch] fix unexpected string interpolation when using the task-definition-tags argument #131

Merged
merged 2 commits into from
Apr 30, 2021

Conversation

dgeorges
Copy link
Contributor

@dgeorges dgeorges commented Apr 22, 2021

remove unnecessary quotes which create confusion around string interpolation.

These quotes make the command difficult to use, for example if you want to quote you values which may have special characters. It also throws an error if more than one key/value is used with the simple format. The json format also requires quotes around key/values which makes the removed quotes troublesome. Below are some of the manual tests I did via inlining the orb to show what I mean.

Before change:

Works:
task-definition-tags: key=key1,value=value1
Doesn't Work

task-definition-tags:     key=key1,value=value1 key=key2,value=value2
...

Error parsing parameter '--tags': Second instance of key "value" encountered for input:
key=key1,value=value1 key=key2,value=value2
                               ^
This is often because there is a preceeding "," instead of a space.

After change:

Works:
task-definition-tags: key=key1,value=value1
task-definition-tags: key=tag,value="${CIRCLE_TAG}" key=BuildURL,value="${CIRCLE_BUILD_URL}"
task-definition-tags: "'[{\"key\": \"ReleaseIDTest\", \"value\": \"test\"}, {\"key\": \"BuildURLTest\",\"value\": \"test\"}]'"

Still Doesn't Work

task-definition-tags: "'[{\"key\": \"ReleaseIDTest\", \"value\": \"${CIRCLE_TAG}\"}, {\"key\": \"BuildURLTest\",\"value\": \"${CIRCLE_BUILD_URL}\"}]'"
...
Error: An error occurred (ClientException) when calling the TagResource operation: Some tags contain invalid characters. Valid characters: UTF-8 letters, spaces, numbers and _ . / = + - : @.

^ Although confusing. I would say is expected. Since the evaluation of the env variable are not happening based on the quotes. In this scenario the json need to be pre processed with jq or something like that. The single quotes are required for the json input by the aws cli. eg.

aws ecs tag-resource   --resource-arn "***"   --tags [{"key": "ReleaseIDTest", "value": "abc"}, {"key": "BuildURLTest","value": "abc"}]

Error parsing parameter '--tags': Invalid JSON:
[{key:

@dgeorges
Copy link
Contributor Author

@KyleTryon @lokst My apologies for not catching this during my manual testing of #127

@KyleTryon KyleTryon self-assigned this Apr 27, 2021
@KyleTryon
Copy link
Contributor

@KyleTryon @lokst My apologies for not catching this during my manual testing of #127

We probably should have had tests for this XP. Running the CI tests now and we'll get this patched in. Thank you again.

@KyleTryon KyleTryon self-requested a review April 30, 2021 20:26
@KyleTryon KyleTryon merged commit cc106a1 into CircleCI-Public:master Apr 30, 2021
@dgeorges
Copy link
Contributor Author

dgeorges commented May 5, 2021

XP

`

@KyleTryon @lokst My apologies for not catching this during my manual testing of #127

We probably should have had tests for this XP. Running the CI tests now and we'll get this patched in. Thank you again.

Yep, Thank you.

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