From a61b411ceb7df49d6ad195da7a2eae774915fe0b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 26 Aug 2022 09:21:29 +0200 Subject: [PATCH 1/8] Dockerfile: update yamllint to v1.27.1 Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 1d7cd76ee9ed678ae00c2a10ec144c3eed0b3dcd) Signed-off-by: Sebastiaan van Stijn --- Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 7cdfbb043b..39c0924e11 100644 --- a/Dockerfile +++ b/Dockerfile @@ -349,7 +349,8 @@ RUN update-alternatives --set iptables /usr/sbin/iptables-legacy || true \ && update-alternatives --set ip6tables /usr/sbin/ip6tables-legacy || true \ && update-alternatives --set arptables /usr/sbin/arptables-legacy || true -RUN pip3 install yamllint==1.26.1 +ARG YAMLLINT_VERSION=1.27.1 +RUN pip3 install yamllint==${YAMLLINT_VERSION} COPY --from=dockercli /build/ /usr/local/cli COPY --from=frozen-images /build/ /docker-frozen-images From 8d43d7fa6b57f2a927cccb7b7e3b2089b1dc7051 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 26 Aug 2022 10:52:27 +0200 Subject: [PATCH 2/8] validate: yamllint rename config-file Don't make the file hidden, and add .yaml extension, so that editors pick up the right formatting :) Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 5f114b65b481337d2524c14a802652edad2db143) Signed-off-by: Sebastiaan van Stijn --- hack/validate/swagger | 2 +- hack/validate/{.swagger-yamllint => yamllint.yaml} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename hack/validate/{.swagger-yamllint => yamllint.yaml} (100%) diff --git a/hack/validate/swagger b/hack/validate/swagger index 5bf9c1cbcc..92ba2c3904 100755 --- a/hack/validate/swagger +++ b/hack/validate/swagger @@ -8,7 +8,7 @@ files=($(validate_diff --diff-filter=ACMR --name-only -- 'api/swagger.yaml' || t unset IFS if [ -n "${TEST_FORCE_VALIDATE:-}" ] || [ ${#files[@]} -gt 0 ]; then - LANG=C.UTF-8 yamllint -c "${SCRIPTDIR}"/.swagger-yamllint api/swagger.yaml + LANG=C.UTF-8 yamllint -c "${SCRIPTDIR}"/yamllint.yaml api/swagger.yaml if out=$(swagger validate api/swagger.yaml); then echo "Congratulations! ${out}" else diff --git a/hack/validate/.swagger-yamllint b/hack/validate/yamllint.yaml similarity index 100% rename from hack/validate/.swagger-yamllint rename to hack/validate/yamllint.yaml From 554a9339447b986969ac50a67a7968a15e59e6f3 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 26 Aug 2022 15:27:45 +0200 Subject: [PATCH 3/8] validate: yamllint: use "parsable" output Before: 10030:81 error line too long (89 > 80 characters) (line-length) After: api/swagger.yaml:10030:81: [error] line too long (89 > 80 characters) (line-length) Signed-off-by: Sebastiaan van Stijn (cherry picked from commit f679d8c821bef2c9c0082bfe9c3a2a07bddf05f8) Signed-off-by: Sebastiaan van Stijn --- hack/validate/swagger | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/validate/swagger b/hack/validate/swagger index 92ba2c3904..840ca0b146 100755 --- a/hack/validate/swagger +++ b/hack/validate/swagger @@ -8,7 +8,7 @@ files=($(validate_diff --diff-filter=ACMR --name-only -- 'api/swagger.yaml' || t unset IFS if [ -n "${TEST_FORCE_VALIDATE:-}" ] || [ ${#files[@]} -gt 0 ]; then - LANG=C.UTF-8 yamllint -c "${SCRIPTDIR}"/yamllint.yaml api/swagger.yaml + LANG=C.UTF-8 yamllint -f parsable -c "${SCRIPTDIR}"/yamllint.yaml api/swagger.yaml if out=$(swagger validate api/swagger.yaml); then echo "Congratulations! ${out}" else From 72156dd7a422a6c0cecab42e4aeeaaf470715279 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 26 Aug 2022 15:51:18 +0200 Subject: [PATCH 4/8] validate: yamllint: set locale in config file Signed-off-by: Sebastiaan van Stijn (cherry picked from commit cc2134ea83a1680832404dd03dba1d4b3770252d) Signed-off-by: Sebastiaan van Stijn --- hack/validate/swagger | 2 +- hack/validate/yamllint.yaml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/hack/validate/swagger b/hack/validate/swagger index 840ca0b146..1ed8d3bcd1 100755 --- a/hack/validate/swagger +++ b/hack/validate/swagger @@ -8,7 +8,7 @@ files=($(validate_diff --diff-filter=ACMR --name-only -- 'api/swagger.yaml' || t unset IFS if [ -n "${TEST_FORCE_VALIDATE:-}" ] || [ ${#files[@]} -gt 0 ]; then - LANG=C.UTF-8 yamllint -f parsable -c "${SCRIPTDIR}"/yamllint.yaml api/swagger.yaml + yamllint -f parsable -c "${SCRIPTDIR}"/yamllint.yaml api/swagger.yaml if out=$(swagger validate api/swagger.yaml); then echo "Congratulations! ${out}" else diff --git a/hack/validate/yamllint.yaml b/hack/validate/yamllint.yaml index 2f00cb666c..e1f8c6b024 100644 --- a/hack/validate/yamllint.yaml +++ b/hack/validate/yamllint.yaml @@ -1,4 +1,5 @@ extends: default +locale: C.UTF-8 rules: document-start: disable line-length: disable From e9f7c05ae180fa358fc937c8c52b71da61ca5f60 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 26 Aug 2022 15:37:42 +0200 Subject: [PATCH 5/8] validate: yamllint: ignore "truthy value should be one of" warnings Suppresses warnings like: LANG=C.UTF-8 yamllint -c hack/validate/yamllint.yaml -f parsable .github/workflows/*.yml .github/workflows/ci.yml:7:1: [warning] truthy value should be one of [false, true] (truthy) .github/workflows/windows.yml:7:1: [warning] truthy value should be one of [false, true] (truthy) Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 91bb776bb85a75e86760b406d5af46b651f044dd) Signed-off-by: Sebastiaan van Stijn --- hack/validate/yamllint.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hack/validate/yamllint.yaml b/hack/validate/yamllint.yaml index e1f8c6b024..93a2b3b787 100644 --- a/hack/validate/yamllint.yaml +++ b/hack/validate/yamllint.yaml @@ -3,3 +3,8 @@ locale: C.UTF-8 rules: document-start: disable line-length: disable + # ignore "warning truthy value should be one of [false, true]" on GitHub + # actions workflows, which use "on:" to specify when to run. + truthy: + ignore: | + .github/workflows/ From f1bd611d41bff3626feb2ec058ce3e6102c9f2fd Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 26 Aug 2022 15:59:53 +0200 Subject: [PATCH 6/8] validate: add yamllint validation validate other YAML files, such as the ones used in the documentation, and GitHub actions workflows, to prevent issues such as; - 30295c1750714d26f3c8fc9c3451f11ac351f2be - 8e8d9a36500fb07fa9d1b68539756b9a93d3509e With this patch: hack/validate/yamllint Congratulations! yamllint config file formatted correctly Congratulations! YAML files are formatted correctly Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 6cef06b94031b15fb1a9dd4b84a0b19d93013b0b) Signed-off-by: Sebastiaan van Stijn --- hack/validate/default | 1 + hack/validate/yamllint | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100755 hack/validate/yamllint diff --git a/hack/validate/default b/hack/validate/default index 88a4b95680..b48d9fa09c 100755 --- a/hack/validate/default +++ b/hack/validate/default @@ -8,6 +8,7 @@ export SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" #. "${SCRIPTDIR}"/dco . "${SCRIPTDIR}"/default-seccomp . "${SCRIPTDIR}"/pkg-imports +. "${SCRIPTDIR}"/yamllint . "${SCRIPTDIR}"/swagger . "${SCRIPTDIR}"/swagger-gen . "${SCRIPTDIR}"/toml diff --git a/hack/validate/yamllint b/hack/validate/yamllint new file mode 100755 index 0000000000..1c663b1748 --- /dev/null +++ b/hack/validate/yamllint @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +set -e +SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${SCRIPTDIR}/.validate" + +if [ -n "${TEST_FORCE_VALIDATE:-}" ]; then + files=(docs/api/*.yaml) +else + IFS=$'\n' + files=($(validate_diff --diff-filter=ACMR --name-only -- docs/*.yaml || true)) + unset IFS +fi + +# validate the yamllint configuration file before anything else +if out=$(yamllint -f parsable -d "{extends: default, rules: {document-start: disable}}" "${SCRIPTDIR}"/yamllint.yaml); then + echo "Congratulations! yamllint config file formatted correctly" +else + echo "${out}" >&2 + false +fi + +# Then validate GitHub actions workflows, and conditionally lint the swagger +# files in the docs directory, as these are large files and take some time. +if out=$(yamllint -f parsable -c "${SCRIPTDIR}"/yamllint.yaml .github/workflows/*.yml "${files[@]}"); then + echo "Congratulations! YAML files are formatted correctly" +else + echo "${out}" >&2 + false +fi From 51bf7da7290dd628a0e266290de065866e5582f3 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 26 Aug 2022 18:31:23 +0200 Subject: [PATCH 7/8] validate: format vendor script with shfmt Signed-off-by: Sebastiaan van Stijn (cherry picked from commit b9fd2cf605dbd15160a7157d801a5551834df0d2) Signed-off-by: Sebastiaan van Stijn --- hack/validate/vendor | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/hack/validate/vendor b/hack/validate/vendor index 1ffbb1a3c2..851bf6e249 100755 --- a/hack/validate/vendor +++ b/hack/validate/vendor @@ -1,20 +1,20 @@ #!/usr/bin/env bash -SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "${SCRIPTDIR}/.validate" -validate_vendor_diff(){ +validate_vendor_diff() { IFS=$'\n' - check_files=( 'vendor.sum' 'vendor.mod' 'vendor/' ) + check_files=('vendor.sum' 'vendor.mod' 'vendor/') # shellcheck disable=SC2207 - changed_files=( $(validate_diff --diff-filter=ACMR --name-only -- "${check_files[@]}" || true) ) + changed_files=($(validate_diff --diff-filter=ACMR --name-only -- "${check_files[@]}" || true)) unset IFS if [ -n "${TEST_FORCE_VALIDATE:-}" ] || [ "${#changed_files[@]}" -gt 0 ]; then # recreate vendor/ ./hack/vendor.sh # check if any files have changed - diffs="$(git status --porcelain -- "${check_files[@]}" 2>/dev/null)" + diffs="$(git status --porcelain -- "${check_files[@]}" 2> /dev/null)" mfiles="$(echo "$diffs" | awk '/^ M / {print $2}')" if [ "$diffs" ]; then { @@ -24,7 +24,7 @@ validate_vendor_diff(){ echo echo 'Please vendor your package with hack/vendor.sh.' echo - if [ -n "$mfiles" ] ; then + if [ -n "$mfiles" ]; then git diff -- "$mfiles" fi } >&2 @@ -41,11 +41,11 @@ validate_vendor_diff(){ # 2. make sure all the packages contain license information (just warning, because it can cause false-positive) validate_vendor_used() { for f in $(mawk '$1 = "#" { print $2 }' 'vendor/modules.txt'); do - if [ -d "vendor/$f" ]; then - if ! echo "vendor/$f"/* | grep -qiEc '/(LICENSE|COPYING)'; then - echo "WARNING: could not find copyright information for $f" + if [ -d "vendor/$f" ]; then + if ! echo "vendor/$f"/* | grep -qiEc '/(LICENSE|COPYING)'; then + echo "WARNING: could not find copyright information for $f" + fi fi - fi done } From 3b9370fcf87956561645d9aeebf6bc7c1da9697b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 26 Aug 2022 16:08:14 +0200 Subject: [PATCH 8/8] validate: address SC2155 (shellcheck) see https://github.com/koalaman/shellcheck/wiki/SC2155 Looking at how these were used, I don't think we even need to export them, so removing that. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 5cfc9c374cbde2fa650ee069d039cd6efeae0197) Signed-off-by: Sebastiaan van Stijn --- hack/validate/all | 2 +- hack/validate/dco | 2 +- hack/validate/default | 2 +- hack/validate/default-seccomp | 2 +- hack/validate/deprecate-integration-cli | 2 +- hack/validate/pkg-imports | 2 +- hack/validate/swagger | 2 +- hack/validate/swagger-gen | 2 +- hack/validate/toml | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/hack/validate/all b/hack/validate/all index 3224fe57c2..281684af0d 100755 --- a/hack/validate/all +++ b/hack/validate/all @@ -2,7 +2,7 @@ # # Run all validation -export SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" . ${SCRIPTDIR}/default . ${SCRIPTDIR}/vendor diff --git a/hack/validate/dco b/hack/validate/dco index 0e61ccca5d..6354d2b57f 100755 --- a/hack/validate/dco +++ b/hack/validate/dco @@ -1,6 +1,6 @@ #!/usr/bin/env bash -export SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "${SCRIPTDIR}/.validate" adds=$(validate_diff --numstat | awk '{ s += $1 } END { print s }') diff --git a/hack/validate/default b/hack/validate/default index b48d9fa09c..bb531770d1 100755 --- a/hack/validate/default +++ b/hack/validate/default @@ -2,7 +2,7 @@ # # Run default validation, exclude vendor because it's slow -export SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # Skip DCO check here, as it's already checked in a separate stage in the Jenkinsfile #. "${SCRIPTDIR}"/dco diff --git a/hack/validate/default-seccomp b/hack/validate/default-seccomp index c5f8c7e3c8..aea27d12af 100755 --- a/hack/validate/default-seccomp +++ b/hack/validate/default-seccomp @@ -1,6 +1,6 @@ #!/usr/bin/env bash -export SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "${SCRIPTDIR}/.validate" IFS=$'\n' diff --git a/hack/validate/deprecate-integration-cli b/hack/validate/deprecate-integration-cli index f53b48eedc..f7b273c9f7 100755 --- a/hack/validate/deprecate-integration-cli +++ b/hack/validate/deprecate-integration-cli @@ -1,7 +1,7 @@ #!/usr/bin/env bash # Check that no new tests are being added to integration-cli -export SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "${SCRIPTDIR}/.validate" new_tests=$( diff --git a/hack/validate/pkg-imports b/hack/validate/pkg-imports index bba762d99f..c8ba223f63 100755 --- a/hack/validate/pkg-imports +++ b/hack/validate/pkg-imports @@ -1,7 +1,7 @@ #!/usr/bin/env bash set -e -export SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "${SCRIPTDIR}/.validate" IFS=$'\n' diff --git a/hack/validate/swagger b/hack/validate/swagger index 1ed8d3bcd1..5313221b7b 100755 --- a/hack/validate/swagger +++ b/hack/validate/swagger @@ -1,6 +1,6 @@ #!/usr/bin/env bash set -e -export SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "${SCRIPTDIR}/.validate" IFS=$'\n' diff --git a/hack/validate/swagger-gen b/hack/validate/swagger-gen index 0d000ea39b..40631703cd 100755 --- a/hack/validate/swagger-gen +++ b/hack/validate/swagger-gen @@ -1,6 +1,6 @@ #!/usr/bin/env bash -export SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "${SCRIPTDIR}/.validate" IFS=$'\n' diff --git a/hack/validate/toml b/hack/validate/toml index 7f1739ab84..fe86cac1a7 100755 --- a/hack/validate/toml +++ b/hack/validate/toml @@ -1,6 +1,6 @@ #!/usr/bin/env bash -export SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "${SCRIPTDIR}/.validate" IFS=$'\n'