Merge branch 'master' of https://github.com/DawoudMahmud/terraform-pr…

…ovider-mikrotik

README.md

@@ -58,6 +58,9 @@ $ export TF_CLI_CONFIG_FILE=path/to/custom.tfrc
 - RouterOS. See which versions are supported by what is tested in [CI](.github/workflows/continuous-integration.yml)
 - Terraform 0.12+
 
+
+For code generation of boilerplate code, see [codegen Readme](./cmd/mikrotik-codegen/internal/codegen/README.md)
+
 ### Testing
 
 The provider is tested with Terraform's acceptance testing framework. As long as you have a RouterOS device you should be able to run them. Please be aware it will create resources on your device! Code that is accepted by the project will not be destructive for anything existing on your router but be careful when changing test code!
@@ -113,5 +116,5 @@ $ make routeros ROUTEROS_VERSION=latest
 
 To cleanup everything, just run:
 ```sh
-$ make routeros clean
+$ make routeros-clean
 ```

client/bgp_instance_test.go

@@ -3,6 +3,8 @@ package client
 import (
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 var bgpName string = "test-bgp"
@@ -120,7 +122,6 @@ func TestFindBgpInstance_onNonExistantBgpInstance(t *testing.T) {
 	name := "bgp instance does not exist"
 	_, err := c.FindBgpInstance(name)
 
-	if _, ok := err.(*NotFound); !ok {
-		t.Errorf("Expecting to receive NotFound error for bgp instance `%s`, instead error was nil.", name)
-	}
+	require.Truef(t, IsNotFoundError(err),
+		"Expecting to receive NotFound error for bgp instance %q", name)
 }

client/bgp_peer_test.go

@@ -3,6 +3,8 @@ package client
 import (
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 // required BGP Peer fields
@@ -129,7 +131,6 @@ func TestFindBgpPeer_onNonExistantBgpPeer(t *testing.T) {
 	name := "bgp peer does not exist"
 	_, err := c.FindBgpPeer(name)
 
-	if _, ok := err.(*NotFound); !ok {
-		t.Errorf("Expecting to receive NotFound error for bgp peer `%s`, instead error was nil.", name)
-	}
+	require.Truef(t, IsNotFoundError(err),
+		"Expecting to receive NotFound error for bgp peer %q.", name)
 }

client/bridge_port_test.go

@@ -1,9 +1,10 @@
 package client
 
 import (
-	"errors"
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestBridgePort_basic(t *testing.T) {
@@ -25,19 +26,14 @@ func TestBridgePort_basic(t *testing.T) {
 		Bridge:    bridge.Name,
 		Interface: "*0",
 	})
-	if err != nil {
-		t.Fatal(err)
-		return
-	}
+	require.NoError(t, err)
+
 	defer func() {
-		if err := c.DeleteBridgePort(bridgePort.Id); err != nil {
-			t.Error(err)
+		c.DeleteBridgePort(bridgePort.Id)
+		require.NoError(t, err)
 
-		}
-		expected := &NotFound{}
-		if _, err := c.FindBridgePort(bridgePort.Id); err == nil || !errors.As(err, &expected) {
-			t.Error(err)
-		}
+		_, err = c.FindBridgePort(bridgePort.Id)
+		require.True(t, IsNotFoundError(err), "expected to get NotFound error")
 	}()
 
 	expected := &BridgePort{

client/client_crud.go

@@ -63,7 +63,7 @@ type (
 )
 
 // Add creates new resource on remote system
-func (client Mikrotik) Add(d Resource) (interface{}, error) {
+func (client Mikrotik) Add(d Resource) (Resource, error) {
 	c, err := client.getMikrotikClient()
 	if err != nil {
 		return nil, err
@@ -84,7 +84,7 @@ func (client Mikrotik) Add(d Resource) (interface{}, error) {
 }
 
 // Find retrieves resource from remote system
-func (client Mikrotik) Find(d Resource) (interface{}, error) {
+func (client Mikrotik) Find(d Resource) (Resource, error) {
 	findField := d.IDField()
 	findFieldValue := d.ID()
 	if finder, ok := d.(Finder); ok {
@@ -95,7 +95,7 @@ func (client Mikrotik) Find(d Resource) (interface{}, error) {
 }
 
 // Update updates existing resource on remote system
-func (client Mikrotik) Update(resource Resource) (interface{}, error) {
+func (client Mikrotik) Update(resource Resource) (Resource, error) {
 	c, err := client.getMikrotikClient()
 	if err != nil {
 		return nil, err
@@ -132,7 +132,7 @@ func (client Mikrotik) Delete(d Resource) error {
 	return err
 }
 
-func (client Mikrotik) findByField(d Resource, field, value string) (interface{}, error) {
+func (client Mikrotik) findByField(d Resource, field, value string) (Resource, error) {
 	cmd := []string{d.ActionToCommand(Find), "?" + field + "=" + value}
 	log.Printf("[INFO] Running the mikrotik command: `%s`", cmd)
 
@@ -152,11 +152,13 @@ func (client Mikrotik) findByField(d Resource, field, value string) (interface{}
 	if err != nil {
 		return nil, err
 	}
-	if targetStructInterface.(Resource).ID() == "" {
+	// assertion is not checked as we are creating the targetStruct from 'd' argument which satisfies Resource interface
+	targetResource := targetStructInterface.(Resource)
+	if targetResource.ID() == "" {
 		return nil, NewNotFound(fmt.Sprintf("resource `%T` with field `%s=%s` not found", targetStruct, field, value))
 	}
 
-	return targetStructInterface, nil
+	return targetResource, nil
 }
 
 func (client Mikrotik) newTargetStruct(d interface{}) reflect.Value {

client/dns_test.go

@@ -1,9 +1,10 @@
 package client
 
 import (
-	"errors"
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestFindDnsRecord_onNonExistantDnsRecord(t *testing.T) {
@@ -12,9 +13,8 @@ func TestFindDnsRecord_onNonExistantDnsRecord(t *testing.T) {
 	name := "dns record does not exist"
 	_, err := c.FindDnsRecord(name)
 
-	if _, ok := err.(*NotFound); !ok {
-		t.Errorf("Expecting to receive NotFound error for dns record `%s`, instead error was nil.", name)
-	}
+	require.Truef(t, IsNotFoundError(err),
+		"Expecting to receive NotFound error for dns record %q", name)
 }
 
 func TestAddFindDeleteDnsRecord(t *testing.T) {
@@ -55,13 +55,8 @@ func TestAddFindDeleteDnsRecord(t *testing.T) {
 	}
 
 	_, err = c.Find(findRecord)
-	if err == nil {
-		t.Errorf("expected error, got nothing")
-		return
-	}
+	require.Error(t, err)
 
-	target := &NotFound{}
-	if !errors.As(err, &target) {
-		t.Errorf("expected error to be of type %T, got %T", &NotFound{}, err)
-	}
+	require.True(t, IsNotFoundError(err),
+		"expected to get NotFound error")
 }

client/errors.go

@@ -1,13 +1,22 @@
 package client
 
+import "errors"
+
 type NotFound struct {
 	s string
 }
 
 func NewNotFound(text string) error {
-	return &NotFound{text}
+	return NotFound{text}
 }
 
-func (e *NotFound) Error() string {
+func (e NotFound) Error() string {
 	return e.s
 }
+
+func IsNotFoundError(err error) bool {
+	var e NotFound
+	var ePtr *NotFound
+
+	return errors.As(err, &e) || errors.As(err, &ePtr)
+}

client/errors_test.go

@@ -0,0 +1,54 @@
+package client
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestIsNotFoundError(t *testing.T) {
+
+	testCases := []struct {
+		name     string
+		err      error
+		expected bool
+	}{
+		{
+			name:     "nil error",
+			expected: false,
+		},
+		{
+			name:     "created via NewNotFoundError()",
+			err:      NewNotFound("not found"),
+			expected: true,
+		},
+		{
+			name:     "created directly via struct initialization",
+			err:      NotFound{},
+			expected: true,
+		},
+		{
+			name:     "chained with other errors",
+			err:      fmt.Errorf("cannot load object info: %w", NewNotFound("no such object")),
+			expected: true,
+		},
+		{
+			name:     "chain of non-matching errors",
+			err:      fmt.Errorf("cannot load object info: %w", errors.New("no such object")),
+			expected: false,
+		},
+		{
+			name:     "generic error",
+			err:      errors.New("no such object"),
+			expected: false,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := IsNotFoundError(tc.err)
+			require.Equal(t, tc.expected, result)
+		})
+	}
+}

client/interface_wireguard_test.go

@@ -1,9 +1,10 @@
 package client
 
 import (
-	"errors"
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestFindInterfaceWireguard_onNonExistantInterfaceWireguard(t *testing.T) {
@@ -13,9 +14,8 @@ func TestFindInterfaceWireguard_onNonExistantInterfaceWireguard(t *testing.T) {
 	name := "Interface wireguard does not exist"
 	_, err := c.FindInterfaceWireguard(name)
 
-	if _, ok := err.(*NotFound); !ok {
-		t.Errorf("Expecting to receive NotFound error for Interface wireguard `%s`, instead error was nil.", name)
-	}
+	require.Truef(t, IsNotFoundError(err),
+		"Expecting to receive NotFound error for Interface wireguard %q.", name)
 }
 
 func TestAddFindDeleteInterfaceWireguard(t *testing.T) {
@@ -39,14 +39,12 @@ func TestAddFindDeleteInterfaceWireguard(t *testing.T) {
 	}
 	defer func() {
 		err = c.Delete(interfaceWireguard)
-		if err != nil {
-			t.Errorf("expected no error, got %v", err)
-		}
-		expected := &NotFound{}
-		if _, err := c.Find(interfaceWireguard); err == nil || !errors.As(err, &expected) {
-			t.Error(err)
-		}
+		require.NoError(t, err)
+
+		_, err := c.Find(interfaceWireguard)
+		require.True(t, IsNotFoundError(err), "expected to get NotFound error")
 	}()
+
 	findInterface := &InterfaceWireguard{}
 	findInterface.Name = name
 	found, err := c.Find(findInterface)

client/pool_test.go

@@ -3,6 +3,8 @@ package client
 import (
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 var name string = "testacc"
@@ -55,9 +57,7 @@ func TestFindPool_forNonExistingPool(t *testing.T) {
 	poolId := "Invalid id"
 	_, err := c.FindPool(poolId)
 
-	if _, ok := err.(*NotFound); !ok {
-		t.Errorf("client should have NotFound error error but instead received '%v'", err)
-	}
+	require.Truef(t, IsNotFoundError(err), "client should have NotFound error error but instead received")
 }
 
 func TestFindPoolByName_forExistingPool(t *testing.T) {
@@ -86,7 +86,6 @@ func TestFindPoolByName_forNonExistingPool(t *testing.T) {
 	poolName := "Invalid name"
 	_, err := c.FindPoolByName(poolName)
 
-	if _, ok := err.(*NotFound); !ok {
-		t.Errorf("client should have NotFound error error but instead received '%v'", err)
-	}
+	require.True(t, IsNotFoundError(err),
+		"client should have NotFound error")
 }

client/scheduler_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/ddelnano/terraform-provider-mikrotik/client/types"
+	"github.com/stretchr/testify/require"
 )
 
 func TestCreateUpdateDeleteAndFindScheduler(t *testing.T) {
@@ -53,7 +54,6 @@ func TestFindScheduler_onNonExistantScript(t *testing.T) {
 	name := "scheduler does not exist"
 	_, err := c.FindScheduler(name)
 
-	if _, ok := err.(*NotFound); !ok {
-		t.Errorf("Expecting to receive NotFound error for scheduler `%s`, instead error was nil.", name)
-	}
+	require.Truef(t, IsNotFoundError(err),
+		"Expecting to receive NotFound error for scheduler %q.", name)
 }

cmd/mikrotik-codegen/internal/codegen/README.md

@@ -1,9 +1,20 @@
 MikroTik code generation
 ========================
 
-This tool allows generating MikroTik resources based on client's struct definition.
+This tool allows generating MikroTik resources for API client and Terraform resources based on Mikrotik struct definition.
 
-## Quickstart
+## MikroTik client resource
+To generate new MikroTik resource definition, simply run
+```sh
+$ go run ./cmd/mikrotik-codegen mikrotik -name BridgeVlan -commandBase "/interface/bridge/vlan"
+```
+where
+
+`name` - a name of MikroTik resource to generate.
+
+`commandBase` - base path to craft commands for CRUD operations.
+
+## Terraform resource
 Just add a `codegen` tag key to struct fields:
 ```go
 type MikrotikResource struct{
@@ -20,7 +31,7 @@ type MikrotikResource struct{
 
 and run:
 ```sh
-$ go run ./cmd/mikrotik-codegen -src client/resource.go -struct MikrotikResource > mikrotik/resource_new.go
+$ go run ./cmd/mikrotik-codegen terraform -src client/resource.go -struct MikrotikResource > mikrotik/resource_new.go
 ```