From e0f08f6ae00355a6fa5471a1f23c940712da449b Mon Sep 17 00:00:00 2001 From: Allen Zhong Date: Tue, 28 Dec 2021 15:58:37 +0800 Subject: [PATCH 1/7] cluster/spec: support secureboot flags for tidb >= v5.3.0 --- components/bench/Makefile | 5 ++--- components/client/Makefile | 5 ++--- embed/templates/scripts/run_tidb.sh.tpl | 3 +++ examples | 2 +- pkg/cluster/spec/tidb.go | 9 ++++++++- pkg/cluster/template/scripts/tidb.go | 25 ++++++++++++++++--------- 6 files changed, 32 insertions(+), 17 deletions(-) diff --git a/components/bench/Makefile b/components/bench/Makefile index 632ba9be71..2cfe3b9b4a 100644 --- a/components/bench/Makefile +++ b/components/bench/Makefile @@ -77,8 +77,7 @@ tidy: clean: @# Target: run the build cleanup steps @rm -rf bin - @rm -rf cover - @rm -rf tests/*/{bin/*.test,logs,cover/*.out} + @rm -rf tests/*/{bin/*.test,logs} test: failpoint-enable run-tests failpoint-disable @# Target: run tests with failpoint enabled @@ -91,7 +90,7 @@ run-tests: unit-test # Run tests unit-test: @# Target: run the code coverage test phase - mkdir -p cover + mkdir -p ../../cover TIUP_HOME=$(shell pwd)/../../tests/tiup $(GOTEST) ./... -covermode=count -coverprofile ../../cover/cov.unit-test.out race: failpoint-enable diff --git a/components/client/Makefile b/components/client/Makefile index e5ccf77803..19d8b38dab 100644 --- a/components/client/Makefile +++ b/components/client/Makefile @@ -77,8 +77,7 @@ tidy: clean: @# Target: run the build cleanup steps @rm -rf bin - @rm -rf cover - @rm -rf tests/*/{bin/*.test,logs,cover/*.out} + @rm -rf tests/*/{bin/*.test,logs} test: failpoint-enable run-tests failpoint-disable @# Target: run tests with failpoint enabled @@ -91,7 +90,7 @@ run-tests: unit-test # Run tests unit-test: @# Target: run the code coverage test phase - mkdir -p cover + mkdir -p ../../cover TIUP_HOME=$(shell pwd)/../../tests/tiup $(GOTEST) ./... -covermode=count -coverprofile ../../cover/cov.unit-test.out race: failpoint-enable diff --git a/embed/templates/scripts/run_tidb.sh.tpl b/embed/templates/scripts/run_tidb.sh.tpl index 7015db78c9..6c30088e9f 100644 --- a/embed/templates/scripts/run_tidb.sh.tpl +++ b/embed/templates/scripts/run_tidb.sh.tpl @@ -27,6 +27,9 @@ exec env GODEBUG=madvdontneed=1 bin/tidb-server \ --host="{{.ListenHost}}" \ --advertise-address="{{.AdvertiseAddr}}" \ --store="tikv" \ +{{- if .SupportSecboot}} + --initialize-insecure \ +{{- end}} --path="{{template "PDList" .Endpoints}}" \ --log-slow-query="{{.LogDir}}/tidb_slow_query.log" \ --config=conf/tidb.toml \ diff --git a/examples b/examples index c87059b096..ed2acf36fe 120000 --- a/examples +++ b/examples @@ -1 +1 @@ -embed/templates/examples \ No newline at end of file +embed/examples \ No newline at end of file diff --git a/pkg/cluster/spec/tidb.go b/pkg/cluster/spec/tidb.go index 399777832e..23ee5d07a2 100644 --- a/pkg/cluster/spec/tidb.go +++ b/pkg/cluster/spec/tidb.go @@ -24,6 +24,7 @@ import ( "github.com/pingcap/tiup/pkg/cluster/ctxt" "github.com/pingcap/tiup/pkg/cluster/template/scripts" "github.com/pingcap/tiup/pkg/meta" + "golang.org/x/mod/semver" ) // TiDBSpec represents the TiDB topology specification in topology.yaml @@ -137,6 +138,11 @@ func (i *TiDBInstance) InitConfig( enableTLS := topo.GlobalOptions.TLSEnabled spec := i.InstanceSpec.(*TiDBSpec) + // check if the tidb version has --initialize-insecure support + hasSecbootSupport := false + if vc := semver.Compare(clusterVersion, "5.3.0"); vc >= 0 { + hasSecbootSupport = true + } cfg := scripts. NewTiDBScript(i.GetHost(), paths.Deploy, paths.Log). WithPort(spec.Port). @@ -144,7 +150,8 @@ func (i *TiDBInstance) InitConfig( WithStatusPort(spec.StatusPort). AppendEndpoints(topo.Endpoints(deployUser)...). WithListenHost(i.GetListenHost()). - WithAdvertiseAddr(spec.Host) + WithAdvertiseAddr(spec.Host). + SupportSecureBootstrap(hasSecbootSupport) fp := filepath.Join(paths.Cache, fmt.Sprintf("run_tidb_%s_%d.sh", i.GetHost(), i.GetPort())) if err := cfg.ConfigToFile(fp); err != nil { return err diff --git a/pkg/cluster/template/scripts/tidb.go b/pkg/cluster/template/scripts/tidb.go index 6501b14d57..04f8c912a3 100644 --- a/pkg/cluster/template/scripts/tidb.go +++ b/pkg/cluster/template/scripts/tidb.go @@ -24,15 +24,16 @@ import ( // TiDBScript represent the data to generate TiDB config type TiDBScript struct { - IP string - ListenHost string - AdvertiseAddr string - Port int - StatusPort int - DeployDir string - LogDir string - NumaNode string - Endpoints []*PDScript + IP string + ListenHost string + AdvertiseAddr string + Port int + StatusPort int + DeployDir string + LogDir string + NumaNode string + SupportSecboot bool + Endpoints []*PDScript } // NewTiDBScript returns a TiDBScript with given arguments @@ -79,6 +80,12 @@ func (c *TiDBScript) WithNumaNode(numa string) *TiDBScript { return c } +// SupportSecureBootstrap set SupportSecboot field of TiDBScript +func (c *TiDBScript) SupportSecureBootstrap(s bool) *TiDBScript { + c.SupportSecboot = s + return c +} + // AppendEndpoints add new PDScript to Endpoints field func (c *TiDBScript) AppendEndpoints(ends ...*PDScript) *TiDBScript { c.Endpoints = append(c.Endpoints, ends...) From a76ae16fedec4c32457e06940495ef42df450d62 Mon Sep 17 00:00:00 2001 From: Allen Zhong Date: Tue, 28 Dec 2021 17:07:33 +0800 Subject: [PATCH 2/7] *: add random password generator --- go.mod | 1 + go.sum | 2 ++ pkg/crypto/rand/passwd.go | 53 ++++++++++++++++++++++++++++++++++ pkg/crypto/rand/passwd_test.go | 36 +++++++++++++++++++++++ 4 files changed, 92 insertions(+) create mode 100644 pkg/crypto/rand/passwd.go create mode 100644 pkg/crypto/rand/passwd_test.go diff --git a/go.mod b/go.mod index ed9af7d91d..4ef19c83b7 100644 --- a/go.mod +++ b/go.mod @@ -49,6 +49,7 @@ require ( github.com/r3labs/diff/v2 v2.14.0 github.com/relex/aini v1.5.0 github.com/sergi/go-diff v1.2.0 + github.com/sethvargo/go-password v0.2.0 github.com/shirou/gopsutil v3.21.8+incompatible github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 github.com/spf13/cobra v1.2.1 diff --git a/go.sum b/go.sum index 57e4646c6f..11635b8c8e 100644 --- a/go.sum +++ b/go.sum @@ -483,6 +483,8 @@ github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAm github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ= github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= +github.com/sethvargo/go-password v0.2.0 h1:BTDl4CC/gjf/axHMaDQtw507ogrXLci6XRiLc7i/UHI= +github.com/sethvargo/go-password v0.2.0/go.mod h1:Ym4Mr9JXLBycr02MFuVQ/0JHidNetSgbzutTr3zsYXE= github.com/shirou/gopsutil v3.21.8+incompatible h1:sh0foI8tMRlCidUJR+KzqWYWxrkuuPIGiO6Vp+KXdCU= github.com/shirou/gopsutil v3.21.8+incompatible/go.mod h1:5b4v6he4MtMOwMlS0TUMTu2PcXUg8+E1lC7eC3UO/RA= github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo= diff --git a/pkg/crypto/rand/passwd.go b/pkg/crypto/rand/passwd.go new file mode 100644 index 0000000000..028b7e8f3d --- /dev/null +++ b/pkg/crypto/rand/passwd.go @@ -0,0 +1,53 @@ +// Copyright 2021 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package rand + +import ( + "github.com/sethvargo/go-password/password" +) + +// charsets with some in similar shapes removed (e.g., O, o, I, l, etc.) +const ( + lowerLetters = "abcdefghijkmnpqrstuvwxyz" + upperLetters = "ABCDEFGHJKLMNPQRSTUVWXYZ" + digits = "0123456789" + symbols = "!@#$%^&*+-=_" +) + +// Password generates a random password +func Password(length int) (string, error) { + if length < 8 { + panic("password length muster be at least 8.") + } + + gi := &password.GeneratorInput{ + LowerLetters: lowerLetters, + UpperLetters: upperLetters, + Digits: digits, + Symbols: symbols, + Reader: Reader, + } + g, err := password.NewGenerator(gi) + if err != nil { + return "", err + } + + // 1/3 of the password are digits and 1/4 of it are symbols + numDigits := length / 3 + numSymbols := length / 4 + // allow repeat if the length is longer than the shortest charset + allowRepeat := (length >= 10) + + return g.Generate(length, numDigits, numSymbols, false, allowRepeat) +} diff --git a/pkg/crypto/rand/passwd_test.go b/pkg/crypto/rand/passwd_test.go new file mode 100644 index 0000000000..f097422163 --- /dev/null +++ b/pkg/crypto/rand/passwd_test.go @@ -0,0 +1,36 @@ +// Copyright 2021 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package rand + +import ( + "testing" +) + +func TestPasswd(t *testing.T) { + for i := 0; i < 100; i++ { + l := Intn(64) + if l < 8 { // make suer it's greater than 8 + l += 8 + } + t.Logf("generating random password of length %d", l) + p, e := Password(l) + if e != nil { + t.Error(e) + } + t.Log(p) + if len(p) != l { + t.Fail() + } + } +} From 234fdf354fdf323de5892f70dd57699d8fcfe01c Mon Sep 17 00:00:00 2001 From: Allen Zhong Date: Tue, 28 Dec 2021 17:19:04 +0800 Subject: [PATCH 3/7] update makefile of components --- components/bench/Makefile | 2 +- components/client/Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/bench/Makefile b/components/bench/Makefile index 2cfe3b9b4a..e534f126f1 100644 --- a/components/bench/Makefile +++ b/components/bench/Makefile @@ -91,7 +91,7 @@ run-tests: unit-test unit-test: @# Target: run the code coverage test phase mkdir -p ../../cover - TIUP_HOME=$(shell pwd)/../../tests/tiup $(GOTEST) ./... -covermode=count -coverprofile ../../cover/cov.unit-test.out + TIUP_HOME=$(shell pwd)/../../tests/tiup $(GOTEST) ./... -covermode=count -coverprofile ../../cover/cov.unit-test.bench.out race: failpoint-enable @# Target: run race check with failpoint enabled diff --git a/components/client/Makefile b/components/client/Makefile index 19d8b38dab..28e5e97339 100644 --- a/components/client/Makefile +++ b/components/client/Makefile @@ -91,7 +91,7 @@ run-tests: unit-test unit-test: @# Target: run the code coverage test phase mkdir -p ../../cover - TIUP_HOME=$(shell pwd)/../../tests/tiup $(GOTEST) ./... -covermode=count -coverprofile ../../cover/cov.unit-test.out + TIUP_HOME=$(shell pwd)/../../tests/tiup $(GOTEST) ./... -covermode=count -coverprofile ../../cover/cov.unit-test.client.out race: failpoint-enable @# Target: run race check with failpoint enabled From 77e5968a7917dc77d738ea778e3a071ecf0e6278 Mon Sep 17 00:00:00 2001 From: Allen Zhong Date: Tue, 28 Dec 2021 19:30:05 +0800 Subject: [PATCH 4/7] implement tiup-cluster start --init-passwd --- components/cluster/command/start.go | 79 ++++++++++++++++++++++++++++- components/cluster/command/test.go | 8 --- pkg/crypto/rand/passwd_test.go | 2 +- 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/components/cluster/command/start.go b/components/cluster/command/start.go index a407c93e15..9a41138d77 100644 --- a/components/cluster/command/start.go +++ b/components/cluster/command/start.go @@ -14,12 +14,23 @@ package command import ( + "database/sql" + "fmt" + + "github.com/fatih/color" "github.com/pingcap/tiup/pkg/cluster/spec" "github.com/pingcap/tiup/pkg/cluster/task" + "github.com/pingcap/tiup/pkg/crypto/rand" + "github.com/pingcap/tiup/pkg/proxy" "github.com/spf13/cobra" + + // for sql/driver + _ "github.com/go-sql-driver/mysql" ) func newStartCmd() *cobra.Command { + var initPasswd bool + cmd := &cobra.Command{ Use: "start ", Short: "Start a TiDB cluster", @@ -36,19 +47,83 @@ func newStartCmd() *cobra.Command { clusterReport.ID = scrubClusterName(clusterName) teleCommand = append(teleCommand, scrubClusterName(clusterName)) - return cm.StartCluster(clusterName, gOpt, func(b *task.Builder, metadata spec.Metadata) { + if err := cm.StartCluster(clusterName, gOpt, func(b *task.Builder, metadata spec.Metadata) { b.UpdateTopology( clusterName, tidbSpec.Path(clusterName), metadata.(*spec.ClusterMeta), nil, /* deleteNodeIds */ ) - }) + }); err != nil { + return err + } + + // init password + if initPasswd { + pwd, err := initPassword(clusterName) + if err != nil { + log.Errorf("failed to set root password of TiDB database to '%s'", pwd) + return err + } + log.Warnf("The root password of TiDB database has been changed to '%s'.", color.HiYellowString(pwd)) + log.Warnf("Copy and record it to somewhere safe, %s, and will not be stored.", color.HiRedString("it is only displayed once")) + log.Warnf("The generated password %s.", color.HiRedString("could NOT be get and shown again")) + } + return nil }, } + cmd.Flags().BoolVar(&initPasswd, "init-passwd", false, "Initialize a secure root password for the database") cmd.Flags().StringSliceVarP(&gOpt.Roles, "role", "R", nil, "Only start specified roles") cmd.Flags().StringSliceVarP(&gOpt.Nodes, "node", "N", nil, "Only start specified nodes") return cmd } + +func initPassword(clusterName string) (string, error) { + metadata, err := spec.ClusterMetadata(clusterName) + if err != nil { + return "", err + } + tcpProxy := proxy.GetTCPProxy() + + // generate password + pwd, err := rand.Password(16) + if err != nil { + return pwd, err + } + + var lastErr error + for _, spec := range metadata.Topology.TiDBServers { + spec := spec + endpoint := fmt.Sprintf("%s:%d", spec.Host, spec.Port) + if tcpProxy != nil { + closeC := tcpProxy.Run([]string{endpoint}) + defer tcpProxy.Close(closeC) + endpoint = tcpProxy.GetEndpoints()[0] + } + db, err := createDB(endpoint) + if err != nil { + lastErr = err + continue + } + defer db.Close() + + sqlStr := fmt.Sprintf("SET PASSWORD FOR 'root'@'%%' = '%s'; FLUSH PRIVILEGES;", pwd) + _, err = db.Exec(sqlStr) + if err != nil { + lastErr = err + continue + } + return pwd, nil + } + + return pwd, lastErr +} + +func createDB(endpoint string) (db *sql.DB, err error) { + dsn := fmt.Sprintf("root:@tcp(%s)/?charset=utf8mb4,utf8&multiStatements=true", endpoint) + db, err = sql.Open("mysql", dsn) + + return +} diff --git a/components/cluster/command/test.go b/components/cluster/command/test.go index 91a5fdabf1..bfcdfd33e9 100644 --- a/components/cluster/command/test.go +++ b/components/cluster/command/test.go @@ -15,7 +15,6 @@ package command import ( "context" - "database/sql" "errors" "fmt" @@ -72,13 +71,6 @@ func newTestCmd() *cobra.Command { return cmd } -func createDB(endpoint string) (db *sql.DB, err error) { - dsn := fmt.Sprintf("root:@tcp(%s)/?charset=utf8mb4,utf8&multiStatements=true", endpoint) - db, err = sql.Open("mysql", dsn) - - return -} - // To check if test.ti_cluster has data func data(topo *spec.Specification, tcpProxy *proxy.TCPProxy) error { errg, _ := errgroup.WithContext(context.Background()) diff --git a/pkg/crypto/rand/passwd_test.go b/pkg/crypto/rand/passwd_test.go index f097422163..20ff21c143 100644 --- a/pkg/crypto/rand/passwd_test.go +++ b/pkg/crypto/rand/passwd_test.go @@ -20,7 +20,7 @@ import ( func TestPasswd(t *testing.T) { for i := 0; i < 100; i++ { l := Intn(64) - if l < 8 { // make suer it's greater than 8 + if l < 8 { // make sure it's greater than 8 l += 8 } t.Logf("generating random password of length %d", l) From 5ff676172ef93a7dd283fbceeb3e42956e3264c6 Mon Sep 17 00:00:00 2001 From: Allen Zhong Date: Wed, 29 Dec 2021 11:22:04 +0800 Subject: [PATCH 5/7] adjust arg & passwd generator --- components/cluster/command/start.go | 4 ++-- pkg/crypto/rand/passwd.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/components/cluster/command/start.go b/components/cluster/command/start.go index 9a41138d77..0fca8c0499 100644 --- a/components/cluster/command/start.go +++ b/components/cluster/command/start.go @@ -73,7 +73,7 @@ func newStartCmd() *cobra.Command { }, } - cmd.Flags().BoolVar(&initPasswd, "init-passwd", false, "Initialize a secure root password for the database") + cmd.Flags().BoolVar(&initPasswd, "init", false, "Initialize a secure root password for the database") cmd.Flags().StringSliceVarP(&gOpt.Roles, "role", "R", nil, "Only start specified roles") cmd.Flags().StringSliceVarP(&gOpt.Nodes, "node", "N", nil, "Only start specified nodes") @@ -88,7 +88,7 @@ func initPassword(clusterName string) (string, error) { tcpProxy := proxy.GetTCPProxy() // generate password - pwd, err := rand.Password(16) + pwd, err := rand.Password(18) if err != nil { return pwd, err } diff --git a/pkg/crypto/rand/passwd.go b/pkg/crypto/rand/passwd.go index 028b7e8f3d..132cd3d2d4 100644 --- a/pkg/crypto/rand/passwd.go +++ b/pkg/crypto/rand/passwd.go @@ -47,7 +47,7 @@ func Password(length int) (string, error) { numDigits := length / 3 numSymbols := length / 4 // allow repeat if the length is longer than the shortest charset - allowRepeat := (length >= 10) + allowRepeat := (numDigits > len(digits) || numSymbols > len(symbols)) return g.Generate(length, numDigits, numSymbols, false, allowRepeat) } From 679c07de78da30fca9d9f237ba0397fd5019bacb Mon Sep 17 00:00:00 2001 From: Allen Zhong Date: Wed, 29 Dec 2021 12:49:07 +0800 Subject: [PATCH 6/7] fix version check --- pkg/cluster/spec/tidb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cluster/spec/tidb.go b/pkg/cluster/spec/tidb.go index 23ee5d07a2..753bb7a444 100644 --- a/pkg/cluster/spec/tidb.go +++ b/pkg/cluster/spec/tidb.go @@ -140,7 +140,7 @@ func (i *TiDBInstance) InitConfig( spec := i.InstanceSpec.(*TiDBSpec) // check if the tidb version has --initialize-insecure support hasSecbootSupport := false - if vc := semver.Compare(clusterVersion, "5.3.0"); vc >= 0 { + if vc := semver.Compare(clusterVersion, "v5.3.0"); vc >= 0 { hasSecbootSupport = true } cfg := scripts. From 3e9e141779e75deb5b58ea2f2d22aabcfa79f982 Mon Sep 17 00:00:00 2001 From: Allen Zhong Date: Wed, 29 Dec 2021 14:30:38 +0800 Subject: [PATCH 7/7] adjust outputs --- components/cluster/command/start.go | 3 ++- pkg/cluster/spec/tidb.go | 7 +------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/components/cluster/command/start.go b/components/cluster/command/start.go index 0fca8c0499..0297d76ced 100644 --- a/components/cluster/command/start.go +++ b/components/cluster/command/start.go @@ -65,7 +65,8 @@ func newStartCmd() *cobra.Command { log.Errorf("failed to set root password of TiDB database to '%s'", pwd) return err } - log.Warnf("The root password of TiDB database has been changed to '%s'.", color.HiYellowString(pwd)) + log.Warnf("The root password of TiDB database has been changed.") + fmt.Printf("The new password is: '%s'.", color.HiYellowString(pwd)) // use fmt to avoid printing to audit log log.Warnf("Copy and record it to somewhere safe, %s, and will not be stored.", color.HiRedString("it is only displayed once")) log.Warnf("The generated password %s.", color.HiRedString("could NOT be get and shown again")) } diff --git a/pkg/cluster/spec/tidb.go b/pkg/cluster/spec/tidb.go index 753bb7a444..046511b242 100644 --- a/pkg/cluster/spec/tidb.go +++ b/pkg/cluster/spec/tidb.go @@ -138,11 +138,6 @@ func (i *TiDBInstance) InitConfig( enableTLS := topo.GlobalOptions.TLSEnabled spec := i.InstanceSpec.(*TiDBSpec) - // check if the tidb version has --initialize-insecure support - hasSecbootSupport := false - if vc := semver.Compare(clusterVersion, "v5.3.0"); vc >= 0 { - hasSecbootSupport = true - } cfg := scripts. NewTiDBScript(i.GetHost(), paths.Deploy, paths.Log). WithPort(spec.Port). @@ -151,7 +146,7 @@ func (i *TiDBInstance) InitConfig( AppendEndpoints(topo.Endpoints(deployUser)...). WithListenHost(i.GetListenHost()). WithAdvertiseAddr(spec.Host). - SupportSecureBootstrap(hasSecbootSupport) + SupportSecureBootstrap(semver.Compare(clusterVersion, "v5.3.0") >= 0) fp := filepath.Join(paths.Cache, fmt.Sprintf("run_tidb_%s_%d.sh", i.GetHost(), i.GetPort())) if err := cfg.ConfigToFile(fp); err != nil { return err