Skip to content

Commit

Permalink
storage: validating the metadata keys so an error's returned
Browse files Browse the repository at this point in the history
  • Loading branch information
tombuildsstuff committed Jul 12, 2019
1 parent 945ba31 commit 1079afa
Show file tree
Hide file tree
Showing 3 changed files with 175 additions and 5 deletions.
115 changes: 112 additions & 3 deletions azurerm/internal/services/storage/metadata.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
package storage

import "github.com/hashicorp/terraform/helper/schema"
import (
"fmt"
"regexp"
"strings"

"github.com/hashicorp/terraform/helper/schema"
)

func MetaDataSchema() *schema.Schema {
return &schema.Schema{
Type: schema.TypeMap,
Optional: true,
Type: schema.TypeMap,
Optional: true,
ValidateFunc: validateMetaDataKeys,
}
}

Expand All @@ -28,3 +35,105 @@ func FlattenMetaData(input map[string]string) map[string]interface{} {

return output
}

func validateMetaDataKeys(value interface{}, fieldName string) (warnings []string, errors []error) {
v, ok := value.(map[string]interface{})
if !ok {
return
}

for k := range v {
isCSharpKeyword := cSharpKeywords[strings.ToLower(k)] != nil
if isCSharpKeyword {
errors = append(errors, fmt.Errorf("%q is not a valid key (C# keyword)", k))
}

// must begin with a letter, underscore
// the rest: letters, digits and underscores
r, _ := regexp.Compile(`^([a-z_]{1}[a-z0-9_]{1,})$`)

This comment has been minimized.

Copy link
@rrmistry

rrmistry Nov 30, 2019

Hi @tombuildsstuff , I'm curious why this regex expression only accepts lower case?

May I send a pull request to use ^([a-zA-Z_]{1}[a-zA-Z0-9_]{1,})$ expression instead?

I tested it locally and everything seems to work with Azure and Terraform v0.12.17-dev.

Related to #4901, where I was using metadata tag with all caps.

This comment has been minimized.

Copy link
@tombuildsstuff

tombuildsstuff Dec 2, 2019

Author Contributor

@rrmistry from memory whilst the API accepts Upper and lower case - the API only returns lower-case; as such to not show a perpetual diff this is why this is limited to lower-case here (since in practice the API converts it to lower-case).

That said - if you can add an acceptance test for the resources using MetaData showing this works in Upper Case too then we're more than happy to accept a PR to enable upper-case here :)

if !r.MatchString(k) {
errors = append(errors, fmt.Errorf("MetaData must start with letters or an underscores. Got %q.", k))
}
}

return
}

var cSharpKeywords = map[string]*struct{}{
"abstract": {},
"as": {},
"base": {},
"bool": {},
"break": {},
"byte": {},
"case": {},
"catch": {},
"char": {},
"checked": {},
"class": {},
"const": {},
"continue": {},
"decimal": {},
"default": {},
"delegate": {},
"do": {},
"double": {},
"else": {},
"enum": {},
"event": {},
"explicit": {},
"extern": {},
"false": {},
"finally": {},
"fixed": {},
"float": {},
"for": {},
"foreach": {},
"goto": {},
"if": {},
"implicit": {},
"in": {},
"int": {},
"interface": {},
"internal": {},
"is": {},
"lock": {},
"long": {},
"namespace": {},
"new": {},
"null": {},
"object": {},
"operator": {},
"out": {},
"override": {},
"params": {},
"private": {},
"protected": {},
"public": {},
"readonly": {},
"ref": {},
"return": {},
"sbyte": {},
"sealed": {},
"short": {},
"sizeof": {},
"stackalloc": {},
"static": {},
"string": {},
"struct": {},
"switch": {},
"this": {},
"throw": {},
"true": {},
"try": {},
"typeof": {},
"uint": {},
"ulong": {},
"unchecked": {},
"unsafe": {},
"ushort": {},
"using": {},
"void": {},
"volatile": {},
"while": {},
}
61 changes: 61 additions & 0 deletions azurerm/internal/services/storage/metadata_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package storage

import "testing"

func TestValidateMetaDataKeys(t *testing.T) {
testData := []struct {
Input string
Expected bool
}{
{
Input: "",
Expected: false,
},
{
Input: "Hello",
Expected: false,
},
{
Input: "hello",
Expected: true,
},
{
Input: "hello",
Expected: true,
},
{
// C# keyword
Input: "using",
Expected: false,
},
{
Input: "0hello",
Expected: false,
},
{
Input: "heLLo",
Expected: false,
},
{
Input: "panda_cycle",
Expected: true,
},
}

for _, v := range testData {
t.Logf("[DEBUG] Testing %q", v.Input)

value := map[string]interface{}{
v.Input: "hello",
}
warnings, errors := validateMetaDataKeys(value, "field")
if len(warnings) != 0 {
t.Fatalf("Expected no warnings but got %d", len(warnings))
}

actual := len(errors) == 0
if v.Expected != actual {
t.Fatalf("Expected %t but got %t", v.Expected, actual)
}
}
}
4 changes: 2 additions & 2 deletions azurerm/resource_arm_storage_share_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ resource "azurerm_storage_share" "test" {
resource_group_name = "${azurerm_resource_group.test.name}"
storage_account_name = "${azurerm_storage_account.test.name}"
metadata {
metadata = {
hello = "world"
}
}
Expand All @@ -336,7 +336,7 @@ resource "azurerm_storage_share" "test" {
resource_group_name = "${azurerm_resource_group.test.name}"
storage_account_name = "${azurerm_storage_account.test.name}"
metadata {
metadata = {
hello = "world"
happy = "birthday"
}
Expand Down

0 comments on commit 1079afa

Please sign in to comment.