From ba21df035b79c907dd1cbc4898e83a7557109553 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Mon, 23 Sep 2024 14:19:43 +0200 Subject: [PATCH 1/6] Enclose array expansions in double quote Fixes the following shellcheck error: SC2068 (error): Double quote array expansions to avoid re-splitting elements. https://www.shellcheck.net/wiki/SC2068 --- hack/deploy-setup.sh | 2 +- hack/vf-netns-switcher.sh | 8 ++++---- test/scripts/enable-kargs_test.sh | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hack/deploy-setup.sh b/hack/deploy-setup.sh index 2c2fc7d8d..807479c38 100755 --- a/hack/deploy-setup.sh +++ b/hack/deploy-setup.sh @@ -22,7 +22,7 @@ load_manifest() { fi files="service_account.yaml role.yaml role_binding.yaml clusterrole.yaml clusterrolebinding.yaml configmap.yaml sriovoperatorconfig.yaml operator.yaml" for m in ${files}; do - if [ "$(echo ${EXCLUSIONS[@]} | grep -o ${m} | wc -w | xargs)" == "0" ] ; then + if [ "$(echo "${EXCLUSIONS[@]}" | grep -o ${m} | wc -w | xargs)" == "0" ] ; then envsubst< ${m} | ${OPERATOR_EXEC} apply ${namespace:-} --validate=false -f - fi done diff --git a/hack/vf-netns-switcher.sh b/hack/vf-netns-switcher.sh index de4e8041a..e842a8dc8 100755 --- a/hack/vf-netns-switcher.sh +++ b/hack/vf-netns-switcher.sh @@ -95,7 +95,7 @@ It must be of the form :,. This flag can be repeated to specify done return_interfaces_to_default_namespace(){ - for netns in ${netnses[@]};do + for netns in "${netnses[@]}";do for pf in ${pfs[$netns]};do return_interface_to_default_namespace "${netns}" "${pf}" done @@ -360,7 +360,7 @@ main(){ trap return_interfaces_to_default_namespace INT EXIT TERM while true;do - for netns in ${netnses[@]};do + for netns in "${netnses[@]}";do switch_pfs "$netns" "${pfs[$netns]}" sleep 2 switch_netns_vfs "$netns" @@ -388,7 +388,7 @@ if [[ "$status" != "0" ]];then exit $status fi -for netns in ${netnses[@]};do +for netns in "${netnses[@]}";do netns_create "$netns" let status=$status+$? if [[ "$status" != "0" ]];then @@ -397,7 +397,7 @@ for netns in ${netnses[@]};do fi done -for netns in ${netnses[@]};do +for netns in "${netnses[@]}";do get_pcis_from_pfs "$netns" "${pfs[$netns]}" get_pf_switch_dev_info "$netns" "${pfs[$netns]}" done diff --git a/test/scripts/enable-kargs_test.sh b/test/scripts/enable-kargs_test.sh index 615f3d2b2..40c2764be 100755 --- a/test/scripts/enable-kargs_test.sh +++ b/test/scripts/enable-kargs_test.sh @@ -46,7 +46,7 @@ setUp() { # Mock chroot calls to the temporary test folder export real_chroot=$(which chroot) chroot() { - $real_chroot $FAKE_HOST ${@:2} + $real_chroot $FAKE_HOST "${@:2}" } export -f chroot From 3d553bfd6985fbd7225f615577148ed1e6a42963 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Mon, 23 Sep 2024 14:22:26 +0200 Subject: [PATCH 2/6] Add missing shebang Fixes the following shellcheck error: SC2148 (error): Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive. https://www.shellcheck.net/wiki/SC2148 --- hack/env.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hack/env.sh b/hack/env.sh index c49c399d8..64f79212d 100755 --- a/hack/env.sh +++ b/hack/env.sh @@ -1,3 +1,5 @@ +#!/bin/bash + if [ -z $SKIP_VAR_SET ]; then export SRIOV_CNI_IMAGE=${SRIOV_CNI_IMAGE:-ghcr.io/k8snetworkplumbingwg/sriov-cni} export SRIOV_INFINIBAND_CNI_IMAGE=${SRIOV_INFINIBAND_CNI_IMAGE:-ghcr.io/k8snetworkplumbingwg/ib-sriov-cni} From 63246d6918a155fc9cbe2aed057274a5dcc9503d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Mon, 23 Sep 2024 15:51:05 +0200 Subject: [PATCH 3/6] Explicitly expand array values Fixes the following shellcheck errors: SC2145 (error): Argument mixes string and array. Use * or separate argument. SC2199 (error): Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @). https://www.shellcheck.net/wiki/SC2145 https://www.shellcheck.net/wiki/SC2199 Also fixes a typo in SUPPORTED_INTERFACE_SWITCHER_MODES. --- hack/run-e2e-test-kind.sh | 6 +++--- hack/vf-netns-switcher.sh | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hack/run-e2e-test-kind.sh b/hack/run-e2e-test-kind.sh index 5cb7750c7..3cc080d9c 100755 --- a/hack/run-e2e-test-kind.sh +++ b/hack/run-e2e-test-kind.sh @@ -6,7 +6,7 @@ export SRIOV_NETWORK_OPERATOR_IMAGE="${SRIOV_NETWORK_OPERATOR_IMAGE:-sriov-netwo export SRIOV_NETWORK_CONFIG_DAEMON_IMAGE="${SRIOV_NETWORK_CONFIG_DAEMON_IMAGE:-origin-sriov-network-config-daemon:e2e-test}" export KUBECONFIG="${KUBECONFIG:-${HOME}/.kube/config}" INTERFACES_SWITCHER="${INTERFACES_SWITCHER:-"test-suite"}" -SUPPORTED_INTERFACE_SWTICHER_MODES=("test-suite", "system-service") +SUPPORTED_INTERFACE_SWITCHER_MODES=("test-suite", "system-service") RETRY_MAX=10 INTERVAL=10 TIMEOUT=300 @@ -16,9 +16,9 @@ while test $# -gt 0; do case "$1" in --device-netns-switcher) INTERFACES_SWITCHER="$2" - if [[ ! "${SUPPORTED_INTERFACE_SWTICHER_MODES[@]}" =~ "${INTERFACES_SWITCHER}" ]]; then + if [[ ! "${SUPPORTED_INTERFACE_SWITCHER_MODES[*]}" =~ "${INTERFACES_SWITCHER}" ]]; then echo "Error: unsupported interface switching mode: ${INTERFACES_SWITCHER}!" - echo "Supported modes are: ${SUPPORTED_INTERFACE_SWTICHER_MODES[@]}" + echo "Supported modes are: ${SUPPORTED_INTERFACE_SWITCHER_MODES[*]}" exit 1 fi shift diff --git a/hack/vf-netns-switcher.sh b/hack/vf-netns-switcher.sh index e842a8dc8..69881da7b 100755 --- a/hack/vf-netns-switcher.sh +++ b/hack/vf-netns-switcher.sh @@ -348,7 +348,7 @@ variables_check(){ check_empty_var(){ local var_name="$1" - if [[ -z "${!var_name[@]}" ]];then + if [[ -z "${!var_name[*]}" ]];then echo "Error: $var_name is empty..." return 1 fi @@ -403,7 +403,7 @@ for netns in "${netnses[@]}";do done if [[ "${#pcis[@]}" == "0" ]];then - echo "Error: could not get pci addresses of interfaces ${pfs[@]}!!" + echo "Error: could not get pci addresses of interfaces ${pfs[*]}!!" exit 1 fi From 3529811b1d3a0833dacc2e7fc27425749562f769 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Mon, 23 Sep 2024 17:43:01 +0200 Subject: [PATCH 4/6] Iterate over globs. Fixes the following shellcheck error: SC2045 (error): Iterating over ls output is fragile. Use globs. https://www.shellcheck.net/wiki/SC2045 --- hack/vf-netns-switcher.sh | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/hack/vf-netns-switcher.sh b/hack/vf-netns-switcher.sh index 69881da7b..c383b5d1e 100755 --- a/hack/vf-netns-switcher.sh +++ b/hack/vf-netns-switcher.sh @@ -277,19 +277,20 @@ switch_interface_vf_representors(){ return 0 fi - for interface in $(ls /sys/class/net);do - phys_switch_id=$(cat /sys/class/net/$interface/phys_switch_id) + for interface in /sys/class/net/*;do + phys_switch_id=$(cat $interface/phys_switch_id) if [[ "$phys_switch_id" != "${pf_switch_ids[$pf_name]}" ]]; then continue fi - phys_port_name=$(cat /sys/class/net/$interface/phys_port_name) + phys_port_name=$(cat $interface/phys_port_name) phys_port_name_pf_index=${phys_port_name%vf*} phys_port_name_pf_index=${phys_port_name_pf_index#pf} if [[ "$phys_port_name_pf_index" != "${pf_port_names[$pf_name]:1}" ]]; then continue fi - echo "Switching VF representor $interface of PF $pf_name to netns $worker_netns" - switch_vf $interface $worker_netns + interface_name=${interface##*/} + echo "Switching VF representor $interface_name of PF $pf_name to netns $worker_netns" + switch_vf $interface_name $worker_netns done } From fb193e80038325b4c9bc8d8012d809eca9bc46da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Thu, 26 Sep 2024 14:23:34 +0200 Subject: [PATCH 5/6] Use grep for matching args with sh Fixes the following shellcheck error: SC2081 (error): [ .. ] can't match globs. Use a case statement. https://www.shellcheck.net/wiki/SC2081 --- test/scripts/enable-kargs_test.sh | 1 + test/scripts/rpm-ostree_mock | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/scripts/enable-kargs_test.sh b/test/scripts/enable-kargs_test.sh index 40c2764be..93a985700 100755 --- a/test/scripts/enable-kargs_test.sh +++ b/test/scripts/enable-kargs_test.sh @@ -40,6 +40,7 @@ setUp() { cp $(which cat) ${FAKE_HOST}/usr/bin/ cp $(which test) ${FAKE_HOST}/usr/bin/ cp $(which sh) ${FAKE_HOST}/usr/bin/ + cp $(which grep) ${FAKE_HOST}/usr/bin/ cp "$SCRIPTPATH/rpm-ostree_mock" ${FAKE_HOST}/usr/bin/rpm-ostree } diff --git a/test/scripts/rpm-ostree_mock b/test/scripts/rpm-ostree_mock index 16e816cc9..db6f66040 100755 --- a/test/scripts/rpm-ostree_mock +++ b/test/scripts/rpm-ostree_mock @@ -5,7 +5,7 @@ # Write invocation with arguments to a file to allow making assertion. echo "$*" >> /rpm-ostree_calls -if [ "$*" != *"--append"* ] +if ! echo "$*" | grep -q "\--append" then # Caller is trying to read kernel arguments. cat /proc/cmdline From 5394d218f8c50ab7acf05558c1777491115fdbaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Mon, 23 Sep 2024 17:58:54 +0200 Subject: [PATCH 6/6] CI: Add a bash linter to pre-submits Warns about shellcheck issues with severity `error`. --- .github/workflows/test.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2fbe84c81..d59e52e47 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -100,6 +100,16 @@ jobs: # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. version: v1.55.2 + shellcheck: + name: Shellcheck + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Run ShellCheck + uses: ludeeus/action-shellcheck@master + with: + severity: error + test-coverage: name: test-coverage runs-on: ubuntu-latest