diff --git a/pkg/systemd/dbus.go b/pkg/systemd/dbus.go index 40c4fc74..bf039470 100644 --- a/pkg/systemd/dbus.go +++ b/pkg/systemd/dbus.go @@ -9,8 +9,14 @@ import ( "github.com/coreos/go-systemd/v22/dbus" ) +type dbusConn interface { + Close() + Connected() bool + GetUnitPropertiesContext(ctx context.Context, unit string) (map[string]interface{}, error) +} + type DbusConn struct { - conn *dbus.Conn + conn dbusConn } // Caller should explicitly close the connection by calling Close() on returned connection object @@ -37,13 +43,26 @@ func (c *DbusConn) IsActive(ctx context.Context, unitName string) (bool, error) if !c.conn.Connected() { return false, fmt.Errorf("connection disconnected") } - if !strings.HasSuffix(unitName, ".target") && !strings.HasSuffix(unitName, ".service") { - unitName = fmt.Sprintf("%s.service", unitName) - } - props, err := c.conn.GetUnitPropertiesContext(ctx, unitName) + + formattedUnitName := normalizeServiceUnitName(unitName) + props, err := c.conn.GetUnitPropertiesContext(ctx, formattedUnitName) if err != nil { - return false, fmt.Errorf("unable to get unit properties for %s: %w", unitName, err) + return false, fmt.Errorf("unable to get unit properties for %s: %w", formattedUnitName, err) + } + + return checkActiveState(props, formattedUnitName) +} + +// normalizeServiceUnitName ensures the unit name has the correct suffix +func normalizeServiceUnitName(unitName string) string { + if !strings.HasSuffix(unitName, ".target") && !strings.HasSuffix(unitName, ".service") { + return fmt.Sprintf("%s.service", unitName) } + return unitName +} + +// checkActiveState checks if a unit is active based on its properties +func checkActiveState(props map[string]interface{}, unitName string) (bool, error) { activeState, ok := props["ActiveState"] if !ok { return false, fmt.Errorf("ActiveState property not found for unit %s", unitName) diff --git a/pkg/systemd/dbus_test.go b/pkg/systemd/dbus_test.go new file mode 100644 index 00000000..d48b2061 --- /dev/null +++ b/pkg/systemd/dbus_test.go @@ -0,0 +1,257 @@ +package systemd + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +// mockDbusConn implements dbusConn interface for testing +type mockDbusConn struct { + connected bool + props map[string]interface{} + err error +} + +func (m *mockDbusConn) Close() {} + +func (m *mockDbusConn) Connected() bool { + return m.connected +} + +func (m *mockDbusConn) GetUnitPropertiesContext(_ context.Context, _ string) (map[string]interface{}, error) { + if m.err != nil { + return nil, m.err + } + return m.props, nil +} + +func TestFormatUnitName(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "bare service name", + input: "nginx", + expected: "nginx.service", + }, + { + name: "already has .service suffix", + input: "nginx.service", + expected: "nginx.service", + }, + { + name: "has .target suffix", + input: "network.target", + expected: "network.target", + }, + { + name: "empty string", + input: "", + expected: ".service", + }, + { + name: "with dots in name", + input: "my.custom.service.name", + expected: "my.custom.service.name.service", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := normalizeServiceUnitName(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestCheckActiveState(t *testing.T) { + tests := []struct { + name string + props map[string]interface{} + unitName string + expected bool + expectError bool + errorMsg string + }{ + { + name: "active service", + props: map[string]interface{}{ + "ActiveState": "active", + }, + unitName: "test.service", + expected: true, + expectError: false, + }, + { + name: "inactive service", + props: map[string]interface{}{ + "ActiveState": "inactive", + }, + unitName: "test.service", + expected: false, + expectError: false, + }, + { + name: "failed service", + props: map[string]interface{}{ + "ActiveState": "failed", + }, + unitName: "test.service", + expected: false, + expectError: false, + }, + { + name: "missing ActiveState", + props: map[string]interface{}{}, + unitName: "test.service", + expected: false, + expectError: true, + errorMsg: "ActiveState property not found for unit test.service", + }, + { + name: "wrong type for ActiveState", + props: map[string]interface{}{ + "ActiveState": 123, + }, + unitName: "test.service", + expected: false, + expectError: true, + errorMsg: "ActiveState property is not a string for unit test.service", + }, + { + name: "wrong type for ActiveState (bool)", + props: map[string]interface{}{ + "ActiveState": true, + }, + unitName: "test.service", + expected: false, + expectError: true, + errorMsg: "ActiveState property is not a string for unit test.service", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := checkActiveState(tt.props, tt.unitName) + if tt.expectError { + assert.Error(t, err) + assert.Equal(t, tt.errorMsg, err.Error()) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} + +func TestIsActive(t *testing.T) { + tests := []struct { + name string + conn *mockDbusConn + unitName string + expected bool + expectError bool + errorMsg string + }{ + { + name: "nil connection", + conn: nil, + unitName: "test.service", + expectError: true, + errorMsg: "connection not initialized", + }, + { + name: "disconnected connection", + conn: &mockDbusConn{ + connected: false, + }, + unitName: "test.service", + expectError: true, + errorMsg: "connection disconnected", + }, + { + name: "error getting properties", + conn: &mockDbusConn{ + connected: true, + err: fmt.Errorf("dbus error"), + }, + unitName: "test.service", + expectError: true, + errorMsg: "unable to get unit properties for test.service: dbus error", + }, + { + name: "active service", + conn: &mockDbusConn{ + connected: true, + props: map[string]interface{}{ + "ActiveState": "active", + }, + }, + unitName: "test.service", + expected: true, + expectError: false, + }, + { + name: "inactive service", + conn: &mockDbusConn{ + connected: true, + props: map[string]interface{}{ + "ActiveState": "inactive", + }, + }, + unitName: "test.service", + expected: false, + expectError: false, + }, + { + name: "service with target suffix", + conn: &mockDbusConn{ + connected: true, + props: map[string]interface{}{ + "ActiveState": "active", + }, + }, + unitName: "test.target", + expected: true, + expectError: false, + }, + { + name: "service without suffix", + conn: &mockDbusConn{ + connected: true, + props: map[string]interface{}{ + "ActiveState": "active", + }, + }, + unitName: "test", + expected: true, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var conn *DbusConn + if tt.conn != nil { + conn = &DbusConn{conn: tt.conn} + } else { + conn = &DbusConn{conn: nil} + } + + result, err := conn.IsActive(context.Background(), tt.unitName) + if tt.expectError { + assert.Error(t, err) + assert.Equal(t, tt.errorMsg, err.Error()) + assert.False(t, result) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +}