Skip to content

Commit df0ccde

Browse files
authored
ci: add linting, code coverage and other useful checks (openshift#27)
chore: introduce hardened binaries and code coverage Add new different targets for CI pipeline + add .golangci.yml file for linter configuration. - fix: potential file inclusion
1 parent 111c084 commit df0ccde

File tree

6 files changed

+141
-9
lines changed

6 files changed

+141
-9
lines changed

.codecov.yml

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
codecov:
2+
notify:
3+
require_ci_to_pass: no
4+
5+
coverage:
6+
precision: 2
7+
round: down
8+
range: "20...100"
9+
10+
status:
11+
project: no
12+
patch: no
13+
changes: no
14+
15+
parsers:
16+
gcov:
17+
branch_detection:
18+
conditional: yes
19+
loop: yes
20+
method: no
21+
macro: no
22+
23+
comment:
24+
layout: "reach,diff,flags,tree"
25+
behavior: default
26+
require_changes: no

.gitignore

+2
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
11
/bin
2+
dist
3+
*.out

.golangci.yml

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
run:
2+
# Our CI expects a vendor directory, hence we have to use -mod=readonly here as well.
3+
modules-download-mode: readonly
4+
5+
issues:
6+
# on default, golangci-lint excludes gosec. Hence, we have to
7+
# explicitly disable the exclude-use-default: https://github.com/golangci/golangci-lint/issues/1504
8+
exclude-use-default: false
9+
10+
# individual linter configs go here
11+
linters-settings:
12+
13+
# default linters are enabled `golangci-lint help linters`
14+
linters:
15+
disable-all: true
16+
enable:
17+
- deadcode
18+
- errcheck
19+
- gosimple
20+
- govet
21+
- gosec
22+
- ineffassign
23+
- staticcheck
24+
- structcheck
25+
- typecheck
26+
- unused
27+
- varcheck

Makefile

+26-8
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,38 @@
11
# Binaries used in Makefile
22
bin/cobra:
3-
GOBIN=$(PWD)/bin go install $(shell go list -m -f '{{ .Path}}/cobra@{{ .Version }}' github.com/spf13/cobra)
3+
GOBIN=$(PWD)/bin go install -mod=readonly $(shell go list -m -f '{{ .Path}}/cobra@{{ .Version }}' github.com/spf13/cobra)
44

55
cadctl/cadctl: cadctl/**/*.go pkg/**/*.go go.mod go.sum
6-
GOBIN=$(PWD)/cadctl go install $(PWD)/cadctl
6+
GOBIN=$(PWD)/cadctl go install -ldflags="-s -w -extldflags=-zrelro -extldflags=-znow" -buildmode=pie -mod=readonly -trimpath $(PWD)/cadctl
7+
8+
bin/gosec:
9+
GOBIN=$(PWD)/bin go install -mod=readonly github.com/securego/gosec/v2/cmd/[email protected]
10+
11+
bin/golangci-lint:
12+
GOBIN=$(PWD)/bin go install -mod=readonly github.com/golangci/golangci-lint/cmd/[email protected]
713

814
# Actions
915
.DEFAULT_GOAL := all
1016
.PHONY: all
1117
all: build test
1218

19+
# build uses the following Go flags:
20+
# -s -w for stripping the binary (making it smaller)
21+
# the extended flags are for enabling ELF hardening features.
22+
# See also: https://www.redhat.com/en/blog/hardening-elf-binaries-using-relocation-read-only-relro
23+
# -mod=readonly and -trimpath are for generating reproducible/verifiable binaries. See also: https://reproducible-builds.org/
24+
# For more information about -buildmode=pie https://www.redhat.com/en/blog/position-independent-executables-pie
1325
.PHONY: build
1426
build:
15-
go build ./...
27+
go build -ldflags="-s -w -extldflags=-zrelro -extldflags=-znow" -buildmode=pie -mod=readonly -trimpath ./...
1628

1729
.PHONY: test
1830
test:
19-
go test ./...
31+
go test -race -mod=readonly ./...
2032

2133
.PHONY: test-with-coverage
2234
test-with-coverage:
23-
go test -cover ./...
35+
go test -cover -mod=readonly ./...
2436

2537
.PHONY: cadctl-install-local
2638
cadctl-install-local: cadctl/cadctl
@@ -37,6 +49,12 @@ cadctl-install-local-force:
3749
isclean: ## Validate the local checkout is clean. Use ALLOW_DIRTY_CHECKOUT=true to nullify
3850
@(test "$(ALLOW_DIRTY_CHECKOUT)" != "false" || test 0 -eq $$(git status --porcelain | wc -l)) || (echo "Local git checkout is not clean, commit changes and try again." >&2 && exit 1)
3951

40-
# not using the 'all' target to make the target independent
41-
.PHONY: ci-check
42-
ci-check: build test isclean
52+
.PHONY: coverage
53+
coverage: hack/codecov.sh
54+
55+
.PHONY: lint
56+
lint: bin/golangci-lint
57+
GOLANGCI_LINT_CACHE=$(shell mktemp -d) ./bin/golangci-lint run
58+
59+
.PHONY: validate
60+
validate: build isclean

hack/codecov.sh

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
#!/usr/bin/env bash
2+
3+
set -o errexit
4+
set -o nounset
5+
set -o pipefail
6+
7+
REPO_ROOT=$(git rev-parse --show-toplevel)
8+
CI_SERVER_URL=https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test
9+
COVER_PROFILE=${COVER_PROFILE:-coverage.out}
10+
JOB_TYPE=${JOB_TYPE:-"local"}
11+
12+
# Default concurrency to four threads. By default it's the number of procs,
13+
# which seems to be 16 in the CI env. Some consumers' coverage jobs were
14+
# regularly getting OOM-killed; so do this rather than boost the pod resources
15+
# unreasonably.
16+
COV_THREAD_COUNT=${COV_THREAD_COUNT:-4}
17+
make -C "${REPO_ROOT}" go-test TESTOPTS="-coverprofile=${COVER_PROFILE}.tmp -covermode=atomic -coverpkg=./... -p ${COV_THREAD_COUNT}"
18+
19+
# Remove generated files from coverage profile
20+
grep -v "zz_generated" "${COVER_PROFILE}.tmp" > "${COVER_PROFILE}"
21+
rm -f "${COVER_PROFILE}.tmp"
22+
23+
# Configure the git refs and job link based on how the job was triggered via prow
24+
if [[ "${JOB_TYPE}" == "presubmit" ]]; then
25+
echo "detected PR code coverage job for #${PULL_NUMBER}"
26+
REF_FLAGS="-P ${PULL_NUMBER} -C ${PULL_PULL_SHA}"
27+
JOB_LINK="${CI_SERVER_URL}/pr-logs/pull/${REPO_OWNER}_${REPO_NAME}/${PULL_NUMBER}/${JOB_NAME}/${BUILD_ID}"
28+
elif [[ "${JOB_TYPE}" == "postsubmit" ]]; then
29+
echo "detected branch code coverage job for ${PULL_BASE_REF}"
30+
REF_FLAGS="-B ${PULL_BASE_REF} -C ${PULL_BASE_SHA}"
31+
JOB_LINK="${CI_SERVER_URL}/logs/${JOB_NAME}/${BUILD_ID}"
32+
elif [[ "${JOB_TYPE}" == "local" ]]; then
33+
echo "coverage report available at ${COVER_PROFILE}"
34+
exit 0
35+
else
36+
echo "${JOB_TYPE} jobs not supported" >&2
37+
exit 1
38+
fi
39+
40+
# Configure certain internal codecov variables with values from prow.
41+
export CI_BUILD_URL="${JOB_LINK}"
42+
export CI_BUILD_ID="${JOB_NAME}"
43+
export CI_JOB_ID="${BUILD_ID}"
44+
45+
if [[ "${JOB_TYPE}" != "local" ]]; then
46+
if [[ -z "${ARTIFACT_DIR:-}" ]] || [[ ! -d "${ARTIFACT_DIR}" ]] || [[ ! -w "${ARTIFACT_DIR}" ]]; then
47+
echo '${ARTIFACT_DIR} must be set for non-local jobs, and must point to a writable directory' >&2
48+
exit 1
49+
fi
50+
curl -sS https://codecov.io/bash -o "${ARTIFACT_DIR}/codecov.sh"
51+
bash <(cat "${ARTIFACT_DIR}/codecov.sh") -Z -K -f "${COVER_PROFILE}" -r "${REPO_OWNER}/${REPO_NAME}" ${REF_FLAGS}
52+
else
53+
bash <(curl -s https://codecov.io/bash) -Z -K -f "${COVER_PROFILE}" -r "${REPO_OWNER}/${REPO_NAME}" ${REF_FLAGS}
54+
fi

pkg/pagerduty/pagerduty.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"io/fs"
99
"net/http"
1010
"os"
11+
"path/filepath"
1112
"reflect"
1213
"time"
1314

@@ -203,7 +204,11 @@ type fileReader interface {
203204
type RealFileReader struct{}
204205

205206
func (_ RealFileReader) ReadFile(name string) ([]byte, error) {
206-
return os.ReadFile(name)
207+
target, err := os.ReadFile(filepath.Clean(name))
208+
if err != nil {
209+
return []byte{}, err
210+
}
211+
return target, nil
207212
}
208213

209214
type WebhookPayloadToIncidentID struct {

0 commit comments

Comments
 (0)