3 Commits

Author SHA1 Message Date
38eb2cc352 Create CHANGELOG.md
All checks were successful
Publish / release (push) Successful in 19s
2026-01-04 13:57:16 -05:00
1754eb6e84 Add thread-safe Add/Del methods and refactor client locking
- Add Add and Del methods to Client for dynamic host management.
- Add RWMutex to Client to protect the devices map.
- Add Transport to Config to allow mocking HTTP transport in tests.
- Add getDeviceByHost helper to centralize device lookup locking.
- Refactor GetAll* methods to snapshot host keys before iteration to avoid concurrent map read/write panic.
- Add tests for thread safety and Add/Del functionality.
2026-01-04 13:56:19 -05:00
906d005edf improves concurrent GetAll operations 2026-01-04 13:49:28 -05:00
6 changed files with 385 additions and 127 deletions

24
CHANGELOG.md Normal file
View File

@@ -0,0 +1,24 @@
# Changelog
All notable changes to this project will be documented in this file.
## [v0.2.0] - 2026-01-04
### Added
- Thread-safe `Add` and `Del` methods to `Client` for dynamic host management.
- `RWMutex` to `Client` struct to protect `devices` map.
- `Transport` field to `Config` to allow mocking HTTP transport in tests.
- `getDeviceByHost` helper to centralize device lookup locking.
- Tests for thread safety and Add/Del functionality.
### Changed
- Refactored `GetAll*` methods to snapshot host keys before iteration to avoid concurrent map read/write panic.
- Improved concurrent `GetAll` operations.
## [v0.1.1] - 2026-01-04
### Fixed
- CI pipeline configuration.
## [v0.1.0] - 2026-01-04
### Added
- Initial CI pipeline setup.
- Initial release of Ubiquiti EdgeOS Go Client.

View File

