diff --git a/.github/workflows/knative-boilerplate.yaml b/.github/workflows/knative-boilerplate.yaml index f79d4401bb..b45afcca57 100644 --- a/.github/workflows/knative-boilerplate.yaml +++ b/.github/workflows/knative-boilerplate.yaml @@ -64,7 +64,7 @@ jobs: go get github.com/mattmoor/boilerplate-check/cmd/boilerplate-check echo '::endgroup::' - echo "::add-path::${TEMP_PATH}" + echo "${TEMP_PATH}" >> $GITHUB_PATH - id: boilerplate_txt uses: andstor/file-existence-action@v1 diff --git a/.github/workflows/knative-go-test.yaml b/.github/workflows/knative-go-test.yaml index bfe6c6ad29..8810ba7dbc 100644 --- a/.github/workflows/knative-go-test.yaml +++ b/.github/workflows/knative-go-test.yaml @@ -55,7 +55,7 @@ jobs: - if: steps.codecov-enabled.outputs.files_exists == 'true' name: Produce Go Coverage - run: echo '::set-env name=COVER_OPTS::-coverprofile=coverage.txt -covermode=atomic' + run: echo 'COVER_OPTS=-coverprofile=coverage.txt -covermode=atomic' >> $GITHUB_ENV - name: Test run: go test -race $COVER_OPTS ./... diff --git a/.github/workflows/knative-security.yaml b/.github/workflows/knative-security.yaml new file mode 100644 index 0000000000..a5f7703d2b --- /dev/null +++ b/.github/workflows/knative-security.yaml @@ -0,0 +1,52 @@ +# Copyright 2020 The Knative Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# This file is automagically synced here from github.com/knative-sandbox/.github +# repo by knobots: https://github.com/mattmoor/knobots and will be overwritten. + +name: 'Security' + +on: + pull_request: + branches: [ 'master', 'release-*' ] + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v2 + with: + # We must fetch at least the immediate parents so that if this is + # a pull request then we can checkout the head. + fetch-depth: 2 + + # If this run was triggered by a pull request event, then checkout + # the head of the pull request instead of the merge commit. + - run: git checkout HEAD^2 + if: ${{ github.event_name == 'pull_request' }} + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + with: + languages: go + + - name: Autobuild + uses: github/codeql-action/autobuild@v1 + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 diff --git a/.github/workflows/knative-style.yaml b/.github/workflows/knative-style.yaml index 31a0aeb2ee..a2603b1e4d 100644 --- a/.github/workflows/knative-style.yaml +++ b/.github/workflows/knative-style.yaml @@ -26,9 +26,20 @@ jobs: autoformat: name: Auto-format and Check runs-on: ubuntu-latest + strategy: + fail-fast: false # Keep running if one leg fails. + matrix: + tool: + - goimports + - gofmt + + include: + - tool: gofmt + options: -s + - tool: goimports + importpath: golang.org/x/tools/cmd/goimports steps: - - name: Set up Go 1.15.x uses: actions/setup-go@v2 with: @@ -39,41 +50,30 @@ jobs: uses: actions/checkout@v2 - name: Install Dependencies + if: ${{ matrix.importpath != '' }} run: | cd $(mktemp -d) - GO111MODULE=on go get golang.org/x/tools/cmd/goimports + GO111MODULE=on go get ${{ matrix.importpath }} - # Run this last because it alters the workspace. - # TODO: add prettier step - - name: Go Imports - shell: bash - run: | - goimports -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print) - echo "::set-env name=FMT_TOOL::goimports" - - name: Verify goimport + - name: ${{ matrix.tool }} ${{ matrix.options }} shell: bash run: | - if [[ $(git diff-index --name-only HEAD --) ]]; then - echo "Found diffs in:" - git diff-index --name-only HEAD -- - echo "${{ github.repository }} is out of style. Please run $FMT_TOOL." - exit 1 - fi - echo "${{ github.repository }} is formatted correctly." + ${{ matrix.tool }} ${{ matrix.options }} -w $(find . -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print) - - name: Go Format + - name: Verify ${{ matrix.tool }} shell: bash run: | - gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print) - echo "::set-env name=FMT_TOOL::gofmt" - - name: Verify gofmt - shell: bash - # TODO(mattmoor): combine with above via anchors when actions support it. - run: | + # From: https://backreference.org/2009/12/23/how-to-match-newlines-in-sed/ + # This is to leverage this workaround: + # https://github.com/actions/toolkit/issues/193#issuecomment-605394935 + function urlencode() { + sed ':begin;$!N;s/\n/%0A/;tbegin' + } if [[ $(git diff-index --name-only HEAD --) ]]; then - echo "Found diffs in:" - git diff-index --name-only HEAD -- - echo "${{ github.repository }} is out of style. Please run $FMT_TOOL." + for x in $(git diff-index --name-only HEAD --); do + echo "::error file=$x::Please run ${{ matrix.tool }} ${{ matrix.options }}.%0A$(git diff $x | urlencode)" + done + echo "${{ github.repository }} is out of style. Please run ${{ matrix.tool }} ${{ matrix.options }}." exit 1 fi echo "${{ github.repository }} is formatted correctly." @@ -109,7 +109,7 @@ jobs: curl -sfL https://raw.githubusercontent.com/get-woke/woke/main/install.sh | sh -s -- -b "${TEMP_PATH}" "${WOKE_VERSION}" 2>&1 echo '::endgroup::' - echo "::add-path::${TEMP_PATH}" + echo "${TEMP_PATH}" >> $GITHUB_PATH - id: golangci_configuration uses: andstor/file-existence-action@v1 @@ -120,7 +120,6 @@ jobs: uses: golangci/golangci-lint-action@v2 with: version: v1.30 - only-new-issues: true # for initial defensiveness - name: misspell shell: bash @@ -180,7 +179,12 @@ jobs: echo '::group:: Flagging missing EOF newlines with reviewdog 🐶 ...' # Don't fail because of misspell set +o pipefail - for x in $(find . -type f -not -path './vendor/*' -not -path './third_party/*' -not -path './.git/*'); do + # Lint exclude rule: + # - nothing in vendor/ + # - nothing in third_party + # - nothing in .git/ + # - no *.ai (Adobe Illustrator) files. + for x in $(find . -type f -not -name '*.ai' -not -path './vendor/*' -not -path './third_party/*' -not -path './.git/*'); do # Based on https://stackoverflow.com/questions/34943632/linux-check-if-there-is-an-empty-line-at-the-end-of-a-file if [[ -f $x && ! ( -s "$x" && -z "$(tail -c 1 $x)" ) ]]; then # We add 1 to `wc -l` here because of this limitation (from the man page): @@ -197,6 +201,52 @@ jobs: echo '::endgroup::' + - name: Redundant Format + shell: bash + if: ${{ always() }} + env: + REVIEWDOG_GITHUB_API_TOKEN: ${{ github.token }} + run: | + set -e + cd "${GITHUB_WORKSPACE}" || exit 1 + + echo '::group:: Flagging possibly unnecessary format calls for trailing argument with reviewdog 🐶 ...' + # Don't fail because of misspell + set +o pipefail + # For all look for formatting calls and extract the ones that + # have only a single formatting argument and that argument is in the last position. + # But exclude the `fmt.Errorf` calls since there's no `fmt.Error` and `errors.New` is not + # as versatile. + # Also ignore `%T` — to print the type and `%q` to quote the final string. + fn=$(find . -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print) + grep -nE '[^.]+\.(Fatalf|Errorf|Warnf|Infof|Debugf|Logf|Sprintf|Fprintf|Printf)[^\%]+%[^Tqx]",[^,]+' $fn | grep -v "fmt.Errorf" | + while read -r ent ; do + file=$(echo $ent | cut -d':' -f 1); + line=$(echo $ent | cut -d':' -f 2); + ch=$(echo $ent | cut -d':' -f3-); + err="Unknown printer tool, please file an issue in knative-sandbox/.github and assign to @vagababov: $ch" + if echo $ch | grep --quiet -E "^t.(Errorf|Fatalf|Logf)" ; then + err=$(echo $ch | sed -E 's/([^.fm]+t\.)(Fatal|Error|Log)f([^\%]+)( %[^Tq]",)([^,]+)/\1\2\3",\5/') + # Not a test. Here we deal with various loggers and fmt helpers. + elif echo $ch | grep --quiet "log" ; then + # Capture (x)?log(er)?. + err=$(echo $ch | sed -E 's/(.*log.*\.)(Print|Fatal|Error|Info|Warn)f([^\%]+)(%[^Tq]",)([^,]+)/\1\2\3",\5/') + elif echo $ch | grep --quiet -E "fmt\.Sprintf" ; then + # Always space after sprintf + err=$(echo $ch | sed -E 's/(fmt\.)(Sprint)f([^%]+) (%s",)([^,]+)/\1\2\3 ",\5/') + elif echo $ch | grep --quiet -E "fmt\." ; then # all other fmt. printers. + err=$(echo $ch | sed -E 's/(fmt\.)(Print|Fprint)f([^%]+) (%[^sTxq]",)([^,]+)/\1\2\3",\5/') + fi + echo "$file:$line: Please consider avoiding tail format like this:%0A$err" + done | + reviewdog -efm="%f:%l: %m" \ + -name="Redundant Format" \ + -reporter="github-pr-check" \ + -filter-mode="added" \ + -fail-on-error="true" \ + -level="error" + echo '::endgroup::' + # This is mostly copied from https://github.com/get-woke/woke-action-reviewdog/blob/main/entrypoint.sh # since their action is not yet released under a stable version. - name: Language diff --git a/.github/workflows/knative-verify.yaml b/.github/workflows/knative-verify.yaml index c07f7462d4..7b9664ad2d 100644 --- a/.github/workflows/knative-verify.yaml +++ b/.github/workflows/knative-verify.yaml @@ -63,12 +63,23 @@ jobs: - name: Verify shell: bash run: | + # From: https://backreference.org/2009/12/23/how-to-match-newlines-in-sed/ + # This is to leverage this workaround: + # https://github.com/actions/toolkit/issues/193#issuecomment-605394935 + function urlencode() { + sed ':begin;$!N;s/\n/%0A/;tbegin' + } + pushd ./src/knative.dev/${{ github.event.repository.name }} if [[ -z "$(git status --porcelain)" ]]; then echo "${{ github.repository }} up to date." else - repoDiff=$(git diff-index --name-only HEAD --) - echo "Found diffs in: $repoDiff" + # Print it both ways because sometimes we haven't modified the file (e.g. zz_generated), + # and sometimes we have (e.g. configmap checksum). + echo "Found diffs in: $(git diff-index --name-only HEAD --)" + for x in $(git diff-index --name-only HEAD --); do + echo "::error file=$x::Please run ./hack/update-codegen.sh.%0A$(git diff $x | urlencode)" + done echo "${{ github.repository }} is out of date. Please run hack/update-codegen.sh" exit 1 fi