Skip to content

Commit

Permalink
AWS account IDS should be strings (#302)
Browse files Browse the repository at this point in the history 
[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
  • Loading branch information
@czimergebot
Eduardo Lopez authored and czimergebot committed Jun 19, 2019
1 parent f1106dc commit 2703db7
Show file tree
Hide file tree
Showing 12 changed files with 76 additions and 53 deletions.
3 changes: 1 addition & 2 deletions cmd/exp/aws_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"os"
"os/exec"
"strconv"
"strings"
"text/template"

Expand Down Expand Up @@ -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),
}

Expand Down
17 changes: 12 additions & 5 deletions config/config_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package config

import (
"encoding/json"
"os"
"strconv"
"testing"

v1 "github.com/chanzuckerberg/fogg/config/v1"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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"),
Expand All @@ -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"),
Expand All @@ -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"),
Expand All @@ -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"),
Expand Down
8 changes: 4 additions & 4 deletions config/v1/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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"`
Expand All @@ -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"`
Expand Down Expand Up @@ -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"`
Expand Down
3 changes: 2 additions & 1 deletion config/v1/config_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package v1

import (
"encoding/json"
"io/ioutil"
"strings"
"testing"
Expand Down Expand Up @@ -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)
Expand Down
23 changes: 7 additions & 16 deletions config/v2/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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),
Expand Down
18 changes: 16 additions & 2 deletions config/v2/resolvers.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package v2

import (
"encoding/json"

v1 "github.com/chanzuckerberg/fogg/config/v1"
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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...),
}
}
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion config/v2/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 10 additions & 9 deletions plan/plan.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package plan

import (
"encoding/json"
"errors"
"fmt"

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions plan/plan_test.go
View file
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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": {
Expand All @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions plan/travisci.go
View file
Original file line number Diff line number Diff line change
@@ -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
}

Expand Down
15 changes: 8 additions & 7 deletions plan/travisci_test.go
View file
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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"),
Expand Down
Loading

0 comments on commit 2703db7

Please sign in to comment.