From f4a2d90e17b0e974d1f648b934f279e4346efa81 Mon Sep 17 00:00:00 2001 From: Vladimir Kozyrev Date: Tue, 21 Mar 2023 13:11:14 +0400 Subject: [PATCH 1/4] properly update routes --- client/route.go | 8 ++++++-- openvpncloud/resource_network.go | 11 +++++++---- openvpncloud/resource_route.go | 26 +++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/client/route.go b/client/route.go index 6c822c6..8e7017f 100644 --- a/client/route.go +++ b/client/route.go @@ -31,7 +31,7 @@ func (c *Client) CreateRoute(networkId string, route Route) (*Route, error) { } var routeToCreate newRoute - routeToCreate.Description = "Managed by Terraform. " + route.Description + routeToCreate.Description = route.Description routeToCreate.Value = route.Value routeJson, err := json.Marshal(routeToCreate) @@ -116,7 +116,11 @@ func (c *Client) UpdateRoute(networkId string, route Route) error { if err != nil { return err } - req, err := http.NewRequest(http.MethodPut, fmt.Sprintf("%s/api/beta/networks/%s/routes/%s", c.BaseURL, networkId, route.Id), bytes.NewBuffer(routeJson)) + req, err := http.NewRequest( + http.MethodPut, + fmt.Sprintf("%s/api/beta/networks/%s/routes/%s", c.BaseURL, networkId, route.Id), + bytes.NewBuffer(routeJson), + ) if err != nil { return err } diff --git a/openvpncloud/resource_network.go b/openvpncloud/resource_network.go index 831b9e4..560b8a3 100644 --- a/openvpncloud/resource_network.go +++ b/openvpncloud/resource_network.go @@ -168,7 +168,8 @@ func resourceNetworkCreate(ctx context.Context, d *schema.ResourceData, m interf } defaultRouteWithIdSlice := make([]map[string]interface{}, 1) defaultRouteWithIdSlice[0] = map[string]interface{}{ - "id": defaultRoute.Id, + "id": defaultRoute.Id, + "description": defaultRoute.Description, } d.Set("default_route", defaultRouteWithIdSlice) return append(diags, diag.Diagnostic{ @@ -217,8 +218,9 @@ func resourceNetworkRead(ctx context.Context, d *schema.ResourceData, m interfac } else { defaultRoute := []map[string]interface{}{ { - "id": configRoute["id"].(string), - "type": route.Type, + "id": configRoute["id"].(string), + "type": route.Type, + "description": route.Description, }, } if route.Type == client.RouteTypeIPV4 || route.Type == client.RouteTypeIPV6 { @@ -294,7 +296,8 @@ func resourceNetworkUpdate(ctx context.Context, d *schema.ResourceData, m interf } defaultRouteWithIdSlice := make([]map[string]interface{}, 1) defaultRouteWithIdSlice[0] = map[string]interface{}{ - "id": defaultRoute.Id, + "id": defaultRoute.Id, + "description": defaultRoute.Description, } err = d.Set("default_route", defaultRouteWithIdSlice) if err != nil { diff --git a/openvpncloud/resource_route.go b/openvpncloud/resource_route.go index b1142cc..8b60bb5 100644 --- a/openvpncloud/resource_route.go +++ b/openvpncloud/resource_route.go @@ -13,7 +13,7 @@ func resourceRoute() *schema.Resource { return &schema.Resource{ Description: "Use `openvpncloud_route` to create a route on an OpenVPN Cloud network.", CreateContext: resourceRouteCreate, - UpdateContext: resourceRouteCreate, + UpdateContext: resourceRouteUpdate, ReadContext: resourceRouteRead, DeleteContext: resourceRouteDelete, Importer: &schema.ResourceImporter{ @@ -42,6 +42,7 @@ func resourceRoute() *schema.Resource { "description": { Type: schema.TypeString, Optional: true, + Default: "Managed by Terraform", }, }, } @@ -94,6 +95,29 @@ func resourceRouteRead(ctx context.Context, d *schema.ResourceData, m interface{ return diags } +func resourceRouteUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics { + c := m.(*client.Client) + var diags diag.Diagnostics + if !d.HasChanges("description", "value") { + return diags + } + + networkItemId := d.Get("network_item_id").(string) + _, description := d.GetChange("description") + _, value := d.GetChange("value") + r := client.Route{ + Id: d.Id(), + Description: description.(string), + Value: value.(string), + } + + err := c.UpdateRoute(networkItemId, r) + if err != nil { + return append(diags, diag.FromErr(err)...) + } + return diags +} + func resourceRouteDelete(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics { c := m.(*client.Client) var diags diag.Diagnostics From 19ccdff5db7bfada85c849fda9ceac880d092634 Mon Sep 17 00:00:00 2001 From: Vladimir Kozyrev Date: Fri, 24 Mar 2023 13:03:10 +0400 Subject: [PATCH 2/4] properly persist route values --- client/route.go | 20 +++++++++++++------- openvpncloud/resource_network.go | 2 ++ openvpncloud/resource_route.go | 1 + 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/client/route.go b/client/route.go index 8e7017f..2096678 100644 --- a/client/route.go +++ b/client/route.go @@ -24,21 +24,25 @@ const ( ) func (c *Client) CreateRoute(networkId string, route Route) (*Route, error) { - type newRoute struct { Description string `json:"description"` Value string `json:"value"` + Type string `json:"type"` + } + routeToCreate := newRoute{ + Description: route.Description, + Value: route.Value, + Type: route.Type, } - - var routeToCreate newRoute - routeToCreate.Description = route.Description - routeToCreate.Value = route.Value - routeJson, err := json.Marshal(routeToCreate) if err != nil { return nil, err } - req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/api/beta/networks/%s/routes", c.BaseURL, networkId), bytes.NewBuffer(routeJson)) + req, err := http.NewRequest( + http.MethodPost, + fmt.Sprintf("%s/api/beta/networks/%s/routes", c.BaseURL, networkId), + bytes.NewBuffer(routeJson), + ) if err != nil { return nil, err } @@ -51,6 +55,8 @@ func (c *Client) CreateRoute(networkId string, route Route) (*Route, error) { if err != nil { return nil, err } + // The API does not return the route Value, so we set it manually. + r.Value = routeToCreate.Value return &r, nil } diff --git a/openvpncloud/resource_network.go b/openvpncloud/resource_network.go index 560b8a3..1b54905 100644 --- a/openvpncloud/resource_network.go +++ b/openvpncloud/resource_network.go @@ -170,6 +170,8 @@ func resourceNetworkCreate(ctx context.Context, d *schema.ResourceData, m interf defaultRouteWithIdSlice[0] = map[string]interface{}{ "id": defaultRoute.Id, "description": defaultRoute.Description, + "type": defaultRoute.Type, + "value": defaultRoute.Value, } d.Set("default_route", defaultRouteWithIdSlice) return append(diags, diag.Diagnostic{ diff --git a/openvpncloud/resource_route.go b/openvpncloud/resource_route.go index 8b60bb5..a48f142 100644 --- a/openvpncloud/resource_route.go +++ b/openvpncloud/resource_route.go @@ -90,6 +90,7 @@ func resourceRouteRead(ctx context.Context, d *schema.ResourceData, m interface{ } else if r.Type == client.RouteTypeDomain { d.Set("resourceRouteRead", r.Domain) } + d.Set("description", r.Description) d.Set("network_item_id", r.NetworkItemId) } return diags From 84f4c014328436bc21dbc4a48e40f893d7e8f6e5 Mon Sep 17 00:00:00 2001 From: Vladimir Kozyrev Date: Fri, 24 Mar 2023 13:03:28 +0400 Subject: [PATCH 3/4] add test for route & network --- openvpncloud/resource_route_test.go | 132 ++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 openvpncloud/resource_route_test.go diff --git a/openvpncloud/resource_route_test.go b/openvpncloud/resource_route_test.go new file mode 100644 index 0000000..cc759e9 --- /dev/null +++ b/openvpncloud/resource_route_test.go @@ -0,0 +1,132 @@ +package openvpncloud + +import ( + "errors" + "fmt" + "testing" + + "github.com/OpenVPN/terraform-provider-openvpn-cloud/client" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/stretchr/testify/require" +) + +func TestAccOpenvpncloudRoute_basic(t *testing.T) { + rn := "openvpncloud_route.test" + ip, err := acctest.RandIpAddress("10.0.0.0/8") + require.NoError(t, err) + route := client.Route{ + Description: "test" + acctest.RandString(10), + Type: client.RouteTypeIPV4, + Value: ip + "/32", + } + routeChanged := route + routeChanged.Description = acctest.RandStringFromCharSet(10, alphabet) + networkRandString := "test" + acctest.RandString(10) + var routeId string + + check := func(r client.Route) resource.TestCheckFunc { + return resource.ComposeTestCheckFunc( + testAccCheckOpenvpncloudRouteExists(rn, &routeId), + resource.TestCheckResourceAttr(rn, "description", r.Description), + resource.TestCheckResourceAttr(rn, "type", r.Type), + resource.TestCheckResourceAttr(rn, "value", r.Value), + ) + } + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProviderFactories: testAccProviderFactories, + CheckDestroy: testAccCheckOpenvpncloudRouteDestroy, + Steps: []resource.TestStep{ + { + Config: testAccOpenvpncloudRouteConfig(route, networkRandString), + Check: check(route), + }, + { + Config: testAccOpenvpncloudRouteConfig(routeChanged, networkRandString), + Check: check(routeChanged), + }, + { + ResourceName: rn, + ImportState: true, + ImportStateIdFunc: testAccOpenvpncloudRouteImportStateIdFunc(rn), + ImportStateVerify: true, + }, + }, + }) +} + +func testAccCheckOpenvpncloudRouteDestroy(s *terraform.State) error { + client := testAccProvider.Meta().(*client.Client) + for _, rs := range s.RootModule().Resources { + if rs.Type != "openvpncloud_route" { + continue + } + routeId := rs.Primary.ID + r, err := client.GetRouteById(routeId) + if err == nil { + return err + } + if r != nil { + return errors.New("route still exists") + } + } + return nil +} + +func testAccCheckOpenvpncloudRouteExists(n string, routeID *string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("not found: %s", n) + } + + if rs.Primary.ID == "" { + return errors.New("no ID is set") + } + + client := testAccProvider.Meta().(*client.Client) + _, err := client.GetRouteById(rs.Primary.ID) + if err != nil { + return err + } + return nil + } +} + +func testAccOpenvpncloudRouteImportStateIdFunc(n string) resource.ImportStateIdFunc { + return func(s *terraform.State) (string, error) { + rs, ok := s.RootModule().Resources[n] + if !ok { + return "", fmt.Errorf("not found: %s", n) + } + return rs.Primary.ID, nil + } +} + +func testAccOpenvpncloudRouteConfig(r client.Route, networkRandStr string) string { + return fmt.Sprintf(` +provider "openvpncloud" { + base_url = "https://%[1]s.api.openvpn.com" +} +resource "openvpncloud_network" "test" { + name = "%[5]s" + default_connector { + name = "%[5]s" + vpn_region_id = "fi-hel" + } + default_route { + value = "10.1.2.0/24" + type = "IP_V4" + } +} +resource "openvpncloud_route" "test" { + network_item_id = openvpncloud_network.test.id + description = "%[2]s" + value = "%[3]s" + type = "%[4]s" +} +`, testCloudID, r.Description, r.Value, r.Type, networkRandStr) +} From e66075961ce8d62af4146674bddb3dae8b697ad8 Mon Sep 17 00:00:00 2001 From: Vladimir Kozyrev Date: Fri, 24 Mar 2023 14:28:59 +0400 Subject: [PATCH 4/4] add concurrency setting for the linter --- .github/workflows/lint.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 660074b..c6a00f8 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -6,6 +6,9 @@ on: - main pull_request: branches: [ main ] +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true jobs: lint: