Skip to content

Conversation

@simondeziel
Copy link
Member

@simondeziel simondeziel commented Jul 11, 2024

This PR is mainly to improve debuggability. Here's why:

# OK, no problem if we get what we expect
echo foo > test-file
cat test-file | grep -Fx "foo"

# Impossible to figure what we got if it's not what we expected
echo bar > test-file
cat test-file | grep -Fx "foo"

Versus:

# OK, no problem if we get what we expect
echo foo > test-file
[ "$(cat test-file)" = "foo" ]

# Easy to figure what we got if it's not what we expected
echo bar > test-file
[ "$(cat test-file)" = "foo" ]

In the last case, it will be easy to figure what was in the test file as we'll see [ "bar" = "foo" ] thanks to the test being run with set -x.

Additional bonus: this caught a bogus invocation of curl where the output was not properly redirected.

…to succeed

Also use `grep -F` as fingerprints are fixed strings.

Signed-off-by: Simon Deziel <[email protected]>
! s3cmdrun "${lxd_backend}" "${roAccessKey}" "${roSecretKey}" delpolicy "s3://${bucketPrefix}.foo" || false
s3cmdrun "${lxd_backend}" "${adAccessKey}" "${adSecretKey}" delpolicy "s3://${bucketPrefix}.foo"
curl -sI --insecure o /dev/null -w "%{http_code}" "${bucketURL}/${lxdTestFile}" | grep -Fx "403"
[ "$(curl -sI --insecure o /dev/null -w "%{http_code}" "${bucketURL}/${lxdTestFile}" = "403" ]

Check warning

Code scanning / shellcheck

SC1009

The mentioned syntax error was in this double quoted string.
! s3cmdrun "${lxd_backend}" "${roAccessKey}" "${roSecretKey}" delpolicy "s3://${bucketPrefix}.foo" || false
s3cmdrun "${lxd_backend}" "${adAccessKey}" "${adSecretKey}" delpolicy "s3://${bucketPrefix}.foo"
curl -sI --insecure o /dev/null -w "%{http_code}" "${bucketURL}/${lxdTestFile}" | grep -Fx "403"
[ "$(curl -sI --insecure o /dev/null -w "%{http_code}" "${bucketURL}/${lxdTestFile}" = "403" ]

Check failure

Code scanning / shellcheck

SC1073

Couldn't parse this command expansion. Fix to allow more checks.
Also fix one of the `curl` call missing `-` when trying to send the output to
`/dev/null`.

Signed-off-by: Simon Deziel <[email protected]>
@simondeziel simondeziel marked this pull request as ready for review July 11, 2024 14:53
@simondeziel simondeziel requested a review from tomponline as a code owner July 11, 2024 14:53
@simondeziel simondeziel requested a review from hamistao July 11, 2024 14:53
@tomponline tomponline merged commit 65869a4 into canonical:main Jul 11, 2024
@simondeziel simondeziel deleted the less-grep-Fx branch July 11, 2024 15:41
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