Skip to content

Conversation

wainersm
Copy link
Contributor

Addressed the following shellcheck advices:

SC2046 (warning): Quote this to prevent word splitting.
SC2248 (style): Prefer double quoting even when variables don't contain special characters
SC2250 (style): Prefer putting braces around variable references even when not strictly required.
SC2292 (style): Prefer [[ ]] over [ ] for tests in Bash/Ksh

Resulting in a free of warning/info/style file.


I have fixed some stuffs on that file recently and seeing the shellcheck advices was making me sick, hence this PR that I hope doesn't introduce unintentional bugs (always are a risk).

@wainersm wainersm added the area/testing Issues related to testing label May 20, 2025
@katacontainersbot katacontainersbot added the size/large Task of significant size label May 20, 2025
@wainersm
Copy link
Contributor Author

The shellcheck action has caught some extra warnings that running locally didn't. I will address these too:

./tests/integration/kubernetes/confidential_kbs.sh:15:8: note: Not following: ./../../../tools/packaging/guest-image/lib_se.sh: openBinaryFile: does not exist (No such file or directory) [SC1091]
./tests/integration/kubernetes/confidential_kbs.sh:210:2: warning: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:214:2: warning: Use 'popd ... || exit' or 'popd ... || return' in case popd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:219:3: warning: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:221:3: warning: Use 'popd ... || exit' or 'popd ... || return' in case popd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:232:2: warning: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:244:2: warning: Use 'popd ... || exit' or 'popd ... || return' in case popd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:293:2: warning: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:296:2: warning: Use 'popd ... || exit' or 'popd ... || return' in case popd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:299:2: warning: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:316:2: warning: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:318:2: warning: Use 'popd ... || exit' or 'popd ... || return' in case popd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:325:3: warning: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:330:3: warning: Use 'popd ... || exit' or 'popd ... || return' in case popd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:338:4: warning: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:342:4: warning: Use 'popd ... || exit' or 'popd ... || return' in case popd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:360:2: warning: Use 'popd ... || exit' or 'popd ... || return' in case popd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:544:2: warning: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:556:2: warning: Use 'popd ... || exit' or 'popd ... || return' in case popd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:593:14: warning: HKD_PATH is referenced but not assigned. [SC2154]
./tests/integration/kubernetes/confidential_kbs.sh:597:2: warning: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails. [SC2164]
./tests/integration/kubernetes/confidential_kbs.sh:606:2: warning: Use 'popd ... || exit' or 'popd ... || return' in case popd fails. [SC2164]

wainersm added 3 commits May 22, 2025 14:52
Addressed the following shellcheck advices:

SC2046 (warning): Quote this to prevent word splitting.
SC2248 (style): Prefer double quoting even when variables don't contain special characters
SC2250 (style): Prefer putting braces around variable references even when not strictly required.
SC2292 (style): Prefer [[ ]] over [ ] for tests in Bash/Ksh

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Although the script will inherit that setting from the caller scripts,
expliciting it in the file will vanish shellcheck "warning: Use 'pushd
... || exit' or 'pushd ... || return' in case pushd fails. [SC2164]"

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Fixed "warning: HKD_PATH is referenced but not assigned. [SC2154]"

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
@wainersm wainersm force-pushed the delint_confidential_kbs branch from 2660376 to c9fb0b9 Compare May 22, 2025 18:04
@wainersm
Copy link
Contributor Author

Added two commits to address #11294 (comment) although I am not able to have shellcheck to emit that warnings neither in VSCode nor running shellcheck (version 0.10.0, fedora packaged)

Fixed "note: Not following: ./../../../tools/packaging/guest-image/lib_se.sh:
openBinaryFile: does not exist (No such file or directory) [SC1091]"

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
@wainersm
Copy link
Contributor Author

one more commit and it's free of shellcheck complains \o/

Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @wainersm!

Copy link
Member

@RuoqingHe RuoqingHe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks @wainersm ❤️

@RuoqingHe RuoqingHe merged commit a9ffdfc into kata-containers:main May 23, 2025
333 of 358 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Issues related to testing ok-to-test size/large Task of significant size
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants