From 30513ab2c3f5b5a855cd5283887f019d3f364939 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Mon, 20 May 2024 11:33:54 -0400 Subject: [PATCH 1/6] Add CODEOWNERS file --- .github/CODEOWNERS | 1 + 1 file changed, 1 insertion(+) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000..21dcdea --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1 @@ +* @joelrebel @mmlb @splaspood @DoctorVin From 2c774aef7447d19284113a8259d04ee379756f30 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Mon, 20 May 2024 13:33:05 -0400 Subject: [PATCH 2/6] Configure chainguard to accept commits signed by GitHub These are commits done by GitHub itself, relevant mostly for commits made when merging a PR. Yoinked from ironlib's repo. --- .chainguard/source.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .chainguard/source.yaml diff --git a/.chainguard/source.yaml b/.chainguard/source.yaml new file mode 100644 index 0000000..a7c39f5 --- /dev/null +++ b/.chainguard/source.yaml @@ -0,0 +1,11 @@ +--- +spec: + authorities: + # Accept all keyless signatures validated from the public sigstore instance. + # This is open source software after all. All we want to know is that the + # person that did the commit has control over their email address. + - keyless: + url: https://fulcio.sigstore.dev + # Add this if you also want to allow commits signed by GitHub. + - key: + kms: https://github.com/web-flow.gpg From 04df28a0f42141e409a30e531b682d3b0d4394cd Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Mon, 20 May 2024 13:42:33 -0400 Subject: [PATCH 3/6] Use go run to run golangci-lint We can even specify the version so that we avoid different versions between ci and local dev machine. I got rid of `${NO_FUTURE}` because that looks to be unset anywhere and looks to be copy/pasted from a github issue having to do with the github action. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b31e121..f8df0e8 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ build: -X $(LDFLAG_LOCATION).Version=$(VERSION) \ -X $(LDFLAG_LOCATION).BuildDate=$(BUILD_DATE)" lint: - golangci-lint run --config .golangci.yml --timeout=5m --out-${NO_FUTURE}format colored-line-number + go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.48.0 run --config .golangci.yml --timeout=5m --out-format colored-line-number test: lint CGO_ENABLED=0 $(GOBINARY) test -timeout 1m -v -covermode=atomic ./... From 5dea985a0db73fadc4c8954b719b5475ccf859e8 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Mon, 20 May 2024 13:39:51 -0400 Subject: [PATCH 4/6] Run golangci-lint in CI using Makefile This way we have one and only-one way of doing things. --- .github/workflows/lint.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 0a5642a..aedbc85 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -15,13 +15,11 @@ jobs: uses: actions/checkout@v4 with: fetch-depth: 0 + - name: Set up Go uses: actions/setup-go@v4 with: go-version: 1.19 - name: Run golangci-lint - uses: golangci/golangci-lint-action@v3 - with: - version: v1.48.0 - args: "--timeout=5m --out-${NO_FUTURE}format colored-line-number" + run: make lint From 0789a9beb17e400588b10f5d80402d7a2909a1ef Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Mon, 20 May 2024 13:59:46 -0400 Subject: [PATCH 5/6] Update Go to latest version The action is able to read the go.mod version to discover what go version to install. So lets use that just like in ironlib to avoid needing to keep multiple files in sync. Needed to update golangci-lint because the previous version fails. I used the same version as in ironlib. New golangci-lint complained about some new stuff, so I fixed it. --- .github/workflows/goreleaser.yml | 6 +++--- .github/workflows/lint.yml | 6 +++--- Makefile | 2 +- cmd/format_partition.go | 2 +- cmd/partition_disk.go | 2 +- cmd/raid_create.go | 2 +- cmd/raid_delete.go | 2 +- cmd/raid_list.go | 2 +- go.mod | 2 +- pkg/model/model_test.go | 2 -- 10 files changed, 13 insertions(+), 15 deletions(-) diff --git a/.github/workflows/goreleaser.yml b/.github/workflows/goreleaser.yml index 6f3979d..bc0d947 100644 --- a/.github/workflows/goreleaser.yml +++ b/.github/workflows/goreleaser.yml @@ -23,10 +23,10 @@ jobs: - name: Checkout uses: actions/checkout@v4 - - name: Set up Go - uses: actions/setup-go@v4 + - name: Install Go + uses: actions/setup-go@v5 with: - go-version: 1.19 + go-version-file: go.mod - name: install cosign uses: sigstore/cosign-installer@main diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index aedbc85..64e7d7e 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -16,10 +16,10 @@ jobs: with: fetch-depth: 0 - - name: Set up Go - uses: actions/setup-go@v4 + - name: Install Go + uses: actions/setup-go@v5 with: - go-version: 1.19 + go-version-file: go.mod - name: Run golangci-lint run: make lint diff --git a/Makefile b/Makefile index f8df0e8..0c6a923 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ build: -X $(LDFLAG_LOCATION).Version=$(VERSION) \ -X $(LDFLAG_LOCATION).BuildDate=$(BUILD_DATE)" lint: - go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.48.0 run --config .golangci.yml --timeout=5m --out-format colored-line-number + go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 run --config .golangci.yml --timeout=5m --out-format colored-line-number test: lint CGO_ENABLED=0 $(GOBINARY) test -timeout 1m -v -covermode=atomic ./... diff --git a/cmd/format_partition.go b/cmd/format_partition.go index 8b9a395..4021b61 100644 --- a/cmd/format_partition.go +++ b/cmd/format_partition.go @@ -12,7 +12,7 @@ var formatPartitionCmd = &cobra.Command{ Use: "format-partition", Short: "Formats a partition", Long: "Formats a partition with your choice of filesystem", - Run: func(cmd *cobra.Command, args []string) { + Run: func(cmd *cobra.Command, _ []string) { ctx := context.Background() if GetString(cmd, "device") == "" && GetString(cmd, "filesystem-device") == "" { diff --git a/cmd/partition_disk.go b/cmd/partition_disk.go index b26050d..ab8e334 100644 --- a/cmd/partition_disk.go +++ b/cmd/partition_disk.go @@ -12,7 +12,7 @@ var partitionDiskCmd = &cobra.Command{ Short: "Partitions a block device", Long: "Partitions a block device with a GPT table", Args: cobra.NoArgs, - Run: func(cmd *cobra.Command, args []string) { + Run: func(cmd *cobra.Command, _ []string) { ctx := context.Background() partitions := GetStringSlice(cmd, "partitions") diff --git a/cmd/raid_create.go b/cmd/raid_create.go index 6d1622c..a9241f7 100644 --- a/cmd/raid_create.go +++ b/cmd/raid_create.go @@ -15,7 +15,7 @@ var createRaidCmd = &cobra.Command{ Use: "create", Short: "Creates a VirtualDisk from one or more PhysicalDisk(s)", Long: "Creates a VirtualDisk from one or more PhysicalDisk(s)", - Run: func(cmd *cobra.Command, args []string) { + Run: func(cmd *cobra.Command, _ []string) { ctx := command.NewContextWithLogger(cmd.Context(), logger) raidType := GetString(cmd, "raid-type") createArray(ctx, GetString(cmd, "name"), raidType, GetString(cmd, "raid-level"), GetStringSlice(cmd, "devices")) diff --git a/cmd/raid_delete.go b/cmd/raid_delete.go index 11734fc..84cdd50 100644 --- a/cmd/raid_delete.go +++ b/cmd/raid_delete.go @@ -14,7 +14,7 @@ var deleteRaidCmd = &cobra.Command{ Use: "delete", Short: "Deletes a VirtualDisk (RAID array)", Long: "Deletes a VirtualDisk (RAID array)", - Run: func(cmd *cobra.Command, args []string) { + Run: func(cmd *cobra.Command, _ []string) { ctx := command.NewContextWithLogger(cmd.Context(), logger) raidType := GetString(cmd, "raid-type") deleteArray(ctx, raidType, GetString(cmd, "name")) diff --git a/cmd/raid_list.go b/cmd/raid_list.go index 29ee14c..24011ce 100644 --- a/cmd/raid_list.go +++ b/cmd/raid_list.go @@ -14,7 +14,7 @@ var listRaidCmd = &cobra.Command{ Use: "list", Short: "Lists existing VirtualDisk(s) (RAID arrays)", Long: "Lists existing VirtualDisk(s) (RAID arrays)", - Run: func(cmd *cobra.Command, args []string) { + Run: func(cmd *cobra.Command, _ []string) { ctx := command.NewContextWithLogger(cmd.Context(), logger) raidType := GetString(cmd, "raid-type") raidObjectType := GetString(cmd, "object-type") diff --git a/go.mod b/go.mod index 98c2e87..2b8fa19 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/metal-toolbox/vogelkop -go 1.19 +go 1.22 require ( github.com/Sytten/logrus-zap-hook v0.1.0 diff --git a/pkg/model/model_test.go b/pkg/model/model_test.go index 360bbdb..c150650 100644 --- a/pkg/model/model_test.go +++ b/pkg/model/model_test.go @@ -51,7 +51,6 @@ func createPartitions(ctx context.Context, bd *model.BlockDevice, partitions []* p.BlockDevice = bd out, err = p.Create(ctx) - if err != nil { return } @@ -68,7 +67,6 @@ func createPartitions(ctx context.Context, bd *model.BlockDevice, partitions []* var partitionBd *model.BlockDevice partitionBd, err = model.NewBlockDevice(p.GetLoopBlockDevice()) - if err != nil { return } From 2fea6fc6309bd2fcf6f55fb4905e353779a017cc Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Mon, 20 May 2024 14:19:24 -0400 Subject: [PATCH 6/6] Update .golangci.yml from metal-toolbox/golangci-lint-config And fix the errors of course, gotta keep CI green! Most of the class of changes are pretty small. --- .golangci.yml | 270 +++++++++++----------------------------- Makefile | 3 + cmd/format_partition.go | 3 +- cmd/raid_create.go | 5 +- cmd/raid_delete.go | 3 +- cmd/raid_list.go | 3 +- cmd/root.go | 1 - pkg/model/model_test.go | 3 +- pkg/model/raid_array.go | 73 ++++++----- 9 files changed, 125 insertions(+), 239 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 46f6a26..8112468 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,211 +1,93 @@ -run: - # The default runtime timeout is 1m, which doesn't work well on Github Actions. - timeout: 4m +# +# This file lives in the github.com/metal-toolbox/golangci-lint-config repo. +# +# Do not edit this file outside of this repo otherwise we will be grumpy. +# Seriously though, this is meant to help promote a "standard" config and coding style. +# If you don't like something, lets have a discussion in GitHub issues! +# -# NOTE: This file is populated by the lint-install tool. Local adjustments may be overwritten. linters-settings: - cyclop: - # NOTE: This is a very high transitional threshold - max-complexity: 37 - package-average: 34.0 - skip-tests: true - - gocognit: - # NOTE: This is a very high transitional threshold - min-complexity: 98 - dupl: - threshold: 200 - + threshold: 125 goconst: - min-len: 4 - min-occurrences: 5 - ignore-tests: true - - gosec: - excludes: - - G107 # Potential HTTP request made with variable url - - G204 # Subprocess launched with function call as argument or cmd arguments - - G404 # Use of weak random number generator (math/rand instead of crypto/rand - - errorlint: - # these are still common in Go: for instance, exit errors. - asserts: false - # Forcing %w in error wrapping forces authors to make errors part of their package APIs. The decision to make - # an error part of a package API should be a concious decision by the author. - # Also see Hyrums Law. - errorf: false - - exhaustive: - default-signifies-exhaustive: true - - nestif: - min-complexity: 8 - - nolintlint: - require-explanation: true - allow-unused: false - require-specific: true - + min-len: 2 + min-occurrences: 2 + gocritic: + enabled-tags: + - experimental + - performance + - style + disabled-checks: + - whyNoLint + - wrapperFunc + gocyclo: + min-complexity: 15 + gofumpt: + extra-rules: true + govet: + enable: + - shadow + lll: + line-length: 140 + misspell: + locale: US revive: - ignore-generated-header: true - severity: warning - rules: - - name: atomic - - name: blank-imports - - name: bool-literal-in-expr - - name: confusing-naming - - name: constant-logical-expr - - name: context-as-argument - - name: context-keys-type - - name: deep-exit - - name: defer - - name: range-val-in-closure - - name: range-val-address - - name: dot-imports - - name: error-naming - - name: error-return - - name: error-strings - - name: errorf - - name: exported - - name: identical-branches - - name: if-return - - name: import-shadowing - - name: increment-decrement - - name: indent-error-flow - - name: indent-error-flow - - name: package-comments - - name: range - - name: receiver-naming - - name: redefines-builtin-id - - name: superfluous-else - - name: struct-tag - - name: time-naming - - name: unexported-naming - - name: unexported-return - - name: unnecessary-stmt - - name: unreachable-code - - name: unused-parameter - - name: var-declaration - - name: var-naming - - name: unconditional-recursion - - name: waitgroup-by-value - - staticcheck: - go: "1.18" - - unused: - go: "1.18" - -output: - sort-results: true + confidence: 0 linters: - disable-all: true - enable: - - asciicheck - - bodyclose + enable-all: true + disable-all: false + # Linters we don't like + # Comments help explain why its disabled or point at ones we should not disable but will take a little work + # If its not commented its likely because its just too annoying or we don't find useful + disable: + - copyloopvar # requires go >=1.22 - cyclop - #- deadcode - - dogsled - - dupl - - durationcheck - - errcheck - - errname - - errorlint - - exhaustive - - exportloopref - - forcetypeassert + - deadcode # deprecated + - depguard + - errname # maybe should be enabled + - exhaustivestruct # deprecated + - exhaustruct + - forbidigo + - funlen + - gochecknoglobals + - gochecknoinits - gocognit - goconst - - gocritic - godot - - gofmt - - gofumpt - - gosec - - goheader - - goimports - - goprintffuncname - - gosimple - - govet - #- ifshort - - importas - - ineffassign - - makezero - - misspell - - nakedret + - godox + - golint # deprecated + - gomnd + - ifshort # deprecated + - inamedparam + - interfacebloat + - interfacer # deprecated + - intrange # requires go >=1.22 + - ireturn # should be enabled, ironlib needs some changes + - lll # not previously enabled, ironlib and mctl both fail this + - maligned # deprecated - nestif - - nilerr - - noctx + - nilnil + - nlreturn - nolintlint - - predeclared - # disabling for the initial iteration of the linting tool - # - promlinter - - revive - #- rowserrcheck - #- sqlclosecheck - - staticcheck - #- structcheck - - stylecheck - - thelper - - tparallel - - typecheck - - unconvert - - unparam - - unused - #- varcheck - #- wastedassign - - whitespace - - # Disabled linters, due to being misaligned with Go practices - # - exhaustivestruct - # - gochecknoglobals - # - gochecknoinits - # - goconst - # - godox - # - goerr113 - # - gomnd - # - lll - # - nlreturn - # - testpackage - # - wsl - # Disabled linters, due to not being relevant to our code base: - # - maligned - # - prealloc "For most programs usage of prealloc will be a premature optimization." - # Disabled linters due to bad error messages or bugs - # - tagliatelle + - nonamedreturns # should be enabled, probably + - nosnakecase # deprecated + - paralleltest + - perfsprint + - scopelint # deprecated + - structcheck # deprecated + - tagliatelle + - tenv # should be enabled + - testpackage + - testifylint # should be enabled + - thelper # should be enabled + - varcheck # deprecated + - varnamelen + - wrapcheck + - wsl issues: - # Excluding configuration per-path, per-linter, per-text and per-source exclude-rules: - - path: _test\.go - linters: - - dupl - - errcheck - - forcetypeassert - - gocyclo - - gosec - - noctx - - - path: .*cmd.* - linters: - - noctx - - - path: main\.go - linters: - - noctx - - - path: .*cmd.* - text: "deep-exit" - - - path: main\.go - text: "deep-exit" - - # This check is of questionable value - linters: - - tparallel - text: "call t.Parallel on the top level as well as its subtests" - - # Don't hide lint issues just because there are many of them - max-same-issues: 0 - max-issues-per-linter: 0 + - stylecheck + text: "ST1016" diff --git a/Makefile b/Makefile index 0c6a923..68c44f0 100644 --- a/Makefile +++ b/Makefile @@ -19,5 +19,8 @@ build: lint: go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 run --config .golangci.yml --timeout=5m --out-format colored-line-number +lint-fix: + go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 run --fix --config .golangci.yml --timeout=5m --out-format colored-line-number + test: lint CGO_ENABLED=0 $(GOBINARY) test -timeout 1m -v -covermode=atomic ./... diff --git a/cmd/format_partition.go b/cmd/format_partition.go index 4021b61..649c6f6 100644 --- a/cmd/format_partition.go +++ b/cmd/format_partition.go @@ -3,9 +3,8 @@ package cmd import ( "context" - "github.com/spf13/cobra" - "github.com/metal-toolbox/vogelkop/pkg/model" + "github.com/spf13/cobra" ) var formatPartitionCmd = &cobra.Command{ diff --git a/cmd/raid_create.go b/cmd/raid_create.go index a9241f7..64cba64 100644 --- a/cmd/raid_create.go +++ b/cmd/raid_create.go @@ -4,11 +4,10 @@ import ( "context" "strconv" - "github.com/spf13/cobra" - "github.com/bmc-toolbox/common" "github.com/metal-toolbox/vogelkop/internal/command" "github.com/metal-toolbox/vogelkop/pkg/model" + "github.com/spf13/cobra" ) var createRaidCmd = &cobra.Command{ @@ -76,7 +75,7 @@ func processDevicesLinuxSw(arrayDevices []string) []*model.BlockDevice { } func processDevicesHardware(arrayDevices []string) []*model.BlockDevice { - var blockDeviceIDs []int + blockDeviceIDs := make([]int, 0, len(arrayDevices)) for _, d := range arrayDevices { intBlockDevice, err := strconv.Atoi(d) diff --git a/cmd/raid_delete.go b/cmd/raid_delete.go index 84cdd50..2d4c1e3 100644 --- a/cmd/raid_delete.go +++ b/cmd/raid_delete.go @@ -4,10 +4,9 @@ import ( "context" "strconv" - "github.com/spf13/cobra" - "github.com/metal-toolbox/vogelkop/internal/command" "github.com/metal-toolbox/vogelkop/pkg/model" + "github.com/spf13/cobra" ) var deleteRaidCmd = &cobra.Command{ diff --git a/cmd/raid_list.go b/cmd/raid_list.go index 24011ce..fcaca71 100644 --- a/cmd/raid_list.go +++ b/cmd/raid_list.go @@ -4,10 +4,9 @@ import ( "context" "fmt" - "github.com/spf13/cobra" - "github.com/metal-toolbox/vogelkop/internal/command" "github.com/metal-toolbox/vogelkop/pkg/model" + "github.com/spf13/cobra" ) var listRaidCmd = &cobra.Command{ diff --git a/cmd/root.go b/cmd/root.go index 43b2141..6ff9561 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -2,7 +2,6 @@ package cmd import ( version "github.com/metal-toolbox/vogelkop/internal/version" - "github.com/spf13/cobra" "go.uber.org/zap" ) diff --git a/pkg/model/model_test.go b/pkg/model/model_test.go index c150650..b1217ef 100644 --- a/pkg/model/model_test.go +++ b/pkg/model/model_test.go @@ -8,10 +8,9 @@ import ( "path/filepath" "testing" + "github.com/bmc-toolbox/common" diskfs "github.com/diskfs/go-diskfs" losetup "github.com/freddierice/go-losetup/v2" - - "github.com/bmc-toolbox/common" "github.com/metal-toolbox/vogelkop/internal/command" "github.com/metal-toolbox/vogelkop/pkg/model" ) diff --git a/pkg/model/raid_array.go b/pkg/model/raid_array.go index 8c5f885..1e1c234 100644 --- a/pkg/model/raid_array.go +++ b/pkg/model/raid_array.go @@ -127,35 +127,39 @@ func (a *RaidArray) DeleteHardware(ctx context.Context) error { } for _, sc := range hardware.StorageControllers { - if sc.Vendor == common.VendorMarvell { - var vds []*common.VirtualDisk - var sca *actions.StorageControllerAction - sca, err = getStorageControllerAction(ctx) - if err != nil { - return err + if sc.Vendor != common.VendorMarvell { + continue + } + + var vds []*common.VirtualDisk + var sca *actions.StorageControllerAction + sca, err = getStorageControllerAction(ctx) + if err != nil { + return err + } + + vds, err = sca.ListVirtualDisks(ctx, sc) + if err != nil { + return err + } + + for _, vd := range vds { + if vd.Name != a.Name && vd.ID != strconv.Itoa(a.ControllerVirtualDiskID) { + continue } - vds, err = sca.ListVirtualDisks(ctx, sc) + options := &model.DestroyVirtualDiskOptions{ + VirtualDiskID: a.ControllerVirtualDiskID, + } + + var sca *actions.StorageControllerAction + sca, err = getStorageControllerAction(ctx) if err != nil { return err } - for _, vd := range vds { - if vd.Name == a.Name || vd.ID == strconv.Itoa(a.ControllerVirtualDiskID) { - options := &model.DestroyVirtualDiskOptions{ - VirtualDiskID: a.ControllerVirtualDiskID, - } - - var sca *actions.StorageControllerAction - sca, err = getStorageControllerAction(ctx) - if err != nil { - return err - } - - err = sca.DestroyVirtualDisk(ctx, sc, options) - return err - } - } + err = sca.DestroyVirtualDisk(ctx, sc, options) + return err } } @@ -188,6 +192,7 @@ func ListPhysicalDisks(ctx context.Context, raidType string) (physicalDisks []*c func listVirtualDisksLinux(_ context.Context) (virtualDisks []*common.VirtualDisk, err error) { // TODO(splaspood) Implement VD listing for mdadm + // mdadm --misc --detail --export /dev/md/* // Seems on my test ubuntu 20.04 host these end up named /dev/md/ROOT_0 (that _0) return @@ -205,17 +210,19 @@ func listVirtualDisksHardware(ctx context.Context) (virtualDisks []*common.Virtu } for _, sc := range hardware.StorageControllers { - if sc.Vendor == common.VendorMarvell { - var sca *actions.StorageControllerAction - sca, err = getStorageControllerAction(ctx) - if err != nil { - return - } + if sc.Vendor != common.VendorMarvell { + continue + } - virtualDisks, err = sca.ListVirtualDisks(ctx, sc) - if err != nil { - return - } + var sca *actions.StorageControllerAction + sca, err = getStorageControllerAction(ctx) + if err != nil { + return + } + + virtualDisks, err = sca.ListVirtualDisks(ctx, sc) + if err != nil { + return } }