Skip to content

Commit

Permalink
cleanup and refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
@mircea-pavel-anton
mircea-pavel-anton committed Oct 26, 2024
1 parent e1e2a81 commit 6d9b37d
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 45 deletions.
2 changes: 1 addition & 1 deletion internal/dnsprovider/dnsprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func Init(config configuration.Config) (provider.Provider, error) {
}
log.Info(createMsg)

mikrotikConfig := mikrotik.Config{}
mikrotikConfig := mikrotik.MikrotikConnectionConfig{}
if err := env.Parse(&mikrotikConfig); err != nil {
return nil, fmt.Errorf("reading mikrotik configuration failed: %v", err)
}
Expand Down
22 changes: 11 additions & 11 deletions internal/mikrotik/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import (
"sigs.k8s.io/external-dns/endpoint"
)

// Config holds the connection details for the API client
type Config struct {
// MikrotikConnectionConfig holds the connection details for the API client
type MikrotikConnectionConfig struct {
BaseUrl string `env:"MIKROTIK_BASEURL,notEmpty"`
Username string `env:"MIKROTIK_USERNAME,notEmpty"`
Password string `env:"MIKROTIK_PASSWORD,notEmpty"`
Expand All @@ -27,13 +27,13 @@ type Config struct {

// MikrotikApiClient encapsulates the client configuration and HTTP client
type MikrotikApiClient struct {
*Config
*MikrotikConnectionConfig
*http.Client
}

// SystemInfo represents MikroTik system information
// MikrotikSystemInfo represents MikroTik system information
// https://help.mikrotik.com/docs/display/ROS/Resource
type SystemInfo struct {
type MikrotikSystemInfo struct {
ArchitectureName string `json:"architecture-name"`
BadBlocks string `json:"bad-blocks"`
BoardName string `json:"board-name"`
Expand All @@ -55,7 +55,7 @@ type SystemInfo struct {
}

// NewMikrotikClient creates a new instance of MikrotikApiClient
func NewMikrotikClient(config *Config) (*MikrotikApiClient, error) {
func NewMikrotikClient(config *MikrotikConnectionConfig) (*MikrotikApiClient, error) {
log.Infof("creating a new Mikrotik API Client")

jar, err := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List})
Expand All @@ -65,7 +65,7 @@ func NewMikrotikClient(config *Config) (*MikrotikApiClient, error) {
}

client := &MikrotikApiClient{
Config: config,
MikrotikConnectionConfig: config,
Client: &http.Client{
Transport: &http.Transport{
TLSClientConfig: &tls.Config{
Expand All @@ -80,7 +80,7 @@ func NewMikrotikClient(config *Config) (*MikrotikApiClient, error) {
}

// GetSystemInfo fetches system information from the MikroTik API
func (c *MikrotikApiClient) GetSystemInfo() (*SystemInfo, error) {
func (c *MikrotikApiClient) GetSystemInfo() (*MikrotikSystemInfo, error) {
log.Debugf("fetching system information.")

// Send the request
Expand All @@ -92,7 +92,7 @@ func (c *MikrotikApiClient) GetSystemInfo() (*SystemInfo, error) {
defer resp.Body.Close()

// Parse the response
var info SystemInfo
var info MikrotikSystemInfo
if err = json.NewDecoder(resp.Body).Decode(&info); err != nil {
log.Errorf("error decoding response body: %v", err)
return nil, err
Expand Down Expand Up @@ -218,7 +218,7 @@ func (c *MikrotikApiClient) lookupDNSRecord(key, recordType string) (*DNSRecord,

// doRequest sends an HTTP request to the MikroTik API with credentials
func (c *MikrotikApiClient) doRequest(method, path string, body io.Reader) (*http.Response, error) {
endpoint_url := fmt.Sprintf("%s/rest/%s", c.Config.BaseUrl, path)
endpoint_url := fmt.Sprintf("%s/rest/%s", c.MikrotikConnectionConfig.BaseUrl, path)
log.Debugf("sending %s request to: %s", method, endpoint_url)

req, err := http.NewRequest(method, endpoint_url, body)
Expand All @@ -227,7 +227,7 @@ func (c *MikrotikApiClient) doRequest(method, path string, body io.Reader) (*htt
return nil, err
}

req.SetBasicAuth(c.Config.Username, c.Config.Password)
req.SetBasicAuth(c.MikrotikConnectionConfig.Username, c.MikrotikConnectionConfig.Password)

resp, err := c.Client.Do(req)
if err != nil {
Expand Down
56 changes: 24 additions & 32 deletions internal/mikrotik/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ var (
)

func TestNewMikrotikClient(t *testing.T) {
config := &Config{
config := &MikrotikConnectionConfig{
BaseUrl: "https://192.168.88.1:443",
Username: "admin",
Password: "password",
Expand All @@ -29,8 +29,8 @@ func TestNewMikrotikClient(t *testing.T) {
t.Fatalf("Expected no error, got %v", err)
}

if client.Config != config {
t.Errorf("Expected config to be %v, got %v", config, client.Config)
if client.MikrotikConnectionConfig != config {
t.Errorf("Expected config to be %v, got %v", config, client.MikrotikConnectionConfig)
}

if client.Client == nil {
Expand All @@ -50,7 +50,7 @@ func TestNewMikrotikClient(t *testing.T) {
}

func TestGetSystemInfo(t *testing.T) {
mockServerInfo := SystemInfo{
mockServerInfo := MikrotikSystemInfo{
ArchitectureName: "arm64",
BadBlocks: "0.1",
BoardName: "RB5009UG+S+",
Expand All @@ -71,7 +71,7 @@ func TestGetSystemInfo(t *testing.T) {
WriteSectTotal: "131658",
}

// Set up your mock server
// Set up mock server
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Validate the Basic Auth header
username, password, ok := r.BasicAuth()
Expand Down Expand Up @@ -102,12 +102,12 @@ func TestGetSystemInfo(t *testing.T) {
// Define test cases
testCases := []struct {
name string
config Config
config MikrotikConnectionConfig
expectedError bool
}{
{
name: "Valid credentials",
config: Config{
config: MikrotikConnectionConfig{
BaseUrl: server.URL,
Username: mockUsername,
Password: mockPassword,
Expand All @@ -117,7 +117,7 @@ func TestGetSystemInfo(t *testing.T) {
},
{
name: "Incorrect password",
config: Config{
config: MikrotikConnectionConfig{
BaseUrl: server.URL,
Username: mockUsername,
Password: "wrongpass",
Expand All @@ -127,7 +127,7 @@ func TestGetSystemInfo(t *testing.T) {
},
{
name: "Incorrect username",
config: Config{
config: MikrotikConnectionConfig{
BaseUrl: server.URL,
Username: "wronguser",
Password: mockPassword,
Expand All @@ -137,7 +137,7 @@ func TestGetSystemInfo(t *testing.T) {
},
{
name: "Incorrect username and password",
config: Config{
config: MikrotikConnectionConfig{
BaseUrl: server.URL,
Username: "wronguser",
Password: "wrongpass",
Expand All @@ -147,7 +147,7 @@ func TestGetSystemInfo(t *testing.T) {
},
{
name: "Missing credentials",
config: Config{
config: MikrotikConnectionConfig{
BaseUrl: server.URL,
Username: "",
Password: "",
Expand All @@ -166,8 +166,6 @@ func TestGetSystemInfo(t *testing.T) {
if err != nil {
t.Fatalf("Failed to create client: %v", err)
}

// Call GetSystemInfo
info, err := client.GetSystemInfo()

if tc.expectedError {
Expand All @@ -187,13 +185,13 @@ func TestGetSystemInfo(t *testing.T) {
if info.Version != mockServerInfo.Version {
t.Errorf("Expected Version %s, got %s", mockServerInfo.Version, info.Version)
}
// i think there's no point in checking any more fields
}
})
}
}

func TestCreateDNSRecord(t *testing.T) {
// Define test cases
testCases := []struct {
name string
initialRecords map[string]DNSRecord
Expand Down Expand Up @@ -267,14 +265,13 @@ func TestCreateDNSRecord(t *testing.T) {
},
expectedError: true,
},

{
name: "Empty target for A record",
initialRecords: map[string]DNSRecord{},
endpoint: &endpoint.Endpoint{
DNSName: "empty-target.example.com",
RecordType: "A",
Targets: endpoint.Targets{""}, // Empty target
Targets: endpoint.Targets{""},
},
expectedError: true,
},
Expand All @@ -284,7 +281,7 @@ func TestCreateDNSRecord(t *testing.T) {
endpoint: &endpoint.Endpoint{
DNSName: "malformed-ip.example.com",
RecordType: "A",
Targets: endpoint.Targets{"999.999.999.999"}, // Invalid IP
Targets: endpoint.Targets{"999.999.999.999"},
},
expectedError: true,
},
Expand All @@ -294,7 +291,7 @@ func TestCreateDNSRecord(t *testing.T) {
endpoint: &endpoint.Endpoint{
DNSName: "malformed-ipv6.example.com",
RecordType: "AAAA",
Targets: endpoint.Targets{"gggg::1"}, // Invalid IPv6
Targets: endpoint.Targets{"gggg::1"},
},
expectedError: true,
},
Expand All @@ -304,7 +301,7 @@ func TestCreateDNSRecord(t *testing.T) {
endpoint: &endpoint.Endpoint{
DNSName: "empty-cname.example.com",
RecordType: "CNAME",
Targets: endpoint.Targets{""}, // Empty target
Targets: endpoint.Targets{""},
},
expectedError: true,
},
Expand All @@ -324,7 +321,7 @@ func TestCreateDNSRecord(t *testing.T) {
endpoint: &endpoint.Endpoint{
DNSName: "empty-txt.example.com",
RecordType: "TXT",
Targets: endpoint.Targets{""}, // Empty text
Targets: endpoint.Targets{""},
},
expectedError: true,
},
Expand All @@ -350,7 +347,7 @@ func TestCreateDNSRecord(t *testing.T) {
recordStore[k] = v
}

// Set up your mock server
// Set up mock server
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Validate the Basic Auth header
username, password, ok := r.BasicAuth()
Expand Down Expand Up @@ -393,7 +390,7 @@ func TestCreateDNSRecord(t *testing.T) {
defer server.Close()

// Set up the client with correct credentials
config := &Config{
config := &MikrotikConnectionConfig{
BaseUrl: server.URL,
Username: mockUsername,
Password: mockPassword,
Expand All @@ -404,16 +401,13 @@ func TestCreateDNSRecord(t *testing.T) {
if err != nil {
t.Fatalf("Failed to create client: %v", err)
}

// Call CreateDNSRecord
record, err := client.CreateDNSRecord(tc.endpoint)

if tc.expectedError {
if err == nil {
t.Fatalf("Expected error, got none")
}
// Optionally, check for specific error messages
return // Error was expected and occurred, test passes
return
}

if err != nil {
Expand Down Expand Up @@ -561,7 +555,7 @@ func TestDeleteDNSRecord(t *testing.T) {
recordStore[k] = v
}

// Set up your mock server
// Set up mock server
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
username, password, ok := r.BasicAuth()
if !ok || username != mockUsername || password != mockPassword {
Expand Down Expand Up @@ -610,7 +604,7 @@ func TestDeleteDNSRecord(t *testing.T) {
}))
defer server.Close()

config := &Config{
config := &MikrotikConnectionConfig{
BaseUrl: server.URL,
Username: mockUsername,
Password: mockPassword,
Expand Down Expand Up @@ -700,7 +694,6 @@ func TestGetAllDNSRecords(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
// Set up the mock server for this test case
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Basic Auth validation
username, password, ok := r.BasicAuth()
Expand All @@ -725,7 +718,7 @@ func TestGetAllDNSRecords(t *testing.T) {
defer server.Close()

// Set up the client
config := &Config{
config := &MikrotikConnectionConfig{
BaseUrl: server.URL,
Username: mockUsername,
Password: mockPassword,
Expand All @@ -735,9 +728,8 @@ func TestGetAllDNSRecords(t *testing.T) {
if err != nil {
t.Fatalf("Failed to create client: %v", err)
}

// Call GetAllDNSRecords
records, err := client.GetAllDNSRecords()

if tc.expectError {
if err == nil {
t.Fatalf("Expected error, got none")
Expand Down
2 changes: 1 addition & 1 deletion internal/mikrotik/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type MikrotikProvider struct {
}

// NewMikrotikProvider initializes a new DNSProvider, of the Mikrotik variety
func NewMikrotikProvider(domainFilter endpoint.DomainFilter, config *Config) (provider.Provider, error) {
func NewMikrotikProvider(domainFilter endpoint.DomainFilter, config *MikrotikConnectionConfig) (provider.Provider, error) {
// Create the Mikrotik API Client
client, err := NewMikrotikClient(config)
if err != nil {
Expand Down

0 comments on commit 6d9b37d

Please sign in to comment.