From 3101794899553b4d6f926c9458bc81b16dbf6aeb Mon Sep 17 00:00:00 2001 From: Raymond Kroeker Date: Tue, 26 May 2026 14:11:37 -0700 Subject: [PATCH] Refactor Build for Go * Add go.just and write generic recipes for build, test and lint for go. * Adjust github workflow for go to use general recipe for testing. * Add meta-recipes go-build and go-test to build and test all go binary and library code. * Adjust go-lint to cover the internal pkg/ package. * Add flag to gofmt to report all issues (-e) versus the first 10. * Adjust recipe dependencies and remove specific recipes in favour of general ones.A * Fix lint issues in the iptables package. --- .github/workflows/go.yml | 2 +- .gitignore | 2 ++ go.just | 39 +++++++++++++++++++++++++++++++++++ justfile | 29 ++------------------------ pkg/iptables/iptables.go | 6 +++--- pkg/iptables/iptables_test.go | 1 + pkg/util/portrange_test.go | 5 ++++- 7 files changed, 52 insertions(+), 32 deletions(-) create mode 100644 go.just diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 86681157..c66e1607 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -43,5 +43,5 @@ jobs: - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c with: go-version-file: go.mod - - run: just proxy-init-test-unit + - run: just go-test diff --git a/.gitignore b/.gitignore index fbbf9c67..0709612d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,6 @@ **/.idea *.iml **/*.swp +*-test.json +*-cover.out target/ diff --git a/go.just b/go.just new file mode 100644 index 00000000..bcd0704b --- /dev/null +++ b/go.just @@ -0,0 +1,39 @@ + +# go-build all packages into target binaries +go-build: \ + (go-build-pkg "linkerd2-cni-plugin" "cni-plugin") \ + (go-build-pkg "linkerd2-proxy-init" "proxy-init") + +# go-fmt-check the root directory for all fmt errors printing the result; +# and exiting non-zero if formatting issues exist. +go-fmt-check: + out=$(gofmt -e -d .) ; [ -z "$out" ] || (echo "$out" ; exit 1) + +# go-lint all packages +go-lint *flags: \ + (go-lint-pkg "cni-plugin" flags) \ + (go-lint-pkg "pkg" flags) \ + (go-lint-pkg "proxy-init" flags) + +# go-test all packages w/ gotestsum and write results to ${pkg}-test.json +# +# cni-plugin is not tested here as it only include integration tests +go-test: \ + (go-test-pkg "pkg") \ + (go-test-pkg "proxy-init") + +# go-build runs go build against a specific package outputting to a specific +# target binary (no optional flags) +go-build-pkg out pkg: + go build -o target/{{ out }} ./{{ pkg }} + +# go-lint-pkg runs go lint against a specific package with optional flags +go-lint-pkg pkg *flags: + golangci-lint run ./{{ pkg }}/... {{ flags }} + +# go-test runs gotestsum against a specific package with optional flags writing +# the output/result to ${pkg}-test.json and coverage detail to ${pkg}-cover.out +go-test-pkg pkg *flags: + gotestsum --jsonfile {{ pkg }}-test.json -- \ + -coverprofile {{ pkg }}-cover.out \ + -race -v -mod=readonly -v ./{{ pkg }}/... {{ flags }} diff --git a/justfile b/justfile index 2eae8de6..c89191ec 100644 --- a/justfile +++ b/justfile @@ -1,3 +1,4 @@ +import "go.just" # # Config # @@ -15,13 +16,7 @@ default: lint test lint: sh-lint md-lint rs-clippy action-lint action-dev-check -go-lint *flags: (proxy-init-lint flags) (cni-plugin-lint flags) - -test: rs-test proxy-init-test-unit proxy-init-test-integration cni-repair-controller-integration - -# Check whether the Go code is formatted. -go-fmt-check: - out=$(gofmt -d .) ; [ -z "$out" ] || (echo "$out" ; exit 1) +test: rs-test go-test proxy-init-test-integration cni-repair-controller-integration ## ## rust @@ -92,24 +87,10 @@ cni-repair-controller-integration: build-cni-plugin-image ## cni-plugin ## -cni-plugin-lint *flags: - golangci-lint run ./cni-plugin/... {{ flags }} - ## ## proxy-init ## -proxy-init-build: - go build -o target/linkerd2-proxy-init ./proxy-init - -proxy-init-lint *flags: - golangci-lint run ./proxy-init/... {{ flags }} - -# Run proxy-init unit tests -proxy-init-test-unit: - go test -v ./proxy-init/... - go test -v ./pkg/... - # Run proxy-init integration tests after preparing dependencies proxy-init-test-integration: proxy-init-test-integration-deps proxy-init-test-integration-run @@ -136,12 +117,6 @@ build-proxy-init-test-image *args='--load': ## cni-plugin ## -cni-plugin-build: - go build -o target/linkerd2-cni-plugin ./cni-plugin - -cni-plugin-test-unit: - go test -v ./cni-plugin/... - # Build docker image for cni-plugin (Development) build-cni-plugin-image *args='--load': docker buildx build . \ diff --git a/pkg/iptables/iptables.go b/pkg/iptables/iptables.go index 9cf9ff75..a661a8d5 100644 --- a/pkg/iptables/iptables.go +++ b/pkg/iptables/iptables.go @@ -274,7 +274,7 @@ func (fc FirewallConfiguration) addRulesForIgnoredSubnets(chainName string, comm func makeMultiportDestinations(portsToIgnore []string) [][]string { destinationSlices := make([][]string, 0) destinationPortCount := 0 - if portsToIgnore == nil || len(portsToIgnore) < 1 { + if len(portsToIgnore) < 1 { return destinationSlices } destinations := make([]string, 0) @@ -422,9 +422,9 @@ func (fc FirewallConfiguration) makeRedirectChainToPortBasedOnDestinationPort(ch "--comment", formatComment(comment)) } -func (fc FirewallConfiguration) makeJumpFromChainToAnotherForAllProtocols(chainName string, targetChain string, comment string, delete bool) *exec.Cmd { +func (fc FirewallConfiguration) makeJumpFromChainToAnotherForAllProtocols(chainName string, targetChain string, comment string, deleteChain bool) *exec.Cmd { action := "-A" - if delete { + if deleteChain { action = "-D" } diff --git a/pkg/iptables/iptables_test.go b/pkg/iptables/iptables_test.go index 8e1f7bd8..8254c1e1 100644 --- a/pkg/iptables/iptables_test.go +++ b/pkg/iptables/iptables_test.go @@ -7,6 +7,7 @@ import ( ) func TestMakeMultiportDestinations(t *testing.T) { + assertEqual(t, makeMultiportDestinations(nil), [][]string{}) assertEqual(t, makeMultiportDestinations([]string{}), [][]string{}) assertEqual(t, makeMultiportDestinations([]string{"22", "25-27", "33"}), [][]string{{"22", "25:27", "33"}}) assertEqual(t, makeMultiportDestinations([]string{"22-22", "25-27", "33"}), [][]string{{"22", "25:27", "33"}}) diff --git a/pkg/util/portrange_test.go b/pkg/util/portrange_test.go index 0b7daf7b..b382daaa 100644 --- a/pkg/util/portrange_test.go +++ b/pkg/util/portrange_test.go @@ -48,7 +48,10 @@ func TestParsePortRange(t *testing.T) { for _, tt := range tests { t.Run(tt.input, func(t *testing.T) { check, _ := ParsePortRange(tt.input) - reflect.DeepEqual(tt.expected, check) + if !reflect.DeepEqual(tt.expected, check) { + t.Fatalf("expected port range does not match check '%v'<>'%v'", + tt.expected, check) + } }) } }