Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Remove unnecessary use of tr in OpenSearch lab #1196

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

matschundbrei
Copy link
Contributor

@matschundbrei matschundbrei commented Dec 13, 2024

What this PR does / why we need it:

This PR removes the unneccesary and potentially destructive use of tr -d '"' to remove enclosing double-quotes from a json-string.

Which issue(s) this PR fixes:

This might be destructive, because tr will remove all quotes, not only the enclosing ones and will not honor JSON escaped quotes.

Check:

jq -cn '$ARGS.named' --arg test 'string_with"quotes"' | jq .test | tr -d '"'

vs

jq -cn '$ARGS.named' --arg test 'string_with"quotes"' | jq -r .test

Quality checks

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

I've noticed that tr is used to mitigate enclosing quotes (") from the result of `aws ssm get-parameter`.

This might be destructive, because tr will remove *all* quotes, not only the enclosing ones and will not honor JSON escaped quotes.

Check:

`jq -cn '$ARGS.named' --arg test 'string_with"quotes"' | jq .test | tr -d '"'`

vs

`jq -cn '$ARGS.named' --arg test 'string_with"quotes"' | jq -r .test`
Copy link

netlify bot commented Dec 13, 2024

Deploy Preview for eks-workshop ready!

Name Link
🔨 Latest commit 6b19bf5
🔍 Latest deploy log https://app.netlify.com/sites/eks-workshop/deploys/675c221f272b730008f9f8e2
😎 Deploy Preview https://deploy-preview-1196--eks-workshop.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@niallthomson
Copy link
Contributor

Looks good, thanks @matschundbrei !

@niallthomson niallthomson merged commit b1f7967 into aws-samples:main Dec 16, 2024
5 checks passed
@niallthomson niallthomson added this to the Release 12/20 milestone Dec 16, 2024
@niallthomson niallthomson changed the title fix: remove unnecessary use of tr in access-opensearch.md fix: Remove unnecessary use of tr in OpenSearch lab Dec 16, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants