From 25ce529bd67198dfefbd4642c96ff4126a400a03 Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Tue, 26 Oct 2021 16:44:10 +0200 Subject: [PATCH 01/11] pkg/client: add Config struct This struct can be used to pass parameters to create the FleetLock client. Signed-off-by: Mathieu Tortuyaux --- pkg/client/config.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 pkg/client/config.go diff --git a/pkg/client/config.go b/pkg/client/config.go new file mode 100644 index 0000000..6fff6ec --- /dev/null +++ b/pkg/client/config.go @@ -0,0 +1,16 @@ +package client + +// Config is the dedicated fleetlock client config. +type Config struct { + // Group of the instance. Defaults to "default" + Group string + // ID of the instance, must be unique and should persist across reboot. + // Required. + ID string + // HTTP client to use - can be used to implement authentication logic. + // Defaults to `http.DefaultClient` + HTTP HTTPClient + // URL of the FleetLock server implementation. + // Required. + URL string +} From 6b9138f96fcbbe993172fbd72725f76f5e435d41 Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Tue, 2 Nov 2021 17:41:27 +0100 Subject: [PATCH 02/11] client: use config to create Client `client.Config` can be passed to FleetLock client when we create it. Signed-off-by: Mathieu Tortuyaux --- cmd/lock.go | 7 ++++++- cmd/unlock.go | 7 ++++++- pkg/client/client.go | 30 ++++++++++++++++-------------- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/cmd/lock.go b/cmd/lock.go index 5b920b8..cd7a737 100644 --- a/cmd/lock.go +++ b/cmd/lock.go @@ -17,7 +17,12 @@ func lock(group, id, url *string) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { httpClient := http.DefaultClient - c, err := client.New(*url, *group, *id, httpClient) + c, err := client.New(&client.Config{ + URL: *url, + Group: *group, + ID: *id, + HTTP: httpClient, + }) if err != nil { return fmt.Errorf("building the client: %w", err) } diff --git a/cmd/unlock.go b/cmd/unlock.go index 7e82ef9..ef29e56 100644 --- a/cmd/unlock.go +++ b/cmd/unlock.go @@ -17,7 +17,12 @@ func unlock(group, id, url *string) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { httpClient := http.DefaultClient - c, err := client.New(*url, *group, *id, httpClient) + c, err := client.New(&client.Config{ + URL: *url, + Group: *group, + ID: *id, + HTTP: httpClient, + }) if err != nil { return fmt.Errorf("building the client: %w", err) } diff --git a/pkg/client/client.go b/pkg/client/client.go index 3b35b6e..1af3bb0 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -47,20 +47,6 @@ type Client struct { http HTTPClient } -// New builds a FleetLock client. -func New(baseServerURL, group, id string, c HTTPClient) (*Client, error) { - if _, err := url.ParseRequestURI(baseServerURL); err != nil { - return nil, fmt.Errorf("parsing URL: %w", err) - } - - return &Client{ - baseServerURL: baseServerURL, - http: c, - group: group, - id: id, - }, nil -} - // RecursiveLock tries to reserve (lock) a slot for rebooting. func (c *Client) RecursiveLock(ctx context.Context) error { req, err := c.generateRequest(ctx, "v1/pre-reboot") @@ -148,3 +134,19 @@ func handleResponse(resp *http.Response) error { return fmt.Errorf("unexpected status code: %d", resp.StatusCode) } } + +// New builds a FleetLock client. +func New(cfg *Config) (*Client, error) { + fleetlock := &Client{ + baseServerURL: cfg.URL, + http: cfg.HTTP, + group: cfg.Group, + id: cfg.ID, + } + + if _, err := url.ParseRequestURI(fleetlock.baseServerURL); err != nil { + return nil, fmt.Errorf("parsing URL: %w", err) + } + + return fleetlock, nil +} From a65477bff9c262dfcb003d2d02c206e57d01fb89 Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Wed, 27 Oct 2021 15:15:07 +0200 Subject: [PATCH 03/11] pkg/client: add default for group and client this values are now optionals. Default values can be overrided with config. ID and URL are mandatory. Signed-off-by: Mathieu Tortuyaux --- pkg/client/client.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/client/client.go b/pkg/client/client.go index 1af3bb0..523ff88 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -144,9 +144,25 @@ func New(cfg *Config) (*Client, error) { id: cfg.ID, } + if fleetlock.id == "" { + return nil, fmt.Errorf("ID is required") + } + + if fleetlock.baseServerURL == "" { + return nil, fmt.Errorf("URL is required") + } + if _, err := url.ParseRequestURI(fleetlock.baseServerURL); err != nil { return nil, fmt.Errorf("parsing URL: %w", err) } + if fleetlock.group == "" { + fleetlock.group = "default" + } + + if fleetlock.http == nil { + fleetlock.http = http.DefaultClient + } + return fleetlock, nil } From 725a17c9855ebaa70f4b702449f99e2ec072bab9 Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Tue, 2 Nov 2021 17:04:33 +0100 Subject: [PATCH 04/11] pkg/client/test: update test to use config we test mandatory config parameters too. Signed-off-by: Mathieu Tortuyaux --- pkg/client/client_test.go | 117 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 111 insertions(+), 6 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index ede89cc..9335222 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -3,6 +3,7 @@ package client_test import ( "bytes" "context" + "encoding/json" "errors" "fmt" "io/ioutil" @@ -26,7 +27,7 @@ func (m *httpClient) Do(req *http.Request) (*http.Response, error) { func TestBadURL(t *testing.T) { t.Parallel() - _, err := client.New("this is not an URL", "default", "1234", nil) + _, err := client.New(&client.Config{URL: "this is not an URL", ID: "1234"}) if err == nil { t.Fatalf("should get an error") } @@ -48,32 +49,70 @@ func TestClient(t *testing.T) { expErr error body []byte doErr error + cfg *client.Config + expCfg *client.Config }{ { statusCode: 200, + cfg: &client.Config{ + ID: "1234", + URL: "http://1.2.3.4", + }, + expCfg: &client.Config{ + Group: "default", + }, }, { statusCode: 500, expErr: errors.New("fleetlock error: this is an error (error_kind)"), body: []byte(`{"kind":"error_kind","value":"this is an error"}`), + cfg: &client.Config{ + ID: "1234", + URL: "http://1.2.3.4", + }, + expCfg: &client.Config{ + Group: "default", + }, }, { statusCode: 500, expErr: errors.New("unmarshalling error: invalid character '\"' after object key:value pair"), body: []byte(`{"kind":"error_kind" "value":"this is an error"}`), + cfg: &client.Config{ + ID: "1234", + URL: "http://1.2.3.4", + Group: "lokomotive", + }, + expCfg: &client.Config{ + Group: "lokomotive", + }, }, { statusCode: 100, expErr: errors.New("unexpected status code: 100"), + cfg: &client.Config{ + ID: "1234", + URL: "http://1.2.3.4", + }, + expCfg: &client.Config{ + Group: "default", + }, }, { expErr: errors.New("doing the request: connection refused"), doErr: errors.New("connection refused"), + cfg: &client.Config{ + ID: "1234", + URL: "http://1.2.3.4", + }, + expCfg: &client.Config{ + Group: "default", + }, }, } { test := test - newClient := func(statusCode int, body []byte, doErr error) (*httpClient, *client.Client) { + newClient := func(cfg *client.Config, statusCode int, body []byte, doErr error) (*httpClient, *client.Client) { h := &httpClient{ do: func(req *http.Request) (*http.Response, error) { return &http.Response{ @@ -83,7 +122,9 @@ func TestClient(t *testing.T) { }, } - c, err := client.New("http://1.2.3.4", "default", "1234", h) + cfg.HTTP = h + + c, err := client.New(cfg) if err != nil { t.Fatalf("Unexpected error creating client: %v", err) } @@ -91,10 +132,29 @@ func TestClient(t *testing.T) { return h, c } + getPayload := func(h *httpClient) *client.Payload { + b, err := h.r.GetBody() + if err != nil { + t.Fatalf("unable to get body from request: %v", err) + } + + payload, err := ioutil.ReadAll(b) + if err != nil { + t.Fatalf("unable to read body: %v", err) + } + + var p client.Payload + if err := json.Unmarshal(payload, &p); err != nil { + t.Fatalf("unable to unmarshal payload: %v", err) + } + + return &p + } + t.Run(fmt.Sprintf("UnlockIfHeld_%d", test.statusCode), func(t *testing.T) { t.Parallel() - h, c := newClient(test.statusCode, test.body, test.doErr) + h, c := newClient(test.cfg, test.statusCode, test.body, test.doErr) err := c.UnlockIfHeld(ctx) if err != nil && err.Error() != test.expErr.Error() { @@ -106,12 +166,18 @@ func TestClient(t *testing.T) { if h.r.URL.String() != expURL { t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) } + + payload := getPayload(h) + + if payload.ClientParams.Group != test.expCfg.Group { + t.Fatalf("payload's group should be : %s, got: %s", test.expCfg.Group, payload.ClientParams.Group) + } }) t.Run(fmt.Sprintf("RecursiveLock_%d", test.statusCode), func(t *testing.T) { t.Parallel() - h, c := newClient(test.statusCode, test.body, test.doErr) + h, c := newClient(test.cfg, test.statusCode, test.body, test.doErr) err := c.RecursiveLock(ctx) if err != nil && err.Error() != test.expErr.Error() { @@ -123,6 +189,12 @@ func TestClient(t *testing.T) { if h.r.URL.String() != expURL { t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) } + + payload := getPayload(h) + + if payload.ClientParams.Group != test.expCfg.Group { + t.Fatalf("payload's group should be : %s, got: %s", test.expCfg.Group, payload.ClientParams.Group) + } }) } } @@ -145,7 +217,11 @@ func Test_Client_use_given_context_for_requests(t *testing.T) { }, } - c, err := client.New("http://1.2.3.4", "default", "1234", h) + c, err := client.New(&client.Config{ + URL: "http://1.2.3.4", + ID: "1234", + HTTP: h, + }) if err != nil { t.Fatalf("Unexpected error creating client: %v", err) } @@ -160,3 +236,32 @@ func Test_Client_use_given_context_for_requests(t *testing.T) { t.Fatalf("Unexpected error while unlocking: %v", err) } } + +func TestRequiredParameters(t *testing.T) { + for _, test := range []struct { + cfg *client.Config + err error + }{ + { + cfg: &client.Config{ + URL: "http://1.2.3.4", + }, + err: errors.New("ID is required"), + }, + { + cfg: &client.Config{ + ID: "1234", + }, + err: errors.New("URL is required"), + }, + } { + _, err := client.New(test.cfg) + if err == nil { + t.Fatal("error should not be nil") + } + + if err.Error() != test.err.Error() { + t.Fatalf("error should be: %v, got: %v", test.err, err) + } + } +} From 131fbe074d6d2ce48465d613c4e6b342f5ebe1be Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Wed, 27 Oct 2021 15:22:42 +0200 Subject: [PATCH 05/11] cmd: get default ID from /etc/machine-id Signed-off-by: Mathieu Tortuyaux --- cmd/lock.go | 14 +++++++++----- cmd/root.go | 16 ++++++++++++++++ cmd/unlock.go | 14 +++++++++----- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/cmd/lock.go b/cmd/lock.go index cd7a737..f59fd25 100644 --- a/cmd/lock.go +++ b/cmd/lock.go @@ -3,7 +3,6 @@ package cmd import ( "context" "fmt" - "net/http" "github.com/spf13/cobra" @@ -15,13 +14,18 @@ func lock(group, id, url *string) *cobra.Command { Use: "recursive-lock", Short: "Try to reserve (lock) a slot for rebooting", RunE: func(cmd *cobra.Command, args []string) error { - httpClient := http.DefaultClient + if id == nil { + var err error + id, err = machineID() + if err != nil { + return fmt.Errorf("getting machine ID: %w", err) + } + } c, err := client.New(&client.Config{ - URL: *url, - Group: *group, ID: *id, - HTTP: httpClient, + Group: *group, + URL: *url, }) if err != nil { return fmt.Errorf("building the client: %w", err) diff --git a/cmd/root.go b/cmd/root.go index 37b9ec7..3444d8a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -2,6 +2,9 @@ package cmd import ( + "fmt" + "io/ioutil" + "github.com/spf13/cobra" ) @@ -20,3 +23,16 @@ func Command() *cobra.Command { return cli } + +// machineID is a helper to return unique ID +// of the machine. +func machineID() (*string, error) { + id, err := ioutil.ReadFile("/etc/machine-id") + if err != nil { + return nil, fmt.Errorf("reading machine ID from file: %w", err) + } + + sid := string(id) + + return &sid, nil +} diff --git a/cmd/unlock.go b/cmd/unlock.go index ef29e56..1349b11 100644 --- a/cmd/unlock.go +++ b/cmd/unlock.go @@ -3,7 +3,6 @@ package cmd import ( "context" "fmt" - "net/http" "github.com/spf13/cobra" @@ -15,13 +14,18 @@ func unlock(group, id, url *string) *cobra.Command { Use: "unlock-if-held", Short: "Try to release (unlock) a slot that it was previously holding", RunE: func(cmd *cobra.Command, args []string) error { - httpClient := http.DefaultClient + if id == nil { + var err error + id, err = machineID() + if err != nil { + return fmt.Errorf("getting machine ID: %w", err) + } + } c, err := client.New(&client.Config{ - URL: *url, - Group: *group, ID: *id, - HTTP: httpClient, + Group: *group, + URL: *url, }) if err != nil { return fmt.Errorf("building the client: %w", err) From e6cfe16f5e247f452808ce6565c520070c1316f5 Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Wed, 27 Oct 2021 19:24:43 +0200 Subject: [PATCH 06/11] pkg/client: add basic auth round tripper This round tripper can be used to do a correct basic authentication Signed-off-by: Mathieu Tortuyaux --- pkg/client/authentication.go | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 pkg/client/authentication.go diff --git a/pkg/client/authentication.go b/pkg/client/authentication.go new file mode 100644 index 0000000..b9ab229 --- /dev/null +++ b/pkg/client/authentication.go @@ -0,0 +1,38 @@ +package client + +import ( + "context" + "net/http" +) + +type basicAuthRoundTripper struct { + username string + password string + rt http.RoundTripper +} + +// RoundTrip is required to implement RoundTripper interface. +// We check if an authorization token is already set, if not we set it. +// We return the initial RoundTripper to chain it in the whole process. +func (b *basicAuthRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + if len(req.Header.Get("Authorization")) != 0 { + return b.rt.RoundTrip(req) + } + + req = req.Clone(context.TODO()) + req.SetBasicAuth(b.username, b.password) + return b.rt.RoundTrip(req) +} + +// NewBasicAuthRoundTripper returns a basicAuthRoundTripper with username and password. +func NewBasicAuthRoundTripper(username, password string, rt http.RoundTripper) http.RoundTripper { + if rt == nil { + rt = &http.Transport{} + } + + return &basicAuthRoundTripper{ + username: username, + password: password, + rt: rt, + } +} From b0096dacd439b81f0c1ab43e1a8903c42cdfcb52 Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Tue, 2 Nov 2021 17:05:50 +0100 Subject: [PATCH 07/11] pkg/client/test: add basic authentication test Signed-off-by: Mathieu Tortuyaux --- pkg/client/authentication.go | 16 +++++++++-- pkg/client/client_test.go | 54 ++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/pkg/client/authentication.go b/pkg/client/authentication.go index b9ab229..a8c5a58 100644 --- a/pkg/client/authentication.go +++ b/pkg/client/authentication.go @@ -2,6 +2,7 @@ package client import ( "context" + "fmt" "net/http" ) @@ -16,12 +17,23 @@ type basicAuthRoundTripper struct { // We return the initial RoundTripper to chain it in the whole process. func (b *basicAuthRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { if len(req.Header.Get("Authorization")) != 0 { - return b.rt.RoundTrip(req) + resp, err := b.rt.RoundTrip(req) + if err != nil { + return nil, fmt.Errorf("inner round trip error: %w", err) + } + + return resp, nil } req = req.Clone(context.TODO()) req.SetBasicAuth(b.username, b.password) - return b.rt.RoundTrip(req) + + resp, err := b.rt.RoundTrip(req) + if err != nil { + return nil, fmt.Errorf("inner round trip error: %w", err) + } + + return resp, nil } // NewBasicAuthRoundTripper returns a basicAuthRoundTripper with username and password. diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 9335222..4a2c12a 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -24,6 +24,12 @@ func (m *httpClient) Do(req *http.Request) (*http.Response, error) { return m.do(req) } +func (m *httpClient) RoundTrip(req *http.Request) (*http.Response, error) { + m.r = req + + return m.do(req) +} + func TestBadURL(t *testing.T) { t.Parallel() @@ -167,6 +173,10 @@ func TestClient(t *testing.T) { t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) } + if _, _, ok := h.r.BasicAuth(); ok { + t.Fatalf("basic auth should not be set") + } + payload := getPayload(h) if payload.ClientParams.Group != test.expCfg.Group { @@ -190,6 +200,10 @@ func TestClient(t *testing.T) { t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) } + if _, _, ok := h.r.BasicAuth(); ok { + t.Fatalf("basic auth should not be set") + } + payload := getPayload(h) if payload.ClientParams.Group != test.expCfg.Group { @@ -237,7 +251,47 @@ func Test_Client_use_given_context_for_requests(t *testing.T) { } } +func TestBasicAuth(t *testing.T) { + t.Parallel() + + var ( + username = "flatcar" + password = "p4ssw0rd" + ) + + ctx := context.Background() + + tr := &httpClient{ + do: func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: 200, + }, nil + }, + } + + h := http.Client{ + Transport: client.NewBasicAuthRoundTripper(username, password, tr), + } + + c, err := client.New(&client.Config{ID: "1234", HTTP: &h, URL: "http://1.2.3.4"}) + if err != nil { + t.Fatalf("Unexpected error creating client: %v", err) + } + + err = c.RecursiveLock(ctx) + if err != nil { + t.Fatalf("should have nil for err, got: %v", err) + } + + u, p, ok := tr.r.BasicAuth() + if u != username || p != password || !ok { + t.Fatalf("basic auth creds do not match") + } +} + func TestRequiredParameters(t *testing.T) { + t.Parallel() + for _, test := range []struct { cfg *client.Config err error From 0a91d9fb703fbdf8c411fff35a200e1376709898 Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Mon, 15 Nov 2021 15:43:12 +0100 Subject: [PATCH 08/11] fixup! pkg/client/test: add basic authentication test --- pkg/client/client_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 4a2c12a..308ab01 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -173,10 +173,6 @@ func TestClient(t *testing.T) { t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) } - if _, _, ok := h.r.BasicAuth(); ok { - t.Fatalf("basic auth should not be set") - } - payload := getPayload(h) if payload.ClientParams.Group != test.expCfg.Group { @@ -200,10 +196,6 @@ func TestClient(t *testing.T) { t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) } - if _, _, ok := h.r.BasicAuth(); ok { - t.Fatalf("basic auth should not be set") - } - payload := getPayload(h) if payload.ClientParams.Group != test.expCfg.Group { From 041a9fb16c3707107ccd1b042376ff2790fb7a88 Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Mon, 15 Nov 2021 15:43:52 +0100 Subject: [PATCH 09/11] pkg/client/test: test fleetlock header Signed-off-by: Mathieu Tortuyaux --- pkg/client/client_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 308ab01..259d631 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -8,11 +8,16 @@ import ( "fmt" "io/ioutil" "net/http" + "reflect" "testing" "github.com/flatcar-linux/fleetlock/pkg/client" ) +var fleetlockHeaders = http.Header{ + "Fleet-Lock-Protocol": []string{"true"}, +} + type httpClient struct { do func(req *http.Request) (*http.Response, error) r *http.Request @@ -173,6 +178,10 @@ func TestClient(t *testing.T) { t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) } + if !reflect.DeepEqual(h.r.Header, fleetlockHeaders) { + t.Fatalf("should have %v for headers, got: %s", fleetlockHeaders, h.r.Header) + } + payload := getPayload(h) if payload.ClientParams.Group != test.expCfg.Group { @@ -196,6 +205,10 @@ func TestClient(t *testing.T) { t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) } + if !reflect.DeepEqual(h.r.Header, fleetlockHeaders) { + t.Fatalf("should have %v for headers, got: %s", fleetlockHeaders, h.r.Header) + } + payload := getPayload(h) if payload.ClientParams.Group != test.expCfg.Group { From e10d5fcc6eabd5e6126c2a89b7a925bf192f0260 Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Mon, 15 Nov 2021 15:46:25 +0100 Subject: [PATCH 10/11] fixup! client: use config to create Client --- pkg/client/client.go | 64 ++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 523ff88..cfd7833 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -47,6 +47,38 @@ type Client struct { http HTTPClient } +// New builds a FleetLock client. +func New(cfg *Config) (*Client, error) { + fleetlock := &Client{ + baseServerURL: cfg.URL, + http: cfg.HTTP, + group: cfg.Group, + id: cfg.ID, + } + + if fleetlock.id == "" { + return nil, fmt.Errorf("ID is required") + } + + if fleetlock.baseServerURL == "" { + return nil, fmt.Errorf("URL is required") + } + + if _, err := url.ParseRequestURI(fleetlock.baseServerURL); err != nil { + return nil, fmt.Errorf("parsing URL: %w", err) + } + + if fleetlock.group == "" { + fleetlock.group = "default" + } + + if fleetlock.http == nil { + fleetlock.http = http.DefaultClient + } + + return fleetlock, nil +} + // RecursiveLock tries to reserve (lock) a slot for rebooting. func (c *Client) RecursiveLock(ctx context.Context) error { req, err := c.generateRequest(ctx, "v1/pre-reboot") @@ -134,35 +166,3 @@ func handleResponse(resp *http.Response) error { return fmt.Errorf("unexpected status code: %d", resp.StatusCode) } } - -// New builds a FleetLock client. -func New(cfg *Config) (*Client, error) { - fleetlock := &Client{ - baseServerURL: cfg.URL, - http: cfg.HTTP, - group: cfg.Group, - id: cfg.ID, - } - - if fleetlock.id == "" { - return nil, fmt.Errorf("ID is required") - } - - if fleetlock.baseServerURL == "" { - return nil, fmt.Errorf("URL is required") - } - - if _, err := url.ParseRequestURI(fleetlock.baseServerURL); err != nil { - return nil, fmt.Errorf("parsing URL: %w", err) - } - - if fleetlock.group == "" { - fleetlock.group = "default" - } - - if fleetlock.http == nil { - fleetlock.http = http.DefaultClient - } - - return fleetlock, nil -} From 03555bc0126cc8d4dd3c4bb3c33ade81549b5870 Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Mon, 15 Nov 2021 18:27:16 +0100 Subject: [PATCH 11/11] fixup! cmd: get default ID from /etc/machine-id --- cmd/lock.go | 8 ++------ cmd/root.go | 24 ++++++++++++++++++++---- cmd/unlock.go | 8 ++------ 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/cmd/lock.go b/cmd/lock.go index f59fd25..b2164f2 100644 --- a/cmd/lock.go +++ b/cmd/lock.go @@ -14,12 +14,8 @@ func lock(group, id, url *string) *cobra.Command { Use: "recursive-lock", Short: "Try to reserve (lock) a slot for rebooting", RunE: func(cmd *cobra.Command, args []string) error { - if id == nil { - var err error - id, err = machineID() - if err != nil { - return fmt.Errorf("getting machine ID: %w", err) - } + if err := checkID(id); err != nil { + return fmt.Errorf("checking ID: %w", err) } c, err := client.New(&client.Config{ diff --git a/cmd/root.go b/cmd/root.go index 3444d8a..56c4cb2 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -26,13 +26,29 @@ func Command() *cobra.Command { // machineID is a helper to return unique ID // of the machine. -func machineID() (*string, error) { +func machineID() (string, error) { id, err := ioutil.ReadFile("/etc/machine-id") if err != nil { - return nil, fmt.Errorf("reading machine ID from file: %w", err) + return "", fmt.Errorf("reading machine ID from file: %w", err) } - sid := string(id) + return string(id), nil +} + +// checkID asserts that the ID is not nil, if it's the case +// it uses `machineID` to generate a default one. +func checkID(id *string) error { + // the ID is set and it's not empty. + if id != nil && *id != "" { + return nil + } + + i, err := machineID() + if err != nil { + return fmt.Errorf("getting default machine ID: %w", err) + } + + *id = i - return &sid, nil + return nil } diff --git a/cmd/unlock.go b/cmd/unlock.go index 1349b11..d13cd15 100644 --- a/cmd/unlock.go +++ b/cmd/unlock.go @@ -14,12 +14,8 @@ func unlock(group, id, url *string) *cobra.Command { Use: "unlock-if-held", Short: "Try to release (unlock) a slot that it was previously holding", RunE: func(cmd *cobra.Command, args []string) error { - if id == nil { - var err error - id, err = machineID() - if err != nil { - return fmt.Errorf("getting machine ID: %w", err) - } + if err := checkID(id); err != nil { + return fmt.Errorf("checking ID: %w", err) } c, err := client.New(&client.Config{