From 5df43ffc7c461899cf046812cd02a470789162d1 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Tue, 18 Mar 2025 23:23:59 +0000 Subject: [PATCH 01/11] tests: k8s/tests_common.sh: Prefer [[ ]] over [ ] Replace [ ] with [[ ]] as advised by shellcheck: note: Prefer [[ ]] over [ ] for tests in Bash/Ksh. [SC2292] Signed-off-by: Dan Mihai --- tests/integration/kubernetes/tests_common.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index bd65cbdee8..cb98cd1668 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -47,17 +47,17 @@ ALLOW_ALL_POLICY="${ALLOW_ALL_POLICY:-$(base64 -w 0 "${K8S_TEST_DIR}/../../../sr # setup_common() { node=$(get_one_kata_node) - [ -n "$node" ] + [[ -n "$node" ]] node_start_time=$(exec_host "$node" date +\"%Y-%m-%d %H:%M:%S\") # If node_start_time is empty, try again 3 times with a 5 seconds sleep between each try. count=0 - while [ -z "$node_start_time" ] && [ $count -lt 3 ]; do + while [[ -z "$node_start_time" ]] && [[ $count -lt 3 ]]; do echo "node_start_time is empty, trying again..." sleep 5 node_start_time=$(exec_host "$node" date +\"%Y-%m-%d %H:%M:%S\") count=$((count + 1)) done - [ -n "$node_start_time" ] + [[ -n "$node_start_time" ]] export node node_start_time k8s_delete_all_pods_if_any_exists || true @@ -77,7 +77,7 @@ get_one_kata_node() { } auto_generate_policy_enabled() { - [ "${AUTO_GENERATE_POLICY}" == "yes" ] + [[ "${AUTO_GENERATE_POLICY}" == "yes" ]] } # adapt common policy settings for tdx or snp @@ -174,7 +174,7 @@ delete_tmp_policy_settings_dir() { auto_generate_policy_enabled || return 0 - if [ -d "${settings_dir}" ]; then + if [[ -d "${settings_dir}" ]]; then info "Deleting ${settings_dir}" rm -rf "${settings_dir}" fi @@ -192,11 +192,11 @@ auto_generate_policy() { genpolicy_command+=" -p ${settings_dir}/rules.rego" genpolicy_command+=" -j ${settings_dir}/genpolicy-settings.json" - if [ ! -z "${config_map_yaml_file}" ]; then + if [[ ! -z "${config_map_yaml_file}" ]]; then genpolicy_command+=" -c ${config_map_yaml_file}" fi - if [ "${GENPOLICY_PULL_METHOD}" == "containerd" ]; then + if [[ "${GENPOLICY_PULL_METHOD}" == "containerd" ]]; then genpolicy_command+=" -d" fi @@ -273,7 +273,7 @@ hard_coded_policy_tests_enabled() { # specifying AUTO_GENERATE_POLICY=yes. local enabled_hypervisors="qemu-coco-dev qemu-sev qemu-snp qemu-tdx" [[ " $enabled_hypervisors " =~ " ${KATA_HYPERVISOR} " ]] || \ - [ "${KATA_HOST_OS}" == "cbl-mariner" ] || \ + [[ "${KATA_HOST_OS}" == "cbl-mariner" ]] || \ auto_generate_policy_enabled } From 0d3f9fcee1c7bc65b7f79d45a9eda8eec3703905 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Tue, 18 Mar 2025 23:29:09 +0000 Subject: [PATCH 02/11] tests: tests_common: export variables used externally ShellCheck: export variables used outside of tests_common.sh - e.g., warning: timeout appears unused. Verify use (or export if used externally). [SC2034] Signed-off-by: Dan Mihai --- tests/integration/kubernetes/tests_common.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index cb98cd1668..a1fd41cc85 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -17,16 +17,16 @@ export container_images_agnhost_version="2.21" # Timeout options, mainly for use with waitForProcess(). Use them unless the # operation needs to wait longer. -wait_time=90 -sleep_time=3 +export wait_time=90 +export sleep_time=3 # Timeout for use with `kubectl wait`, unless it needs to wait longer. # Note: try to keep timeout and wait_time equal. -timeout=90s +export timeout=90s # issues that can't test yet. -fc_limitations="https://github.com/kata-containers/documentation/issues/351" -dragonball_limitations="https://github.com/kata-containers/kata-containers/issues/6621" +export fc_limitations="https://github.com/kata-containers/documentation/issues/351" +export dragonball_limitations="https://github.com/kata-containers/kata-containers/issues/6621" # Path to the kubeconfig file which is used by kubectl and other tools. # Note: the init script sets that variable but if you want to run the tests in @@ -64,7 +64,7 @@ setup_common() { } get_pod_config_dir() { - pod_config_dir="${BATS_TEST_DIRNAME}/runtimeclass_workloads_work" + export pod_config_dir="${BATS_TEST_DIRNAME}/runtimeclass_workloads_work" info "k8s configured to use runtimeclass" } From cc5f8d31d2a0a6e9b458e4ac23f3d68ad76e6d76 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Tue, 18 Mar 2025 23:37:41 +0000 Subject: [PATCH 03/11] tests: k8s/tests_common.sh: add braces ShellCheck: add braces around variable references: note: Prefer putting braces around variable references even when not strictly required. [SC2250] Signed-off-by: Dan Mihai --- tests/integration/kubernetes/tests_common.sh | 22 ++++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index a1fd41cc85..77ec29f55f 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -32,7 +32,7 @@ export dragonball_limitations="https://github.com/kata-containers/kata-container # Note: the init script sets that variable but if you want to run the tests in # your own provisioned cluster and you know what you are doing then you should # overwrite it. -export KUBECONFIG="${KUBECONFIG:-$HOME/.kube/config}" +export KUBECONFIG="${KUBECONFIG:-${HOME}/.kube/config}" # ALLOW_ALL_POLICY is a Rego policy that allows all the Agent ttrpc requests. K8S_TEST_DIR="${kubernetes_dir:-"${BATS_TEST_DIRNAME}"}" @@ -47,17 +47,17 @@ ALLOW_ALL_POLICY="${ALLOW_ALL_POLICY:-$(base64 -w 0 "${K8S_TEST_DIR}/../../../sr # setup_common() { node=$(get_one_kata_node) - [[ -n "$node" ]] - node_start_time=$(exec_host "$node" date +\"%Y-%m-%d %H:%M:%S\") + [[ -n "${node}" ]] + node_start_time=$(exec_host "${node}" date +\"%Y-%m-%d %H:%M:%S\") # If node_start_time is empty, try again 3 times with a 5 seconds sleep between each try. count=0 - while [[ -z "$node_start_time" ]] && [[ $count -lt 3 ]]; do + while [[ -z "${node_start_time}" ]] && [[ ${count} -lt 3 ]]; do echo "node_start_time is empty, trying again..." sleep 5 - node_start_time=$(exec_host "$node" date +\"%Y-%m-%d %H:%M:%S\") + node_start_time=$(exec_host "${node}" date +\"%Y-%m-%d %H:%M:%S\") count=$((count + 1)) done - [[ -n "$node_start_time" ]] + [[ -n "${node_start_time}" ]] export node node_start_time k8s_delete_all_pods_if_any_exists || true @@ -272,7 +272,7 @@ hard_coded_policy_tests_enabled() { # users can enable testing of the same policies (plus the auto-generated policies) by # specifying AUTO_GENERATE_POLICY=yes. local enabled_hypervisors="qemu-coco-dev qemu-sev qemu-snp qemu-tdx" - [[ " $enabled_hypervisors " =~ " ${KATA_HYPERVISOR} " ]] || \ + [[ " ${enabled_hypervisors} " =~ " ${KATA_HYPERVISOR} " ]] || \ [[ "${KATA_HOST_OS}" == "cbl-mariner" ]] || \ auto_generate_policy_enabled } @@ -324,7 +324,7 @@ wait_for_blocked_request() { local -r command="kubectl describe pod ${pod} | grep \"${endpoint} is blocked by policy\"" info "Waiting ${wait_time} seconds for: ${command}" - waitForProcess "${wait_time}" "$sleep_time" "${command}" >/dev/null 2>/dev/null + waitForProcess "${wait_time}" "${sleep_time}" "${command}" >/dev/null 2>/dev/null } # Execute in a pod a command that is allowed by policy. @@ -370,9 +370,9 @@ teardown_common() { k8s_delete_all_pods_if_any_exists || true # Print the node journal since the test start time if a bats test is not completed - if [[ -n "${node_start_time}" && -z "$BATS_TEST_COMPLETED" ]]; then - echo "DEBUG: system logs of node '$node' since test start time ($node_start_time)" - exec_host "${node}" journalctl -x -t "kata" --since '"'$node_start_time'"' || true + if [[ -n "${node_start_time}" && -z "${BATS_TEST_COMPLETED}" ]]; then + echo "DEBUG: system logs of node '${node}' since test start time (${node_start_time})" + exec_host "${node}" journalctl -x -t "kata" --since '"'${node_start_time}'"' || true fi } From 4589dc96efa69cd1b0663339b81fd498c107f31f Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Tue, 18 Mar 2025 23:41:13 +0000 Subject: [PATCH 04/11] tests: k8s/tests_common.sh: add double quoting ShellCheck: note: Prefer double quoting even when variables don't contain special characters. [SC2248] Signed-off-by: Dan Mihai --- tests/integration/kubernetes/tests_common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index 77ec29f55f..670d57fd8e 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -51,7 +51,7 @@ setup_common() { node_start_time=$(exec_host "${node}" date +\"%Y-%m-%d %H:%M:%S\") # If node_start_time is empty, try again 3 times with a 5 seconds sleep between each try. count=0 - while [[ -z "${node_start_time}" ]] && [[ ${count} -lt 3 ]]; do + while [[ -z "${node_start_time}" ]] && [[ "${count}" -lt 3 ]]; do echo "node_start_time is empty, trying again..." sleep 5 node_start_time=$(exec_host "${node}" date +\"%Y-%m-%d %H:%M:%S\") From 15961b03f7e3676ca9c63b851c6fcbc1c379ca9b Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Tue, 18 Mar 2025 23:45:59 +0000 Subject: [PATCH 05/11] tests: k8s/tests_common.sh: -n instead of ! -z ShellCheck: note: Use -n instead of ! -z. [SC2236] Signed-off-by: Dan Mihai --- tests/integration/kubernetes/tests_common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index 670d57fd8e..62fd5e0d82 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -192,7 +192,7 @@ auto_generate_policy() { genpolicy_command+=" -p ${settings_dir}/rules.rego" genpolicy_command+=" -j ${settings_dir}/genpolicy-settings.json" - if [[ ! -z "${config_map_yaml_file}" ]]; then + if [[ -n "${config_map_yaml_file}" ]]; then genpolicy_command+=" -c ${config_map_yaml_file}" fi From 9c0d069ac79790ebaeca96349b6051fc6c104ac7 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Tue, 18 Mar 2025 23:50:39 +0000 Subject: [PATCH 06/11] tests: tests_common: prevent globbing and word splitting ShellCheck: note: Double quote to prevent globbing and word splitting. [SC2086] Signed-off-by: Dan Mihai --- tests/integration/kubernetes/tests_common.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index 62fd5e0d82..e72afff771 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -284,7 +284,7 @@ add_allow_all_policy_to_yaml() { # Previous version of yq was not ready to handle multiple objects in a single yaml. # By default was changing only the first object. # With yq>4 we need to make it explicit during the read and write. - local resource_kind="$(yq .kind ${yaml_file} | head -1)" + local resource_kind=$(yq .kind "${yaml_file}" | head -1) case "${resource_kind}" in @@ -372,7 +372,7 @@ teardown_common() { # Print the node journal since the test start time if a bats test is not completed if [[ -n "${node_start_time}" && -z "${BATS_TEST_COMPLETED}" ]]; then echo "DEBUG: system logs of node '${node}' since test start time (${node_start_time})" - exec_host "${node}" journalctl -x -t "kata" --since '"'${node_start_time}'"' || true + exec_host "${node}" journalctl -x -t "kata" --since '"'"${node_start_time}"'"' || true fi } From 0f4de1c94a828d6c902495c527ecc8424360f167 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Tue, 18 Mar 2025 23:55:40 +0000 Subject: [PATCH 07/11] tests: tests_common: remove useless assignment ShellCheck: warning: This assignment is only seen by the forked process. [SC2097] warning: This expansion will not see the mentioned assignment. [SC2098] Signed-off-by: Dan Mihai --- tests/integration/kubernetes/tests_common.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index e72afff771..853789439e 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -290,14 +290,14 @@ add_allow_all_policy_to_yaml() { Pod) info "Adding allow all policy to ${resource_kind} from ${yaml_file}" - ALLOW_ALL_POLICY="${ALLOW_ALL_POLICY}" yq -i \ + yq -i \ ".metadata.annotations.\"io.katacontainers.config.agent.policy\" = \"${ALLOW_ALL_POLICY}\"" \ "${yaml_file}" ;; Deployment|Job|ReplicationController) info "Adding allow all policy to ${resource_kind} from ${yaml_file}" - ALLOW_ALL_POLICY="${ALLOW_ALL_POLICY}" yq -i \ + yq -i \ ".spec.template.metadata.annotations.\"io.katacontainers.config.agent.policy\" = \"${ALLOW_ALL_POLICY}\"" \ "${yaml_file}" ;; From b895e3b3e5abc40c4f6fb4a2d38827df00193c3e Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Wed, 19 Mar 2025 00:03:14 +0000 Subject: [PATCH 08/11] tests: k8s/tests_common.sh: add variable assignments Pick the the values exported by other scripts. ShellCheck: warning: AUTO_GENERATE_POLICY is referenced but not assigned. [SC2154] Signed-off-by: Dan Mihai --- tests/integration/kubernetes/tests_common.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index 853789439e..9ab9b3b1f7 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -38,6 +38,11 @@ export KUBECONFIG="${KUBECONFIG:-${HOME}/.kube/config}" K8S_TEST_DIR="${kubernetes_dir:-"${BATS_TEST_DIRNAME}"}" ALLOW_ALL_POLICY="${ALLOW_ALL_POLICY:-$(base64 -w 0 "${K8S_TEST_DIR}/../../../src/kata-opa/allow-all.rego")}" +AUTO_GENERATE_POLICY="${AUTO_GENERATE_POLICY:-}" +GENPOLICY_PULL_METHOD="${GENPOLICY_PULL_METHOD:-}" +KATA_HYPERVISOR="${KATA_HYPERVISOR:-}" +KATA_HOST_OS="${KATA_HOST_OS:-}" + # Common setup for tests. # # Global variables exported: From 59a70a2b28757dabb2ad2dfe3379a00364a520d3 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Wed, 19 Mar 2025 00:09:32 +0000 Subject: [PATCH 09/11] tests: k8s/tests_common: avoid masking return values Avoid masking command return values by declaring and only then assigning. ShellCheck: warning: Declare and assign separately to avoid masking return values. [SC2155] Signed-off-by: Dan Mihai --- tests/integration/kubernetes/tests_common.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index 9ab9b3b1f7..e30972a786 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -220,7 +220,8 @@ add_exec_to_policy_settings() { shift # Create a JSON array of strings containing all the args of the command to be allowed. - local exec_args=$(printf "%s\n" "$@" | jq -R | jq -sc) + local exec_args + exec_args=$(printf "%s\n" "$@" | jq -R | jq -sc) # Change genpolicy settings to allow kubectl to exec the command specified by the caller. local jq_command=".request_defaults.ExecProcessRequest.allowed_commands |= . + [${exec_args}]" @@ -289,7 +290,8 @@ add_allow_all_policy_to_yaml() { # Previous version of yq was not ready to handle multiple objects in a single yaml. # By default was changing only the first object. # With yq>4 we need to make it explicit during the read and write. - local resource_kind=$(yq .kind "${yaml_file}" | head -1) + local resource_kind + resource_kind=$(yq .kind "${yaml_file}" | head -1) case "${resource_kind}" in From d83b8349a2fcbe099514cc98de6cf70ddd3c7463 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Wed, 19 Mar 2025 00:13:46 +0000 Subject: [PATCH 10/11] tests: policy: avoid using caller's variable Fix unintended use of caller's variable. Use the corresponding function parameter instead. ShellCheck: warning: policy_settings_dir is referenced but not assigned. [SC2154] Signed-off-by: Dan Mihai --- tests/integration/kubernetes/tests_common.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index e30972a786..1f2ede004e 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -258,9 +258,9 @@ add_copy_from_host_to_policy_settings() { local -r genpolicy_settings_dir="$1" local exec_command=(test -d /tmp) - add_exec_to_policy_settings "${policy_settings_dir}" "${exec_command[@]}" + add_exec_to_policy_settings "${genpolicy_settings_dir}" "${exec_command[@]}" exec_command=(tar -xmf - -C /tmp) - add_exec_to_policy_settings "${policy_settings_dir}" "${exec_command[@]}" + add_exec_to_policy_settings "${genpolicy_settings_dir}" "${exec_command[@]}" } # Change genpolicy settings to allow executing on the Guest VM the commands @@ -270,7 +270,7 @@ add_copy_from_guest_to_policy_settings() { local -r copied_file="$2" exec_command=(tar cf - "${copied_file}") - add_exec_to_policy_settings "${policy_settings_dir}" "${exec_command[@]}" + add_exec_to_policy_settings "${genpolicy_settings_dir}" "${exec_command[@]}" } hard_coded_policy_tests_enabled() { From 835c6814d76ac9a1a00d36661efcb43826df2eea Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Wed, 19 Mar 2025 00:20:20 +0000 Subject: [PATCH 11/11] tests: k8s/tests_common: avoid using regex More straightforward implementation of hard_coded_policy_tests_enabled, that avoids ShellCheck warning: warning: Remove quotes from right-hand side of =~ to match as a regex rather than literally. [SC2076] Signed-off-by: Dan Mihai --- tests/integration/kubernetes/tests_common.sh | 23 ++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index 1f2ede004e..d87c9fad43 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -274,13 +274,28 @@ add_copy_from_guest_to_policy_settings() { } hard_coded_policy_tests_enabled() { + local enabled="no" # CI is testing hard-coded policies just on a the platforms listed here. Outside of CI, # users can enable testing of the same policies (plus the auto-generated policies) by # specifying AUTO_GENERATE_POLICY=yes. - local enabled_hypervisors="qemu-coco-dev qemu-sev qemu-snp qemu-tdx" - [[ " ${enabled_hypervisors} " =~ " ${KATA_HYPERVISOR} " ]] || \ - [[ "${KATA_HOST_OS}" == "cbl-mariner" ]] || \ - auto_generate_policy_enabled + local -r enabled_hypervisors=("qemu-coco-dev" "qemu-sev" "qemu-snp" "qemu-tdx") + for enabled_hypervisor in "${enabled_hypervisors[@]}" + do + if [[ "${enabled_hypervisor}" == "${KATA_HYPERVISOR}" ]]; then + enabled="yes" + break + fi + done + + if [[ "${enabled}" == "no" && "${KATA_HOST_OS}" == "cbl-mariner" ]]; then + enabled="yes" + fi + + if [[ "${enabled}" == "no" ]] && auto_generate_policy_enabled; then + enabled="yes" + fi + + [[ "${enabled}" == "yes" ]] } add_allow_all_policy_to_yaml() {