Skip to content

Commit

Permalink
[minor] Accept siteID key as either int or string
Browse files Browse the repository at this point in the history
  • Loading branch information
@pwtyler
pwtyler committed Apr 12, 2023
1 parent 912244d commit 0961d9d
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 43 deletions.
7 changes: 7 additions & 0 deletions fixtures/sites/valid_both_string_and_int_key.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
api_version: 1
domain_maps:
dev:
1: dev-srtest.pantheonsite.io
"2": about.dev-srtest.pantheonsite.io
"3": employee-resources.dev-srtest.pantheonsite.io
7 changes: 7 additions & 0 deletions fixtures/sites/valid_string_as_key.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
api_version: 1
domain_maps:
dev:
"30": dev-srtest.pantheonsite.io
"2": about.dev-srtest.pantheonsite.io
"3": employee-resources.dev-srtest.pantheonsite.io
7 changes: 6 additions & 1 deletion pkg/model/sites.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,9 @@ type SitesYml struct {
type DomainMaps map[string]DomainMapByEnvironment

// DomainMapByEnvironment is a collection of site domains keyed by site ID.
type DomainMapByEnvironment map[int]string
//
// Given an int is valid in yaml but not json, and it is difficult to validate
// if a key is an int or string in PHP (c/f terminus-yml-validator-plugin), it
// is easiest to unmarshal into a string, and check that the key is also a
// valid integer as part of the validator.
type DomainMapByEnvironment map[string]string
11 changes: 10 additions & 1 deletion pkg/validator/sites.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"regexp"
"sites-yml-validator/pkg/model"
"strconv"

"gopkg.in/yaml.v3"
)
Expand Down Expand Up @@ -64,7 +65,10 @@ func validateDomainMaps(domainMaps map[string]model.DomainMapByEnvironment) erro
if domainMapCount > maxDomainMaps {
return fmt.Errorf("%q has too many domains listed (%d). Maximum is %d", env, domainMapCount, maxDomainMaps)
}
for _, domain := range domainMap {
for siteID, domain := range domainMap {
if !isValidSiteID(siteID) {
return fmt.Errorf("%q is not a valid site ID", siteID)
}
if !validHostname.MatchString(domain) {
return fmt.Errorf("%q is not a valid hostname", domain)
}
Expand All @@ -81,3 +85,8 @@ func validateAPIVersion(apiVersion int) error {
}
return nil
}

func isValidSiteID(s string) bool {
_, err := strconv.Atoi(s)
return err == nil
}
116 changes: 75 additions & 41 deletions pkg/validator/sites_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,16 @@ func TestValidate(t *testing.T) {
APIVersion: 1,
DomainMaps: model.DomainMaps{
"dev": model.DomainMapByEnvironment{
1: "site1.dev-mysite.pantheonsite.io",
"1": "site1.dev-mysite.pantheonsite.io",
},
"test": model.DomainMapByEnvironment{
1: "site1.dev-mysite.pantheonsite.io",
"1": "site1.dev-mysite.pantheonsite.io",
},
"live": model.DomainMapByEnvironment{
1: "site1.mysite.com",
"1": "site1.mysite.com",
},
"autopilot": model.DomainMapByEnvironment{
1: "site1.autopilot-mysite.pantheonsite.io",
"1": "site1.autopilot-mysite.pantheonsite.io",
},
},
},
Expand All @@ -70,10 +70,10 @@ func TestValidate(t *testing.T) {
APIVersion: 1,
DomainMaps: model.DomainMaps{
"dev": model.DomainMapByEnvironment{
1: "site1.dev-mysite.pantheonsite.io",
"1": "site1.dev-mysite.pantheonsite.io",
},
"mylongmultidevname": model.DomainMapByEnvironment{
1: "site1.mylongmultidevname-mysite.pantheonsite.io",
"1": "site1.mylongmultidevname-mysite.pantheonsite.io",
},
},
},
Expand All @@ -85,10 +85,10 @@ func TestValidate(t *testing.T) {
APIVersion: 1,
DomainMaps: model.DomainMaps{
"dev": model.DomainMapByEnvironment{
1: "site1.dev-mysite.pantheonsite.io",
"1": "site1.dev-mysite.pantheonsite.io",
},
"feat_branch": model.DomainMapByEnvironment{
1: "site1.feat-branch-mysite.pantheonsite.io",
"1": "site1.feat-branch-mysite.pantheonsite.io",
},
},
},
Expand All @@ -100,38 +100,38 @@ func TestValidate(t *testing.T) {
APIVersion: 1,
DomainMaps: model.DomainMaps{
"dev": model.DomainMapByEnvironment{
1: "site1.dev-mysite.pantheonsite.io",
2: "site2.dev-mysite.pantheonsite.io",
3: "site3.dev-mysite.pantheonsite.io",
4: "site4.dev-mysite.pantheonsite.io",
5: "site5.dev-mysite.pantheonsite.io",
6: "site6.dev-mysite.pantheonsite.io",
7: "site7.dev-mysite.pantheonsite.io",
8: "site8.dev-mysite.pantheonsite.io",
9: "site9.dev-mysite.pantheonsite.io",
10: "site10.dev-mysite.pantheonsite.io",
11: "site11.dev-mysite.pantheonsite.io",
12: "site12.dev-mysite.pantheonsite.io",
13: "site13.dev-mysite.pantheonsite.io",
14: "site14.dev-mysite.pantheonsite.io",
15: "site15.dev-mysite.pantheonsite.io",
16: "site16.dev-mysite.pantheonsite.io",
17: "site17.dev-mysite.pantheonsite.io",
18: "site18.dev-mysite.pantheonsite.io",
19: "site19.dev-mysite.pantheonsite.io",
20: "site20.dev-mysite.pantheonsite.io",
21: "site21.dev-mysite.pantheonsite.io",
22: "site22.dev-mysite.pantheonsite.io",
23: "site23.dev-mysite.pantheonsite.io",
24: "site24.dev-mysite.pantheonsite.io",
25: "site25.dev-mysite.pantheonsite.io",
26: "site26.dev-mysite.pantheonsite.io",
27: "site27.dev-mysite.pantheonsite.io",
28: "site28.dev-mysite.pantheonsite.io",
29: "site29.dev-mysite.pantheonsite.io",
"1": "site1.dev-mysite.pantheonsite.io",
"2": "site2.dev-mysite.pantheonsite.io",
"3": "site3.dev-mysite.pantheonsite.io",
"4": "site4.dev-mysite.pantheonsite.io",
"5": "site5.dev-mysite.pantheonsite.io",
"6": "site6.dev-mysite.pantheonsite.io",
"7": "site7.dev-mysite.pantheonsite.io",
"8": "site8.dev-mysite.pantheonsite.io",
"9": "site9.dev-mysite.pantheonsite.io",
"10": "site10.dev-mysite.pantheonsite.io",
"11": "site11.dev-mysite.pantheonsite.io",
"12": "site12.dev-mysite.pantheonsite.io",
"13": "site13.dev-mysite.pantheonsite.io",
"14": "site14.dev-mysite.pantheonsite.io",
"15": "site15.dev-mysite.pantheonsite.io",
"16": "site16.dev-mysite.pantheonsite.io",
"17": "site17.dev-mysite.pantheonsite.io",
"18": "site18.dev-mysite.pantheonsite.io",
"19": "site19.dev-mysite.pantheonsite.io",
"20": "site20.dev-mysite.pantheonsite.io",
"21": "site21.dev-mysite.pantheonsite.io",
"22": "site22.dev-mysite.pantheonsite.io",
"23": "site23.dev-mysite.pantheonsite.io",
"24": "site24.dev-mysite.pantheonsite.io",
"25": "site25.dev-mysite.pantheonsite.io",
"26": "site26.dev-mysite.pantheonsite.io",
"27": "site27.dev-mysite.pantheonsite.io",
"28": "site28.dev-mysite.pantheonsite.io",
"29": "site29.dev-mysite.pantheonsite.io",
},
"mdev": model.DomainMapByEnvironment{
1: "site1.mdev-mysite.pantheonsite.io",
"1": "site1.mdev-mysite.pantheonsite.io",
},
},
},
Expand All @@ -143,18 +143,33 @@ func TestValidate(t *testing.T) {
APIVersion: 1,
DomainMaps: model.DomainMaps{
"dev": model.DomainMapByEnvironment{
1: "site1.dev-mysite.pantheonsite.io",
"1": "site1.dev-mysite.pantheonsite.io",
},
"test": model.DomainMapByEnvironment{
1: "$(sudo do something dangerous)",
"1": "$(sudo do something dangerous)",
},
"live": model.DomainMapByEnvironment{
1: "site1.mysite.com",
"1": "site1.mysite.com",
},
},
},
errors.New(`"$(sudo do something dangerous)" is not a valid hostname`),
},
{
"invalid site id",
model.SitesYml{
APIVersion: 1,
DomainMaps: model.DomainMaps{
"dev": model.DomainMapByEnvironment{
"foo": "site1.dev-mysite.pantheonsite.io",
},
"test": model.DomainMapByEnvironment{
"1": "$(sudo do something dangerous)",
},
},
},
errors.New(`"foo" is not a valid site ID`),
},
} {
t.Run(tc.name, func(t *testing.T) {
v, err := ValidatorFactory("sites")
Expand Down Expand Up @@ -232,6 +247,8 @@ func TestValidateSitesFromFilePath(t *testing.T) {
"error reading YAML file: open ../../fixtures/sites/this_file_does_not_exist.yml: no such file or directory",
),
},
{"valid_string_as_key", nil},
{"valid_both_string_and_int_key", nil},
} {
t.Run(tc.fixtureName, func(t *testing.T) {
v, err := ValidatorFactory("sites")
Expand All @@ -249,3 +266,20 @@ func TestValidateSitesFromFilePath(t *testing.T) {
})
}
}

func TestIsValidSiteID(t *testing.T) {
for _, tc := range []struct {
input string
expected bool
}{
{"1", true},
{"300", true},
{"foo", false},
{"1a", false},
{"", false},
} {
t.Run(tc.input, func(t *testing.T) {
assert.Equal(t, tc.expected, isValidSiteID(tc.input))
})
}
}

0 comments on commit 0961d9d

Please sign in to comment.