From 6b16d0051fb29bb55ab9de2e989adb60c0a92622 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Feb 2022 11:05:46 -0800 Subject: [PATCH 1/7] shfmt: add more files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …and fix a single format issue found. Signed-off-by: Kir Kolyshkin --- Makefile | 4 +++- contrib/cmd/seccompagent/gen-seccomp-example-cfg.sh | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index f9045df615a..6b2b43f2d8f 100644 --- a/Makefile +++ b/Makefile @@ -138,7 +138,9 @@ shellcheck: shfmt: shfmt -ln bats -d -w tests/integration/*.bats - shfmt -ln bash -d -w man/*.sh script/* tests/*.sh tests/integration/*.bash + shfmt -ln bash -d -w man/*.sh script/* \ + tests/*.sh tests/integration/*.bash tests/fuzzing/*.sh \ + contrib/completions/bash/runc contrib/cmd/seccompagent/*.sh vendor: $(GO) mod tidy diff --git a/contrib/cmd/seccompagent/gen-seccomp-example-cfg.sh b/contrib/cmd/seccompagent/gen-seccomp-example-cfg.sh index bd4e209cf52..e48f9fd99b4 100755 --- a/contrib/cmd/seccompagent/gen-seccomp-example-cfg.sh +++ b/contrib/cmd/seccompagent/gen-seccomp-example-cfg.sh @@ -12,7 +12,7 @@ fi # exits when not running inside bats. We can do hacks, but just to redefine # update_config() seems clearer. We don't even really need to keep them in sync. function update_config() { - jq "$1" "./config.json" | awk 'BEGIN{RS="";getline<"-";print>ARGV[1]}' "./config.json" + jq "$1" "./config.json" | awk 'BEGIN{RS="";getline<"-";print>ARGV[1]}' "./config.json" } update_config '.linux.seccomp = { From dc73d236eac7bd523e0bd3aa3fa38c2c7f3b7dec Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Feb 2022 13:19:52 -0800 Subject: [PATCH 2/7] script/check-config.sh: fix wrap_color usage 1. Allow wrap_bad and wrap_good to have an optional arguments. 2. Remove unneeded echos; this fixes the shellcheck warnings like In ./script/check-config.sh line 178: echo "$(wrap_bad 'cgroup hierarchy' 'nonexistent??')" ^-- SC2005 (style): Useless echo? Instead of 'echo $(cmd)', just use 'cmd'. 3. Fix missing color argument in calls to wrap_color (when printing the hint about how to install apparmor). Signed-off-by: Kir Kolyshkin --- script/check-config.sh | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/script/check-config.sh b/script/check-config.sh index 0fe13912524..97ec3ef0540 100755 --- a/script/check-config.sh +++ b/script/check-config.sh @@ -73,10 +73,19 @@ wrap_color() { } wrap_good() { - echo "$(wrap_color "$1" white): $(wrap_color "$2" green)" + local name="$1" + shift + local val="$1" + shift + echo "$(wrap_color "$name" white): $(wrap_color "$val" green)" "$@" } + wrap_bad() { - echo "$(wrap_color "$1" bold): $(wrap_color "$2" bold red)" + local name="$1" + shift + local val="$1" + shift + echo "$(wrap_color "$name" bold): $(wrap_color "$val" bold red)" "$@" } wrap_warning() { wrap_color >&2 "$*" red @@ -165,35 +174,35 @@ echo 'Generally Necessary:' echo -n '- ' if [ "$(stat -f -c %t /sys/fs/cgroup 2>/dev/null)" = "63677270" ]; then - echo "$(wrap_good 'cgroup hierarchy' 'cgroupv2')" + wrap_good 'cgroup hierarchy' 'cgroupv2' else cgroupSubsystemDir="$(awk '/[, ](cpu|cpuacct|cpuset|devices|freezer|memory)[, ]/ && $3 == "cgroup" { print $2 }' /proc/mounts | head -n1)" cgroupDir="$(dirname "$cgroupSubsystemDir")" if [ -d "$cgroupDir/cpu" -o -d "$cgroupDir/cpuacct" -o -d "$cgroupDir/cpuset" -o -d "$cgroupDir/devices" -o -d "$cgroupDir/freezer" -o -d "$cgroupDir/memory" ]; then - echo "$(wrap_good 'cgroup hierarchy' 'properly mounted') [$cgroupDir]" + wrap_good 'cgroup hierarchy' 'properly mounted' "[$cgroupDir]" else if [ "$cgroupSubsystemDir" ]; then - echo "$(wrap_bad 'cgroup hierarchy' 'single mountpoint!') [$cgroupSubsystemDir]" + wrap_bad 'cgroup hierarchy' 'single mountpoint!' "[$cgroupSubsystemDir]" else - echo "$(wrap_bad 'cgroup hierarchy' 'nonexistent??')" + wrap_bad 'cgroup hierarchy' 'nonexistent??' fi - echo " $(wrap_color '(see https://github.com/tianon/cgroupfs-mount)' yellow)" + wrap_color ' (see https://github.com/tianon/cgroupfs-mount)' yellow fi fi if [ "$(cat /sys/module/apparmor/parameters/enabled 2>/dev/null)" = 'Y' ]; then echo -n '- ' if command -v apparmor_parser &>/dev/null; then - echo "$(wrap_good 'apparmor' 'enabled and tools installed')" + wrap_good 'apparmor' 'enabled and tools installed' else - echo "$(wrap_bad 'apparmor' 'enabled, but apparmor_parser missing')" + wrap_bad 'apparmor' 'enabled, but apparmor_parser missing' echo -n ' ' if command -v apt-get &>/dev/null; then - echo "$(wrap_color '(use "apt-get install apparmor" to fix this)')" + wrap_color '(use "apt-get install apparmor" to fix this)' yellow elif command -v yum &>/dev/null; then - echo "$(wrap_color '(your best bet is "yum install apparmor-parser")')" + wrap_color '(your best bet is "yum install apparmor-parser")' yellow else - echo "$(wrap_color '(look for an "apparmor" package for your distribution)')" + wrap_color '(look for an "apparmor" package for your distribution)' yellow fi fi fi @@ -236,7 +245,7 @@ echo 'Optional Features:' if [ "$kernelMajor" -lt 5 ] || [ "$kernelMajor" -eq 5 -a "$kernelMinor" -le 8 ]; then check_flags MEMCG_SWAP_ENABLED if is_set MEMCG_SWAP && ! is_set MEMCG_SWAP_ENABLED; then - echo " $(wrap_color '(note that cgroup swap accounting is not enabled in your kernel config, you can enable it by setting boot option "swapaccount=1")' bold black)" + wrap_color ' (note that cgroup swap accounting is not enabled in your kernel config, you can enable it by setting boot option "swapaccount=1")' bold black fi fi } From baa06227a4ec49ae0354c6aa43f1bcf7f372001c Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Feb 2022 13:25:27 -0800 Subject: [PATCH 3/7] script/check-config.sh: fix SC2166 warnings Like this one: In ./script/check-config.sh line 215: if [ "$kernelMajor" -lt 5 ] || [ "$kernelMajor" -eq 5 -a "$kernelMinor" -le 1 ]; then ^-- SC2166 (warning): Prefer [ p ] && [ q ] as [ p -a q ] is not well defined. Signed-off-by: Kir Kolyshkin --- script/check-config.sh | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/script/check-config.sh b/script/check-config.sh index 97ec3ef0540..e8279330418 100755 --- a/script/check-config.sh +++ b/script/check-config.sh @@ -27,6 +27,11 @@ kernelMajor="${kernelVersion%%.*}" kernelMinor="${kernelVersion#$kernelMajor.}" kernelMinor="${kernelMinor%%.*}" +kernel_lt() { + [ "$kernelMajor" -lt "$1" ] && return + [ "$kernelMajor" -eq "$1" ] && [ "$kernelMinor" -le "$2" ] +} + is_set() { zgrep "CONFIG_$1=[y|m]" "$CONFIG" >/dev/null } @@ -178,7 +183,7 @@ if [ "$(stat -f -c %t /sys/fs/cgroup 2>/dev/null)" = "63677270" ]; then else cgroupSubsystemDir="$(awk '/[, ](cpu|cpuacct|cpuset|devices|freezer|memory)[, ]/ && $3 == "cgroup" { print $2 }' /proc/mounts | head -n1)" cgroupDir="$(dirname "$cgroupSubsystemDir")" - if [ -d "$cgroupDir/cpu" -o -d "$cgroupDir/cpuacct" -o -d "$cgroupDir/cpuset" -o -d "$cgroupDir/devices" -o -d "$cgroupDir/freezer" -o -d "$cgroupDir/memory" ]; then + if [ -d "$cgroupDir/cpu" ] || [ -d "$cgroupDir/cpuacct" ] || [ -d "$cgroupDir/cpuset" ] || [ -d "$cgroupDir/devices" ] || [ -d "$cgroupDir/freezer" ] || [ -d "$cgroupDir/memory" ]; then wrap_good 'cgroup hierarchy' 'properly mounted' "[$cgroupDir]" else if [ "$cgroupSubsystemDir" ]; then @@ -221,11 +226,11 @@ flags=( ) check_flags "${flags[@]}" -if [ "$kernelMajor" -lt 5 ] || [ "$kernelMajor" -eq 5 -a "$kernelMinor" -le 1 ]; then +if kernel_lt 5 1; then check_flags NF_NAT_IPV4 fi -if [ "$kernelMajor" -lt 5 ] || [ "$kernelMajor" -eq 5 -a "$kernelMinor" -le 2 ]; then +if kernel_lt 5 2; then check_flags NF_NAT_NEEDED fi @@ -242,7 +247,7 @@ echo 'Optional Features:' check_flags MEMCG_SWAP - if [ "$kernelMajor" -lt 5 ] || [ "$kernelMajor" -eq 5 -a "$kernelMinor" -le 8 ]; then + if kernel_lt 5 8; then check_flags MEMCG_SWAP_ENABLED if is_set MEMCG_SWAP && ! is_set MEMCG_SWAP_ENABLED; then wrap_color ' (note that cgroup swap accounting is not enabled in your kernel config, you can enable it by setting boot option "swapaccount=1")' bold black @@ -250,21 +255,21 @@ echo 'Optional Features:' fi } -if [ "$kernelMajor" -lt 4 ] || [ "$kernelMajor" -eq 4 -a "$kernelMinor" -le 5 ]; then +if kernel_lt 4 5; then check_flags MEMCG_KMEM fi -if [ "$kernelMajor" -lt 3 ] || [ "$kernelMajor" -eq 3 -a "$kernelMinor" -le 18 ]; then +if kernel_lt 3 18; then check_flags RESOURCE_COUNTERS fi -if [ "$kernelMajor" -lt 3 ] || [ "$kernelMajor" -eq 3 -a "$kernelMinor" -le 13 ]; then +if kernel_lt 3 13; then netprio=NETPRIO_CGROUP else netprio=CGROUP_NET_PRIO fi -if [ "$kernelMajor" -lt 5 ]; then +if kernel_lt 5 0; then check_flags IOSCHED_CFQ CFQ_GROUP_IOSCHED fi From d66498e7716732a478b40c8136a9fcf4642673fe Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Feb 2022 13:26:53 -0800 Subject: [PATCH 4/7] script/check-config.sh: fix remaining shellcheck warnings ... and add this file to shellcheck target in Makefile. These: In script/check-config.sh line 27: kernelMinor="${kernelVersion#$kernelMajor.}" ^----------^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns. Did you mean: kernelMinor="${kernelVersion#"$kernelMajor".}" In script/check-config.sh line 103: source /etc/os-release 2>/dev/null || /bin/true ^-------------^ SC1091 (info): Not following: /etc/os-release was not specified as input (see shellcheck -x). In script/check-config.sh line 267: NET_CLS_CGROUP $netprio ^------^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a. Signed-off-by: Kir Kolyshkin --- Makefile | 2 +- script/check-config.sh | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 6b2b43f2d8f..bc6b80bef03 100644 --- a/Makefile +++ b/Makefile @@ -133,7 +133,7 @@ cfmt: shellcheck: shellcheck tests/integration/*.bats tests/integration/*.sh \ tests/integration/*.bash tests/*.sh \ - script/release_*.sh script/seccomp.sh script/lib.sh + script/* # TODO: add shellcheck for more sh files shfmt: diff --git a/script/check-config.sh b/script/check-config.sh index e8279330418..7a700af23d9 100755 --- a/script/check-config.sh +++ b/script/check-config.sh @@ -24,7 +24,7 @@ fi kernelVersion="$(uname -r)" kernelMajor="${kernelVersion%%.*}" -kernelMinor="${kernelVersion#$kernelMajor.}" +kernelMinor="${kernelVersion#"$kernelMajor".}" kernelMinor="${kernelMinor%%.*}" kernel_lt() { @@ -113,7 +113,9 @@ check_flags() { } check_distro_userns() { - source /etc/os-release 2>/dev/null || /bin/true + [ -r /etc/os-release ] || return 0 + # shellcheck source=/dev/null + . /etc/os-release 2>/dev/null || return 0 if [[ "${ID}" =~ ^(centos|rhel)$ && "${VERSION_ID}" =~ ^7 ]]; then # this is a CentOS7 or RHEL7 system grep -q "user_namespace.enable=1" /proc/cmdline || { @@ -277,7 +279,7 @@ flags=( BLK_CGROUP BLK_DEV_THROTTLING CGROUP_PERF CGROUP_HUGETLB - NET_CLS_CGROUP $netprio + NET_CLS_CGROUP "$netprio" CFS_BANDWIDTH FAIR_GROUP_SCHED RT_GROUP_SCHED IP_NF_TARGET_REDIRECT IP_VS From 5d1ef78cadaeb0708d9ea46c545c3e821517caaf Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Feb 2022 13:35:50 -0800 Subject: [PATCH 5/7] script/check-config.sh: enable set -u, fix issues One particularly bad one is ${codes[@]} which is fine in bash 4.4+, but gives "codes[@]: unbound variable" with older bash versions, such as with bash 4.2 used on CentOS 6. It's good that this is the only array in the script that can potentially be empty. Signed-off-by: Kir Kolyshkin --- script/check-config.sh | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/script/check-config.sh b/script/check-config.sh index 7a700af23d9..ec8bc63a573 100755 --- a/script/check-config.sh +++ b/script/check-config.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -set -e +set -e -u # bits of this were adapted from check_config.sh in docker # see also https://github.com/docker/docker/blob/master/contrib/check-config.sh @@ -45,11 +45,11 @@ is_set_as_module() { color() { local codes=() if [ "$1" = 'bold' ]; then - codes=("${codes[@]}" '1') + codes=("${codes[@]-}" '1') shift fi if [ "$#" -gt 0 ]; then - local code + local code='' case "$1" in # see https://en.wikipedia.org/wiki/ANSI_escape_code#Colors black) code=30 ;; @@ -62,11 +62,11 @@ color() { white) code=37 ;; esac if [ "$code" ]; then - codes=("${codes[@]}" "$code") + codes=("${codes[@]-}" "$code") fi fi local IFS=';' - echo -en '\033['"${codes[*]}"'m' + echo -en '\033['"${codes[*]-}"'m' } wrap_color() { text="$1" @@ -134,8 +134,7 @@ is_config() { } search_config() { - local target_dir="$1" - [[ "$target_dir" ]] || target_dir=("${possibleConfigs[@]}") + local target_dir=("${1:-${possibleConfigs[@]}}") local tryConfig for tryConfig in "${target_dir[@]}"; do @@ -159,7 +158,7 @@ search_config() { exit 1 } -CONFIG="$1" +CONFIG="${1:-}" is_config "$CONFIG" || { if [[ ! "$CONFIG" ]]; then From cacc823724d9b7447c9747cd19d007565fc1f520 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Feb 2022 13:45:40 -0800 Subject: [PATCH 6/7] ci: add call to check-config.sh This is done to make sure the script is working correctly in different environments (distro and kernel versions). In addition, we can see in test logs which kernel features are enabled. Note that I didn't want to have a separate job for GHA CI, so I just added this to the end of shellcheck one. Signed-off-by: Kir Kolyshkin --- .cirrus.yml | 4 ++++ .github/workflows/validate.yml | 2 ++ 2 files changed, 6 insertions(+) diff --git a/.cirrus.yml b/.cirrus.yml index f72637a3980..e2e7a3dc43f 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -51,6 +51,8 @@ task: vagrant ssh-config >> /root/.ssh/config guest_info_script: | ssh default 'sh -exc "uname -a && systemctl --version && df -T && cat /etc/os-release && go version"' + check_config_script: | + ssh default /vagrant/script/check-config.sh unit_tests_script: | ssh default 'sudo -i make -C /vagrant localunittest' integration_systemd_script: | @@ -144,6 +146,8 @@ task: df -T echo "-----" systemctl --version + check_config_script: | + /home/runc/script/check-config.sh unit_tests_script: | ssh -tt localhost "make -C /home/runc localunittest" integration_systemd_script: | diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index c21efd3dad5..20facee1280 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -118,6 +118,8 @@ jobs: - name: shellcheck run: | make shellcheck + - name: check-config.sh + run : ./script/check-config.sh deps: runs-on: ubuntu-20.04 From ae6cb653f4250a9d4c2f190613241c85b5a1c125 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Feb 2022 13:52:46 -0800 Subject: [PATCH 7/7] man/*sh: fix shellcheck warnings, add to shellcheck Now the only remaining file that needs shellcheck warnings to be fixed is bash-completion. Note that in Makefile's TODO. Signed-off-by: Kir Kolyshkin --- Makefile | 4 ++-- man/md2man-all.sh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index bc6b80bef03..dcceb0b1693 100644 --- a/Makefile +++ b/Makefile @@ -133,8 +133,8 @@ cfmt: shellcheck: shellcheck tests/integration/*.bats tests/integration/*.sh \ tests/integration/*.bash tests/*.sh \ - script/* - # TODO: add shellcheck for more sh files + man/*.sh script/* + # TODO: add shellcheck for more sh files (contrib/completions/bash/runc). shfmt: shfmt -ln bats -d -w tests/integration/*.bats diff --git a/man/md2man-all.sh b/man/md2man-all.sh index eaee58ee5af..04d9c70a4e1 100755 --- a/man/md2man-all.sh +++ b/man/md2man-all.sh @@ -2,7 +2,7 @@ set -e # get into this script's directory -cd "$(dirname "$(readlink -f "$BASH_SOURCE")")" +cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" [ "$1" = '-q' ] || { set -x @@ -18,7 +18,7 @@ for FILE in *.md; do base="$(basename "$FILE")" name="${base%.md}" num="${name##*.}" - if [ -z "$num" -o "$name" = "$num" ]; then + if [ -z "$num" ] || [ "$name" = "$num" ]; then # skip files that aren't of the format xxxx.N.md (like README.md) continue fi