Repository: chanzuckerberg/fogg
Pull request #302 (2019-06-19 21:18:36)

[bug] AWS account IDS should be strings
### Summary
AWS account IDS should be strings (since they can start with 0). We use json.Number (a type alias for string) to allow for backwards compatibility with fogg configs that have ints already.

### Test Plan
unittests + apply in existing repos.

### References


codecov[bot]: # [Codecov](https://codecov.io/gh/chanzuckerberg/fogg/pull/302?src=pr&el=h1) Report
> Merging [#302](https://codecov.io/gh/chanzuckerberg/fogg/pull/302?src=pr&el=desc) into [master](https://codecov.io/gh/chanzuckerberg/fogg/commit/f1106dc98cc62e97bebdfe05a7595782b0897807?src=pr&el=desc) will **decrease** coverage by `0.55%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/chanzuckerberg/fogg/pull/302/graphs/tree.svg?width=650&token=57DhtZ7g2A&height=150&src=pr)](https://codecov.io/gh/chanzuckerberg/fogg/pull/302?src=pr&el=tree)

```diff
@@           Coverage Diff            @@
##           master   #302      +/-   ##
========================================
- Coverage   80.55%    80%   -0.56%     
========================================
  Files          17     17              
  Lines        1466   1470       +4     
========================================
- Hits         1181   1176       -5     
- Misses        163    172       +9     
  Partials      122    122
```


| [Impacted Files](https://codecov.io/gh/chanzuckerberg/fogg/pull/302?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [config/v1/config.go](https://codecov.io/gh/chanzuckerberg/fogg/pull/302/diff?src=pr&el=tree#diff-Y29uZmlnL3YxL2NvbmZpZy5nbw==) | `84.61% <ø> (ø)` | :arrow_up: |
| [plan/travisci.go](https://codecov.io/gh/chanzuckerberg/fogg/pull/302/diff?src=pr&el=tree#diff-cGxhbi90cmF2aXNjaS5nbw==) | `94.44% <ø> (ø)` | :arrow_up: |
| [util/testing.go](https://codecov.io/gh/chanzuckerberg/fogg/pull/302/diff?src=pr&el=tree#diff-dXRpbC90ZXN0aW5nLmdv) | `77.77% <100%> (-5.56%)` | :arrow_down: |
| [config/v2/validation.go](https://codecov.io/gh/chanzuckerberg/fogg/pull/302/diff?src=pr&el=tree#diff-Y29uZmlnL3YyL3ZhbGlkYXRpb24uZ28=) | `82.24% <100%> (ø)` | :arrow_up: |
| [config/v2/config.go](https://codecov.io/gh/chanzuckerberg/fogg/pull/302/diff?src=pr&el=tree#diff-Y29uZmlnL3YyL2NvbmZpZy5nbw==) | `100% <100%> (ø)` | :arrow_up: |
| [plan/plan.go](https://codecov.io/gh/chanzuckerberg/fogg/pull/302/diff?src=pr&el=tree#diff-cGxhbi9wbGFuLmdv) | `91.35% <100%> (ø)` | :arrow_up: |
| [config/v2/resolvers.go](https://codecov.io/gh/chanzuckerberg/fogg/pull/302/diff?src=pr&el=tree#diff-Y29uZmlnL3YyL3Jlc29sdmVycy5nbw==) | `95.13% <100%> (-3.75%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/chanzuckerberg/fogg/pull/302?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/chanzuckerberg/fogg/pull/302?src=pr&el=footer). Last update [f1106dc...2e323a4](https://codecov.io/gh/chanzuckerberg/fogg/pull/302?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/cmd/exp/aws_config.go b/cmd/exp/aws_config.go
index ce4bd1a0..d34ad799 100644
--- a/cmd/exp/aws_config.go
+++ b/cmd/exp/aws_config.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"strconv"
 	"strings"
 	"text/template"
 
@@ -87,7 +86,7 @@ output = json
 			roleARN := arn.ARN{
 				Partition: "aws",
 				Service:   "iam",
-				AccountID: strconv.Itoa(int(*account.Providers.AWS.AccountID)),
+				AccountID: account.Providers.AWS.AccountID.String(),
 				Resource:  fmt.Sprintf("role/%s", role),
 			}
 
diff --git a/config/v1/config.go b/config/v1/config.go
index bb599990..ad1cbc93 100644
--- a/config/v1/config.go
+++ b/config/v1/config.go
@@ -17,7 +17,7 @@ type TfLint struct {
 }
 
 type Defaults struct {
-	AccountID          int64             `json:"account_id,omitempty" validate:"required"`
+	AccountID          json.Number       `json:"account_id,omitempty" validate:"required"`
 	AWSProfileBackend  string            `json:"aws_profile_backend" validate:"required"`
 	AWSProfileProvider string            `json:"aws_profile_provider" validate:"required"`
 	AWSProviderVersion string            `json:"aws_provider_version" validate:"required"`
@@ -34,7 +34,7 @@ type Defaults struct {
 }
 
 type Account struct {
-	AccountID          *int64            `json:"account_id"`
+	AccountID          *json.Number      `json:"account_id"`
 	AWSProfileBackend  *string           `json:"aws_profile_backend"`
 	AWSProfileProvider *string           `json:"aws_profile_provider"`
 	AWSProviderVersion *string           `json:"aws_provider_version,omitempty"`
@@ -51,7 +51,7 @@ type Account struct {
 }
 
 type Env struct {
-	AccountID          *int64            `json:"account_id"`
+	AccountID          *json.Number      `json:"account_id"`
 	AWSProfileBackend  *string           `json:"aws_profile_backend"`
 	AWSProfileProvider *string           `json:"aws_profile_provider"`
 	AWSProviderVersion *string           `json:"aws_provider_version,omitempty"`
@@ -95,7 +95,7 @@ type EKSConfig struct {
 }
 
 type Component struct {
-	AccountID          *int64            `json:"account_id"`
+	AccountID          *json.Number      `json:"account_id"`
 	AWSProfileBackend  *string           `json:"aws_profile_backend"`
 	AWSProfileProvider *string           `json:"aws_profile_provider"`
 	AWSProviderVersion *string           `json:"aws_provider_version,omitempty"`
diff --git a/config/v2/config.go b/config/v2/config.go
index 8ee92523..d7620832 100644
--- a/config/v2/config.go
+++ b/config/v2/config.go
@@ -92,11 +92,11 @@ type BlessProvider struct {
 
 type AWSProvider struct {
 	// the aws provider is optional (above) but if supplied you must set account id and region
-	AccountID         *int64   `json:"account_id,omitempty"`
-	AdditionalRegions []string `json:"additional_regions,omitempty"`
-	Profile           *string  `json:"profile,omitempty"`
-	Region            *string  `json:"region,omitempty"`
-	Version           *string  `json:"version,omitempty"`
+	AccountID         *json.Number `json:"account_id,omitempty"`
+	AdditionalRegions []string     `json:"additional_regions,omitempty"`
+	Profile           *string      `json:"profile,omitempty"`
+	Region            *string      `json:"region,omitempty"`
+	Version           *string      `json:"version,omitempty"`
 }
 
 type SnowflakeProvider struct {
@@ -150,16 +150,6 @@ func (c *Config) Generate(r *rand.Rand, size int) reflect.Value {
 		return map[string]string{}
 	}
 
-	randInt64Ptr := func(r *rand.Rand, s int) *int64 {
-		if r.Float32() < 0.5 {
-			i := r.Int63n(int64(size))
-			return &i
-		} else {
-			var i *int64
-			return i
-		}
-	}
-
 	randOktaProvider := func(r *rand.Rand, s int) *OktaProvider {
 		if r.Float32() < 0.5 {
 			return nil
@@ -184,8 +174,9 @@ func (c *Config) Generate(r *rand.Rand, size int) reflect.Value {
 
 	randAWSProvider := func(r *rand.Rand, s int) *AWSProvider {
 		if r.Float32() < 0.5 {
+			accountID := json.Number(randString(r, s))
 			return &AWSProvider{
-				AccountID: randInt64Ptr(r, size),
+				AccountID: &accountID,
 				Region:    randStringPtr(r, s),
 				Profile:   randStringPtr(r, s),
 				Version:   randStringPtr(r, s),
diff --git a/config/v2/resolvers.go b/config/v2/resolvers.go
index af4ce4cc..57fe4484 100644
--- a/config/v2/resolvers.go
+++ b/config/v2/resolvers.go
@@ -1,6 +1,8 @@
 package v2
 
 import (
+	"encoding/json"
+
 	v1 "github.com/chanzuckerberg/fogg/config/v1"
 )
 
@@ -28,6 +30,18 @@ func lastNonNilInt64(getter func(Common) *int64, commons ...Common) *int64 {
 	return s
 }
 
+// lastNonNilJsonNumber, despite its name can return nil if all results are nil
+func lastNonNilJsonNumber(getter func(Common) *json.Number, commons ...Common) *json.Number {
+	var jsonNumber *json.Number
+	for _, c := range commons {
+		j := getter(c)
+		if j != nil {
+			jsonNumber = j
+		}
+	}
+	return jsonNumber
+}
+
 // lastNonNilStringSlice, despite its name can return nil if all results are nil
 func lastNonNilStringSlice(getter func(Common) []string, commons ...Common) []string {
 	var s []string
@@ -93,7 +107,7 @@ func ResolveAWSProvider(commons ...Common) *AWSProvider {
 			Version: version,
 
 			// optional fields
-			AccountID:         lastNonNilInt64(AWSProviderAccountIdGetter, commons...),
+			AccountID:         lastNonNilJsonNumber(AWSProviderAccountIdGetter, commons...),
 			AdditionalRegions: ResolveOptionalStringSlice(AWSProviderAdditionalRegionsGetter, commons...),
 		}
 	}
@@ -221,7 +235,7 @@ func AWSProviderProfileGetter(comm Common) *string {
 	return nil
 }
 
-func AWSProviderAccountIdGetter(comm Common) *int64 {
+func AWSProviderAccountIdGetter(comm Common) *json.Number {
 	if comm.Providers != nil && comm.Providers.AWS != nil {
 		return comm.Providers.AWS.AccountID
 	}
diff --git a/config/v2/validation.go b/config/v2/validation.go
index 9746e729..8b90ff13 100644
--- a/config/v2/validation.go
+++ b/config/v2/validation.go
@@ -86,7 +86,7 @@ func ValidateAWSProvider(p *AWSProvider, component string) error {
 		errs = multierror.Append(errs, fmt.Errorf("aws provider version for %s ", component))
 	}
 
-	if p.AccountID == nil || *p.AccountID == 0 {
+	if p.AccountID == nil || *p.AccountID == "" {
 		errs = multierror.Append(errs, fmt.Errorf("aws provider account id for %s", component))
 	}
 	return errs
diff --git a/plan/plan.go b/plan/plan.go
index 4a8055a0..016ff4fa 100644
--- a/plan/plan.go
+++ b/plan/plan.go
@@ -1,6 +1,7 @@
 package plan
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 
@@ -46,11 +47,11 @@ type Providers struct {
 }
 
 type AWSProvider struct {
-	AccountID         int64    `yaml:"account_id"`
-	Profile           string   `yaml:"profile"`
-	Version           string   `yaml:"version"`
-	Region            string   `yaml:"region"`
-	AdditionalRegions []string `yaml:"additional_regions"`
+	AccountID         json.Number `yaml:"account_id"`
+	Profile           string      `yaml:"profile"`
+	Version           string      `yaml:"version"`
+	Region            string      `yaml:"region"`
+	AdditionalRegions []string    `yaml:"additional_regions"`
 }
 
 type SnowflakeProvider struct {
@@ -90,8 +91,8 @@ type Module struct {
 type Account struct {
 	ComponentCommon `yaml:",inline"`
 
-	AllAccounts map[string]int64 `yaml:"all_accounts"`
-	AccountName string           `yaml:"account_name"`
+	AllAccounts map[string]json.Number `yaml:"all_accounts"`
+	AccountName string                 `yaml:"account_name"`
 	Global      *Component
 }
 
@@ -361,8 +362,8 @@ func resolveExtraVars(vars ...map[string]string) map[string]string {
 	return resolved
 }
 
-func resolveAccounts(accounts map[string]v2.Account) map[string]int64 {
-	a := make(map[string]int64)
+func resolveAccounts(accounts map[string]v2.Account) map[string]json.Number {
+	a := make(map[string]json.Number)
 	for name, account := range accounts {
 		if account.Providers != nil && account.Providers.AWS != nil && account.Providers.AWS.AccountID != nil {
 			a[name] = *account.Providers.AWS.AccountID
diff --git a/plan/travisci.go b/plan/travisci.go
index 59638274..9aa3c9e5 100644
--- a/plan/travisci.go
+++ b/plan/travisci.go
@@ -1,15 +1,16 @@
 package plan
 
 import (
+	"encoding/json"
 	"path"
 
-	"github.com/chanzuckerberg/fogg/config/v2"
+	v2 "github.com/chanzuckerberg/fogg/config/v2"
 	"github.com/chanzuckerberg/fogg/util"
 )
 
 type AWSProfile struct {
 	Name string
-	ID   int64
+	ID   json.Number
 	Role string
 }
 


Test patch:
diff --git a/config/config_test.go b/config/config_test.go
index b58072da..a15de522 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -1,7 +1,9 @@
 package config
 
 import (
+	"encoding/json"
 	"os"
+	"strconv"
 	"testing"
 
 	v1 "github.com/chanzuckerberg/fogg/config/v1"
@@ -61,6 +63,11 @@ func intptr(i int64) *int64 {
 	return &i
 }
 
+func jsonNumberPtr(i int) *json.Number {
+	j := json.Number(strconv.Itoa(i))
+	return &j
+}
+
 func strptr(s string) *string {
 	return &s
 }
@@ -88,7 +95,7 @@ func TestUpgradeConfigVersion(t *testing.T) {
 
 				Providers: &v2.Providers{
 					AWS: &v2.AWSProvider{
-						AccountID:         intptr(1),
+						AccountID:         jsonNumberPtr(1),
 						Profile:           strptr("czi"),
 						Region:            strptr("us-west-1"),
 						Version:           strptr("0.1.0"),
@@ -124,7 +131,7 @@ func TestUpgradeConfigVersion(t *testing.T) {
 
 					Providers: &v2.Providers{
 						AWS: &v2.AWSProvider{
-							AccountID:         intptr(2),
+							AccountID:         jsonNumberPtr(2),
 							Profile:           strptr("czi-foo"),
 							Region:            strptr("us-west-foo1"),
 							Version:           strptr("0.12.0"),
@@ -148,7 +155,7 @@ func TestUpgradeConfigVersion(t *testing.T) {
 
 					Providers: &v2.Providers{
 						AWS: &v2.AWSProvider{
-							AccountID:         intptr(3),
+							AccountID:         jsonNumberPtr(3),
 							Profile:           strptr("czi-bar"),
 							Region:            strptr("us-west-bar1"),
 							Version:           strptr("0.13.0"),
@@ -175,7 +182,7 @@ func TestUpgradeConfigVersion(t *testing.T) {
 
 					Providers: &v2.Providers{
 						AWS: &v2.AWSProvider{
-							AccountID:         intptr(4),
+							AccountID:         jsonNumberPtr(4),
 							Profile:           strptr("czi-stage"),
 							Region:            strptr("us-west-stage1"),
 							Version:           strptr("0.14.0"),
@@ -199,7 +206,7 @@ func TestUpgradeConfigVersion(t *testing.T) {
 
 							Providers: &v2.Providers{
 								AWS: &v2.AWSProvider{
-									AccountID:         intptr(5),
+									AccountID:         jsonNumberPtr(5),
 									Profile:           strptr("czi-env"),
 									Region:            strptr("us-west-env1"),
 									Version:           strptr("0.15.0"),
diff --git a/config/v1/config_test.go b/config/v1/config_test.go
index f96e7a90..05f1efe1 100644
--- a/config/v1/config_test.go
+++ b/config/v1/config_test.go
@@ -1,6 +1,7 @@
 package v1
 
 import (
+	"encoding/json"
 	"io/ioutil"
 	"strings"
 	"testing"
@@ -57,7 +58,7 @@ func TestParse(t *testing.T) {
 	c, e := ReadConfig(b)
 	assert.Nil(t, e)
 	assert.NotNil(t, c.Defaults)
-	assert.Equal(t, int64(1), c.Defaults.AccountID)
+	assert.Equal(t, json.Number("1"), c.Defaults.AccountID)
 	assert.Equal(t, "us-west-2", c.Defaults.AWSRegionBackend)
 	assert.Equal(t, "us-west-1", c.Defaults.AWSRegionProvider)
 	assert.Equal(t, "0.1.0", c.Defaults.AWSProviderVersion)
diff --git a/plan/plan_test.go b/plan/plan_test.go
index bb541649..64a264b0 100644
--- a/plan/plan_test.go
+++ b/plan/plan_test.go
@@ -1,11 +1,12 @@
 package plan
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/chanzuckerberg/fogg/config"
-	"github.com/chanzuckerberg/fogg/config/v1"
-	"github.com/chanzuckerberg/fogg/config/v2"
+	v1 "github.com/chanzuckerberg/fogg/config/v1"
+	v2 "github.com/chanzuckerberg/fogg/config/v2"
 	"github.com/chanzuckerberg/fogg/util"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
@@ -29,7 +30,7 @@ func init() {
 // }
 
 func TestResolveAccounts(t *testing.T) {
-	foo, bar := int64(123), int64(456)
+	foo, bar := json.Number("123"), json.Number("456")
 
 	accounts := map[string]v2.Account{
 		"foo": {
@@ -55,7 +56,7 @@ func TestResolveAccounts(t *testing.T) {
 
 	other := resolveAccounts(accounts)
 	assert.NotNil(t, other)
-	assert.Equal(t, map[string]int64{"bar": bar, "foo": foo}, other)
+	assert.Equal(t, map[string]json.Number{"bar": bar, "foo": foo}, other)
 }
 
 func TestPlanBasicV1(t *testing.T) {
diff --git a/plan/travisci_test.go b/plan/travisci_test.go
index cc08f1a3..634f95a5 100644
--- a/plan/travisci_test.go
+++ b/plan/travisci_test.go
@@ -1,19 +1,20 @@
 package plan
 
 import (
+	"encoding/json"
 	"testing"
 
-	"github.com/chanzuckerberg/fogg/config/v1"
-	"github.com/chanzuckerberg/fogg/config/v2"
+	v1 "github.com/chanzuckerberg/fogg/config/v1"
+	v2 "github.com/chanzuckerberg/fogg/config/v2"
 	"github.com/chanzuckerberg/fogg/util"
 	"github.com/stretchr/testify/assert"
 )
 
-var id1, id2 int64
+var id1, id2 json.Number
 
 func init() {
-	id1 = int64(123456789)
-	id2 = int64(987654321)
+	id1 = json.Number("123456789")
+	id2 = json.Number("987654321")
 }
 
 func Test_buildTravisCI_Disabled(t *testing.T) {
@@ -50,7 +51,7 @@ func Test_buildTravisCI_Profiles(t *testing.T) {
 				TerraformVersion: util.StrPtr("0.1.0"),
 				Providers: &v2.Providers{
 					AWS: &v2.AWSProvider{
-						AccountID: util.Intptr(123),
+						AccountID: util.JsonNumberPtr(123),
 						Region:    util.StrPtr("us-west-2"),
 						Profile:   util.StrPtr("foo"),
 						Version:   util.StrPtr("0.12.0"),
@@ -99,7 +100,7 @@ func Test_buildTravisCI_TestBuckets(t *testing.T) {
 				TerraformVersion: util.StrPtr("0.1.0"),
 				Providers: &v2.Providers{
 					AWS: &v2.AWSProvider{
-						AccountID: util.Intptr(123),
+						AccountID: util.JsonNumberPtr(123),
 						Region:    util.StrPtr("us-west-2"),
 						Profile:   util.StrPtr("foo"),
 						Version:   util.StrPtr("0.12.0"),
diff --git a/util/testing.go b/util/testing.go
index e9fef4e2..10e04ce9 100644
--- a/util/testing.go
+++ b/util/testing.go
@@ -1,10 +1,12 @@
 package util
 
 import (
+	"encoding/json"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"runtime"
+	"strconv"
 
 	"github.com/spf13/afero"
 )
@@ -13,6 +15,11 @@ func Intptr(i int64) *int64 {
 	return &i
 }
 
+func JsonNumberPtr(i int) *json.Number {
+	j := json.Number(strconv.Itoa(i))
+	return &j
+}
+
 func StrPtr(s string) *string {
 	if s == "" {
 		return nil