config: iam create aws check zone contains availability zone (#1913)

* init

* make zone flag mandatory again

* add info about zone update + refactor

* add comment in docs about zone update

* Update cli/internal/cmd/iamcreate_test.go

Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>

* thomas feedback

* add format check to config validation

* remove TODO

* Update docs/docs/workflows/config.md

Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>

* thomas nit

---------

Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>
This commit is contained in:
Adrian Stobbe 2023-07-04 13:55:52 +02:00
parent 5749e885d9
commit 4b16139f07
6 changed files with 274 additions and 36 deletions

View file

@ -26,9 +26,10 @@ import (
var (
// GCP-specific validation regexes
// Source: https://cloud.google.com/compute/docs/regions-zones
zoneRegex = regexp.MustCompile(`^\w+-\w+-[abc]$`)
regionRegex = regexp.MustCompile(`^\w+-\w+[0-9]$`)
// Source: https://cloud.google.com/resource-manager/reference/rest/v1/projects.
zoneRegex = regexp.MustCompile(`^\w+-\w+-[abc]$`)
regionRegex = regexp.MustCompile(`^\w+-\w+[0-9]$`)
projectIDRegex = regexp.MustCompile(`^[a-z][-a-z0-9]{4,28}[a-z0-9]{1}$`)
serviceAccIDRegex = regexp.MustCompile(`^[a-z](?:[-a-z0-9]{4,28}[a-z0-9])$`)
)
@ -83,7 +84,6 @@ func newIAMCreateAWSCmd() *cobra.Command {
"Find available zones here: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-availability-zones. "+
"Note that we do not support every zone / region. You can find a list of all supported regions in our docs.")
must(cobra.MarkFlagRequired(cmd.Flags(), "zone"))
return cmd
}
@ -103,7 +103,6 @@ func newIAMCreateAzureCmd() *cobra.Command {
must(cobra.MarkFlagRequired(cmd.Flags(), "region"))
cmd.Flags().String("servicePrincipal", "", "name of the service principal that will be created (required)")
must(cobra.MarkFlagRequired(cmd.Flags(), "servicePrincipal"))
return cmd
}
@ -232,11 +231,14 @@ func (c *iamCreator) create(ctx context.Context) error {
var conf config.Config
if flags.updateConfig {
c.cmd.Printf("The configuration file %q will be automatically updated and populated with the IAM values.\n", flags.configPath)
c.log.Debugf("Parsing config %s", flags.configPath)
if err = c.fileHandler.ReadYAML(flags.configPath, &conf); err != nil {
return fmt.Errorf("error reading the configuration file: %w", err)
}
if err := validateConfigWithFlagCompatibility(c.provider, conf, flags); err != nil {
return err
}
c.cmd.Printf("The configuration file %q will be automatically updated with the IAM values and zone/region information.\n", flags.configPath)
}
c.spinner.Start("Creating", false)
@ -368,14 +370,19 @@ func (c *awsIAMCreator) parseFlagsAndSetupConfig(cmd *cobra.Command, flags iamFl
return iamFlags{}, fmt.Errorf("parsing zone string: %w", err)
}
if !config.ValidateAWSZone(zone) {
return iamFlags{}, fmt.Errorf("invalid AWS zone. To find a valid zone, please refer to our docs and https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-availability-zones")
}
// Infer region from zone.
region := zone[:len(zone)-1]
if !config.ValidateAWSRegion(region) {
return iamFlags{}, fmt.Errorf("invalid AWS region: %s", region)
}
flags.aws = awsFlags{
prefix: prefix,
zone: zone,
}
flags.aws.region, err = awsZoneToRegion(zone)
if err != nil {
return iamFlags{}, fmt.Errorf("invalid AWS zone. To find a valid zone, please refer to our docs and https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-availability-zones")
region: region,
}
// Setup IAM config.
@ -569,16 +576,31 @@ func parseIDFile(serviceAccountKeyBase64 string) (map[string]string, error) {
return out, nil
}
// awsZoneToRegion converts an AWS zone string to a region string.
// Example: "us-east-1a" -> "us-east-1"
// It does not check against a list of valid zones.
// Instead, it just checks that the zone string is in the correct format:
// "The code for Availability Zone is its Region code followed by a letter identifier."
// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-availability-zones .
func awsZoneToRegion(zone string) (string, error) {
parts := strings.Split(zone, "-")
if len(parts) < 3 || len(parts[2]) < 1 {
return "", fmt.Errorf("invalid zone string: %s", zone)
// validateConfigWithFlagCompatibility checks if the config is compatible with the flags.
func validateConfigWithFlagCompatibility(iamProvider cloudprovider.Provider, cfg config.Config, flags iamFlags) error {
if !cfg.HasProvider(iamProvider) {
return fmt.Errorf("cloud provider from the the configuration file differs from the one provided via the command %q", iamProvider)
}
return fmt.Sprintf("%s-%s-%c", parts[0], parts[1], parts[2][0]), nil
return checkIfCfgZoneAndFlagZoneDiffer(iamProvider, flags, cfg)
}
func checkIfCfgZoneAndFlagZoneDiffer(iamProvider cloudprovider.Provider, flags iamFlags, cfg config.Config) error {
flagZone := flagZoneOrAzRegion(iamProvider, flags)
configZone := cfg.GetZone()
if configZone != "" && flagZone != configZone {
return fmt.Errorf("zone/region from the configuration file %q differs from the one provided via flags %q", configZone, flagZone)
}
return nil
}
func flagZoneOrAzRegion(provider cloudprovider.Provider, flags iamFlags) string {
switch provider {
case cloudprovider.AWS:
return flags.aws.zone
case cloudprovider.Azure:
return flags.azure.region
case cloudprovider.GCP:
return flags.gcp.zone
}
return ""
}

View file

@ -67,17 +67,7 @@ func TestParseIDFile(t *testing.T) {
}
func TestIAMCreateAWS(t *testing.T) {
defaultFs := func(require *require.Assertions, provider cloudprovider.Provider, existingConfigFiles []string, existingDirs []string) afero.Fs {
fs := afero.NewMemMapFs()
fileHandler := file.NewHandler(fs)
for _, f := range existingConfigFiles {
require.NoError(fileHandler.WriteYAML(f, createConfig(cloudprovider.AWS), file.OptNone))
}
for _, d := range existingDirs {
require.NoError(fs.MkdirAll(d, 0o755))
}
return fs
}
defaultFs := createFSWithConfig(*createConfig(cloudprovider.AWS))
readOnlyFs := func(require *require.Assertions, provider cloudprovider.Provider, existingConfigFiles []string, existingDirs []string) afero.Fs {
fs := afero.NewReadOnlyFs(afero.NewMemMapFs())
return fs
@ -114,6 +104,16 @@ func TestIAMCreateAWS(t *testing.T) {
yesFlag: true,
existingConfigFiles: []string{constants.ConfigFilename},
},
"iam create aws fails when --zone has no availability zone": {
setupFs: defaultFs,
creator: &stubIAMCreator{id: validIAMIDFile},
provider: cloudprovider.AWS,
zoneFlag: "us-east-1",
prefixFlag: "test",
yesFlag: true,
existingConfigFiles: []string{constants.ConfigFilename},
wantErr: true,
},
"iam create aws --update-config": {
setupFs: defaultFs,
creator: &stubIAMCreator{id: validIAMIDFile},
@ -125,6 +125,33 @@ func TestIAMCreateAWS(t *testing.T) {
updateConfigFlag: true,
existingConfigFiles: []string{constants.ConfigFilename},
},
"iam create aws --update-config fails when --zone is different from zone in config": {
setupFs: createFSWithConfig(func() config.Config {
cfg := createConfig(cloudprovider.AWS)
cfg.Provider.AWS.Zone = "eu-central-1a"
cfg.Provider.AWS.Region = "eu-central-1"
return *cfg
}()),
creator: &stubIAMCreator{id: validIAMIDFile},
provider: cloudprovider.AWS,
zoneFlag: "us-east-1a",
prefixFlag: "test",
yesFlag: true,
existingConfigFiles: []string{constants.ConfigFilename},
updateConfigFlag: true,
wantErr: true,
},
"iam create aws --update-config fails when config has different provider": {
setupFs: createFSWithConfig(*createConfig(cloudprovider.GCP)),
creator: &stubIAMCreator{id: validIAMIDFile},
provider: cloudprovider.AWS,
zoneFlag: "us-east-1a",
prefixFlag: "test",
yesFlag: true,
existingConfigFiles: []string{constants.ConfigFilename},
updateConfigFlag: true,
wantErr: true,
},
"iam create aws no config": {
setupFs: defaultFs,
creator: &stubIAMCreator{id: validIAMIDFile},
@ -278,6 +305,7 @@ func TestIAMCreateAWS(t *testing.T) {
assert.Error(err)
return
}
require.NoError(err)
if tc.wantAbort {
assert.False(tc.creator.createCalled)
@ -829,3 +857,93 @@ func TestIAMCreateGCP(t *testing.T) {
})
}
}
func TestValidateConfigWithFlagCompatibility(t *testing.T) {
testCases := map[string]struct {
iamProvider cloudprovider.Provider
cfg config.Config
flags iamFlags
wantErr bool
}{
"AWS valid when cfg.zone == flag.zone": {
iamProvider: cloudprovider.AWS,
cfg: func() config.Config {
cfg := createConfig(cloudprovider.AWS)
cfg.Provider.AWS.Zone = "europe-west-1a"
return *cfg
}(),
flags: iamFlags{
aws: awsFlags{
zone: "europe-west-1a",
},
},
},
"AWS valid when cfg.zone not set": {
iamProvider: cloudprovider.AWS,
cfg: *createConfig(cloudprovider.AWS),
flags: iamFlags{
aws: awsFlags{
zone: "europe-west-1a",
},
},
},
"GCP invalid when cfg.zone != flag.zone": {
iamProvider: cloudprovider.GCP,
cfg: func() config.Config {
cfg := createConfig(cloudprovider.GCP)
cfg.Provider.GCP.Zone = "europe-west-1a"
return *cfg
}(),
flags: iamFlags{
aws: awsFlags{
zone: "us-west-1a",
},
},
wantErr: true,
},
"Azure invalid when cfg.zone != flag.zone": {
iamProvider: cloudprovider.GCP,
cfg: func() config.Config {
cfg := createConfig(cloudprovider.Azure)
cfg.Provider.Azure.Location = "europe-west-1a"
return *cfg
}(),
flags: iamFlags{
aws: awsFlags{
zone: "us-west-1a",
},
},
wantErr: true,
},
"GCP invalid when cfg.provider different from iam provider": {
iamProvider: cloudprovider.GCP,
cfg: *createConfig(cloudprovider.AWS),
wantErr: true,
},
}
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
assert := assert.New(t)
err := validateConfigWithFlagCompatibility(tc.iamProvider, tc.cfg, tc.flags)
if tc.wantErr {
assert.Error(err)
return
}
assert.NoError(err)
})
}
}
func createFSWithConfig(cfg config.Config) func(require *require.Assertions, provider cloudprovider.Provider, existingConfigFiles []string, existingDirs []string) afero.Fs {
return func(require *require.Assertions, provider cloudprovider.Provider, existingConfigFiles []string, existingDirs []string) afero.Fs {
fs := afero.NewMemMapFs()
fileHandler := file.NewHandler(fs)
for _, f := range existingConfigFiles {
require.NoError(fileHandler.WriteYAML(f, cfg, file.OptNone))
}
for _, d := range existingDirs {
require.NoError(fs.MkdirAll(d, 0o755))
}
return fs
}
}