@@ -2,15 +2,15 @@ package edgeos
import ( import (
"context" "context"
"fmt" "errors"
"sync" "sync"
) )
// GetInterfaces retrieves the interfaces for a specific device. // GetInterfaces retrieves the interfaces for a specific device.
func (c *Client) GetInterfaces(ctx context.Context, host string) ([]Interface, error) { func (c *Client) GetInterfaces(ctx context.Context, host string) ([]Interface, error) {
d, ok := c.devices[host] d, err := c.getDeviceByHost(host)
if !ok { if err != nil {
return nil, fmt.Errorf("device not found: %s", host) return nil, err
} }
var out []Interface var out []Interface
@@ -24,43 +24,42 @@ func (c *Client) GetInterfaces(ctx context.Context, host string) ([]Interface, e
// GetAllInterfaces retrieves interfaces for all devices. // GetAllInterfaces retrieves interfaces for all devices.
func (c *Client) GetAllInterfaces(ctx context.Context) (map[string][]Interface, error) { func (c *Client) GetAllInterfaces(ctx context.Context) (map[string][]Interface, error) {
results := make(map[string][]Interface) results := make(map[string][]Interface)
var mu sync.Mutex var (
var wg sync.WaitGroup mu sync.Mutex
wg sync.WaitGroup
// Use a buffered channel or just loop? errs error
// Since we return error if any fails? Or partial results? )
// Usually partial results + error or composite error.
// I will return partial results and the last error for now, or just stop on error?
// "methods to get ... for either all device"
// I will implement parallel fetch.
for host := range c.devices { c.mu.RLock()
wg.Add(1) hosts := make([]string, 0, len(c.devices))
go func(h string) { for h := range c.devices {
defer wg.Done() hosts = append(hosts, h)
res, err := c.GetInterfaces(ctx, h) }
c.mu.RUnlock()
for _, host := range hosts {
wg.Go(func() {
res, err := c.GetInterfaces(ctx, host)
if err != nil { if err != nil {
// For now, log error or ignore? mu.Lock()
// We should probably return an error map or just return what we have? errs = errors.Join(errs, err)
// I will just skip failed ones for this implementation or log? mu.Unlock()
// I'll return what succeeds.
// The prompt doesn't specify error handling strategy for "all".
return return
} }
mu.Lock() mu.Lock()
results[h] = res results[host] = res
mu.Unlock() mu.Unlock()
}(host) })
} }
wg.Wait() wg.Wait()
return results, nil return results, errs
} }
// GetDevice retrieves the device info for a specific device. // GetDevice retrieves the device info for a specific device.
func (c *Client) GetDevice(ctx context.Context, host string) (*Device, error) { func (c *Client) GetDevice(ctx context.Context, host string) (*Device, error) {
d, ok := c.devices[host] d, err := c.getDeviceByHost(host)
if !ok { if err != nil {
return nil, fmt.Errorf("device not found: %s", host) return nil, err
} }
var out Device var out Device
@@ -74,31 +73,42 @@ func (c *Client) GetDevice(ctx context.Context, host string) (*Device, error) {
// GetAllDevices retrieves device info for all devices. // GetAllDevices retrieves device info for all devices.
func (c *Client) GetAllDevices(ctx context.Context) (map[string]*Device, error) { func (c *Client) GetAllDevices(ctx context.Context) (map[string]*Device, error) {
results := make(map[string]*Device) results := make(map[string]*Device)
var mu sync.Mutex var (
var wg sync.WaitGroup mu sync.Mutex
wg sync.WaitGroup
errs error
)
for host := range c.devices { c.mu.RLock()
wg.Add(1) hosts := make([]string, 0, len(c.devices))
go func(h string) { for h := range c.devices {
defer wg.Done() hosts = append(hosts, h)
res, err := c.GetDevice(ctx, h) }
c.mu.RUnlock()
for _, host := range hosts {
wg.Go(func() {
res, err := c.GetDevice(ctx, host)
if err != nil { if err != nil {
mu.Lock()
errs = errors.Join(errs, err)
mu.Unlock()
return return
} }
mu.Lock() mu.Lock()
results[h] = res results[host] = res
mu.Unlock() mu.Unlock()
}(host) })
} }
wg.Wait() wg.Wait()
return results, nil return results, errs
} }
// GetSystem retrieves the system info for a specific device. // GetSystem retrieves the system info for a specific device.
func (c *Client) GetSystem(ctx context.Context, host string) (*System, error) { func (c *Client) GetSystem(ctx context.Context, host string) (*System, error) {
d, ok := c.devices[host] d, err := c.getDeviceByHost(host)
if !ok { if err != nil {
return nil, fmt.Errorf("device not found: %s", host) return nil, err
} }
var out System var out System
@@ -112,31 +122,42 @@ func (c *Client) GetSystem(ctx context.Context, host string) (*System, error) {
// GetAllSystems retrieves system info for all devices. // GetAllSystems retrieves system info for all devices.
func (c *Client) GetAllSystems(ctx context.Context) (map[string]*System, error) { func (c *Client) GetAllSystems(ctx context.Context) (map[string]*System, error) {
results := make(map[string]*System) results := make(map[string]*System)
var mu sync.Mutex var (
var wg sync.WaitGroup mu sync.Mutex
wg sync.WaitGroup
errs error
)
for host := range c.devices { c.mu.RLock()
wg.Add(1) hosts := make([]string, 0, len(c.devices))
go func(h string) { for h := range c.devices {
defer wg.Done() hosts = append(hosts, h)
res, err := c.GetSystem(ctx, h) }
c.mu.RUnlock()
for _, host := range hosts {
wg.Go(func() {
res, err := c.GetSystem(ctx, host)
if err != nil { if err != nil {
mu.Lock()
errs = errors.Join(errs, err)
mu.Unlock()
return return
} }
mu.Lock() mu.Lock()
results[h] = res results[host] = res
mu.Unlock() mu.Unlock()
}(host) })
} }
wg.Wait() wg.Wait()
return results, nil return results, errs
} }
// GetVLANs retrieves the VLANs for a specific device. // GetVLANs retrieves the VLANs for a specific device.
func (c *Client) GetVLANs(ctx context.Context, host string) (*VLANs, error) { func (c *Client) GetVLANs(ctx context.Context, host string) (*VLANs, error) {
d, ok := c.devices[host] d, err := c.getDeviceByHost(host)
if !ok { if err != nil {
return nil, fmt.Errorf("device not found: %s", host) return nil, err
} }
var out VLANs var out VLANs
@@ -150,31 +171,42 @@ func (c *Client) GetVLANs(ctx context.Context, host string) (*VLANs, error) {
// GetAllVLANs retrieves VLANs for all devices. // GetAllVLANs retrieves VLANs for all devices.
func (c *Client) GetAllVLANs(ctx context.Context) (map[string]*VLANs, error) { func (c *Client) GetAllVLANs(ctx context.Context) (map[string]*VLANs, error) {
results := make(map[string]*VLANs) results := make(map[string]*VLANs)
var mu sync.Mutex var (
var wg sync.WaitGroup mu sync.Mutex
wg sync.WaitGroup
errs error
)
for host := range c.devices { c.mu.RLock()
wg.Add(1) hosts := make([]string, 0, len(c.devices))
go func(h string) { for h := range c.devices {
defer wg.Done() hosts = append(hosts, h)
res, err := c.GetVLANs(ctx, h) }
c.mu.RUnlock()
for _, host := range hosts {
wg.Go(func() {
res, err := c.GetVLANs(ctx, host)
if err != nil { if err != nil {
mu.Lock()
errs = errors.Join(errs, err)
mu.Unlock()
return return
} }
mu.Lock() mu.Lock()
results[h] = res results[host] = res
mu.Unlock() mu.Unlock()
}(host) })
} }
wg.Wait() wg.Wait()
return results, nil return results, errs
} }
// GetServices retrieves the services for a specific device. // GetServices retrieves the services for a specific device.
func (c *Client) GetServices(ctx context.Context, host string) (*Services, error) { func (c *Client) GetServices(ctx context.Context, host string) (*Services, error) {
d, ok := c.devices[host] d, err := c.getDeviceByHost(host)
if !ok { if err != nil {
return nil, fmt.Errorf("device not found: %s", host) return nil, err
} }
var out Services var out Services
@@ -188,31 +220,42 @@ func (c *Client) GetServices(ctx context.Context, host string) (*Services, error
// GetAllServices retrieves services for all devices. // GetAllServices retrieves services for all devices.
func (c *Client) GetAllServices(ctx context.Context) (map[string]*Services, error) { func (c *Client) GetAllServices(ctx context.Context) (map[string]*Services, error) {
results := make(map[string]*Services) results := make(map[string]*Services)
var mu sync.Mutex var (
var wg sync.WaitGroup mu sync.Mutex
wg sync.WaitGroup
errs error
)
for host := range c.devices { c.mu.RLock()
wg.Add(1) hosts := make([]string, 0, len(c.devices))
go func(h string) { for h := range c.devices {
defer wg.Done() hosts = append(hosts, h)
res, err := c.GetServices(ctx, h) }
c.mu.RUnlock()
for _, host := range hosts {
wg.Go(func() {
res, err := c.GetServices(ctx, host)
if err != nil { if err != nil {
mu.Lock()
errs = errors.Join(errs, err)
mu.Unlock()
return return
} }
mu.Lock() mu.Lock()
results[h] = res results[host] = res
mu.Unlock() mu.Unlock()
}(host) })
} }
wg.Wait() wg.Wait()
return results, nil return results, errs
} }
// GetStatistics retrieves the statistics for a specific device. // GetStatistics retrieves the statistics for a specific device.
func (c *Client) GetStatistics(ctx context.Context, host string) ([]Statistics, error) { func (c *Client) GetStatistics(ctx context.Context, host string) ([]Statistics, error) {
d, ok := c.devices[host] d, err := c.getDeviceByHost(host)
if !ok { if err != nil {
return nil, fmt.Errorf("device not found: %s", host) return nil, err
} }
var out []Statistics var out []Statistics
@@ -226,31 +269,42 @@ func (c *Client) GetStatistics(ctx context.Context, host string) ([]Statistics,
// GetAllStatistics retrieves statistics for all devices. // GetAllStatistics retrieves statistics for all devices.
func (c *Client) GetAllStatistics(ctx context.Context) (map[string][]Statistics, error) { func (c *Client) GetAllStatistics(ctx context.Context) (map[string][]Statistics, error) {
results := make(map[string][]Statistics) results := make(map[string][]Statistics)
var mu sync.Mutex var (
var wg sync.WaitGroup mu sync.Mutex
wg sync.WaitGroup
errs error
)
for host := range c.devices { c.mu.RLock()
wg.Add(1) hosts := make([]string, 0, len(c.devices))
go func(h string) { for h := range c.devices {
defer wg.Done() hosts = append(hosts, h)
res, err := c.GetStatistics(ctx, h) }
c.mu.RUnlock()
for _, host := range hosts {
wg.Go(func() {
res, err := c.GetStatistics(ctx, host)
if err != nil { if err != nil {
mu.Lock()
errs = errors.Join(errs, err)
mu.Unlock()
return return
} }
mu.Lock() mu.Lock()
results[h] = res results[host] = res
mu.Unlock() mu.Unlock()
}(host) })
} }
wg.Wait() wg.Wait()
return results, nil return results, errs
} }
// GetNeighbors retrieves the neighbors for a specific device. // GetNeighbors retrieves the neighbors for a specific device.
func (c *Client) GetNeighbors(ctx context.Context, host string) ([]Neighbor, error) { func (c *Client) GetNeighbors(ctx context.Context, host string) ([]Neighbor, error) {
d, ok := c.devices[host] d, err := c.getDeviceByHost(host)
if !ok { if err != nil {
return nil, fmt.Errorf("device not found: %s", host) return nil, err
} }
var out []Neighbor var out []Neighbor
@@ -264,22 +318,33 @@ func (c *Client) GetNeighbors(ctx context.Context, host string) ([]Neighbor, err
// GetAllNeighbors retrieves neighbors for all devices. // GetAllNeighbors retrieves neighbors for all devices.
func (c *Client) GetAllNeighbors(ctx context.Context) (map[string][]Neighbor, error) { func (c *Client) GetAllNeighbors(ctx context.Context) (map[string][]Neighbor, error) {
results := make(map[string][]Neighbor) results := make(map[string][]Neighbor)
var mu sync.Mutex var (
var wg sync.WaitGroup mu sync.Mutex
wg sync.WaitGroup
errs error
)
for host := range c.devices { c.mu.RLock()
wg.Add(1) hosts := make([]string, 0, len(c.devices))
go func(h string) { for h := range c.devices {
defer wg.Done() hosts = append(hosts, h)
res, err := c.GetNeighbors(ctx, h) }
c.mu.RUnlock()
for _, host := range hosts {
wg.Go(func() {
res, err := c.GetNeighbors(ctx, host)
if err != nil { if err != nil {
mu.Lock()
errs = errors.Join(errs, err)
mu.Unlock()
return return
} }
mu.Lock() mu.Lock()
results[h] = res results[host] = res
mu.Unlock() mu.Unlock()
}(host) })
} }
wg.Wait() wg.Wait()
return results, nil return results, errs
} }

View File

@@ -21,6 +21,7 @@ import (
// Client handles communication with EdgeOS devices. // Client handles communication with EdgeOS devices.
type Client struct { type Client struct {
mu sync.RWMutex
devices map[string]*deviceClient devices map[string]*deviceClient
} }
@@ -31,33 +32,42 @@ type deviceClient struct {
mu sync.Mutex mu sync.Mutex
} }
func newDeviceClient(cfg Config) *deviceClient {
// Ensure scheme is set
if cfg.Scheme == "" {
cfg.Scheme = "https"
}
var tr http.RoundTripper
if cfg.Transport != nil {
tr = cfg.Transport
} else {
defaultTr := http.DefaultTransport.(*http.Transport).Clone()
if defaultTr.TLSClientConfig == nil {
defaultTr.TLSClientConfig = &tls.Config{}
}
defaultTr.TLSClientConfig.InsecureSkipVerify = cfg.Insecure
tr = defaultTr
}
client := &http.Client{
Transport: tr,
Timeout: cfg.Timeout,
}
return &deviceClient{
config: cfg,
client: client,
}
}
// MustNew creates a new Client with the given configurations. // MustNew creates a new Client with the given configurations.
// It panics if a configuration is invalid (though currently we just accept all). // It panics if a configuration is invalid (though currently we just accept all).
func MustNew(ctx context.Context, configs []Config) *Client { func MustNew(ctx context.Context, configs []Config) *Client {
devices := make(map[string]*deviceClient) devices := make(map[string]*deviceClient)
for _, cfg := range configs { for _, cfg := range configs {
// Use Host as the key. devices[cfg.Host] = newDeviceClient(cfg)
// Ensure scheme is set
if cfg.Scheme == "" {
cfg.Scheme = "https"
}
tr := http.DefaultTransport.(*http.Transport).Clone()
if tr.TLSClientConfig == nil {
tr.TLSClientConfig = &tls.Config{}
}
tr.TLSClientConfig.InsecureSkipVerify = cfg.Insecure
client := &http.Client{
Transport: tr,
Timeout: cfg.Timeout,
}
devices[cfg.Host] = &deviceClient{
config: cfg,
client: client,
}
} }
return &Client{ return &Client{
@@ -65,6 +75,51 @@ func MustNew(ctx context.Context, configs []Config) *Client {
} }
} }
// Add adds a new device to the client.
// It returns an error if a device with the same host already exists.
func (c *Client) Add(cfg *Config) error {
if cfg == nil {
return fmt.Errorf("config cannot be nil")
}
d := newDeviceClient(*cfg)
c.mu.Lock()
defer c.mu.Unlock()
if _, ok := c.devices[cfg.Host]; ok {
return fmt.Errorf("device already exists: %s", cfg.Host)
}
c.devices[cfg.Host] = d
return nil
}
// Del removes a device from the client.
// It returns an error if the device does not exist.
func (c *Client) Del(host string) error {
c.mu.Lock()
defer c.mu.Unlock()
if _, ok := c.devices[host]; !ok {
return fmt.Errorf("device not found: %s", host)
}
delete(c.devices, host)
return nil
}
func (c *Client) getDeviceByHost(host string) (*deviceClient, error) {
c.mu.RLock()
defer c.mu.RUnlock()
d, ok := c.devices[host]
if !ok {
return nil, fmt.Errorf("device not found: %s", host)
}
return d, nil
}
func (d *deviceClient) login(ctx context.Context) error { func (d *deviceClient) login(ctx context.Context) error {
d.mu.Lock() d.mu.Lock()
defer d.mu.Unlock() defer d.mu.Unlock()

109
pkg/edgeos/client_test.go Normal file
View File

@@ -0,0 +1,109 @@
package edgeos
import (
"bytes"
"context"
"fmt"
"io"
"net/http"
"sync"
"testing"
)
type mockTransport struct {
RoundTripFunc func(req *http.Request) (*http.Response, error)
}
func (m *mockTransport) RoundTrip(req *http.Request) (*http.Response, error) {
if m.RoundTripFunc != nil {
return m.RoundTripFunc(req)
}
// Default mock response
return &http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString("{}")),
Header: make(http.Header),
}, nil
}
func TestClient_ThreadSafety(t *testing.T) {
ctx := context.Background()
client := MustNew(ctx, []Config{})
var wg sync.WaitGroup
start := make(chan struct{})
// Writer: Adds and deletes hosts
wg.Add(1)
go func() {
defer wg.Done()
<-start
for i := 0; i < 100; i++ {
host := fmt.Sprintf("host-%d", i)
cfg := &Config{
Host: host,
Transport: &mockTransport{},
}
if err := client.Add(cfg); err != nil {
// verify we don't error on valid add
t.Logf("Add error: %v", err)
}
// We invoke Del immediately.
if err := client.Del(host); err != nil {
t.Logf("Del error: %v", err)
}
}
}()
// Reader: Iterates hosts
wg.Add(1)
go func() {
defer wg.Done()
<-start
for i := 0; i < 10; i++ {
// GetAllInterfaces iterates keys.
// With mock transport, this will succeed (returning empty structs)
// checking for race conditions.
_, _ = client.GetAllInterfaces(ctx)
}
}()
close(start)
wg.Wait()
}
func TestClient_AddDel(t *testing.T) {
ctx := context.Background()
client := MustNew(ctx, []Config{})
cfg := &Config{
Host: "test-host",
Transport: &mockTransport{},
}
if err := client.Add(cfg); err != nil {
t.Fatalf("Add failed: %v", err)
}
if err := client.Add(cfg); err == nil {
t.Fatal("Expected error adding duplicate host, got nil")
}
// Verify we can retrieve it
// Mock transport returns 200 OK with empty body, so GetInterfaces should return empty slice (or error decoding if empty body is not valid JSON array? actually "{}" is valid object, but GetInterfaces expects array for /interfaces?)
// Let's check api.go: GetInterfaces calls /interfaces.
// We can customize the mock if we want to test success return.
// For this test, we just care that it doesn't return "device not found".
_, err := client.GetInterfaces(ctx, "test-host")
if err != nil && err.Error() == "device not found: test-host" {
t.Fatal("Device should exist")
}
if err := client.Del("test-host"); err != nil {
t.Fatalf("Del failed: %v", err)
}
if err := client.Del("test-host"); err == nil {
t.Fatal("Expected error deleting non-existent host, got nil")
}
}

View File

@@ -1,6 +1,9 @@
package edgeos package edgeos
import "time" import (
"net/http"
"time"
)
// Config represents the configuration for an EdgeOS device. // Config represents the configuration for an EdgeOS device.
type Config struct { type Config struct {
@@ -10,4 +13,6 @@ type Config struct {
Username string Username string
Password string Password string
Timeout time.Duration Timeout time.Duration
// Transport allows customizing the http transport (useful for testing)
Transport http.RoundTripper
} }

View File

@@ -361,4 +361,4 @@ type Neighbor struct {
IP string `json:"ip"` IP string `json:"ip"`
ZoneID string `json:"zoneID"` ZoneID string `json:"zoneID"`
Addresses []NeighborAddress `json:"addresses"` Addresses []NeighborAddress `json:"addresses"`
} }