From 9d1c0665809f4831f8f573116565d102ba9a2d6a Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Tue, 11 Jul 2023 20:05:33 +0200 Subject: [PATCH] make the node fail to start on invalid version --- .github/workflows/ci.yml | 5 ++++- cmd/bootstrap/transit/cmd/utils.go | 2 +- cmd/build/version.go | 21 +++++++++++-------- cmd/build/version_test.go | 2 +- cmd/execution_builder.go | 17 ++++++++------- cmd/scaffold.go | 8 +++---- .../export_report.json | 6 ------ engine/access/access_test.go | 2 +- .../rest/routes/node_version_info_test.go | 2 +- engine/access/rpc/backend/backend.go | 2 +- engine/access/rpc/backend/backend_network.go | 2 +- engine/testutil/nodes.go | 6 ++++-- integration/localnet/builder/bootstrap.go | 2 +- 13 files changed, 41 insertions(+), 36 deletions(-) delete mode 100644 cmd/util/cmd/execution-state-extract/export_report.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 26d14496fb5..e41e747e0e0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ on: env: GO_VERSION: "1.20" -concurrency: +concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true @@ -203,6 +203,9 @@ jobs: steps: - name: Checkout repo uses: actions/checkout@v3 + with: + # all tags are needed for integration tests + fetch-depth: 0 - name: Setup Go uses: actions/setup-go@v3 with: diff --git a/cmd/bootstrap/transit/cmd/utils.go b/cmd/bootstrap/transit/cmd/utils.go index 380646671c0..1f8f2f7920b 100644 --- a/cmd/bootstrap/transit/cmd/utils.go +++ b/cmd/bootstrap/transit/cmd/utils.go @@ -36,7 +36,7 @@ var ( // commit and semver vars commit = build.Commit() - semver = build.Semver() + semver = build.Version() ) // readNodeID reads the NodeID file diff --git a/cmd/build/version.go b/cmd/build/version.go index 238385846fc..d089c27f3fb 100644 --- a/cmd/build/version.go +++ b/cmd/build/version.go @@ -7,6 +7,7 @@ package build import ( + "fmt" "strings" smv "github.com/coreos/go-semver/semver" @@ -21,8 +22,8 @@ var ( commit string ) -// Semver returns the semantic version of this build. -func Semver() string { +// Version returns the raw version string of this build. +func Version() string { return semver } @@ -48,24 +49,26 @@ func init() { } } -// SemverV2 returns the semantic version of this build as a semver.Version -// if it is defined, or nil otherwise. +var UndefinedVersionError = fmt.Errorf("version is undefined") + +// Semver returns the semantic version of this build as a semver.Version +// if it is defined, or UndefinedVersionError otherwise. // The version string is converted to a semver compliant one if it isn't already // but this might fail if the version string is still not semver compliant. In that // case, an error is returned. -func SemverV2() (*smv.Version, error) { +func Semver() (*smv.Version, error) { if !IsDefined(semver) { - return nil, nil + return nil, UndefinedVersionError } - ver, err := smv.NewVersion(makeSemverV2Compliant(semver)) + ver, err := smv.NewVersion(makeSemverCompliant(semver)) return ver, err } -// makeSemverV2Compliant converts a non-semver version string to a semver compliant one. +// makeSemverCompliant converts a non-semver version string to a semver compliant one. // This removes the leading 'v'. // In the past we sometimes omitted the patch version, e.g. v1.0.0 became v1.0 so this // also adds a 0 patch version if there's no patch version. -func makeSemverV2Compliant(version string) string { +func makeSemverCompliant(version string) string { if !IsDefined(version) { return version } diff --git a/cmd/build/version_test.go b/cmd/build/version_test.go index 3e130cadde8..4f3232b56d2 100644 --- a/cmd/build/version_test.go +++ b/cmd/build/version_test.go @@ -17,7 +17,7 @@ func TestMakeSemverV2Compliant(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - output := makeSemverV2Compliant(tc.input) + output := makeSemverCompliant(tc.input) if output != tc.expected { t.Errorf("Got %s; expected %s", output, tc.expected) } diff --git a/cmd/execution_builder.go b/cmd/execution_builder.go index 90186b5aa62..871ab90b8f6 100644 --- a/cmd/execution_builder.go +++ b/cmd/execution_builder.go @@ -666,15 +666,18 @@ func (exeNode *ExecutionNode) LoadStopControl( module.ReadyDoneAware, error, ) { - ver, err := build.SemverV2() + ver, err := build.Semver() if err != nil { - ver = nil - // TODO: In the future we want to error here, but for now we just log a warning. - // This is because we currently have no strong guarantee that then node version - // tag is semver compliant. - exeNode.builder.Logger.Warn(). + err = fmt.Errorf("could not set semver version for stop control. "+ + "version %s is not semver compliant: %w", build.Version(), err) + + // The node would not know its own version. Without this the node would not know + // how to reach to version boundaries. + exeNode.builder.Logger. Err(err). - Msg("could not set semver version for stop control") + Msg("error starting stop control") + + return nil, err } latestFinalizedBlock, err := node.State.Final().Head() diff --git a/cmd/scaffold.go b/cmd/scaffold.go index e6043fd1801..07e2d63011e 100644 --- a/cmd/scaffold.go +++ b/cmd/scaffold.go @@ -202,7 +202,7 @@ func (fnb *FlowNodeBuilder) EnqueuePingService() { // setup the Ping provider to return the software version and the sealed block height pingInfoProvider := &ping.InfoProvider{ SoftwareVersionFun: func() string { - return build.Semver() + return build.Version() }, SealedBlockHeightFun: func() (uint64, error) { head, err := node.State.Sealed().Head() @@ -610,7 +610,7 @@ func (fnb *FlowNodeBuilder) ValidateFlags(f func() error) NodeBuilder { } func (fnb *FlowNodeBuilder) PrintBuildVersionDetails() { - fnb.Logger.Info().Str("version", build.Semver()).Str("commit", build.Commit()).Msg("build details") + fnb.Logger.Info().Str("version", build.Version()).Str("commit", build.Commit()).Msg("build details") } func (fnb *FlowNodeBuilder) initNodeInfo() error { @@ -733,7 +733,7 @@ func (fnb *FlowNodeBuilder) initMetrics() error { if err != nil { return fmt.Errorf("could not query root snapshoot protocol version: %w", err) } - nodeInfoMetrics.NodeInfo(build.Semver(), build.Commit(), nodeConfig.SporkID.String(), protocolVersion) + nodeInfoMetrics.NodeInfo(build.Version(), build.Commit(), nodeConfig.SporkID.String(), protocolVersion) return nil }) } @@ -761,7 +761,7 @@ func (fnb *FlowNodeBuilder) createGCEProfileUploader(client *gcemd.Client, opts ProjectID: projectID, ChainID: chainID, Role: fnb.NodeConfig.NodeRole, - Version: build.Semver(), + Version: build.Version(), Commit: build.Commit(), Instance: instance, } diff --git a/cmd/util/cmd/execution-state-extract/export_report.json b/cmd/util/cmd/execution-state-extract/export_report.json deleted file mode 100644 index f33cbf40cb9..00000000000 --- a/cmd/util/cmd/execution-state-extract/export_report.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "EpochCounter": 0, - "PreviousStateCommitment": "1c9f9d343cb8d4610e0b2c1eb74d6ea2f2f8aef2d666281dc22870e3efaa607b", - "CurrentStateCommitment": "1c9f9d343cb8d4610e0b2c1eb74d6ea2f2f8aef2d666281dc22870e3efaa607b", - "ReportSucceeded": true -} diff --git a/engine/access/access_test.go b/engine/access/access_test.go index b6d4051e9ba..b3979979fb9 100644 --- a/engine/access/access_test.go +++ b/engine/access/access_test.go @@ -1142,7 +1142,7 @@ func (suite *Suite) TestAPICallNodeVersionInfo() { respNodeVersionInfo := resp.Info suite.Require().Equal(respNodeVersionInfo, &entitiesproto.NodeVersionInfo{ - Semver: build.Semver(), + Semver: build.Version(), Commit: build.Commit(), SporkId: sporkId[:], ProtocolVersion: uint64(protocolVersion), diff --git a/engine/access/rest/routes/node_version_info_test.go b/engine/access/rest/routes/node_version_info_test.go index 25f19ae1f3c..179d339f94f 100644 --- a/engine/access/rest/routes/node_version_info_test.go +++ b/engine/access/rest/routes/node_version_info_test.go @@ -29,7 +29,7 @@ func TestGetNodeVersionInfo(t *testing.T) { req := getNodeVersionInfoRequest(t) params := &access.NodeVersionInfo{ - Semver: build.Semver(), + Semver: build.Version(), Commit: build.Commit(), SporkId: unittest.IdentifierFixture(), ProtocolVersion: unittest.Uint64InRange(10, 30), diff --git a/engine/access/rpc/backend/backend.go b/engine/access/rpc/backend/backend.go index b6720242717..9b10cc5b539 100644 --- a/engine/access/rpc/backend/backend.go +++ b/engine/access/rpc/backend/backend.go @@ -303,7 +303,7 @@ func (b *Backend) GetNodeVersionInfo(ctx context.Context) (*access.NodeVersionIn } return &access.NodeVersionInfo{ - Semver: build.Semver(), + Semver: build.Version(), Commit: build.Commit(), SporkId: sporkId, ProtocolVersion: uint64(protocolVersion), diff --git a/engine/access/rpc/backend/backend_network.go b/engine/access/rpc/backend/backend_network.go index d88c36db070..577ebaa8a84 100644 --- a/engine/access/rpc/backend/backend_network.go +++ b/engine/access/rpc/backend/backend_network.go @@ -57,7 +57,7 @@ func (b *backendNetwork) GetNodeVersionInfo(ctx context.Context) (*access.NodeVe } return &access.NodeVersionInfo{ - Semver: build.Semver(), + Semver: build.Version(), Commit: build.Commit(), SporkId: sporkId, ProtocolVersion: uint64(protocolVersion), diff --git a/engine/testutil/nodes.go b/engine/testutil/nodes.go index fc5df47f25d..8639e1ee1f7 100644 --- a/engine/testutil/nodes.go +++ b/engine/testutil/nodes.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/coreos/go-semver/semver" "github.com/ipfs/go-datastore" dssync "github.com/ipfs/go-datastore/sync" blockstore "github.com/ipfs/go-ipfs-blockstore" @@ -687,8 +688,9 @@ func ExecutionNode(t *testing.T, hub *stub.Hub, identity *flow.Identity, identit // disabled by default uploader := uploader.NewManager(node.Tracer) - ver, err := build.SemverV2() - require.NoError(t, err, "failed to parse semver version from build info") + _, err = build.Semver() + require.ErrorIs(t, err, build.UndefinedVersionError) + ver := semver.New("0.0.0") latestFinalizedBlock, err := node.State.Final().Head() require.NoError(t, err) diff --git a/integration/localnet/builder/bootstrap.go b/integration/localnet/builder/bootstrap.go index dc54dba8cdb..2a42963461f 100644 --- a/integration/localnet/builder/bootstrap.go +++ b/integration/localnet/builder/bootstrap.go @@ -518,7 +518,7 @@ func defaultService(name, role, dataDir, profilerDir string, i int) Service { Dockerfile: "cmd/Dockerfile", Args: map[string]string{ "TARGET": fmt.Sprintf("./cmd/%s", role), - "VERSION": build.Semver(), + "VERSION": build.Version(), "COMMIT": build.Commit(), "GOARCH": runtime.GOARCH, },