From 812389f086cdbfaa467f3085de0c228a78566f54 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Thu, 2 Dec 2021 16:49:50 +0100 Subject: [PATCH 1/4] pkg/dbus: initial commit This commit adds initial commit for dbus package containing helpers for creating a D-Bus client. Signed-off-by: Mateusz Gozdek --- pkg/dbus/conn.go | 64 ++++++++++ pkg/dbus/conn_integration_test.go | 30 +++++ pkg/dbus/conn_test.go | 189 ++++++++++++++++++++++++++++++ pkg/dbus/mock.go | 138 ++++++++++++++++++++++ 4 files changed, 421 insertions(+) create mode 100644 pkg/dbus/conn.go create mode 100644 pkg/dbus/conn_integration_test.go create mode 100644 pkg/dbus/conn_test.go create mode 100644 pkg/dbus/mock.go diff --git a/pkg/dbus/conn.go b/pkg/dbus/conn.go new file mode 100644 index 000000000..c5339c280 --- /dev/null +++ b/pkg/dbus/conn.go @@ -0,0 +1,64 @@ +// Package dbus provides a helper function for creating new D-Bus client. +package dbus + +import ( + "fmt" + "os" + "strconv" + + godbus "github.com/godbus/dbus/v5" +) + +// Client is an interface describing capabilities of internal D-Bus client. +type Client interface { + AddMatchSignal(matchOptions ...godbus.MatchOption) error + Signal(ch chan<- *godbus.Signal) + Object(dest string, path godbus.ObjectPath) godbus.BusObject + Close() error +} + +// Connection is an interface describing how much functionality we need from object providing D-Bus connection. +type Connection interface { + Auth(authMethods []godbus.Auth) error + Hello() error + + Client +} + +// Connector is a constructor function providing D-Bus connection. +type Connector func() (Connection, error) + +// SystemPrivateConnector is a standard connector using system bus. +func SystemPrivateConnector() (Connection, error) { + return godbus.SystemBusPrivate() +} + +// New creates new D-Bus client using given connector. +func New(connector Connector) (Client, error) { + if connector == nil { + return nil, fmt.Errorf("no connection creator given") + } + + conn, err := connector() + if err != nil { + return nil, fmt.Errorf("connecting to D-Bus: %w", err) + } + + methods := []godbus.Auth{godbus.AuthExternal(strconv.Itoa(os.Getuid()))} + + if err := conn.Auth(methods); err != nil { + // Best effort closing the connection. + _ = conn.Close() + + return nil, fmt.Errorf("authenticating to D-Bus: %w", err) + } + + if err := conn.Hello(); err != nil { + // Best effort closing the connection. + _ = conn.Close() + + return nil, fmt.Errorf("sending hello to D-Bus: %w", err) + } + + return conn, nil +} diff --git a/pkg/dbus/conn_integration_test.go b/pkg/dbus/conn_integration_test.go new file mode 100644 index 000000000..fcfbe457a --- /dev/null +++ b/pkg/dbus/conn_integration_test.go @@ -0,0 +1,30 @@ +//go:build integration +// +build integration + +package dbus_test + +import ( + "fmt" + "os" + "testing" + + "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/dbus" +) + +const ( + testDbusSocketEnv = "FLUO_TEST_DBUS_SOCKET" +) + +//nolint:paralleltest // This test use environment variables. +func Test_System_private_connector_successfully_connects_to_running_system_bus(t *testing.T) { + t.Setenv("DBUS_SYSTEM_BUS_ADDRESS", fmt.Sprintf("unix:path=%s", os.Getenv(testDbusSocketEnv))) + + client, err := dbus.New(dbus.SystemPrivateConnector) + if err != nil { + t.Fatalf("Failed creating client: %v", err) + } + + if client == nil { + t.Fatalf("Expected not nil client when new succeeds") + } +} diff --git a/pkg/dbus/conn_test.go b/pkg/dbus/conn_test.go new file mode 100644 index 000000000..8e5e8203a --- /dev/null +++ b/pkg/dbus/conn_test.go @@ -0,0 +1,189 @@ +package dbus_test + +import ( + "encoding/hex" + "errors" + "fmt" + "os" + "strconv" + "testing" + + godbus "github.com/godbus/dbus/v5" + + "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/dbus" +) + +func Test_Creating_client_authenticates_using_user_id(t *testing.T) { + t.Parallel() + + authCheckingConnection := &mockConnection{ + authF: func(authMethods []godbus.Auth) error { + uidAuthFound := false + + for i, method := range authMethods { + _, data, _ := method.FirstData() + + decodedData, err := hex.DecodeString(string(data)) + if err != nil { + t.Fatalf("Received auth method %d has bad hex data %v: %v", i, data, err) + } + + potentialUID, err := strconv.Atoi(string(decodedData)) + if err != nil { + t.Logf("Data %q couldn't be converted to UID: %v", string(decodedData), err) + } + + if potentialUID == os.Getuid() { + uidAuthFound = true + } + } + + if !uidAuthFound { + t.Fatalf("Expected auth method with user id") + } + + return nil + }, + } + + client, err := dbus.New(func() (dbus.Connection, error) { return authCheckingConnection, nil }) + if err != nil { + t.Fatalf("Unexpected error creating client: %v", err) + } + + if client == nil { + t.Fatalf("When new succeeds, returned client should not be nil") + } +} + +//nolint:funlen // Just many subtests. +func Test_Creating_client_returns_error_when(t *testing.T) { + t.Parallel() + + t.Run("no_connector_is_given", func(t *testing.T) { + t.Parallel() + + testNewError(t, nil, nil) + }) + + t.Run("connecting_to_D-Bus_socket_fails", func(t *testing.T) { + t.Parallel() + + expectedErr := fmt.Errorf("connection error") + + failingConnectionConnector := func() (dbus.Connection, error) { return nil, expectedErr } + + testNewError(t, failingConnectionConnector, expectedErr) + }) + + t.Run("authenticating_to_D-Bus_fails", func(t *testing.T) { + t.Parallel() + + expectedErr := fmt.Errorf("auth error") + + closeCalled := false + + failingAuthConnection := &mockConnection{ + authF: func([]godbus.Auth) error { + return expectedErr + }, + closeF: func() error { + closeCalled = true + + return fmt.Errorf("closing error") + }, + } + + testNewError(t, func() (dbus.Connection, error) { return failingAuthConnection, nil }, expectedErr) + + t.Run("and_tries_to_close_the_client_while_ignoring_closing_error", func(t *testing.T) { + if !closeCalled { + t.Fatalf("Expected close function to be called") + } + }) + }) + + t.Run("sending_hello_to_D-Bus_fails", func(t *testing.T) { + t.Parallel() + + expectedErr := fmt.Errorf("hello error") + + closeCalled := false + + failingHelloConnection := &mockConnection{ + helloF: func() error { + return expectedErr + }, + closeF: func() error { + closeCalled = true + + return fmt.Errorf("closing error") + }, + } + + testNewError(t, func() (dbus.Connection, error) { return failingHelloConnection, nil }, expectedErr) + + t.Run("and_tries_to_close_the_client_while_ignoring_closing_error", func(t *testing.T) { + if !closeCalled { + t.Fatalf("Expected close function to be called") + } + }) + }) +} + +func testNewError(t *testing.T, connector dbus.Connector, expectedErr error) { + t.Helper() + + client, err := dbus.New(connector) + if err == nil { + t.Fatalf("Expected error creating client") + } + + if client != nil { + t.Fatalf("Client should not be returned when creation error occurs") + } + + if expectedErr != nil && !errors.Is(err, expectedErr) { + t.Fatalf("Unexpected error occurred, expected %q, got %q", expectedErr, err) + } +} + +type mockConnection struct { + authF func(authMethods []godbus.Auth) error + helloF func() error + closeF func() error +} + +func (m *mockConnection) Auth(authMethods []godbus.Auth) error { + if m.authF == nil { + return nil + } + + return m.authF(authMethods) +} + +func (m *mockConnection) Hello() error { + if m.helloF == nil { + return nil + } + + return m.helloF() +} + +func (m *mockConnection) AddMatchSignal(matchOptions ...godbus.MatchOption) error { + return nil +} + +func (m *mockConnection) Signal(ch chan<- *godbus.Signal) {} + +func (m *mockConnection) Object(dest string, path godbus.ObjectPath) godbus.BusObject { + return nil +} + +func (m *mockConnection) Close() error { + if m.closeF == nil { + return nil + } + + return m.closeF() +} diff --git a/pkg/dbus/mock.go b/pkg/dbus/mock.go new file mode 100644 index 000000000..b1a5f33d5 --- /dev/null +++ b/pkg/dbus/mock.go @@ -0,0 +1,138 @@ +package dbus + +import ( + "context" + + godbus "github.com/godbus/dbus/v5" +) + +// MockConnection is a test helper for mocking godbus.Conn. +type MockConnection struct { + AuthF func(authMethods []godbus.Auth) error + HelloF func() error + CloseF func() error + AddMatchSignalF func(...godbus.MatchOption) error + SignalF func(chan<- *godbus.Signal) + ObjectF func(string, godbus.ObjectPath) godbus.BusObject +} + +// Auth ... +func (m *MockConnection) Auth(authMethods []godbus.Auth) error { + if m.AuthF == nil { + return nil + } + + return m.AuthF(authMethods) +} + +// Hello ... +func (m *MockConnection) Hello() error { + if m.HelloF == nil { + return nil + } + + return m.HelloF() +} + +// AddMatchSignal ... +func (m *MockConnection) AddMatchSignal(matchOptions ...godbus.MatchOption) error { + if m.AddMatchSignalF == nil { + return nil + } + + return m.AddMatchSignalF(matchOptions...) +} + +// Signal ... +func (m *MockConnection) Signal(ch chan<- *godbus.Signal) { + if m.SignalF == nil { + return + } + + m.SignalF(ch) +} + +// Object ... +func (m *MockConnection) Object(dest string, path godbus.ObjectPath) godbus.BusObject { + if m.ObjectF == nil { + return nil + } + + return m.ObjectF(dest, path) +} + +// Close ... +func (m *MockConnection) Close() error { + if m.CloseF == nil { + return nil + } + + return m.CloseF() +} + +// MockObject is a mock of godbus.BusObject. +type MockObject struct { + CallWithContextF func(context.Context, string, godbus.Flags, ...interface{}) *godbus.Call + CallF func(string, godbus.Flags, ...interface{}) *godbus.Call +} + +// MockObject must implement godbus.BusObject to be usable for other packages in tests, though not +// all methods must actually be mockable. See https://github.com/godbus/dbus/issues/252 for details. +var _ godbus.BusObject = &MockObject{} + +// CallWithContext ... +// +//nolint:lll // Upstream signature, can't do much with that. +func (m *MockObject) CallWithContext(ctx context.Context, method string, flags godbus.Flags, args ...interface{}) *godbus.Call { + if m.CallWithContextF == nil { + return &godbus.Call{} + } + + return m.CallWithContextF(ctx, method, flags, args...) +} + +// Call ... +func (m *MockObject) Call(method string, flags godbus.Flags, args ...interface{}) *godbus.Call { + if m.CallF == nil { + return &godbus.Call{} + } + + return m.CallF(method, flags, args...) +} + +// Go ... +func (m *MockObject) Go(method string, flags godbus.Flags, ch chan *godbus.Call, args ...interface{}) *godbus.Call { + return &godbus.Call{} +} + +// GoWithContext ... +// +//nolint:lll // Upstream signature, can't do much with that. +func (m *MockObject) GoWithContext(ctx context.Context, method string, flags godbus.Flags, ch chan *godbus.Call, args ...interface{}) *godbus.Call { + return &godbus.Call{} +} + +// AddMatchSignal ... +func (m *MockObject) AddMatchSignal(iface, member string, options ...godbus.MatchOption) *godbus.Call { + return &godbus.Call{} +} + +// RemoveMatchSignal ... +func (m *MockObject) RemoveMatchSignal(iface, member string, options ...godbus.MatchOption) *godbus.Call { + return &godbus.Call{} +} + +// GetProperty ... +func (m *MockObject) GetProperty(p string) (godbus.Variant, error) { return godbus.Variant{}, nil } + +// StoreProperty ... +func (m *MockObject) StoreProperty(p string, value interface{}) error { return nil } + +// SetProperty ... +func (m *MockObject) SetProperty(p string, v interface{}) error { return nil } + +// Destination ... +func (m *MockObject) Destination() string { return "" } + +// Path ... +func (m *MockObject) Path() godbus.ObjectPath { return "" } From 3ac1d2985a2a71aefbc0d9346cc28a736035d16e Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Thu, 2 Dec 2021 16:55:21 +0100 Subject: [PATCH 2/4] pkg/updateengine/client_test.go: fix import sorting Signed-off-by: Mateusz Gozdek --- pkg/updateengine/client_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/updateengine/client_test.go b/pkg/updateengine/client_test.go index 3875a9b47..050e4df99 100644 --- a/pkg/updateengine/client_test.go +++ b/pkg/updateengine/client_test.go @@ -5,8 +5,9 @@ import ( "fmt" "testing" - "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/updateengine" dbus "github.com/godbus/dbus/v5" + + "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/updateengine" ) //nolint:paralleltest,funlen,tparallel // Test uses environment variables, which are global. From b3b7acf8e2802a235092d692faf7a8836071b059 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Thu, 2 Dec 2021 16:56:00 +0100 Subject: [PATCH 3/4] pkg/updateengine: rename dbus import to godbus So we can introduce internal dbus package. Signed-off-by: Mateusz Gozdek --- pkg/updateengine/client.go | 28 ++++++++++++++-------------- pkg/updateengine/client_test.go | 30 +++++++++++++++--------------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/pkg/updateengine/client.go b/pkg/updateengine/client.go index 74f194d90..1409c4501 100644 --- a/pkg/updateengine/client.go +++ b/pkg/updateengine/client.go @@ -19,7 +19,7 @@ import ( "os" "strconv" - dbus "github.com/godbus/dbus/v5" + godbus "github.com/godbus/dbus/v5" ) const ( @@ -52,12 +52,12 @@ type Client interface { // DBusConnection is set of methods which client expects D-Bus connection to implement. type DBusConnection interface { - Auth([]dbus.Auth) error + Auth([]godbus.Auth) error Hello() error Close() error - AddMatchSignal(...dbus.MatchOption) error - Signal(chan<- *dbus.Signal) - Object(string, dbus.ObjectPath) dbus.BusObject + AddMatchSignal(...godbus.MatchOption) error + Signal(chan<- *godbus.Signal) + Object(string, godbus.ObjectPath) godbus.BusObject } // DBusConnector is a constructor function providing D-Bus connection. @@ -65,13 +65,13 @@ type DBusConnector func() (DBusConnection, error) // DBusSystemPrivateConnector is a standard update_engine connector using system bus. func DBusSystemPrivateConnector() (DBusConnection, error) { - return dbus.SystemBusPrivate() + return godbus.SystemBusPrivate() } type client struct { conn DBusConnection - object dbus.BusObject - ch chan *dbus.Signal + object godbus.BusObject + ch chan *godbus.Signal } // New creates new instance of Client and initializes it. @@ -81,7 +81,7 @@ func New(newConnection DBusConnector) (Client, error) { return nil, fmt.Errorf("opening private connection to system bus: %w", err) } - methods := []dbus.Auth{dbus.AuthExternal(strconv.Itoa(os.Getuid()))} + methods := []godbus.Auth{godbus.AuthExternal(strconv.Itoa(os.Getuid()))} if err := conn.Auth(methods); err != nil { // Best effort closing the connection. @@ -97,22 +97,22 @@ func New(newConnection DBusConnector) (Client, error) { return nil, fmt.Errorf("sending hello to system bus: %w", err) } - matchOptions := []dbus.MatchOption{ - dbus.WithMatchInterface(DBusInterface), - dbus.WithMatchMember(DBusSignalNameStatusUpdate), + matchOptions := []godbus.MatchOption{ + godbus.WithMatchInterface(DBusInterface), + godbus.WithMatchMember(DBusSignalNameStatusUpdate), } if err := conn.AddMatchSignal(matchOptions...); err != nil { return nil, fmt.Errorf("adding filter: %w", err) } - ch := make(chan *dbus.Signal, signalBuffer) + ch := make(chan *godbus.Signal, signalBuffer) conn.Signal(ch) return &client{ ch: ch, conn: conn, - object: conn.Object(DBusDestination, dbus.ObjectPath(DBusPath)), + object: conn.Object(DBusDestination, godbus.ObjectPath(DBusPath)), }, nil } diff --git a/pkg/updateengine/client_test.go b/pkg/updateengine/client_test.go index 050e4df99..874253372 100644 --- a/pkg/updateengine/client_test.go +++ b/pkg/updateengine/client_test.go @@ -5,7 +5,7 @@ import ( "fmt" "testing" - dbus "github.com/godbus/dbus/v5" + godbus "github.com/godbus/dbus/v5" "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/updateengine" ) @@ -28,7 +28,7 @@ func Test_Creating_client_fails_when(t *testing.T) { closeCalled := false connector := newMockDBusConnection() - connector.authF = func([]dbus.Auth) error { return expectedError } + connector.authF = func([]godbus.Auth) error { return expectedError } connector.closeF = func() error { closeCalled = true @@ -88,7 +88,7 @@ func Test_Creating_client_fails_when(t *testing.T) { expectedError := fmt.Errorf("match signal failed") connector := newMockDBusConnection() - connector.addMatchSignalF = func(...dbus.MatchOption) error { return expectedError } + connector.addMatchSignalF = func(...godbus.MatchOption) error { return expectedError } client, err := updateengine.New(func() (updateengine.DBusConnection, error) { return connector, nil }) if !errors.Is(err, expectedError) { @@ -120,15 +120,15 @@ func Test_Closing_client_returns_error_when_closing_DBus_client_fails(t *testing } type mockDBusConnection struct { - authF func([]dbus.Auth) error + authF func([]godbus.Auth) error helloF func() error closeF func() error - addMatchSignalF func(...dbus.MatchOption) error - signalF func(chan<- *dbus.Signal) - objectF func(string, dbus.ObjectPath) dbus.BusObject + addMatchSignalF func(...godbus.MatchOption) error + signalF func(chan<- *godbus.Signal) + objectF func(string, godbus.ObjectPath) godbus.BusObject } -func (m *mockDBusConnection) Auth(methods []dbus.Auth) error { +func (m *mockDBusConnection) Auth(methods []godbus.Auth) error { return m.authF(methods) } @@ -140,25 +140,25 @@ func (m *mockDBusConnection) Close() error { return m.closeF() } -func (m *mockDBusConnection) AddMatchSignal(options ...dbus.MatchOption) error { +func (m *mockDBusConnection) AddMatchSignal(options ...godbus.MatchOption) error { return m.addMatchSignalF(options...) } -func (m *mockDBusConnection) Signal(ch chan<- *dbus.Signal) { +func (m *mockDBusConnection) Signal(ch chan<- *godbus.Signal) { m.signalF(ch) } -func (m *mockDBusConnection) Object(dest string, path dbus.ObjectPath) dbus.BusObject { +func (m *mockDBusConnection) Object(dest string, path godbus.ObjectPath) godbus.BusObject { return m.objectF(dest, path) } func newMockDBusConnection() *mockDBusConnection { return &mockDBusConnection{ - authF: func([]dbus.Auth) error { return nil }, + authF: func([]godbus.Auth) error { return nil }, helloF: func() error { return nil }, closeF: func() error { return nil }, - addMatchSignalF: func(...dbus.MatchOption) error { return nil }, - signalF: func(chan<- *dbus.Signal) {}, - objectF: func(string, dbus.ObjectPath) dbus.BusObject { return &dbus.Object{} }, + addMatchSignalF: func(...godbus.MatchOption) error { return nil }, + signalF: func(chan<- *godbus.Signal) {}, + objectF: func(string, godbus.ObjectPath) godbus.BusObject { return &godbus.Object{} }, } } From 373e9b1350a346ef6e42650945c9f559b1cebf6f Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Thu, 2 Dec 2021 22:54:29 +0100 Subject: [PATCH 4/4] pkg/updateengine: rewrite tests and code to use new dbus package This is a safe step before moving creation of D-Bus connection to agent and eventually agent CLI code, which however requires development of tests on the agent code itself. Signed-off-by: Mateusz Gozdek --- pkg/agent/agent.go | 3 +- pkg/updateengine/client.go | 38 +- pkg/updateengine/client_integration_test.go | 244 ------------ pkg/updateengine/client_test.go | 397 +++++++++++++++----- 4 files changed, 310 insertions(+), 372 deletions(-) delete mode 100644 pkg/updateengine/client_integration_test.go diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index ae4a758e8..3095d9422 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -27,6 +27,7 @@ import ( "k8s.io/klog/v2" "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/constants" + "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/dbus" "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/k8sutil" "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/updateengine" ) @@ -67,7 +68,7 @@ func New(node string, reapTimeout time.Duration) (*Klocksmith, error) { nc := kc.CoreV1().Nodes() // Set up update_engine client. - updateEngineClient, err := updateengine.New(updateengine.DBusSystemPrivateConnector) + updateEngineClient, err := updateengine.New(dbus.SystemPrivateConnector) if err != nil { return nil, fmt.Errorf("establishing connection to update_engine dbus: %w", err) } diff --git a/pkg/updateengine/client.go b/pkg/updateengine/client.go index 1409c4501..dd411c546 100644 --- a/pkg/updateengine/client.go +++ b/pkg/updateengine/client.go @@ -16,10 +16,10 @@ package updateengine import ( "fmt" - "os" - "strconv" godbus "github.com/godbus/dbus/v5" + + "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/dbus" ) const ( @@ -52,49 +52,27 @@ type Client interface { // DBusConnection is set of methods which client expects D-Bus connection to implement. type DBusConnection interface { - Auth([]godbus.Auth) error - Hello() error Close() error AddMatchSignal(...godbus.MatchOption) error Signal(chan<- *godbus.Signal) Object(string, godbus.ObjectPath) godbus.BusObject } -// DBusConnector is a constructor function providing D-Bus connection. -type DBusConnector func() (DBusConnection, error) - -// DBusSystemPrivateConnector is a standard update_engine connector using system bus. -func DBusSystemPrivateConnector() (DBusConnection, error) { - return godbus.SystemBusPrivate() +type caller interface { + Call(method string, flags godbus.Flags, args ...interface{}) *godbus.Call } type client struct { conn DBusConnection - object godbus.BusObject + object caller ch chan *godbus.Signal } // New creates new instance of Client and initializes it. -func New(newConnection DBusConnector) (Client, error) { - conn, err := newConnection() +func New(connector dbus.Connector) (Client, error) { + conn, err := dbus.New(connector) if err != nil { - return nil, fmt.Errorf("opening private connection to system bus: %w", err) - } - - methods := []godbus.Auth{godbus.AuthExternal(strconv.Itoa(os.Getuid()))} - - if err := conn.Auth(methods); err != nil { - // Best effort closing the connection. - _ = conn.Close() - - return nil, fmt.Errorf("authenticating to system bus: %w", err) - } - - if err := conn.Hello(); err != nil { - // Best effort closing the connection. - _ = conn.Close() - - return nil, fmt.Errorf("sending hello to system bus: %w", err) + return nil, fmt.Errorf("creating D-Bus client: %w", err) } matchOptions := []godbus.MatchOption{ diff --git a/pkg/updateengine/client_integration_test.go b/pkg/updateengine/client_integration_test.go deleted file mode 100644 index 4be81eca3..000000000 --- a/pkg/updateengine/client_integration_test.go +++ /dev/null @@ -1,244 +0,0 @@ -//go:build integration -// +build integration - -package updateengine_test - -import ( - "fmt" - "os" - "strconv" - "testing" - "time" - - dbus "github.com/godbus/dbus/v5" - "github.com/google/go-cmp/cmp" - - "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/updateengine" -) - -//nolint:paralleltest,funlen,cyclop // Test uses environment variables and D-Bus which are global. -// Just many subtests. -func Test_Receiving_status(t *testing.T) { - t.Run("emits_first_status_immediately_after_start", func(t *testing.T) { - ch, _ := testGetStatusReceiver(t, updateengine.Status{}) - - timeout := time.NewTimer(time.Second) - select { - case <-ch: - case <-timeout.C: - t.Fatal("Failed getting status within expected timeframe") - } - }) - - t.Run("parses_received_values_in_order_defined_by_update_engine", func(t *testing.T) { - expectedStatus := testStatus() - - ch, _ := testGetStatusReceiver(t, expectedStatus) - - timeout := time.NewTimer(time.Second) - select { - case status := <-ch: - if diff := cmp.Diff(expectedStatus, status); diff != "" { - t.Fatalf("Unexpectected status values received:\n%s", diff) - } - case <-timeout.C: - t.Fatal("Failed getting status within expected timeframe") - } - }) - - t.Run("forwards_status_updates_received_from_update_engine_to_given_receiver_channel", func(t *testing.T) { - firstExpectedStatus := updateengine.Status{} - - statusCh, conn := testGetStatusReceiver(t, firstExpectedStatus) - - secondExpectedStatus := testStatus() - - lastCheckedTime, progress, currentOperation, newVersion, newSize, _ := statusToRawValues(secondExpectedStatus, nil) - - withMockStatusUpdate(t, conn, lastCheckedTime, progress, currentOperation, newVersion, newSize) - - timeout := time.NewTimer(time.Second) - - select { - case status := <-statusCh: - if diff := cmp.Diff(firstExpectedStatus, status); diff != "" { - t.Fatalf("Unexpectected first status values received (-expected/+got):\n%s", diff) - } - case <-timeout.C: - t.Fatal("Failed getting first status within expected timeframe") - } - - timeout.Reset(time.Second) - - select { - case status := <-statusCh: - if diff := cmp.Diff(secondExpectedStatus, status); diff != "" { - t.Fatalf("Unexpectected second status values received:\n%s", diff) - } - case <-timeout.C: - t.Fatal("Failed getting second status within expected timeframe") - } - }) - - t.Run("returns_empty_status_when_getting_initial_status_fails", func(t *testing.T) { - expectedStatus := updateengine.Status{} - - statusCh, conn := testGetStatusReceiver(t, expectedStatus) - - // Reset method table so GetStatus updates immediately returns error. - // Once https://github.com/flatcar-linux/flatcar-linux-update-operator/issues/100 is solved, - // we can make method call to actually return an error. - tbl := map[string]interface{}{} - if err := conn.ExportMethodTable(tbl, updateengine.DBusPath, updateengine.DBusInterface); err != nil { - t.Fatalf("Failed resetting method table: %v", err) - } - - timeout := time.NewTimer(time.Second) - - select { - case status := <-statusCh: - if diff := cmp.Diff(expectedStatus, status); diff != "" { - t.Fatalf("Unexpectected status values received:\n%s", diff) - } - case <-timeout.C: - t.Fatal("Failed getting status within expected timeframe") - } - }) -} - -func testGetStatusReceiver(t *testing.T, status updateengine.Status) (chan updateengine.Status, *dbus.Conn) { - t.Helper() - - getStatusTestResponse := func(message dbus.Message) (int64, float64, string, string, int64, *dbus.Error) { - return statusToRawValues(status, nil) - } - - conn := withMockGetStatus(t, getStatusTestResponse) - - client, err := updateengine.New(updateengine.DBusSystemPrivateConnector) - if err != nil { - t.Fatalf("Creating client should succeed, got: %v", err) - } - - stop := make(chan struct{}) - - t.Cleanup(func() { - // Stopping receiver routine must be done before closing the client. See - // https://github.com/flatcar-linux/flatcar-linux-update-operator/issues/101 for more details. - close(stop) - if err := client.Close(); err != nil { - t.Fatalf("Failed closing client: %v", err) - } - }) - - ch := make(chan updateengine.Status, 1) - - go client.ReceiveStatuses(ch, stop) - - return ch, conn -} - -const ( - testDbusSocketEnv = "FLUO_TEST_DBUS_SOCKET" -) - -func testStatus() updateengine.Status { - return updateengine.Status{ - LastCheckedTime: 10, - Progress: 20, - CurrentOperation: updateengine.UpdateStatusVerifying, - NewVersion: "1.2.3", - NewSize: 30, - } -} - -func statusToRawValues(s updateengine.Status, err *dbus.Error) (int64, float64, string, string, int64, *dbus.Error) { - return s.LastCheckedTime, s.Progress, s.CurrentOperation, s.NewVersion, s.NewSize, err -} - -func testSystemConnection(t *testing.T) *dbus.Conn { - t.Helper() - - t.Setenv("DBUS_SYSTEM_BUS_ADDRESS", fmt.Sprintf("unix:path=%s", os.Getenv(testDbusSocketEnv))) - - conn, err := dbus.SystemBusPrivate() - if err != nil { - t.Fatalf("Opening private connection to system bus: %v", err) - } - - return conn -} - -func withMockGetStatus(t *testing.T, getStatusF interface{}) *dbus.Conn { - t.Helper() - - conn := testSystemConnection(t) - - methods := []dbus.Auth{dbus.AuthExternal(strconv.Itoa(os.Getuid()))} - - if err := conn.Auth(methods); err != nil { - t.Fatalf("Failed authenticating to system bus: %v", err) - } - - if err := conn.Hello(); err != nil { - t.Fatalf("Failed sending hello to system bus: %v", err) - } - - if _, err := conn.RequestName(updateengine.DBusDestination, 0); err != nil { - t.Fatalf("Requesting name: %v", err) - } - - t.Cleanup(func() { - if _, err := conn.ReleaseName(updateengine.DBusDestination); err != nil { - t.Fatalf("Failed releasing name: %v", err) - } - }) - - tbl := map[string]interface{}{ - updateengine.DBusMethodNameGetStatus: getStatusF, - } - - if err := conn.ExportMethodTable(tbl, updateengine.DBusPath, updateengine.DBusInterface); err != nil { - t.Fatalf("Exporting method table: %v", err) - } - - t.Cleanup(func() { - tbl := map[string]interface{}{} - if err := conn.ExportMethodTable(tbl, updateengine.DBusPath, updateengine.DBusInterface); err != nil { - t.Fatalf("Failed resetting method table: %v", err) - } - }) - - return conn -} - -func withMockStatusUpdate(t *testing.T, conn *dbus.Conn, values ...interface{}) { - t.Helper() - - emitName := fmt.Sprintf("%s.%s", updateengine.DBusInterface, updateengine.DBusSignalNameStatusUpdate) - - ticker := time.NewTicker(100 * time.Millisecond) - stopCh := make(chan struct{}) - doneCh := make(chan struct{}) - - t.Cleanup(func() { - close(stopCh) - <-doneCh - }) - - go func() { - for { - select { - case <-ticker.C: - if err := conn.Emit(updateengine.DBusPath, emitName, values...); err != nil { - t.Logf("Failed emitting mock status: %v", err) - t.Fail() - } - case <-stopCh: - close(doneCh) - - return - } - } - }() -} diff --git a/pkg/updateengine/client_test.go b/pkg/updateengine/client_test.go index 874253372..959da846b 100644 --- a/pkg/updateengine/client_test.go +++ b/pkg/updateengine/client_test.go @@ -3,162 +3,365 @@ package updateengine_test import ( "errors" "fmt" + "reflect" "testing" + "time" godbus "github.com/godbus/dbus/v5" + "github.com/google/go-cmp/cmp" + "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/dbus" "github.com/flatcar-linux/flatcar-linux-update-operator/pkg/updateengine" ) -//nolint:paralleltest,funlen,tparallel // Test uses environment variables, which are global. -func Test_Creating_client_fails_when(t *testing.T) { - t.Run("connecting_to_system_bus_fails", func(t *testing.T) { - t.Setenv("DBUS_SYSTEM_BUS_ADDRESS", "foo") +//nolint:funlen,cyclop // Just many test cases. +func Test_Receiving_status(t *testing.T) { + t.Parallel() + + t.Run("emits_first_status_immediately_after_start", func(t *testing.T) { + t.Parallel() + + mockConnection := &dbus.MockConnection{ + ObjectF: func(string, godbus.ObjectPath) godbus.BusObject { + return &dbus.MockObject{ + CallF: func(method string, flags godbus.Flags, args ...interface{}) *godbus.Call { + return &godbus.Call{ + Body: statusToSignalBody(updateengine.Status{}), + } + }, + } + }, + } + + client, err := updateengine.New(func() (dbus.Connection, error) { return mockConnection, nil }) + if err != nil { + t.Fatalf("Got unexpected error while creating client: %v", err) + } + + stop := make(chan struct{}) + + t.Cleanup(func() { + close(stop) + }) + + statusCh := make(chan updateengine.Status, 1) + + go client.ReceiveStatuses(statusCh, stop) - if _, err := updateengine.New(updateengine.DBusSystemPrivateConnector); err == nil { - t.Fatalf("Creating client should fail when unable to connect to system bus") + timeout := time.NewTimer(time.Second) + select { + case <-statusCh: + case <-timeout.C: + t.Fatal("Failed getting status within expected timeframe") } }) - t.Run("D-Bus_authentication_fails", func(t *testing.T) { + t.Run("parses_received_values_in_order_defined_by_update_engine", func(t *testing.T) { t.Parallel() - expectedError := fmt.Errorf("auth failed") + expectedStatus := testStatus() + + mockConnection := &dbus.MockConnection{ + ObjectF: func(string, godbus.ObjectPath) godbus.BusObject { + return &dbus.MockObject{ + CallF: func(method string, flags godbus.Flags, args ...interface{}) *godbus.Call { + return &godbus.Call{ + Body: statusToSignalBody(expectedStatus), + } + }, + } + }, + } - closeCalled := false + client, err := updateengine.New(func() (dbus.Connection, error) { return mockConnection, nil }) + if err != nil { + t.Fatalf("Got unexpected error while creating client: %v", err) + } + + stop := make(chan struct{}) + + t.Cleanup(func() { + close(stop) + }) - connector := newMockDBusConnection() - connector.authF = func([]godbus.Auth) error { return expectedError } - connector.closeF = func() error { - closeCalled = true + statusCh := make(chan updateengine.Status, 1) - return fmt.Errorf("closing error") + go client.ReceiveStatuses(statusCh, stop) + + timeout := time.NewTimer(time.Second) + select { + case status := <-statusCh: + if diff := cmp.Diff(expectedStatus, status); diff != "" { + t.Fatalf("Unexpectected status values received:\n%s", diff) + } + case <-timeout.C: + t.Fatal("Failed getting status within expected timeframe") } + }) - client, err := updateengine.New(func() (updateengine.DBusConnection, error) { return connector, nil }) - if !errors.Is(err, expectedError) { - t.Fatalf("Got unexpected error while creating client, expected %q, got %q", expectedError, err) + t.Run("forwards_status_updates_received_from_update_engine_to_given_receiver_channel", func(t *testing.T) { + t.Parallel() + + firstExpectedStatus := updateengine.Status{} + + secondExpectedStatus := testStatus() + + mockConnection := &dbus.MockConnection{ + ObjectF: func(string, godbus.ObjectPath) godbus.BusObject { + return &dbus.MockObject{ + CallF: func(method string, flags godbus.Flags, args ...interface{}) *godbus.Call { + return &godbus.Call{ + Body: statusToSignalBody(firstExpectedStatus), + } + }, + } + }, + SignalF: func(ch chan<- *godbus.Signal) { + ch <- &godbus.Signal{ + Body: statusToSignalBody(secondExpectedStatus), + } + }, } - if client != nil { - t.Fatalf("Expected client to be nil when creating fails") + client, err := updateengine.New(func() (dbus.Connection, error) { return mockConnection, nil }) + if err != nil { + t.Fatalf("Got unexpected error while creating client: %v", err) } - t.Run("and_tries_to_close_the_client_while_ignoring_closing_error", func(t *testing.T) { - if !closeCalled { - t.Fatalf("Expected close function to be called") - } + stop := make(chan struct{}) + + t.Cleanup(func() { + close(stop) }) + + statusCh := make(chan updateengine.Status, 1) + + go client.ReceiveStatuses(statusCh, stop) + + timeout := time.NewTimer(time.Second) + + select { + case status := <-statusCh: + if diff := cmp.Diff(firstExpectedStatus, status); diff != "" { + t.Fatalf("Unexpectected first status values received (-expected/+got):\n%s", diff) + } + case <-timeout.C: + t.Fatal("Failed getting first status within expected timeframe") + } + + timeout.Reset(time.Second) + + select { + case status := <-statusCh: + if diff := cmp.Diff(secondExpectedStatus, status); diff != "" { + t.Fatalf("Unexpectected second status values received:\n%s", diff) + } + case <-timeout.C: + t.Fatal("Failed getting second status within expected timeframe") + } }) - t.Run("D-Bus_hello_fails", func(t *testing.T) { + t.Run("returns_empty_status_when_getting_initial_status_fails", func(t *testing.T) { t.Parallel() - expectedError := fmt.Errorf("hello failed") + expectedStatus := updateengine.Status{} + + mockConnection := &dbus.MockConnection{ + ObjectF: func(string, godbus.ObjectPath) godbus.BusObject { + return &dbus.MockObject{ + CallF: func(method string, flags godbus.Flags, args ...interface{}) *godbus.Call { + return &godbus.Call{ + Body: statusToSignalBody(testStatus()), + Err: fmt.Errorf("some error"), + } + }, + } + }, + } - closeCalled := false + client, err := updateengine.New(func() (dbus.Connection, error) { return mockConnection, nil }) + if err != nil { + t.Fatalf("Got unexpected error while creating client: %v", err) + } + + stop := make(chan struct{}) + + t.Cleanup(func() { + close(stop) + }) + + statusCh := make(chan updateengine.Status, 1) - connector := newMockDBusConnection() - connector.helloF = func() error { return expectedError } - connector.closeF = func() error { - closeCalled = true + go client.ReceiveStatuses(statusCh, stop) - return fmt.Errorf("closing error") + timeout := time.NewTimer(time.Second) + + select { + case status := <-statusCh: + if diff := cmp.Diff(expectedStatus, status); diff != "" { + t.Fatalf("Unexpectected status values received:\n%s", diff) + } + case <-timeout.C: + t.Fatal("Failed getting status within expected timeframe") } + }) +} - client, err := updateengine.New(func() (updateengine.DBusConnection, error) { return connector, nil }) - if !errors.Is(err, expectedError) { - t.Fatalf("Got unexpected error while creating client, expected %q, got %q", expectedError, err) +//nolint:funlen,gocognit,cyclop // Just many test cases. +func Test_Creating_client(t *testing.T) { + t.Parallel() + + t.Run("subscribes_to_status_update_signals", func(t *testing.T) { + t.Parallel() + + mockConnection := &dbus.MockConnection{ + AddMatchSignalF: func(matchOptions ...godbus.MatchOption) error { + foundInterface := false + foundMember := false + + for _, option := range matchOptions { + optionValue := reflect.ValueOf(&option).Elem() + key := optionValue.Field(0) + value := optionValue.Field(1) + + switch key.String() { + case "interface": + if value.String() == updateengine.DBusInterface { + foundInterface = true + } + case "member": + if value.String() == updateengine.DBusSignalNameStatusUpdate { + foundMember = true + } + } + } + + if !foundInterface { + t.Fatal("Did not receive match option with interface specified") + } + + if !foundMember { + t.Fatal("Did not receive match option with member specified") + } + + return nil + }, } - if client != nil { - t.Fatalf("Expected client to be nil when creating fails") + if _, err := updateengine.New(func() (dbus.Connection, error) { return mockConnection, nil }); err != nil { + t.Fatalf("Got unexpected error while creating client: %v", err) } + }) + + t.Run("fails_when", func(t *testing.T) { + t.Parallel() - t.Run("and_tries_to_close_the_client_while_ignoring_closing_error", func(t *testing.T) { - if !closeCalled { - t.Fatalf("Expected close function to be called") + t.Run("no_D-Bus_connection_is_given", func(t *testing.T) { + t.Parallel() + + if _, err := updateengine.New(nil); err == nil { + t.Fatalf("Creating client should fail when unable to connect to system bus") } }) - }) - t.Run("adding_D-Bus_filter_fails", func(t *testing.T) { - t.Parallel() + t.Run("creating_D-Bus_client_fails", func(t *testing.T) { + t.Parallel() - expectedError := fmt.Errorf("match signal failed") + expectedError := fmt.Errorf("D-Bus connection error") - connector := newMockDBusConnection() - connector.addMatchSignalF = func(...godbus.MatchOption) error { return expectedError } + client, err := updateengine.New(func() (dbus.Connection, error) { return nil, expectedError }) + if !errors.Is(err, expectedError) { + t.Fatalf("Got unexpected error while creating client, expected %q, got %q", expectedError, err) + } - client, err := updateengine.New(func() (updateengine.DBusConnection, error) { return connector, nil }) - if !errors.Is(err, expectedError) { - t.Fatalf("Got unexpected error while creating client, expected %q, got %q", expectedError, err) - } + if client != nil { + t.Fatalf("Expected client to be nil when creating fails") + } + }) - if client != nil { - t.Fatalf("Expected client to be nil when creating fails") - } + t.Run("adding_D-Bus_filter_fails", func(t *testing.T) { + t.Parallel() + + expectedError := fmt.Errorf("match signal failed") + + failingAddMatchSignalConnection := &dbus.MockConnection{ + AddMatchSignalF: func(...godbus.MatchOption) error { return expectedError }, + } + + client, err := updateengine.New(func() (dbus.Connection, error) { return failingAddMatchSignalConnection, nil }) + if !errors.Is(err, expectedError) { + t.Fatalf("Got unexpected error while creating client, expected %q, got %q", expectedError, err) + } + + if client != nil { + t.Fatalf("Expected client to be nil when creating fails") + } + }) }) } -func Test_Closing_client_returns_error_when_closing_DBus_client_fails(t *testing.T) { +func Test_Closing_client(t *testing.T) { t.Parallel() - expectedError := fmt.Errorf("closing failed") + t.Run("closes_underlying_D-Bus_connection", func(t *testing.T) { + t.Parallel() + + closeCalled := false - connector := newMockDBusConnection() - connector.closeF = func() error { return expectedError } + mockConnection := &dbus.MockConnection{ + CloseF: func() error { + closeCalled = true - client, err := updateengine.New(func() (updateengine.DBusConnection, error) { return connector, nil }) - if err != nil { - t.Fatalf("Unexpected error creating client: %v", err) - } + return nil + }, + } - if err := client.Close(); !errors.Is(err, expectedError) { - t.Fatalf("Got unexpected error closing the client, expected %q, got %q", expectedError, err) - } -} + client, err := updateengine.New(func() (dbus.Connection, error) { return mockConnection, nil }) + if err != nil { + t.Fatalf("Got unexpected error while creating client: %v", err) + } -type mockDBusConnection struct { - authF func([]godbus.Auth) error - helloF func() error - closeF func() error - addMatchSignalF func(...godbus.MatchOption) error - signalF func(chan<- *godbus.Signal) - objectF func(string, godbus.ObjectPath) godbus.BusObject -} + if err := client.Close(); err != nil { + t.Fatalf("Unexpected error closing client: %v", err) + } -func (m *mockDBusConnection) Auth(methods []godbus.Auth) error { - return m.authF(methods) -} + if !closeCalled { + t.Fatalf("Expected client to close D-Bus connection") + } + }) -func (m *mockDBusConnection) Hello() error { - return m.helloF() -} + t.Run("returns_error_when_closing_D-Bus_connection_fails", func(t *testing.T) { + t.Parallel() -func (m *mockDBusConnection) Close() error { - return m.closeF() -} + expectedError := fmt.Errorf("closing error") -func (m *mockDBusConnection) AddMatchSignal(options ...godbus.MatchOption) error { - return m.addMatchSignalF(options...) -} + mockConnection := &dbus.MockConnection{ + CloseF: func() error { + return expectedError + }, + } -func (m *mockDBusConnection) Signal(ch chan<- *godbus.Signal) { - m.signalF(ch) -} + client, err := updateengine.New(func() (dbus.Connection, error) { return mockConnection, nil }) + if err != nil { + t.Fatalf("Got unexpected error while creating client: %v", err) + } -func (m *mockDBusConnection) Object(dest string, path godbus.ObjectPath) godbus.BusObject { - return m.objectF(dest, path) + if err := client.Close(); !errors.Is(err, expectedError) { + t.Fatalf("Expected error closing client %q, got %q", expectedError, err) + } + }) } -func newMockDBusConnection() *mockDBusConnection { - return &mockDBusConnection{ - authF: func([]godbus.Auth) error { return nil }, - helloF: func() error { return nil }, - closeF: func() error { return nil }, - addMatchSignalF: func(...godbus.MatchOption) error { return nil }, - signalF: func(chan<- *godbus.Signal) {}, - objectF: func(string, godbus.ObjectPath) godbus.BusObject { return &godbus.Object{} }, +func testStatus() updateengine.Status { + return updateengine.Status{ + LastCheckedTime: 10, + Progress: 20, + CurrentOperation: updateengine.UpdateStatusVerifying, + NewVersion: "1.2.3", + NewSize: 30, } } + +func statusToSignalBody(s updateengine.Status) []interface{} { + return []interface{}{s.LastCheckedTime, s.Progress, s.CurrentOperation, s.NewVersion, s.NewSize} +}