From 6d2607d284f946b5b6c972d4849894301a26ef21 Mon Sep 17 00:00:00 2001 From: Andrew Gouin Date: Thu, 27 Apr 2023 10:14:40 -0600 Subject: [PATCH] go 1.20 (#141) * Bump to go v1.20 * Implement errors.Join suggestions * Simplify errors.Join returns * Fix TestIsRunning for go 1.20 * lint * bump gomod * Feedback * lint --- .github/workflows/docker-publish.yaml | 12 +++--- .github/workflows/lint.yml | 4 +- .github/workflows/release.yml | 10 ++--- .github/workflows/tests.yml | 8 ++-- .golangci.yml | 2 + docker/horcrux/Dockerfile | 2 +- docker/horcrux/native.Dockerfile | 2 +- go.mod | 3 +- go.sum | 2 + signer/config.go | 48 ++++++++++++++------- signer/config_test.go | 8 ++-- signer/services_test.go | 60 +++++++++++++++++++++------ 12 files changed, 109 insertions(+), 52 deletions(-) diff --git a/.github/workflows/docker-publish.yaml b/.github/workflows/docker-publish.yaml index 36e995e4..e3df3aae 100644 --- a/.github/workflows/docker-publish.yaml +++ b/.github/workflows/docker-publish.yaml @@ -20,16 +20,16 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Set up QEMU - uses: docker/setup-qemu-action@v1 + uses: docker/setup-qemu-action@v2 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v2 - name: Log in to the Container registry - uses: docker/login-action@v1 + uses: docker/login-action@v2 with: registry: ${{ env.REGISTRY }} username: ${{ github.actor }} @@ -37,12 +37,12 @@ jobs: - name: Extract metadata (tags, labels) for Docker id: meta - uses: docker/metadata-action@v3 + uses: docker/metadata-action@v4 with: images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} - name: Build and push Docker image - uses: docker/build-push-action@v2.7.0 + uses: docker/build-push-action@v4 with: context: . platforms: linux/amd64,linux/arm64 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 3a04e351..bacb81e7 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -12,9 +12,9 @@ jobs: name: lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: golangci-lint - uses: golangci/golangci-lint-action@v2 + uses: golangci/golangci-lint-action@v3 with: version: latest only-new-issues: true diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 834143a3..fb0ae72e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -14,13 +14,13 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v2.3.4 + uses: actions/checkout@v3 with: fetch-depth: 0 - - uses: actions/setup-go@v2 + - uses: actions/setup-go@v4 with: - go-version: '1.19' + go-version: '^1.20.0' # TODO:figure out how to add the supported SDK and tendermint versions # here we need to run the following and save the results to ENV in the goreleaser action @@ -32,13 +32,13 @@ jobs: if: startsWith(github.ref, 'refs/tags/') - name: Build - uses: goreleaser/goreleaser-action@v2 + uses: goreleaser/goreleaser-action@v4 with: version: latest args: build --skip-validate # skip validate skips initial sanity checks in order to be able to fully run - name: Release - uses: goreleaser/goreleaser-action@v2 + uses: goreleaser/goreleaser-action@v4 if: startsWith(github.ref, 'refs/tags/') with: version: latest diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d6fa69ef..75d1afb0 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -18,14 +18,14 @@ jobs: runs-on: [self-hosted, linux] steps: # Install and setup go - - name: Set up Go 1.19 - uses: actions/setup-go@v2 + - name: Set up Go 1.20 + uses: actions/setup-go@v4 with: - go-version: 1.19 + go-version: '^1.20.0' # checkout horcrux - name: checkout horcrux - uses: actions/checkout@v2 + uses: actions/checkout@v3 # cleanup docker environment on self-hosted test runner - name: prepare fresh docker environment diff --git a/.golangci.yml b/.golangci.yml index bd262e10..f7823edc 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -36,6 +36,8 @@ linters-settings: alias: cryptotypes - pkg: github.com/cosmos/cosmos-sdk/x/slashing/types alias: slashingtypes + - pkg : github.com/kraken-hpc/go-fork + alias: fork - pkg: github.com/tendermint/tendermint/types alias: tm - pkg: github.com/tendermint/tendermint/config diff --git a/docker/horcrux/Dockerfile b/docker/horcrux/Dockerfile index 5c173655..a054597e 100644 --- a/docker/horcrux/Dockerfile +++ b/docker/horcrux/Dockerfile @@ -1,4 +1,4 @@ -FROM --platform=$BUILDPLATFORM golang:1.19-alpine AS build-env +FROM --platform=$BUILDPLATFORM golang:1.20-alpine AS build-env ENV PACKAGES make git diff --git a/docker/horcrux/native.Dockerfile b/docker/horcrux/native.Dockerfile index e54a6343..d9ae66cd 100644 --- a/docker/horcrux/native.Dockerfile +++ b/docker/horcrux/native.Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.19-alpine AS build-env +FROM golang:1.20-alpine AS build-env ENV PACKAGES make git diff --git a/go.mod b/go.mod index 4e2c5c78..c057eeb2 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/strangelove-ventures/horcrux -go 1.19 +go 1.20 require ( github.com/Jille/grpc-multi-resolver v1.1.0 @@ -14,6 +14,7 @@ require ( github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 github.com/hashicorp/raft v1.3.10 github.com/hashicorp/raft-boltdb/v2 v2.2.2 + github.com/kraken-hpc/go-fork v0.1.1 github.com/mitchellh/go-homedir v1.1.0 github.com/ory/dockertest v3.3.5+incompatible github.com/prometheus/client_golang v1.11.0 diff --git a/go.sum b/go.sum index 750a3e2d..b0cca039 100644 --- a/go.sum +++ b/go.sum @@ -550,6 +550,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/kraken-hpc/go-fork v0.1.1 h1:O3X/ynoNy/eS7UIcZYef8ndFq2RXEIOue9kZqyzF0Sk= +github.com/kraken-hpc/go-fork v0.1.1/go.mod h1:uu0e5h+V4ONH5Qk/xuVlyNXJXy/swhqGIEMK7w+9dNc= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII= github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= diff --git a/signer/config.go b/signer/config.go index eb017407..b437f2c7 100644 --- a/signer/config.go +++ b/signer/config.go @@ -1,6 +1,7 @@ package signer import ( + "errors" "fmt" "net/url" "os" @@ -49,36 +50,46 @@ func (c *Config) MustMarshalYaml() []byte { } func (c *Config) ValidateSingleSignerConfig() error { + var errs []error if len(c.ChainNodes) == 0 { - return fmt.Errorf("need to have chain-nodes configured for priv-val connection") + errs = append(errs, fmt.Errorf("need to have chain-nodes configured for priv-val connection")) } - return c.ChainNodes.Validate() + if err := c.ChainNodes.Validate(); err != nil { + errs = append(errs, err) + } + return errors.Join(errs...) } func (c *Config) ValidateCosignerConfig() error { + var errs []error if err := c.ValidateSingleSignerConfig(); err != nil { - return err + errs = append(errs, err) } if c.CosignerConfig == nil { - return fmt.Errorf("cosigner config can't be empty") + errs = append(errs, fmt.Errorf("cosigner config can't be empty")) + // the rest of the checks depend on non-nil c.CosignerConfig + return errors.Join(errs...) } if c.CosignerConfig.Threshold <= c.CosignerConfig.Shares/2 { - return fmt.Errorf("threshold (%d) must be greater than number of shares (%d) / 2", - c.CosignerConfig.Threshold, c.CosignerConfig.Shares) + errs = append(errs, fmt.Errorf("threshold (%d) must be greater than number of shares (%d) / 2", + c.CosignerConfig.Threshold, c.CosignerConfig.Shares)) } if c.CosignerConfig.Shares < c.CosignerConfig.Threshold { - return fmt.Errorf("number of shares (%d) must be greater or equal to threshold (%d)", - c.CosignerConfig.Shares, c.CosignerConfig.Threshold) + errs = append(errs, fmt.Errorf("number of shares (%d) must be greater or equal to threshold (%d)", + c.CosignerConfig.Shares, c.CosignerConfig.Threshold)) } _, err := time.ParseDuration(c.CosignerConfig.Timeout) if err != nil { - return fmt.Errorf("invalid --timeout: %w", err) + errs = append(errs, fmt.Errorf("invalid --timeout: %w", err)) } if _, err := url.Parse(c.CosignerConfig.P2PListen); err != nil { - return fmt.Errorf("failed to parse p2p listen address: %w", err) + errs = append(errs, fmt.Errorf("failed to parse p2p listen address: %w", err)) + } + if err := c.CosignerConfig.Peers.Validate(c.CosignerConfig.Shares); err != nil { + errs = append(errs, err) } - return c.CosignerConfig.Peers.Validate(c.CosignerConfig.Shares) + return errors.Join(errs...) } type RuntimeConfig struct { @@ -218,17 +229,23 @@ func duplicatePeers(peers []CosignerPeerConfig) (duplicates map[int][]string) { } func PeersFromFlag(peers []string) (out []CosignerPeerConfig, err error) { + var errs []error for _, p := range peers { ps := strings.Split(p, "|") if len(ps) != 2 { - return nil, fmt.Errorf("invalid peer string %s, expected format: tcp://{addr}:{port}|{share-id}", p) + errs = append(errs, fmt.Errorf("invalid peer string %s, expected format: tcp://{addr}:{port}|{share-id}", p)) + continue } shareid, err := strconv.ParseInt(ps[1], 10, 64) if err != nil { - return nil, fmt.Errorf("failed to parse share ID: %w", err) + errs = append(errs, fmt.Errorf("failed to parse share ID: %w", err)) + continue } out = append(out, CosignerPeerConfig{ShareID: int(shareid), P2PAddr: ps[0]}) } + if len(errs) > 0 { + return nil, errors.Join(errs...) + } return out, nil } @@ -244,12 +261,13 @@ func (cn ChainNode) Validate() error { type ChainNodes []ChainNode func (cns ChainNodes) Validate() error { + var errs []error for _, cn := range cns { if err := cn.Validate(); err != nil { - return err + errs = append(errs, err) } } - return nil + return errors.Join(errs...) } func ChainNodesFromArg(arg string) (out ChainNodes, err error) { diff --git a/signer/config_test.go b/signer/config_test.go index 22ae8e2b..18787fed 100644 --- a/signer/config_test.go +++ b/signer/config_test.go @@ -73,7 +73,7 @@ func TestValidateSingleSignerConfig(t *testing.T) { require.NoError(t, err, tc.name) } else { require.Error(t, err, tc.name) - require.Equal(t, tc.expectErr, err, tc.name) + require.EqualError(t, err, tc.expectErr.Error(), tc.name) } } } @@ -293,7 +293,7 @@ func TestValidateCosignerConfig(t *testing.T) { require.NoError(t, err, tc.name) } else { require.Error(t, err, tc.name) - require.Equal(t, tc.expectErr, err, tc.name) + require.EqualError(t, err, tc.expectErr.Error(), tc.name) } } } @@ -528,7 +528,7 @@ func TestCosignerPeersConfigValidate(t *testing.T) { require.NoError(t, err, tc.name) } else { require.Error(t, err, tc.name) - require.Equal(t, tc.expectErr, err, tc.name) + require.EqualError(t, err, tc.expectErr.Error(), tc.name) } } } @@ -570,7 +570,7 @@ func TestPeersFromFlag(t *testing.T) { require.NoError(t, err, tc.name) } else { require.Error(t, err, tc.name) - require.Equal(t, tc.expectErr, err, tc.name) + require.EqualError(t, err, tc.expectErr.Error(), tc.name) } } } diff --git a/signer/services_test.go b/signer/services_test.go index ab69d4a3..c443d7c9 100644 --- a/signer/services_test.go +++ b/signer/services_test.go @@ -17,29 +17,63 @@ import ( tmlog "github.com/tendermint/tendermint/libs/log" tmservice "github.com/tendermint/tendermint/libs/service" + fork "github.com/kraken-hpc/go-fork" "github.com/stretchr/testify/require" ) -func TestIsRunning(t *testing.T) { - if runtime.GOOS != "linux" { - t.Skip("test only valid on Linux") +func init() { + fork.RegisterFunc("child", mockHorcruxChildProcess) + fork.Init() +} + +func mockHorcruxChildProcess(pidFilePath string) { + _ = os.WriteFile( + pidFilePath, + []byte(fmt.Sprintf("%d\n", os.Getpid())), + 0600, + ) +} + +func waitForFileToExist(file string, timeout time.Duration) error { + exp := time.After(timeout) + tick := time.Tick(20 * time.Millisecond) + for { + select { + case <-exp: + return fmt.Errorf("timed out") + case <-tick: + if _, err := os.Stat(file); err != nil { + if os.IsNotExist(err) { + // file does not exist yet + continue + } + // unexpected error + return err + } + // file exists + return nil + } } +} +func TestIsRunning(t *testing.T) { homeDir := t.TempDir() pidFilePath := filepath.Join(homeDir, "horcrux.pid") - // this child process exists for linux, but not macOS - // TODO: make this test work with macOS, maybe spawn child process or dummy horcrux process. - pid := os.Getpid() + 1 - err := os.WriteFile( - pidFilePath, - []byte(fmt.Sprintf("%d\n", pid)), - 0600, - ) - require.NoError(t, err, "error writing pid file") + // github.com/kraken-hpc/go-fork package used (in tests only) to create a new pid with args[0] of horcrux. + // This lets us mock a horcrux process to test the "horcrux is already running" case. + err := fork.Fork("child", pidFilePath) + require.NoError(t, err) + + // wait for child process to start and write pidFilePath + err = waitForFileToExist(pidFilePath, 1*time.Second) + require.NoError(t, err) + + pidBz, err := os.ReadFile(pidFilePath) + require.NoError(t, err) err = signer.RequireNotRunning(pidFilePath) - expectedErrorMsg := fmt.Sprintf("horcrux is already running on PID: %d", pid) + expectedErrorMsg := fmt.Sprintf("horcrux is already running on PID: %s", strings.TrimSpace(string(pidBz))) require.EqualError(t, err, expectedErrorMsg) }