From 489c394b12825c78aac2ae316f467b957d683da0 Mon Sep 17 00:00:00 2001 From: Volodymyr Manilo Date: Fri, 10 Nov 2023 04:34:26 +0100 Subject: [PATCH 1/8] added security_policy_id to resource definition --- .github/workflows/ci.yml | 6 +- docs/resources/resource.md | 1 + .../internal/client/query/resource-create.go | 2 +- .../internal/client/query/resource-read.go | 7 ++ .../internal/client/query/resource-update.go | 2 +- twingate/internal/client/resource.go | 10 +++ twingate/internal/client/variables.go | 5 ++ twingate/internal/model/resource.go | 1 + .../internal/provider/resource/resource.go | 46 +++++++---- .../test/acctests/resource/resource_test.go | 76 +++++++++++++++++++ 10 files changed, 139 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee1bfee8..7394cd65 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,6 +7,7 @@ on: pull_request: branches: - main + - feature/add-security_policy-to-resource paths-ignore: - 'README.md' @@ -15,6 +16,7 @@ on: - 'README.md' branches: - main + - feature/add-security_policy-to-resource # Ensures only 1 action runs per PR and previous is canceled on new trigger concurrency: @@ -118,7 +120,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 +171,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/docs/resources/resource.md b/docs/resources/resource.md index 7c5a03e1..8330976c 100644 --- a/docs/resources/resource.md +++ b/docs/resources/resource.md @@ -70,6 +70,7 @@ resource "twingate_resource" "resource" { - `is_browser_shortcut_enabled` (Boolean) Controls whether an "Open in Browser" shortcut will be shown for this Resource in the Twingate Client. - `is_visible` (Boolean) Controls whether this Resource will be visible in the main Resource list in the Twingate Client. - `protocols` (Block List, Max: 1) Restrict access to certain protocols and ports. By default or when this argument is not defined, there is no restriction, and all protocols and ports are allowed. (see [below for nested schema](#nestedblock--protocols)) +- `security_policy_id` (String) The ID of a `twingate_security_policy` to set as this Resource's Security Policy. ### Read-Only diff --git a/twingate/internal/client/query/resource-create.go b/twingate/internal/client/query/resource-create.go index 17c697b5..7611a8e2 100644 --- a/twingate/internal/client/query/resource-create.go +++ b/twingate/internal/client/query/resource-create.go @@ -1,7 +1,7 @@ package query type CreateResource struct { - ResourceEntityResponse `graphql:"resourceCreate(name: $name, address: $address, remoteNetworkId: $remoteNetworkId, groupIds: $groupIds, protocols: $protocols, isVisible: $isVisible, isBrowserShortcutEnabled: $isBrowserShortcutEnabled, alias: $alias)"` + ResourceEntityResponse `graphql:"resourceCreate(name: $name, address: $address, remoteNetworkId: $remoteNetworkId, groupIds: $groupIds, protocols: $protocols, isVisible: $isVisible, isBrowserShortcutEnabled: $isBrowserShortcutEnabled, alias: $alias, securityPolicyId: $securityPolicyId)"` } func (q CreateResource) IsEmpty() bool { diff --git a/twingate/internal/client/query/resource-read.go b/twingate/internal/client/query/resource-read.go index d641b291..83087561 100644 --- a/twingate/internal/client/query/resource-read.go +++ b/twingate/internal/client/query/resource-read.go @@ -56,6 +56,7 @@ type ResourceNode struct { IsVisible bool IsBrowserShortcutEnabled bool Alias string + SecurityPolicy *gqlSecurityPolicy } type Protocols struct { @@ -90,6 +91,11 @@ func (r gqlResource) ToModel() *model.Resource { } func (r ResourceNode) ToModel() *model.Resource { + var securityPolicy string + if r.SecurityPolicy != nil { + securityPolicy = string(r.SecurityPolicy.ID) + } + return &model.Resource{ ID: string(r.ID), Name: r.Name, @@ -100,6 +106,7 @@ func (r ResourceNode) ToModel() *model.Resource { IsVisible: &r.IsVisible, IsBrowserShortcutEnabled: &r.IsBrowserShortcutEnabled, Alias: optionalString(r.Alias), + SecurityPolicyID: optionalString(securityPolicy), } } diff --git a/twingate/internal/client/query/resource-update.go b/twingate/internal/client/query/resource-update.go index 419b4f57..4bdc0e5e 100644 --- a/twingate/internal/client/query/resource-update.go +++ b/twingate/internal/client/query/resource-update.go @@ -1,7 +1,7 @@ package query type UpdateResource struct { - ResourceEntityResponse `graphql:"resourceUpdate(id: $id, name: $name, address: $address, remoteNetworkId: $remoteNetworkId, protocols: $protocols, isVisible: $isVisible, isBrowserShortcutEnabled: $isBrowserShortcutEnabled, alias: $alias)"` + ResourceEntityResponse `graphql:"resourceUpdate(id: $id, name: $name, address: $address, remoteNetworkId: $remoteNetworkId, protocols: $protocols, isVisible: $isVisible, isBrowserShortcutEnabled: $isBrowserShortcutEnabled, alias: $alias, securityPolicyId: $securityPolicyId)"` } func (q UpdateResource) IsEmpty() bool { diff --git a/twingate/internal/client/resource.go b/twingate/internal/client/resource.go index 8518c30e..d3e9950c 100644 --- a/twingate/internal/client/resource.go +++ b/twingate/internal/client/resource.go @@ -70,6 +70,7 @@ func (client *Client) CreateResource(ctx context.Context, input *model.Resource) gqlNullable(input.IsVisible, "isVisible"), gqlNullable(input.IsBrowserShortcutEnabled, "isBrowserShortcutEnabled"), gqlNullable(input.Alias, "alias"), + gqlNullableID(input.SecurityPolicyID, "securityPolicyId"), cursor(query.CursorAccess), pageLimit(client.pageLimit), ) @@ -92,6 +93,10 @@ func (client *Client) CreateResource(ctx context.Context, input *model.Resource) resource.IsBrowserShortcutEnabled = nil } + if input.SecurityPolicyID == nil { + resource.SecurityPolicyID = nil + } + return resource, nil } @@ -180,6 +185,7 @@ func (client *Client) UpdateResource(ctx context.Context, input *model.Resource) gqlNullable(input.IsVisible, "isVisible"), gqlNullable(input.IsBrowserShortcutEnabled, "isBrowserShortcutEnabled"), gqlNullable(input.Alias, "alias"), + gqlNullableID(input.SecurityPolicyID, "securityPolicyId"), cursor(query.CursorAccess), pageLimit(client.pageLimit), ) @@ -204,6 +210,10 @@ func (client *Client) UpdateResource(ctx context.Context, input *model.Resource) resource.IsBrowserShortcutEnabled = nil } + if input.SecurityPolicyID == nil { + resource.SecurityPolicyID = nil + } + return resource, nil } diff --git a/twingate/internal/client/variables.go b/twingate/internal/client/variables.go index b9e6a945..e76daa91 100644 --- a/twingate/internal/client/variables.go +++ b/twingate/internal/client/variables.go @@ -105,6 +105,7 @@ func getValue(val any) any { } } +//nolint:unparam func gqlNullableID(val interface{}, name string) gqlVarOption { return func(values map[string]interface{}) map[string]interface{} { var ( @@ -112,6 +113,10 @@ func gqlNullableID(val interface{}, name string) gqlVarOption { defaultID *graphql.ID ) + if value, ok := val.(*string); ok && value != nil { + val = *value + } + if isZeroValue(val) { gqlValue = defaultID } else { diff --git a/twingate/internal/model/resource.go b/twingate/internal/model/resource.go index c073bb60..8990ca17 100644 --- a/twingate/internal/model/resource.go +++ b/twingate/internal/model/resource.go @@ -34,6 +34,7 @@ type Resource struct { IsVisible *bool IsBrowserShortcutEnabled *bool Alias *string + SecurityPolicyID *string } func (r Resource) AccessToTerraform() []interface{} { diff --git a/twingate/internal/provider/resource/resource.go b/twingate/internal/provider/resource/resource.go index 82ba876b..e16cd252 100644 --- a/twingate/internal/provider/resource/resource.go +++ b/twingate/internal/provider/resource/resource.go @@ -136,6 +136,11 @@ func Resource() *schema.Resource { //nolint:funlen Description: "Restrict access to certain groups or service accounts", Elem: accessSchema, }, + attr.SecurityPolicyID: { + Type: schema.TypeString, + Optional: true, + Description: "The ID of a `twingate_security_policy` to set as this Resource's Security Policy.", + }, // computed attr.IsVisible: { Type: schema.TypeBool, @@ -222,6 +227,7 @@ func resourceUpdate(ctx context.Context, resourceData *schema.ResourceData, meta attr.IsVisible, attr.IsBrowserShortcutEnabled, attr.Alias, + attr.SecurityPolicyID, ) { resource, err = client.UpdateResource(ctx, resource) } else { @@ -239,9 +245,15 @@ func resourceUpdate(ctx context.Context, resourceData *schema.ResourceData, meta func resourceRead(ctx context.Context, resourceData *schema.ResourceData, meta interface{}) diag.Diagnostics { client := meta.(*client.Client) + securityPolicyID := resourceData.Get(attr.SecurityPolicyID) + resource, err := client.ReadResource(ctx, resourceData.Id()) if resource != nil { resource.IsAuthoritative = convertAuthoritativeFlagLegacy(resourceData) + + if securityPolicyID == "" { + resource.SecurityPolicyID = nil + } } return resourceResourceReadHelper(ctx, client, resourceData, resource, err) @@ -348,13 +360,12 @@ func readDiagnostics(resourceData *schema.ResourceData, resource *model.Resource } } - var alias interface{} - if resource.Alias != nil { - alias = *resource.Alias + if err := resourceData.Set(attr.Alias, resource.Alias); err != nil { + return ErrAttributeSet(err, attr.Alias) } - if err := resourceData.Set(attr.Alias, alias); err != nil { - return ErrAttributeSet(err, attr.Alias) + if err := resourceData.Set(attr.SecurityPolicyID, resource.SecurityPolicyID); err != nil { + return ErrAttributeSet(err, attr.SecurityPolicyID) } return nil @@ -483,14 +494,15 @@ func convertResource(data *schema.ResourceData) (*model.Resource, error) { groups, serviceAccounts := convertAccess(data) res := &model.Resource{ - Name: data.Get(attr.Name).(string), - RemoteNetworkID: data.Get(attr.RemoteNetworkID).(string), - Address: data.Get(attr.Address).(string), - Protocols: protocols, - Groups: groups, - ServiceAccounts: serviceAccounts, - IsAuthoritative: convertAuthoritativeFlagLegacy(data), - Alias: getOptionalString(data, attr.Alias), + Name: data.Get(attr.Name).(string), + RemoteNetworkID: data.Get(attr.RemoteNetworkID).(string), + Address: data.Get(attr.Address).(string), + Protocols: protocols, + Groups: groups, + ServiceAccounts: serviceAccounts, + IsAuthoritative: convertAuthoritativeFlagLegacy(data), + Alias: getOptionalString(data, attr.Alias), + SecurityPolicyID: getOptionalString(data, attr.SecurityPolicyID), } isVisible, ok := data.GetOkExists(attr.IsVisible) //nolint @@ -524,9 +536,17 @@ func isAttrKnown(data *schema.ResourceData, attr string) bool { } func getOptionalString(data *schema.ResourceData, attr string) *string { + if data == nil { + return nil + } + var result *string cfg := data.GetRawConfig() + if cfg.IsNull() { + return nil + } + val := cfg.GetAttr(attr) if !val.IsNull() { diff --git a/twingate/internal/test/acctests/resource/resource_test.go b/twingate/internal/test/acctests/resource/resource_test.go index a5a97811..8de6ba07 100644 --- a/twingate/internal/test/acctests/resource/resource_test.go +++ b/twingate/internal/test/acctests/resource/resource_test.go @@ -2542,3 +2542,79 @@ func createResourceWithBrowserOption(name, networkName, resourceName, address st } `, name, networkName, resourceName, address, browserOption) } + +func TestAccTwingateResourceUpdateSecurityPolicy(t *testing.T) { + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() + + policies, err := acctests.ListSecurityPolicies() + if err != nil { + t.Skipf("failed to retrieve security policies: %v", err) + } + + if len(policies) < 2 { + t.Skip("requires at least 2 security policy for the test") + } + + sdk.Test(t, sdk.TestCase{ + ProtoV6ProviderFactories: acctests.ProviderFactories, + PreCheck: func() { acctests.PreCheck(t) }, + CheckDestroy: acctests.CheckTwingateResourceDestroy, + Steps: []sdk.TestStep{ + { + Config: createResourceWithSecurityPolicy(remoteNetworkName, resourceName, policies[0].ID), + Check: acctests.ComposeTestCheckFunc( + acctests.CheckTwingateResourceExists(theResource), + sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, policies[0].ID), + ), + }, + { + Config: createResourceWithSecurityPolicy(remoteNetworkName, resourceName, policies[1].ID), + Check: acctests.ComposeTestCheckFunc( + acctests.CheckTwingateResourceExists(theResource), + sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, policies[1].ID), + ), + }, + { + Config: createResourceWithoutSecurityPolicy(remoteNetworkName, resourceName), + Check: acctests.ComposeTestCheckFunc( + acctests.CheckTwingateResourceExists(theResource), + sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, ""), + ), + }, + { + Config: createResourceWithSecurityPolicy(remoteNetworkName, resourceName, ""), + // no changes + PlanOnly: true, + }, + }, + }) +} + +func createResourceWithSecurityPolicy(remoteNetwork, resource, policyID string) string { + return fmt.Sprintf(` + resource "twingate_remote_network" "%[1]s" { + name = "%[1]s" + } + resource "twingate_resource" "%[2]s" { + name = "%[2]s" + address = "acc-test-address.com" + remote_network_id = twingate_remote_network.%[1]s.id + security_policy_id = "%[3]s" + } + `, remoteNetwork, resource, policyID) +} + +func createResourceWithoutSecurityPolicy(remoteNetwork, resource string) string { + return fmt.Sprintf(` + resource "twingate_remote_network" "%[1]s" { + name = "%[1]s" + } + resource "twingate_resource" "%[2]s" { + name = "%[2]s" + address = "acc-test-address.com" + remote_network_id = twingate_remote_network.%[1]s.id + } + `, remoteNetwork, resource) +} From aaa61e6c349399219bc82a3af88ffc079e4668f4 Mon Sep 17 00:00:00 2001 From: Volodymyr Manilo Date: Fri, 10 Nov 2023 04:46:32 +0100 Subject: [PATCH 2/8] remove feature branch from CI --- .github/workflows/ci.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7394cd65..ee1bfee8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,6 @@ on: pull_request: branches: - main - - feature/add-security_policy-to-resource paths-ignore: - 'README.md' @@ -16,7 +15,6 @@ on: - 'README.md' branches: - main - - feature/add-security_policy-to-resource # Ensures only 1 action runs per PR and previous is canceled on new trigger concurrency: @@ -120,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 @@ -171,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 From 4c9b97f2b4c82e4d6c050e27c2224f589db77918 Mon Sep 17 00:00:00 2001 From: Volodymyr Manilo Date: Tue, 14 Nov 2023 19:10:38 +0100 Subject: [PATCH 3/8] setting default policy on disabling security_policy_id --- docs/resources/resource.md | 6 +++ .../resources/twingate_resource/resource.tf | 6 +++ .../internal/provider/resource/resource.go | 34 ++++++++++++ twingate/internal/test/acctests/helper.go | 20 +++++++ .../test/acctests/resource/resource_test.go | 52 +++++++++++++++++++ 5 files changed, 118 insertions(+) diff --git a/docs/resources/resource.md b/docs/resources/resource.md index 8330976c..665f4372 100644 --- a/docs/resources/resource.md +++ b/docs/resources/resource.md @@ -30,11 +30,17 @@ resource "twingate_service_account" "github_actions_prod" { name = "Github Actions PROD" } +data "twingate_security_policy" "test_policy" { + name = "Test Policy" +} + resource "twingate_resource" "resource" { name = "network" address = "internal.int" remote_network_id = twingate_remote_network.aws_network.id + security_policy_id = data.twingate_security_policy.test_policy.id + protocols { allow_icmp = true tcp { diff --git a/examples/resources/twingate_resource/resource.tf b/examples/resources/twingate_resource/resource.tf index 7c5ae842..5c7ca027 100644 --- a/examples/resources/twingate_resource/resource.tf +++ b/examples/resources/twingate_resource/resource.tf @@ -15,11 +15,17 @@ resource "twingate_service_account" "github_actions_prod" { name = "Github Actions PROD" } +data "twingate_security_policy" "test_policy" { + name = "Test Policy" +} + resource "twingate_resource" "resource" { name = "network" address = "internal.int" remote_network_id = twingate_remote_network.aws_network.id + security_policy_id = data.twingate_security_policy.test_policy.id + protocols { allow_icmp = true tcp { diff --git a/twingate/internal/provider/resource/resource.go b/twingate/internal/provider/resource/resource.go index e16cd252..ecd0f277 100644 --- a/twingate/internal/provider/resource/resource.go +++ b/twingate/internal/provider/resource/resource.go @@ -17,6 +17,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) +const defaultSecurityPolicyName = "Default Policy" + var ( ErrPortsWithPolicyAllowAll = errors.New(model.PolicyAllowAll + " policy does not allow specifying ports.") ErrPortsWithPolicyDenyAll = errors.New(model.PolicyDenyAll + " policy does not allow specifying ports.") @@ -194,6 +196,7 @@ func resourceCreate(ctx context.Context, resourceData *schema.ResourceData, meta return resourceResourceReadHelper(ctx, client, resourceData, resource, nil) } +//nolint:cyclop func resourceUpdate(ctx context.Context, resourceData *schema.ResourceData, meta interface{}) diag.Diagnostics { client := meta.(*client.Client) @@ -229,7 +232,16 @@ func resourceUpdate(ctx context.Context, resourceData *schema.ResourceData, meta attr.Alias, attr.SecurityPolicyID, ) { + hasOverride, diagErr := overrideSecurityPolicy(ctx, resource, client) + if diagErr.HasError() { + return diagErr + } + resource, err = client.UpdateResource(ctx, resource) + + if hasOverride && resource != nil { + resource.SecurityPolicyID = nil + } } else { resource, err = client.ReadResource(ctx, resource.ID) } @@ -242,6 +254,28 @@ func resourceUpdate(ctx context.Context, resourceData *schema.ResourceData, meta return resourceResourceReadHelper(ctx, client, resourceData, resource, err) } +func overrideSecurityPolicy(ctx context.Context, resource *model.Resource, client *client.Client) (bool, diag.Diagnostics) { + var securityPolicyOverride bool + + remoteResource, err := client.ReadResource(ctx, resource.ID) + if err != nil { + return securityPolicyOverride, diag.FromErr(err) + } + + defaultPolicy, err := client.ReadSecurityPolicy(ctx, "", defaultSecurityPolicyName) + if err != nil { + return securityPolicyOverride, diag.FromErr(err) + } + + if remoteResource.SecurityPolicyID != nil && resource.SecurityPolicyID == nil && + *remoteResource.SecurityPolicyID != defaultPolicy.ID { + securityPolicyOverride = true + resource.SecurityPolicyID = &defaultPolicy.ID + } + + return securityPolicyOverride, nil +} + func resourceRead(ctx context.Context, resourceData *schema.ResourceData, meta interface{}) diag.Diagnostics { client := meta.(*client.Client) diff --git a/twingate/internal/test/acctests/helper.go b/twingate/internal/test/acctests/helper.go index b3eef07c..0a4b9a4a 100644 --- a/twingate/internal/test/acctests/helper.go +++ b/twingate/internal/test/acctests/helper.go @@ -615,6 +615,26 @@ func CheckResourceServiceAccountsLen(resourceName string, expectedServiceAccount } } +func CheckResourceSecurityPolicy(resourceName string, expectedSecurityPolicyID string) sdk.TestCheckFunc { + return func(state *terraform.State) error { + resourceID, err := getResourceID(state, resourceName) + if err != nil { + return err + } + + resource, err := providerClient.ReadResource(context.Background(), resourceID) + if err != nil { + return fmt.Errorf("resource with ID %s failed to read: %w", resourceID, err) + } + + if resource.SecurityPolicyID != nil && *resource.SecurityPolicyID != expectedSecurityPolicyID { + return fmt.Errorf("expected security_policy_id %s, got %s", expectedSecurityPolicyID, *resource.SecurityPolicyID) //nolint + } + + return nil + } +} + func AddGroupUser(groupResource, groupName, terraformUserID string) sdk.TestCheckFunc { return func(state *terraform.State) error { userID, err := getResourceID(state, getResourceNameFromID(terraformUserID)) diff --git a/twingate/internal/test/acctests/resource/resource_test.go b/twingate/internal/test/acctests/resource/resource_test.go index 8de6ba07..c0d9b83b 100644 --- a/twingate/internal/test/acctests/resource/resource_test.go +++ b/twingate/internal/test/acctests/resource/resource_test.go @@ -2618,3 +2618,55 @@ func createResourceWithoutSecurityPolicy(remoteNetwork, resource string) string } `, remoteNetwork, resource) } + +func TestAccTwingateResourceSetDefaultSecurityPolicyOnDisablingSecurityPolicy(t *testing.T) { + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() + + policies, err := acctests.ListSecurityPolicies() + if err != nil { + t.Skipf("failed to retrieve security policies: %v", err) + } + + if len(policies) < 2 { + t.Skip("requires at least 2 security policy for the test") + } + + var defaultPolicy, testPolicy string + if policies[0].Name == "Default Policy" { + defaultPolicy = policies[0].ID + testPolicy = policies[1].ID + } else { + testPolicy = policies[0].ID + defaultPolicy = policies[1].ID + } + + sdk.Test(t, sdk.TestCase{ + ProtoV6ProviderFactories: acctests.ProviderFactories, + PreCheck: func() { acctests.PreCheck(t) }, + CheckDestroy: acctests.CheckTwingateResourceDestroy, + Steps: []sdk.TestStep{ + { + Config: createResourceWithSecurityPolicy(remoteNetworkName, resourceName, testPolicy), + Check: acctests.ComposeTestCheckFunc( + acctests.CheckTwingateResourceExists(theResource), + sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, testPolicy), + ), + }, + { + Config: createResourceWithoutSecurityPolicy(remoteNetworkName, resourceName), + Check: acctests.ComposeTestCheckFunc( + acctests.CheckTwingateResourceExists(theResource), + sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, ""), + acctests.CheckResourceSecurityPolicy(theResource, defaultPolicy), + ), + }, + { + Config: createResourceWithSecurityPolicy(remoteNetworkName, resourceName, ""), + // no changes + PlanOnly: true, + }, + }, + }) +} From 0e8bf88a041d1fe84a34a151c5dd1a2f9a5cf9dd Mon Sep 17 00:00:00 2001 From: Volodymyr Manilo Date: Wed, 15 Nov 2023 17:48:14 +0100 Subject: [PATCH 4/8] update resource logic for security policy --- .../internal/provider/resource/resource.go | 49 ++++++++++--------- twingate/internal/test/acctests/helper.go | 23 +++++++++ .../test/acctests/resource/resource_test.go | 49 +++++++++++-------- twingate/provider.go | 20 +++++--- 4 files changed, 92 insertions(+), 49 deletions(-) diff --git a/twingate/internal/provider/resource/resource.go b/twingate/internal/provider/resource/resource.go index ecd0f277..e7ebea69 100644 --- a/twingate/internal/provider/resource/resource.go +++ b/twingate/internal/provider/resource/resource.go @@ -17,9 +17,10 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) -const defaultSecurityPolicyName = "Default Policy" +const DefaultSecurityPolicyName = "Default Policy" var ( + DefaultSecurityPolicyID string //nolint:gochecknoglobals ErrPortsWithPolicyAllowAll = errors.New(model.PolicyAllowAll + " policy does not allow specifying ports.") ErrPortsWithPolicyDenyAll = errors.New(model.PolicyDenyAll + " policy does not allow specifying ports.") ErrPolicyRestrictedWithoutPorts = errors.New(model.PolicyRestricted + " policy requires specifying ports.") @@ -139,9 +140,11 @@ func Resource() *schema.Resource { //nolint:funlen Elem: accessSchema, }, attr.SecurityPolicyID: { - Type: schema.TypeString, - Optional: true, - Description: "The ID of a `twingate_security_policy` to set as this Resource's Security Policy.", + Type: schema.TypeString, + Optional: true, + Description: "The ID of a `twingate_security_policy` to set as this Resource's Security Policy.", + DiffSuppressOnRefresh: true, + DiffSuppressFunc: defaultPolicyNotChanged, }, // computed attr.IsVisible: { @@ -196,7 +199,6 @@ func resourceCreate(ctx context.Context, resourceData *schema.ResourceData, meta return resourceResourceReadHelper(ctx, client, resourceData, resource, nil) } -//nolint:cyclop func resourceUpdate(ctx context.Context, resourceData *schema.ResourceData, meta interface{}) diag.Diagnostics { client := meta.(*client.Client) @@ -232,16 +234,12 @@ func resourceUpdate(ctx context.Context, resourceData *schema.ResourceData, meta attr.Alias, attr.SecurityPolicyID, ) { - hasOverride, diagErr := overrideSecurityPolicy(ctx, resource, client) + diagErr := setDefaultSecurityPolicy(ctx, resource, client) if diagErr.HasError() { return diagErr } resource, err = client.UpdateResource(ctx, resource) - - if hasOverride && resource != nil { - resource.SecurityPolicyID = nil - } } else { resource, err = client.ReadResource(ctx, resource.ID) } @@ -254,26 +252,29 @@ func resourceUpdate(ctx context.Context, resourceData *schema.ResourceData, meta return resourceResourceReadHelper(ctx, client, resourceData, resource, err) } -func overrideSecurityPolicy(ctx context.Context, resource *model.Resource, client *client.Client) (bool, diag.Diagnostics) { - var securityPolicyOverride bool +func setDefaultSecurityPolicy(ctx context.Context, resource *model.Resource, client *client.Client) diag.Diagnostics { + if DefaultSecurityPolicyID == "" { + policy, _ := client.ReadSecurityPolicy(ctx, "", DefaultSecurityPolicyName) + if policy != nil { + DefaultSecurityPolicyID = policy.ID + } + } - remoteResource, err := client.ReadResource(ctx, resource.ID) - if err != nil { - return securityPolicyOverride, diag.FromErr(err) + if DefaultSecurityPolicyID == "" { + return diag.Errorf("default policy not set") } - defaultPolicy, err := client.ReadSecurityPolicy(ctx, "", defaultSecurityPolicyName) + remoteResource, err := client.ReadResource(ctx, resource.ID) if err != nil { - return securityPolicyOverride, diag.FromErr(err) + return diag.FromErr(err) } - if remoteResource.SecurityPolicyID != nil && resource.SecurityPolicyID == nil && - *remoteResource.SecurityPolicyID != defaultPolicy.ID { - securityPolicyOverride = true - resource.SecurityPolicyID = &defaultPolicy.ID + if remoteResource.SecurityPolicyID != nil && (resource.SecurityPolicyID == nil || *resource.SecurityPolicyID == "") && + *remoteResource.SecurityPolicyID != DefaultSecurityPolicyID { + resource.SecurityPolicyID = &DefaultSecurityPolicyID } - return securityPolicyOverride, nil + return nil } func resourceRead(ctx context.Context, resourceData *schema.ResourceData, meta interface{}) diag.Diagnostics { @@ -485,6 +486,10 @@ func protocolsNotChanged(attribute, oldValue, newValue string, data *schema.Reso return false } +func defaultPolicyNotChanged(attribute, oldValue, newValue string, data *schema.ResourceData) bool { + return oldValue == DefaultSecurityPolicyID && (newValue == "" || newValue == DefaultSecurityPolicyID) +} + func getChangedAccessIDs(ctx context.Context, resourceData *schema.ResourceData, resource *model.Resource, client *client.Client) ([]string, []string, error) { remote, err := client.ReadResource(ctx, resource.ID) if err != nil { diff --git a/twingate/internal/test/acctests/helper.go b/twingate/internal/test/acctests/helper.go index 0a4b9a4a..c58f9517 100644 --- a/twingate/internal/test/acctests/helper.go +++ b/twingate/internal/test/acctests/helper.go @@ -635,6 +635,29 @@ func CheckResourceSecurityPolicy(resourceName string, expectedSecurityPolicyID s } } +func UpdateResourceSecurityPolicy(resourceName, securityPolicyID string) sdk.TestCheckFunc { + return func(state *terraform.State) error { + resourceID, err := getResourceID(state, resourceName) + if err != nil { + return err + } + + resource, err := providerClient.ReadResource(context.Background(), resourceID) + if err != nil { + return fmt.Errorf("resource with ID %s failed to read: %w", resourceID, err) + } + + resource.SecurityPolicyID = &securityPolicyID + + _, err = providerClient.UpdateResource(context.Background(), resource) + if err != nil { + return fmt.Errorf("resource with ID %s failed to update security_policy: %w", resourceID, err) + } + + return nil + } +} + func AddGroupUser(groupResource, groupName, terraformUserID string) sdk.TestCheckFunc { return func(state *terraform.State) error { userID, err := getResourceID(state, getResourceNameFromID(terraformUserID)) diff --git a/twingate/internal/test/acctests/resource/resource_test.go b/twingate/internal/test/acctests/resource/resource_test.go index c0d9b83b..97246a08 100644 --- a/twingate/internal/test/acctests/resource/resource_test.go +++ b/twingate/internal/test/acctests/resource/resource_test.go @@ -2548,14 +2548,7 @@ func TestAccTwingateResourceUpdateSecurityPolicy(t *testing.T) { theResource := acctests.TerraformResource(resourceName) remoteNetworkName := test.RandomName() - policies, err := acctests.ListSecurityPolicies() - if err != nil { - t.Skipf("failed to retrieve security policies: %v", err) - } - - if len(policies) < 2 { - t.Skip("requires at least 2 security policy for the test") - } + defaultPolicy, testPolicy := preparePolicies(t) sdk.Test(t, sdk.TestCase{ ProtoV6ProviderFactories: acctests.ProviderFactories, @@ -2563,24 +2556,24 @@ func TestAccTwingateResourceUpdateSecurityPolicy(t *testing.T) { CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithSecurityPolicy(remoteNetworkName, resourceName, policies[0].ID), + Config: createResourceWithSecurityPolicy(remoteNetworkName, resourceName, defaultPolicy), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), - sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, policies[0].ID), + sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, defaultPolicy), ), }, { - Config: createResourceWithSecurityPolicy(remoteNetworkName, resourceName, policies[1].ID), + Config: createResourceWithSecurityPolicy(remoteNetworkName, resourceName, testPolicy), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), - sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, policies[1].ID), + sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, testPolicy), ), }, { Config: createResourceWithoutSecurityPolicy(remoteNetworkName, resourceName), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), - sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, ""), + sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, defaultPolicy), ), }, { @@ -2619,11 +2612,7 @@ func createResourceWithoutSecurityPolicy(remoteNetwork, resource string) string `, remoteNetwork, resource) } -func TestAccTwingateResourceSetDefaultSecurityPolicyOnDisablingSecurityPolicy(t *testing.T) { - resourceName := test.RandomResourceName() - theResource := acctests.TerraformResource(resourceName) - remoteNetworkName := test.RandomName() - +func preparePolicies(t *testing.T) (string, string) { policies, err := acctests.ListSecurityPolicies() if err != nil { t.Skipf("failed to retrieve security policies: %v", err) @@ -2634,7 +2623,7 @@ func TestAccTwingateResourceSetDefaultSecurityPolicyOnDisablingSecurityPolicy(t } var defaultPolicy, testPolicy string - if policies[0].Name == "Default Policy" { + if policies[0].Name == resource.DefaultSecurityPolicyName { defaultPolicy = policies[0].ID testPolicy = policies[1].ID } else { @@ -2642,6 +2631,16 @@ func TestAccTwingateResourceSetDefaultSecurityPolicyOnDisablingSecurityPolicy(t defaultPolicy = policies[1].ID } + return defaultPolicy, testPolicy +} + +func TestAccTwingateResourceSetDefaultSecurityPolicyByDefault(t *testing.T) { + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() + + defaultPolicy, testPolicy := preparePolicies(t) + sdk.Test(t, sdk.TestCase{ ProtoV6ProviderFactories: acctests.ProviderFactories, PreCheck: func() { acctests.PreCheck(t) }, @@ -2658,12 +2657,22 @@ func TestAccTwingateResourceSetDefaultSecurityPolicyOnDisablingSecurityPolicy(t Config: createResourceWithoutSecurityPolicy(remoteNetworkName, resourceName), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), - sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, ""), + sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, defaultPolicy), acctests.CheckResourceSecurityPolicy(theResource, defaultPolicy), + // set new policy via API + acctests.UpdateResourceSecurityPolicy(theResource, testPolicy), ), + ExpectNonEmptyPlan: true, }, { Config: createResourceWithSecurityPolicy(remoteNetworkName, resourceName, ""), + Check: acctests.ComposeTestCheckFunc( + sdk.TestCheckResourceAttr(theResource, attr.SecurityPolicyID, defaultPolicy), + acctests.CheckResourceSecurityPolicy(theResource, defaultPolicy), + ), + }, + { + Config: createResourceWithoutSecurityPolicy(remoteNetworkName, resourceName), // no changes PlanOnly: true, }, diff --git a/twingate/provider.go b/twingate/provider.go index a21e4c78..40cfcd38 100644 --- a/twingate/provider.go +++ b/twingate/provider.go @@ -96,13 +96,19 @@ func configure(version string, _ *schema.Provider) func(context.Context, *schema httpMaxRetry = withDefault(data.Get(attr.HTTPMaxRetry).(int), httpMaxRetry) if network != "" { - return client.NewClient(url, - apiToken, - network, - time.Duration(httpTimeout)*time.Second, - httpMaxRetry, - version), - nil + client := client.NewClient(url, + apiToken, + network, + time.Duration(httpTimeout)*time.Second, + httpMaxRetry, + version) + + policy, _ := client.ReadSecurityPolicy(ctx, "", resource.DefaultSecurityPolicyName) + if policy != nil { + resource.DefaultSecurityPolicyID = policy.ID + } + + return client, nil } return nil, diag.Diagnostics{ From a900645bec2be27f974ab57f8026fc35ebaff81e Mon Sep 17 00:00:00 2001 From: Volodymyr Manilo Date: Fri, 17 Nov 2023 05:40:24 +0100 Subject: [PATCH 5/8] re-run acctests on feature branch --- .github/workflows/ci.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee1bfee8..7394cd65 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,6 +7,7 @@ on: pull_request: branches: - main + - feature/add-security_policy-to-resource paths-ignore: - 'README.md' @@ -15,6 +16,7 @@ on: - 'README.md' branches: - main + - feature/add-security_policy-to-resource # Ensures only 1 action runs per PR and previous is canceled on new trigger concurrency: @@ -118,7 +120,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 +171,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 From 990acfa5297c7f72cca8808d237edfd7ec98fd67 Mon Sep 17 00:00:00 2001 From: Volodymyr Manilo Date: Fri, 17 Nov 2023 06:06:40 +0100 Subject: [PATCH 6/8] allow tests run in parallel --- .../test/acctests/resource/resource_test.go | 145 ++++++++++-------- 1 file changed, 81 insertions(+), 64 deletions(-) diff --git a/twingate/internal/test/acctests/resource/resource_test.go b/twingate/internal/test/acctests/resource/resource_test.go index 97246a08..067f5b9d 100644 --- a/twingate/internal/test/acctests/resource/resource_test.go +++ b/twingate/internal/test/acctests/resource/resource_test.go @@ -2087,7 +2087,8 @@ func TestAccTwingateResourceUpdateWithPort(t *testing.T) { } func TestAccTwingateResourceWithPortsFailsForAllowAllAndDenyAllPolicy(t *testing.T) { - const terraformResourceName = "test28" + t.Parallel() + remoteNetworkName := test.RandomName() resourceName := test.RandomResourceName() @@ -2097,11 +2098,11 @@ func TestAccTwingateResourceWithPortsFailsForAllowAllAndDenyAllPolicy(t *testing CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), + Config: createResourceWithPorts(resourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), ExpectError: regexp.MustCompile(resource.ErrPortsWithPolicyAllowAll.Error()), }, { - Config: createResourceWithPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), + Config: createResourceWithPorts(resourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), ExpectError: regexp.MustCompile(resource.ErrPortsWithPolicyDenyAll.Error()), }, }, @@ -2133,10 +2134,11 @@ func createResourceWithPorts(name, networkName, resourceName, policy string) str } func TestAccTwingateResourceWithoutPortsOkForAllowAllAndDenyAllPolicy(t *testing.T) { - const terraformResourceName = "test29" - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() - theResource := acctests.TerraformResource(terraformResourceName) + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() sdk.Test(t, sdk.TestCase{ ProtoV6ProviderFactories: acctests.ProviderFactories, @@ -2144,7 +2146,7 @@ func TestAccTwingateResourceWithoutPortsOkForAllowAllAndDenyAllPolicy(t *testing CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithoutPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), + Config: createResourceWithoutPorts(resourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyAllowAll), @@ -2152,7 +2154,7 @@ func TestAccTwingateResourceWithoutPortsOkForAllowAllAndDenyAllPolicy(t *testing ), }, { - Config: createResourceWithoutPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), + Config: createResourceWithoutPorts(resourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyDenyAll), @@ -2187,10 +2189,11 @@ func createResourceWithoutPorts(name, networkName, resourceName, policy string) } func TestAccTwingateResourceWithRestrictedPolicy(t *testing.T) { - const terraformResourceName = "test30" - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() - theResource := acctests.TerraformResource(terraformResourceName) + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() sdk.Test(t, sdk.TestCase{ ProtoV6ProviderFactories: acctests.ProviderFactories, @@ -2198,7 +2201,7 @@ func TestAccTwingateResourceWithRestrictedPolicy(t *testing.T) { CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyRestricted), + Config: createResourceWithPorts(resourceName, remoteNetworkName, resourceName, model.PolicyRestricted), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyRestricted), @@ -2210,10 +2213,11 @@ func TestAccTwingateResourceWithRestrictedPolicy(t *testing.T) { } func TestAccTwingateResourcePolicyTransitionDenyAllToRestricted(t *testing.T) { - const terraformResourceName = "test31" - theResource := acctests.TerraformResource(terraformResourceName) - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() sdk.Test(t, sdk.TestCase{ ProtoV6ProviderFactories: acctests.ProviderFactories, @@ -2221,7 +2225,7 @@ func TestAccTwingateResourcePolicyTransitionDenyAllToRestricted(t *testing.T) { CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithoutPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), + Config: createResourceWithoutPorts(resourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyDenyAll), @@ -2229,7 +2233,7 @@ func TestAccTwingateResourcePolicyTransitionDenyAllToRestricted(t *testing.T) { ), }, { - Config: createResourceWithPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyRestricted), + Config: createResourceWithPorts(resourceName, remoteNetworkName, resourceName, model.PolicyRestricted), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyRestricted), @@ -2241,10 +2245,11 @@ func TestAccTwingateResourcePolicyTransitionDenyAllToRestricted(t *testing.T) { } func TestAccTwingateResourcePolicyTransitionDenyAllToAllowAll(t *testing.T) { - const terraformResourceName = "test32" - theResource := acctests.TerraformResource(terraformResourceName) - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() sdk.Test(t, sdk.TestCase{ ProtoV6ProviderFactories: acctests.ProviderFactories, @@ -2252,7 +2257,7 @@ func TestAccTwingateResourcePolicyTransitionDenyAllToAllowAll(t *testing.T) { CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithoutPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), + Config: createResourceWithoutPorts(resourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyDenyAll), @@ -2260,7 +2265,7 @@ func TestAccTwingateResourcePolicyTransitionDenyAllToAllowAll(t *testing.T) { ), }, { - Config: createResourceWithoutPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), + Config: createResourceWithoutPorts(resourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyAllowAll), @@ -2272,10 +2277,11 @@ func TestAccTwingateResourcePolicyTransitionDenyAllToAllowAll(t *testing.T) { } func TestAccTwingateResourcePolicyTransitionRestrictedToDenyAll(t *testing.T) { - const terraformResourceName = "test33" - theResource := acctests.TerraformResource(terraformResourceName) - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() sdk.Test(t, sdk.TestCase{ ProtoV6ProviderFactories: acctests.ProviderFactories, @@ -2283,7 +2289,7 @@ func TestAccTwingateResourcePolicyTransitionRestrictedToDenyAll(t *testing.T) { CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyRestricted), + Config: createResourceWithPorts(resourceName, remoteNetworkName, resourceName, model.PolicyRestricted), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyRestricted), @@ -2291,7 +2297,7 @@ func TestAccTwingateResourcePolicyTransitionRestrictedToDenyAll(t *testing.T) { ), }, { - Config: createResourceWithoutPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), + Config: createResourceWithoutPorts(resourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyDenyAll), @@ -2303,10 +2309,11 @@ func TestAccTwingateResourcePolicyTransitionRestrictedToDenyAll(t *testing.T) { } func TestAccTwingateResourcePolicyTransitionRestrictedToAllowAll(t *testing.T) { - const terraformResourceName = "test34" - theResource := acctests.TerraformResource(terraformResourceName) - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() sdk.Test(t, sdk.TestCase{ ProtoV6ProviderFactories: acctests.ProviderFactories, @@ -2314,7 +2321,7 @@ func TestAccTwingateResourcePolicyTransitionRestrictedToAllowAll(t *testing.T) { CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyRestricted), + Config: createResourceWithPorts(resourceName, remoteNetworkName, resourceName, model.PolicyRestricted), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyRestricted), @@ -2322,7 +2329,7 @@ func TestAccTwingateResourcePolicyTransitionRestrictedToAllowAll(t *testing.T) { ), }, { - Config: createResourceWithoutPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), + Config: createResourceWithoutPorts(resourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyAllowAll), @@ -2334,10 +2341,11 @@ func TestAccTwingateResourcePolicyTransitionRestrictedToAllowAll(t *testing.T) { } func TestAccTwingateResourcePolicyTransitionRestrictedToAllowAllWithPortsShouldFail(t *testing.T) { - const terraformResourceName = "test35" - theResource := acctests.TerraformResource(terraformResourceName) - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() sdk.Test(t, sdk.TestCase{ ProtoV6ProviderFactories: acctests.ProviderFactories, @@ -2345,7 +2353,7 @@ func TestAccTwingateResourcePolicyTransitionRestrictedToAllowAllWithPortsShouldF CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyRestricted), + Config: createResourceWithPorts(resourceName, remoteNetworkName, resourceName, model.PolicyRestricted), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyRestricted), @@ -2353,7 +2361,7 @@ func TestAccTwingateResourcePolicyTransitionRestrictedToAllowAllWithPortsShouldF ), }, { - Config: createResourceWithPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), + Config: createResourceWithPorts(resourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), ExpectError: regexp.MustCompile(resource.ErrPortsWithPolicyAllowAll.Error()), }, }, @@ -2361,10 +2369,11 @@ func TestAccTwingateResourcePolicyTransitionRestrictedToAllowAllWithPortsShouldF } func TestAccTwingateResourcePolicyTransitionAllowAllToRestricted(t *testing.T) { - const terraformResourceName = "test36" - theResource := acctests.TerraformResource(terraformResourceName) - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() sdk.Test(t, sdk.TestCase{ ProtoV6ProviderFactories: acctests.ProviderFactories, @@ -2372,7 +2381,7 @@ func TestAccTwingateResourcePolicyTransitionAllowAllToRestricted(t *testing.T) { CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithoutPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), + Config: createResourceWithoutPorts(resourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyAllowAll), @@ -2380,7 +2389,7 @@ func TestAccTwingateResourcePolicyTransitionAllowAllToRestricted(t *testing.T) { ), }, { - Config: createResourceWithPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyRestricted), + Config: createResourceWithPorts(resourceName, remoteNetworkName, resourceName, model.PolicyRestricted), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyRestricted), @@ -2392,10 +2401,11 @@ func TestAccTwingateResourcePolicyTransitionAllowAllToRestricted(t *testing.T) { } func TestAccTwingateResourcePolicyTransitionAllowAllToDenyAll(t *testing.T) { - const terraformResourceName = "test37" - theResource := acctests.TerraformResource(terraformResourceName) - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() sdk.Test(t, sdk.TestCase{ ProtoV6ProviderFactories: acctests.ProviderFactories, @@ -2403,7 +2413,7 @@ func TestAccTwingateResourcePolicyTransitionAllowAllToDenyAll(t *testing.T) { CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithoutPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), + Config: createResourceWithoutPorts(resourceName, remoteNetworkName, resourceName, model.PolicyAllowAll), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyAllowAll), @@ -2411,7 +2421,7 @@ func TestAccTwingateResourcePolicyTransitionAllowAllToDenyAll(t *testing.T) { ), }, { - Config: createResourceWithoutPorts(terraformResourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), + Config: createResourceWithoutPorts(resourceName, remoteNetworkName, resourceName, model.PolicyDenyAll), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), sdk.TestCheckResourceAttr(theResource, tcpPolicy, model.PolicyDenyAll), @@ -2423,10 +2433,11 @@ func TestAccTwingateResourcePolicyTransitionAllowAllToDenyAll(t *testing.T) { } func TestAccTwingateResourceWithBrowserOption(t *testing.T) { - const terraformResourceName = "test40" - theResource := acctests.TerraformResource(terraformResourceName) - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() wildcardAddress := "*.acc-test.com" sdk.Test(t, sdk.TestCase{ @@ -2435,19 +2446,19 @@ func TestAccTwingateResourceWithBrowserOption(t *testing.T) { CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithoutBrowserOption(terraformResourceName, remoteNetworkName, resourceName, wildcardAddress), + Config: createResourceWithoutBrowserOption(resourceName, remoteNetworkName, resourceName, wildcardAddress), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), ), }, { - Config: createResourceWithBrowserOption(terraformResourceName, remoteNetworkName, resourceName, wildcardAddress, false), + Config: createResourceWithBrowserOption(resourceName, remoteNetworkName, resourceName, wildcardAddress, false), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), ), }, { - Config: createResourceWithBrowserOption(terraformResourceName, remoteNetworkName, resourceName, wildcardAddress, true), + Config: createResourceWithBrowserOption(resourceName, remoteNetworkName, resourceName, wildcardAddress, true), ExpectError: regexp.MustCompile(resource.ErrWildcardAddressWithEnabledShortcut.Error()), }, }, @@ -2455,10 +2466,11 @@ func TestAccTwingateResourceWithBrowserOption(t *testing.T) { } func TestAccTwingateResourceWithBrowserOptionFailOnUpdate(t *testing.T) { - const terraformResourceName = "test41" - theResource := acctests.TerraformResource(terraformResourceName) - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() wildcardAddress := "*.acc-test.com" simpleAddress := "acc-test.com" @@ -2468,19 +2480,19 @@ func TestAccTwingateResourceWithBrowserOptionFailOnUpdate(t *testing.T) { CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithoutBrowserOption(terraformResourceName, remoteNetworkName, resourceName, simpleAddress), + Config: createResourceWithoutBrowserOption(resourceName, remoteNetworkName, resourceName, simpleAddress), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), ), }, { - Config: createResourceWithBrowserOption(terraformResourceName, remoteNetworkName, resourceName, simpleAddress, true), + Config: createResourceWithBrowserOption(resourceName, remoteNetworkName, resourceName, simpleAddress, true), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), ), }, { - Config: createResourceWithBrowserOption(terraformResourceName, remoteNetworkName, resourceName, wildcardAddress, true), + Config: createResourceWithBrowserOption(resourceName, remoteNetworkName, resourceName, wildcardAddress, true), ExpectError: regexp.MustCompile(resource.ErrWildcardAddressWithEnabledShortcut.Error()), }, }, @@ -2488,10 +2500,11 @@ func TestAccTwingateResourceWithBrowserOptionFailOnUpdate(t *testing.T) { } func TestAccTwingateResourceWithBrowserOptionRecovered(t *testing.T) { - const terraformResourceName = "test42" - theResource := acctests.TerraformResource(terraformResourceName) - remoteNetworkName := test.RandomName() + t.Parallel() + resourceName := test.RandomResourceName() + theResource := acctests.TerraformResource(resourceName) + remoteNetworkName := test.RandomName() wildcardAddress := "*.acc-test.com" simpleAddress := "acc-test.com" @@ -2501,13 +2514,13 @@ func TestAccTwingateResourceWithBrowserOptionRecovered(t *testing.T) { CheckDestroy: acctests.CheckTwingateResourceDestroy, Steps: []sdk.TestStep{ { - Config: createResourceWithBrowserOption(terraformResourceName, remoteNetworkName, resourceName, simpleAddress, true), + Config: createResourceWithBrowserOption(resourceName, remoteNetworkName, resourceName, simpleAddress, true), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), ), }, { - Config: createResourceWithoutBrowserOption(terraformResourceName, remoteNetworkName, resourceName, wildcardAddress), + Config: createResourceWithoutBrowserOption(resourceName, remoteNetworkName, resourceName, wildcardAddress), Check: acctests.ComposeTestCheckFunc( acctests.CheckTwingateResourceExists(theResource), ), @@ -2544,6 +2557,8 @@ func createResourceWithBrowserOption(name, networkName, resourceName, address st } func TestAccTwingateResourceUpdateSecurityPolicy(t *testing.T) { + t.Parallel() + resourceName := test.RandomResourceName() theResource := acctests.TerraformResource(resourceName) remoteNetworkName := test.RandomName() @@ -2635,6 +2650,8 @@ func preparePolicies(t *testing.T) (string, string) { } func TestAccTwingateResourceSetDefaultSecurityPolicyByDefault(t *testing.T) { + t.Parallel() + resourceName := test.RandomResourceName() theResource := acctests.TerraformResource(resourceName) remoteNetworkName := test.RandomName() From a8d02e8689b088c42a263f68f2ecc3272fa1d8bd Mon Sep 17 00:00:00 2001 From: Volodymyr Manilo Date: Fri, 17 Nov 2023 06:32:52 +0100 Subject: [PATCH 7/8] remove feature branch from ci --- .github/workflows/ci.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7394cd65..ee1bfee8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,6 @@ on: pull_request: branches: - main - - feature/add-security_policy-to-resource paths-ignore: - 'README.md' @@ -16,7 +15,6 @@ on: - 'README.md' branches: - main - - feature/add-security_policy-to-resource # Ensures only 1 action runs per PR and previous is canceled on new trigger concurrency: @@ -120,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 @@ -171,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 From 26e9f25e5761e30ea9c19df51fdbc78277b4f941 Mon Sep 17 00:00:00 2001 From: Volodymyr Manilo Date: Mon, 20 Nov 2023 20:54:46 +0100 Subject: [PATCH 8/8] updated doc --- docs/resources/resource.md | 2 +- twingate/internal/provider/resource/resource.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/resources/resource.md b/docs/resources/resource.md index 665f4372..9e9fbdf0 100644 --- a/docs/resources/resource.md +++ b/docs/resources/resource.md @@ -76,7 +76,7 @@ resource "twingate_resource" "resource" { - `is_browser_shortcut_enabled` (Boolean) Controls whether an "Open in Browser" shortcut will be shown for this Resource in the Twingate Client. - `is_visible` (Boolean) Controls whether this Resource will be visible in the main Resource list in the Twingate Client. - `protocols` (Block List, Max: 1) Restrict access to certain protocols and ports. By default or when this argument is not defined, there is no restriction, and all protocols and ports are allowed. (see [below for nested schema](#nestedblock--protocols)) -- `security_policy_id` (String) The ID of a `twingate_security_policy` to set as this Resource's Security Policy. +- `security_policy_id` (String) The ID of a `twingate_security_policy` to set as this Resource's Security Policy. Default is `Default Policy` ### Read-Only diff --git a/twingate/internal/provider/resource/resource.go b/twingate/internal/provider/resource/resource.go index e7ebea69..147f3e1c 100644 --- a/twingate/internal/provider/resource/resource.go +++ b/twingate/internal/provider/resource/resource.go @@ -142,7 +142,7 @@ func Resource() *schema.Resource { //nolint:funlen attr.SecurityPolicyID: { Type: schema.TypeString, Optional: true, - Description: "The ID of a `twingate_security_policy` to set as this Resource's Security Policy.", + Description: "The ID of a `twingate_security_policy` to set as this Resource's Security Policy. Default is `Default Policy`", DiffSuppressOnRefresh: true, DiffSuppressFunc: defaultPolicyNotChanged, },