Skip to content

Commit

Permalink
Resolve bug in validation of a chrome_policy resource with multiple s…
Browse files Browse the repository at this point in the history 
…chema_values of differing types (hashicorp#3)

* add chrome policy test case: schema_value correctly contains multiple values of different types

* Do not iterate over and validate against all schema field definitions during validation. Instead, access only the relevant def from schemaFields map.

* make fmt

* rename test case for clarity

* fix test name in invocation
  • Loading branch information
@w0de
w0de authored Nov 27, 2023
1 parent b55be9e commit 2d50b07
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 53 deletions.
101 changes: 48 additions & 53 deletions internal/provider/resource_chrome_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,10 @@ func validateChromePolicies(ctx context.Context, d *schema.ResourceData, client
})
}

schemaFieldMap := map[string][]*chromepolicy.Proto2FieldDescriptorProto{}
schemaFieldMap := map[string]*chromepolicy.Proto2FieldDescriptorProto{}
for _, schemaField := range schemaDef.Definition.MessageType {
for _, schemaNestedField := range schemaField.Field {
schemaFieldMap[schemaNestedField.Name] = schemaField.Field
for i, schemaNestedField := range schemaField.Field {
schemaFieldMap[schemaNestedField.Name] = schemaField.Field[i]
}
}

Expand All @@ -348,43 +348,40 @@ func validateChromePolicies(ctx context.Context, d *schema.ResourceData, client
return diag.FromErr(err)
}

for _, schemaField := range schemaFieldMap[polKey] {
schemaField := schemaFieldMap[polKey]
if schemaField == nil {
return append(diags, diag.Diagnostic{
Summary: fmt.Sprintf("field type is not defined for field name (%s)", polKey),
Severity: diag.Warning,
})
}

if schemaField == nil {
if schemaField.Label == "LABEL_REPEATED" {
polValType := reflect.ValueOf(polVal).Kind()
if !((polValType == reflect.Array) || (polValType == reflect.Slice)) {
return append(diags, diag.Diagnostic{
Summary: fmt.Sprintf("field type is not defined for field name (%s)", polKey),
Severity: diag.Warning,
Summary: fmt.Sprintf("value provided for %s is of incorrect type %v (expected type: []%v)", schemaField.Name, polValType, schemaField.Type),
Severity: diag.Error,
})
}

if schemaField.Label == "LABEL_REPEATED" {
polValType := reflect.ValueOf(polVal).Kind()
if !((polValType == reflect.Array) || (polValType == reflect.Slice)) {
return append(diags, diag.Diagnostic{
Summary: fmt.Sprintf("value provided for %s is of incorrect type %v (expected type: []%v)", schemaField.Name, polValType, schemaField.Type),
Severity: diag.Error,
})
} else {
if polValArray, ok := polVal.([]interface{}); ok {
for _, polValItem := range polValArray {
if !validatePolicyFieldValueType(schemaField.Type, polValItem) {
return append(diags, diag.Diagnostic{
Summary: fmt.Sprintf("array value %v provided for %s is of incorrect type (expected type: %s)", polValItem, schemaField.Name, schemaField.Type),
Severity: diag.Error,
})
}
} else {
if polValArray, ok := polVal.([]interface{}); ok {
for _, polValItem := range polValArray {
if !validatePolicyFieldValueType(schemaField.Type, polValItem) {
return append(diags, diag.Diagnostic{
Summary: fmt.Sprintf("array value %v provided for %s is of incorrect type (expected type: %s)", polValItem, schemaField.Name, schemaField.Type),
Severity: diag.Error,
})
}
}
}
} else {
if !validatePolicyFieldValueType(schemaField.Type, polVal) {
return append(diags, diag.Diagnostic{
Summary: fmt.Sprintf("value %v provided for %s is of incorrect type (expected type: %s)", polVal, schemaField.Name, schemaField.Type),
Severity: diag.Error,
})
}
}

} else {
if !validatePolicyFieldValueType(schemaField.Type, polVal) {
return append(diags, diag.Diagnostic{
Summary: fmt.Sprintf("value %v provided for %s is of incorrect type (expected type: %s)", polVal, schemaField.Name, schemaField.Type),
Severity: diag.Error,
})
}
}
}
}
Expand Down Expand Up @@ -566,10 +563,10 @@ func flattenChromePolicies(ctx context.Context, policiesObj []*chromepolicy.Goog
})
}

schemaFieldMap := map[string][]*chromepolicy.Proto2FieldDescriptorProto{}
schemaFieldMap := map[string]*chromepolicy.Proto2FieldDescriptorProto{}
for _, schemaField := range schemaDef.Definition.MessageType {
for _, schemaNestedField := range schemaField.Field {
schemaFieldMap[schemaNestedField.Name] = schemaField.Field
for i, schemaNestedField := range schemaField.Field {
schemaFieldMap[schemaNestedField.Name] = schemaField.Field[i]
}
}

Expand All @@ -589,26 +586,24 @@ func flattenChromePolicies(ctx context.Context, policiesObj []*chromepolicy.Goog
})
}

for _, schemaField := range schemaFieldMap[k] {

if schemaField == nil {
return nil, append(diags, diag.Diagnostic{
Summary: fmt.Sprintf("field type is not defined for field name (%s)", k),
Severity: diag.Warning,
})
}
schemaField := schemaFieldMap[k]
if schemaField == nil {
return nil, append(diags, diag.Diagnostic{
Summary: fmt.Sprintf("field type is not defined for field name (%s)", k),
Severity: diag.Warning,
})
}

val, err := convertPolicyFieldValueType(schemaField.Type, v)
if err != nil {
return nil, diag.FromErr(err)
}
val, err := convertPolicyFieldValueType(schemaField.Type, v)
if err != nil {
return nil, diag.FromErr(err)
}

jsonVal, err := json.Marshal(val)
if err != nil {
return nil, diag.FromErr(err)
}
schemaValues[k] = string(jsonVal)
jsonVal, err := json.Marshal(val)
if err != nil {
return nil, diag.FromErr(err)
}
schemaValues[k] = string(jsonVal)
}

policies = append(policies, map[string]interface{}{
Expand Down
30 changes: 30 additions & 0 deletions internal/provider/resource_chrome_policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,16 @@ func TestAccResourceChromePolicy_multiple(t *testing.T) {
testCheck,
),
},
{
Config: testAccResourceChromePolicy_multipleValueTypes(ouName, true, "POLICY_MODE_RECOMMENDED"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("googleworkspace_chrome_policy.test", "policies.#", "1"),
resource.TestCheckResourceAttr("googleworkspace_chrome_policy.test", "policies.0.schema_name", "chrome.users.DomainReliabilityAllowed"),
resource.TestCheckResourceAttr("googleworkspace_chrome_policy.test", "policies.0.schema_values.domainReliabilityAllowed", "true"),
resource.TestCheckResourceAttr("googleworkspace_chrome_policy.test", "policies.0.schema_values.domainReliabilityAllowedSettingGroupPolicyMode", encode("POLICY_MODE_RECOMMENDED")),
testCheck,
),
},
},
})
}
Expand Down Expand Up @@ -253,6 +263,26 @@ resource "googleworkspace_chrome_policy" "test" {
`, ouName, enabled, pattern)
}

func testAccResourceChromePolicy_multipleValueTypes(ouName string, enabled bool, policyMode string) string {
return fmt.Sprintf(`
resource "googleworkspace_org_unit" "test" {
name = "%s"
parent_org_unit_path = "/"
}
resource "googleworkspace_chrome_policy" "test" {
org_unit_id = googleworkspace_org_unit.test.id
policies {
schema_name = "chrome.users.DomainReliabilityAllowed"
schema_values = {
domainReliabilityAllowed = jsonencode(%t)
domainReliabilityAllowedSettingGroupPolicyMode = jsonencode("%s")
}
}
}
`, ouName, enabled, policyMode)
}

func testAccResourceChromePolicy_basic(ouName string, conns int) string {
return fmt.Sprintf(`
resource "googleworkspace_org_unit" "test" {
Expand Down

0 comments on commit 2d50b07

Please sign in to comment.