From 4816ea76eee0dbcca04c7c6f47f76d3ae79e1a9e Mon Sep 17 00:00:00 2001 From: Volodymyr Manilo Date: Wed, 20 Sep 2023 20:49:42 +0200 Subject: [PATCH 1/3] added resource protocols validation --- .github/workflows/ci.yml | 4 +- .../internal/client/query/resource-read.go | 4 +- .../internal/provider/resource/resource.go | 46 +++++++++------ .../test/acctests/resource/resource_test.go | 56 +++++++++++++++++++ 4 files changed, 89 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee1bfee8..3bf3ef99 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -118,7 +118,7 @@ jobs: name: Matrix Acceptance Tests needs: build runs-on: ubuntu-latest - if: "!github.event.pull_request.head.repo.fork" +# if: "!github.event.pull_request.head.repo.fork" timeout-minutes: 15 strategy: fail-fast: false @@ -169,7 +169,7 @@ jobs: cleanup: name: Cleanup - if: "!github.event.pull_request.head.repo.fork" +# if: "!github.event.pull_request.head.repo.fork" needs: tests-acceptance runs-on: ubuntu-latest timeout-minutes: 15 diff --git a/twingate/internal/client/query/resource-read.go b/twingate/internal/client/query/resource-read.go index 1219f567..d641b291 100644 --- a/twingate/internal/client/query/resource-read.go +++ b/twingate/internal/client/query/resource-read.go @@ -105,7 +105,7 @@ func (r ResourceNode) ToModel() *model.Resource { func protocolsToModel(protocols *Protocols) *model.Protocols { if protocols == nil { - return nil + return model.DefaultProtocols() } return &model.Protocols{ @@ -117,7 +117,7 @@ func protocolsToModel(protocols *Protocols) *model.Protocols { func protocolToModel(protocol *Protocol) *model.Protocol { if protocol == nil { - return nil + return model.DefaultProtocol() } return &model.Protocol{ diff --git a/twingate/internal/provider/resource/resource.go b/twingate/internal/provider/resource/resource.go index 7555031d..616dc7d7 100644 --- a/twingate/internal/provider/resource/resource.go +++ b/twingate/internal/provider/resource/resource.go @@ -228,10 +228,9 @@ func resourceUpdate(ctx context.Context, resourceData *schema.ResourceData, meta if resource != nil { resource.IsAuthoritative = convertAuthoritativeFlagLegacy(resourceData) + log.Printf("[INFO] Updated resource %s", resource.Name) } - log.Printf("[INFO] Updated resource %s", resource.Name) - return resourceResourceReadHelper(ctx, client, resourceData, resource, err) } @@ -320,14 +319,15 @@ func readDiagnostics(resourceData *schema.ResourceData, resource *model.Resource return ErrAttributeSet(err, attr.Access) } - protocols, _ := convertProtocols(resourceData) - - if portRangeEqual(protocols.TCP.Ports, resource.Protocols.TCP.Ports) { - resource.Protocols.TCP.Ports = protocols.TCP.Ports - } + protocols, err := convertProtocols(resourceData) + if err == nil && protocols != nil && protocols.TCP != nil && protocols.UDP != nil { + if portRangeEqual(protocols.TCP.Ports, resource.Protocols.TCP.Ports) { + resource.Protocols.TCP.Ports = protocols.TCP.Ports + } - if portRangeEqual(protocols.UDP.Ports, resource.Protocols.UDP.Ports) { - resource.Protocols.UDP.Ports = protocols.UDP.Ports + if portRangeEqual(protocols.UDP.Ports, resource.Protocols.UDP.Ports) { + resource.Protocols.UDP.Ports = protocols.UDP.Ports + } } if err := resourceData.Set(attr.Protocols, resource.Protocols.ToTerraform()); err != nil { @@ -573,33 +573,45 @@ func convertProtocol(rawList []interface{}) (*model.Protocol, error) { rawMap := rawList[0].(map[string]interface{}) policy := rawMap[attr.Policy].(string) + if policy == "" { + policy = model.PolicyAllowAll + } + ports, err := convertPorts(rawMap[attr.Ports].([]interface{})) if err != nil { return nil, err } + if err := validateProtocol(policy, ports); err != nil { + return nil, err + } + + if policy == model.PolicyDenyAll { + policy = model.PolicyRestricted + } + + return model.NewProtocol(policy, ports), nil +} + +func validateProtocol(policy string, ports []*model.PortRange) error { switch policy { case model.PolicyAllowAll: if len(ports) > 0 { - return nil, ErrPortsWithPolicyAllowAll + return ErrPortsWithPolicyAllowAll } case model.PolicyDenyAll: if len(ports) > 0 { - return nil, ErrPortsWithPolicyDenyAll + return ErrPortsWithPolicyDenyAll } case model.PolicyRestricted: if len(ports) == 0 { - return nil, ErrPolicyRestrictedWithoutPorts + return ErrPolicyRestrictedWithoutPorts } } - if policy == model.PolicyDenyAll { - policy = model.PolicyRestricted - } - - return model.NewProtocol(policy, ports), nil + return nil } func convertPorts(rawList []interface{}) ([]*model.PortRange, error) { diff --git a/twingate/internal/test/acctests/resource/resource_test.go b/twingate/internal/test/acctests/resource/resource_test.go index 74c2fd4d..7e5efcf4 100644 --- a/twingate/internal/test/acctests/resource/resource_test.go +++ b/twingate/internal/test/acctests/resource/resource_test.go @@ -51,6 +51,39 @@ func TestAccTwingateResourceCreate(t *testing.T) { }) } +func TestAccTwingateResourceUpdateProtocols(t *testing.T) { + const terraformResourceName = "test1u" + theResource := acctests.TerraformResource(terraformResourceName) + remoteNetworkName := test.RandomName() + resourceName := test.RandomResourceName() + + sdk.Test(t, sdk.TestCase{ + ProtoV6ProviderFactories: acctests.ProviderFactories, + PreCheck: func() { acctests.PreCheck(t) }, + CheckDestroy: acctests.CheckTwingateResourceDestroy, + Steps: []sdk.TestStep{ + { + Config: createResourceOnlyWithNetwork(terraformResourceName, remoteNetworkName, resourceName), + Check: acctests.ComposeTestCheckFunc( + acctests.CheckTwingateResourceExists(theResource), + ), + }, + { + Config: createResourceWithSimpleProtocols(terraformResourceName, remoteNetworkName, resourceName), + Check: acctests.ComposeTestCheckFunc( + acctests.CheckTwingateResourceExists(theResource), + ), + }, + { + Config: createResourceOnlyWithNetwork(terraformResourceName, remoteNetworkName, resourceName), + Check: acctests.ComposeTestCheckFunc( + acctests.CheckTwingateResourceExists(theResource), + ), + }, + }, + }) +} + func createResourceOnlyWithNetwork(terraformResourceName, networkName, resourceName string) string { return fmt.Sprintf(` resource "twingate_remote_network" "%s" { @@ -64,6 +97,29 @@ func createResourceOnlyWithNetwork(terraformResourceName, networkName, resourceN `, terraformResourceName, networkName, terraformResourceName, resourceName, terraformResourceName) } +func createResourceWithSimpleProtocols(terraformResourceName, networkName, resourceName string) string { + return fmt.Sprintf(` + resource "twingate_remote_network" "%s" { + name = "%s" + } + resource "twingate_resource" "%s" { + name = "%s" + address = "acc-test.com" + remote_network_id = twingate_remote_network.%s.id + + protocols { + allow_icmp = true + tcp { + policy = "DENY_ALL" + } + udp { + policy = "DENY_ALL" + } + } + } + `, terraformResourceName, networkName, terraformResourceName, resourceName, terraformResourceName) +} + func TestAccTwingateResourceCreateWithProtocolsAndGroups(t *testing.T) { const theResource = "twingate_resource.test2" remoteNetworkName := test.RandomName() From 7082ce6c2a495e7d5fad95fce09d831a4300cbcd Mon Sep 17 00:00:00 2001 From: Volodymyr Manilo Date: Wed, 20 Sep 2023 21:01:55 +0200 Subject: [PATCH 2/3] fix unit tests --- twingate/internal/client/query/query_test.go | 3 ++- twingate/internal/test/client/resource_test.go | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/twingate/internal/client/query/query_test.go b/twingate/internal/client/query/query_test.go index 211f399b..9720cbae 100644 --- a/twingate/internal/client/query/query_test.go +++ b/twingate/internal/client/query/query_test.go @@ -123,6 +123,7 @@ func TestReadResourcesByNameQueryToModel(t *testing.T) { RemoteNetworkID: "resource-network-id", IsVisible: &boolTrue, IsBrowserShortcutEnabled: &boolFalse, + Protocols: model.DefaultProtocols(), }, }, }, @@ -796,7 +797,7 @@ func TestProtocolToModel(t *testing.T) { }{ { protocol: nil, - expected: nil, + expected: model.DefaultProtocol(), }, { protocol: &Protocol{ diff --git a/twingate/internal/test/client/resource_test.go b/twingate/internal/test/client/resource_test.go index f40bfbfd..0b533749 100644 --- a/twingate/internal/test/client/resource_test.go +++ b/twingate/internal/test/client/resource_test.go @@ -668,11 +668,11 @@ func TestClientResourcesReadAllOk(t *testing.T) { var defaultBool bool expected := []*model.Resource{ - {ID: "resource1", Name: "tf-acc-resource1", IsVisible: &defaultBool, IsBrowserShortcutEnabled: &defaultBool}, - {ID: "resource2", Name: "resource2", IsVisible: &defaultBool, IsBrowserShortcutEnabled: &defaultBool}, - {ID: "resource3", Name: "tf-acc-resource3", IsVisible: &defaultBool, IsBrowserShortcutEnabled: &defaultBool}, - {ID: "resource4", Name: "tf-acc-resource4", IsVisible: &defaultBool, IsBrowserShortcutEnabled: &defaultBool}, - {ID: "resource5", Name: "tf-acc-resource5", IsVisible: &defaultBool, IsBrowserShortcutEnabled: &defaultBool}, + {ID: "resource1", Name: "tf-acc-resource1", IsVisible: &defaultBool, IsBrowserShortcutEnabled: &defaultBool, Protocols: model.DefaultProtocols()}, + {ID: "resource2", Name: "resource2", IsVisible: &defaultBool, IsBrowserShortcutEnabled: &defaultBool, Protocols: model.DefaultProtocols()}, + {ID: "resource3", Name: "tf-acc-resource3", IsVisible: &defaultBool, IsBrowserShortcutEnabled: &defaultBool, Protocols: model.DefaultProtocols()}, + {ID: "resource4", Name: "tf-acc-resource4", IsVisible: &defaultBool, IsBrowserShortcutEnabled: &defaultBool, Protocols: model.DefaultProtocols()}, + {ID: "resource5", Name: "tf-acc-resource5", IsVisible: &defaultBool, IsBrowserShortcutEnabled: &defaultBool, Protocols: model.DefaultProtocols()}, } // response JSON From da9b0f116be9ee31e97b6aa217f589767e742219 Mon Sep 17 00:00:00 2001 From: Volodymyr Manilo Date: Wed, 20 Sep 2023 21:14:58 +0200 Subject: [PATCH 3/3] revert ci changes --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3bf3ef99..ee1bfee8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -118,7 +118,7 @@ jobs: name: Matrix Acceptance Tests needs: build runs-on: ubuntu-latest -# if: "!github.event.pull_request.head.repo.fork" + if: "!github.event.pull_request.head.repo.fork" timeout-minutes: 15 strategy: fail-fast: false @@ -169,7 +169,7 @@ jobs: cleanup: name: Cleanup -# if: "!github.event.pull_request.head.repo.fork" + if: "!github.event.pull_request.head.repo.fork" needs: tests-acceptance runs-on: ubuntu-latest timeout-minutes: 15