Skip to content

Commit

Permalink
Fix identity type comparison for service perimeters (#12267) (#20221)
Browse files Browse the repository at this point in the history 
[upstream:343132cfcaa07a03a385461bfe261580d4624ac4]

Signed-off-by: Modular Magician <magic-modules@google.com>
  • Loading branch information
@modular-magician
modular-magician authored Nov 7, 2024
1 parent 738ea64 commit 19a78a7
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 28 deletions.
3 changes: 3 additions & 0 deletions .changelog/12267.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note: bug
accesscontextmanager: fixed comparison of `identity_type` in `ingress_from` and `egress_from` when the `IDENTITY_TYPE_UNSPECIFIED` is set
```
80 changes: 68 additions & 12 deletions google/services/accesscontextmanager/resource_access_context_manager_service_perimeter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"log"
"net/http"
"reflect"
"slices"
"sort"
"strings"
"time"

Expand All @@ -32,6 +34,56 @@ import (
"github.com/hashicorp/terraform-provider-google/google/verify"
)

func AccessContextManagerServicePerimeterEgressToResourcesDiffSupressFunc(_, _, _ string, d *schema.ResourceData) bool {
old, new := d.GetChange("egress_to.0.resources")

oldResources, err := tpgresource.InterfaceSliceToStringSlice(old)
if err != nil {
log.Printf("[ERROR] Failed to convert config value: %s", err)
return false
}

newResources, err := tpgresource.InterfaceSliceToStringSlice(new)
if err != nil {
log.Printf("[ERROR] Failed to convert config value: %s", err)
return false
}

sort.Strings(oldResources)
sort.Strings(newResources)

return slices.Equal(oldResources, newResources)
}

func AccessContextManagerServicePerimeterIngressToResourcesDiffSupressFunc(_, _, _ string, d *schema.ResourceData) bool {
old, new := d.GetChange("ingress_to.0.resources")

oldResources, err := tpgresource.InterfaceSliceToStringSlice(old)
if err != nil {
log.Printf("[ERROR] Failed to convert config value: %s", err)
return false
}

newResources, err := tpgresource.InterfaceSliceToStringSlice(new)
if err != nil {
log.Printf("[ERROR] Failed to convert config value: %s", err)
return false
}

sort.Strings(oldResources)
sort.Strings(newResources)

return slices.Equal(oldResources, newResources)
}

func AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc(_, old, new string, _ *schema.ResourceData) bool {
if old == "" && new == "IDENTITY_TYPE_UNSPECIFIED" {
return true
}

return old == new
}

func ResourceAccessContextManagerServicePerimeter() *schema.Resource {
return &schema.Resource{
Create: resourceAccessContextManagerServicePerimeterCreate,
Expand Down Expand Up @@ -156,9 +208,10 @@ represent individual user or service account only.`,
Set: schema.HashString,
},
"identity_type": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: verify.ValidateEnum([]string{"IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
Type: schema.TypeString,
Optional: true,
ValidateFunc: verify.ValidateEnum([]string{"IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
DiffSuppressFunc: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc,
Description: `Specifies the type of identities that are allowed access to outside the
perimeter. If left unspecified, then members of 'identities' field will
be allowed access. Possible values: ["IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT"]`,
Expand Down Expand Up @@ -295,9 +348,10 @@ individual user or service account only.`,
Set: schema.HashString,
},
"identity_type": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: verify.ValidateEnum([]string{"IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
Type: schema.TypeString,
Optional: true,
ValidateFunc: verify.ValidateEnum([]string{"IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
DiffSuppressFunc: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc,
Description: `Specifies the type of identities that are allowed access from outside the
perimeter. If left unspecified, then members of 'identities' field will be
allowed access. Possible values: ["IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT"]`,
Expand Down Expand Up @@ -520,9 +574,10 @@ represent individual user or service account only.`,
Set: schema.HashString,
},
"identity_type": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: verify.ValidateEnum([]string{"IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
Type: schema.TypeString,
Optional: true,
ValidateFunc: verify.ValidateEnum([]string{"IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
DiffSuppressFunc: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc,
Description: `Specifies the type of identities that are allowed access to outside the
perimeter. If left unspecified, then members of 'identities' field will
be allowed access. Possible values: ["IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT"]`,
Expand Down Expand Up @@ -659,9 +714,10 @@ individual user or service account only.`,
Set: schema.HashString,
},
"identity_type": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: verify.ValidateEnum([]string{"IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
Type: schema.TypeString,
Optional: true,
ValidateFunc: verify.ValidateEnum([]string{"IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
DiffSuppressFunc: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc,
Description: `Specifies the type of identities that are allowed access from outside the
perimeter. If left unspecified, then members of 'identities' field will be
allowed access. Possible values: ["IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT"]`,
Expand Down
17 changes: 13 additions & 4 deletions ...contextmanager/resource_access_context_manager_service_perimeter_dry_run_egress_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ func AccessContextManagerServicePerimeterDryRunEgressPolicyIngressToResourcesDif
return slices.Equal(oldResources, newResources)
}

func AccessContextManagerServicePerimeterDryRunEgressPolicyIdentityTypeDiffSupressFunc(_, old, new string, _ *schema.ResourceData) bool {
if old == "" && new == "IDENTITY_TYPE_UNSPECIFIED" {
return true
}

return old == new
}

func ResourceAccessContextManagerServicePerimeterDryRunEgressPolicy() *schema.Resource {
return &schema.Resource{
Create: resourceAccessContextManagerServicePerimeterDryRunEgressPolicyCreate,
Expand Down Expand Up @@ -114,10 +122,11 @@ represent individual user or service account only.`,
},
},
"identity_type": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: verify.ValidateEnum([]string{"ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: verify.ValidateEnum([]string{"ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
DiffSuppressFunc: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc,
Description: `Specifies the type of identities that are allowed access to outside the
perimeter. If left unspecified, then members of 'identities' field will
be allowed access. Possible values: ["ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT"]`,
Expand Down
17 changes: 13 additions & 4 deletions ...ontextmanager/resource_access_context_manager_service_perimeter_dry_run_ingress_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ func AccessContextManagerServicePerimeterDryRunIngressPolicyIngressToResourcesDi
return slices.Equal(oldResources, newResources)
}

func AccessContextManagerServicePerimeterDryRunIngressPolicyIdentityTypeDiffSupressFunc(_, old, new string, _ *schema.ResourceData) bool {
if old == "" && new == "IDENTITY_TYPE_UNSPECIFIED" {
return true
}

return old == new
}

func ResourceAccessContextManagerServicePerimeterDryRunIngressPolicy() *schema.Resource {
return &schema.Resource{
Create: resourceAccessContextManagerServicePerimeterDryRunIngressPolicyCreate,
Expand Down Expand Up @@ -115,10 +123,11 @@ individual user or service account only.`,
},
},
"identity_type": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: verify.ValidateEnum([]string{"ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: verify.ValidateEnum([]string{"ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
DiffSuppressFunc: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc,
Description: `Specifies the type of identities that are allowed access from outside the
perimeter. If left unspecified, then members of 'identities' field will be
allowed access. Possible values: ["ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT"]`,
Expand Down
17 changes: 13 additions & 4 deletions ...s/accesscontextmanager/resource_access_context_manager_service_perimeter_egress_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ func AccessContextManagerServicePerimeterEgressPolicyIngressToResourcesDiffSupre
return slices.Equal(oldResources, newResources)
}

func AccessContextManagerServicePerimeterEgressPolicyIdentityTypeDiffSupressFunc(_, old, new string, _ *schema.ResourceData) bool {
if old == "" && new == "IDENTITY_TYPE_UNSPECIFIED" {
return true
}

return old == new
}

func ResourceAccessContextManagerServicePerimeterEgressPolicy() *schema.Resource {
return &schema.Resource{
Create: resourceAccessContextManagerServicePerimeterEgressPolicyCreate,
Expand Down Expand Up @@ -114,10 +122,11 @@ represent individual user or service account only.`,
},
},
"identity_type": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: verify.ValidateEnum([]string{"ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: verify.ValidateEnum([]string{"ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
DiffSuppressFunc: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc,
Description: `Specifies the type of identities that are allowed access to outside the
perimeter. If left unspecified, then members of 'identities' field will
be allowed access. Possible values: ["ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT"]`,
Expand Down
17 changes: 13 additions & 4 deletions .../accesscontextmanager/resource_access_context_manager_service_perimeter_ingress_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ func AccessContextManagerServicePerimeterIngressPolicyIngressToResourcesDiffSupr
return slices.Equal(oldResources, newResources)
}

func AccessContextManagerServicePerimeterIngressPolicyIdentityTypeDiffSupressFunc(_, old, new string, _ *schema.ResourceData) bool {
if old == "" && new == "IDENTITY_TYPE_UNSPECIFIED" {
return true
}

return old == new
}

func ResourceAccessContextManagerServicePerimeterIngressPolicy() *schema.Resource {
return &schema.Resource{
Create: resourceAccessContextManagerServicePerimeterIngressPolicyCreate,
Expand Down Expand Up @@ -115,10 +123,11 @@ individual user or service account only.`,
},
},
"identity_type": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: verify.ValidateEnum([]string{"ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: verify.ValidateEnum([]string{"ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT", ""}),
DiffSuppressFunc: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc,
Description: `Specifies the type of identities that are allowed access from outside the
perimeter. If left unspecified, then members of 'identities' field will be
allowed access. Possible values: ["ANY_IDENTITY", "ANY_USER_ACCOUNT", "ANY_SERVICE_ACCOUNT"]`,
Expand Down
56 changes: 56 additions & 0 deletions ...e/services/accesscontextmanager/resource_access_context_manager_service_perimeter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/hashicorp/terraform-provider-google/google/acctest"
"github.com/hashicorp/terraform-provider-google/google/envvar"
"github.com/hashicorp/terraform-provider-google/google/services/accesscontextmanager"
"github.com/hashicorp/terraform-provider-google/google/tpgresource"
transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport"
)
Expand Down Expand Up @@ -411,3 +412,58 @@ resource "google_access_context_manager_service_perimeter" "test-access" {
}
`, org, policyTitle, levelTitleName, levelTitleName, perimeterTitleName, perimeterTitleName)
}

type IdentityTypeDiffSupressFuncDiffSuppressTestCase struct {
Name string
AreEqual bool
Before string
After string
}

var identityTypeDiffSuppressTestCases = []IdentityTypeDiffSupressFuncDiffSuppressTestCase{
{
AreEqual: false,
Before: "A",
After: "B",
},
{
AreEqual: true,
Before: "A",
After: "A",
},
{
AreEqual: false,
Before: "",
After: "A",
},
{
AreEqual: false,
Before: "A",
After: "",
},
{
AreEqual: true,
Before: "",
After: "IDENTITY_TYPE_UNSPECIFIED",
},
{
AreEqual: false,
Before: "IDENTITY_TYPE_UNSPECIFIED",
After: "",
},
}

func TestUnitAccessContextManagerServicePerimeter_identityTypeDiff(t *testing.T) {
for _, tc := range identityTypeDiffSuppressTestCases {
tc.Test(t)
}
}

func (tc *IdentityTypeDiffSupressFuncDiffSuppressTestCase) Test(t *testing.T) {
actual := accesscontextmanager.AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc("", tc.Before, tc.After, nil)
if actual != tc.AreEqual {
t.Errorf(
"Unexpected difference found. Before: \"%s\", after: \"%s\", actual: %t, expected: %t",
tc.Before, tc.After, actual, tc.AreEqual)
}
}

0 comments on commit 19a78a7

Please sign in to comment.