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>

cli/internal/cmd/iamcreate.go

@@ -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 ""
 }

cli/internal/cmd/iamcreate_test.go

@@ -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
+	}
+}

docs/docs/workflows/config.md

@@ -88,7 +88,7 @@ See also Constellation's [Kubernetes support policy](../architecture/versions.md
 ## Creating an IAM configuration
 
 You can create an IAM configuration for your cluster automatically using the `constellation iam create` command.
-If you already have a constellation configuration file, you can add the `--update-config` flag to the command. This writes the needed IAM fields into your configuration.
+If you already have a Constellation configuration file, you can add the `--update-config` flag to the command. This writes the needed IAM fields into your configuration. Furthermore, the flag updates the zone/region of the configuration if it hasn't been set yet.
 
 <tabs groupId="csp">
 <tabItem value="azure" label="Azure">

internal/config/config.go

@@ -112,10 +112,10 @@ type ProviderConfig struct {
 type AWSConfig struct {
 	// description: |
 	//   AWS data center region. See: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-available-regions
-	Region string `yaml:"region" validate:"required"`
+	Region string `yaml:"region" validate:"required,aws_region"`
 	// description: |
 	//   AWS data center zone name in defined region. See: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-availability-zones
-	Zone string `yaml:"zone" validate:"required"`
+	Zone string `yaml:"zone" validate:"required,aws_zone"`
 	// description: |
 	//   VM instance type to use for Constellation nodes. Needs to support NitroTPM. See: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/enable-nitrotpm-prerequisites.html
 	InstanceType string `yaml:"instanceType" validate:"lowercase,aws_instance_type"`
@@ -631,6 +631,19 @@ func (c *Config) GetRegion() string {
 	return ""
 }
 
+// GetZone returns the configured zone or location for providers without zone support (Azure).
+func (c *Config) GetZone() string {
+	switch c.GetProvider() {
+	case cloudprovider.AWS:
+		return c.Provider.AWS.Zone
+	case cloudprovider.Azure:
+		return c.Provider.Azure.Location
+	case cloudprovider.GCP:
+		return c.Provider.GCP.Zone
+	}
+	return ""
+}
+
 // UpdateMAAURL updates the MAA URL in the config.
 func (c *Config) UpdateMAAURL(maaURL string) {
 	if c.Attestation.AzureSEVSNP != nil {
@@ -759,6 +772,19 @@ func (c *Config) Validate(force bool) error {
 		return err
 	}
 
+	if err := validate.RegisterValidation("aws_region", validateAWSRegionField); err != nil {
+		return err
+	}
+	if err := validate.RegisterValidation("aws_zone", validateAWSZoneField); err != nil {
+		return err
+	}
+	if err := validate.RegisterTranslation("aws_region", trans, registerAWSRegionError, translateAWSRegionError); err != nil {
+		return err
+	}
+	if err := validate.RegisterTranslation("aws_zone", trans, registerAWSZoneError, translateAWSZoneError); err != nil {
+		return err
+	}
+
 	validate.RegisterStructValidation(validateMeasurement, measurements.Measurement{})
 	validate.RegisterStructValidation(validateAttestation, AttestationConfig{})
 

internal/config/config_test.go

@@ -264,6 +264,7 @@ func TestFromFile(t *testing.T) {
 func TestValidate(t *testing.T) {
 	const defaultErrCount = 33 // expect this number of error messages by default because user-specific values are not set and multiple providers are defined by default
 	const azErrCount = 7
+	const awsErrCount = 6
 	const gcpErrCount = 6
 
 	// TODO(AB#3132,3u13r): refactor config validation tests
@@ -346,7 +347,37 @@ func TestValidate(t *testing.T) {
 				return cnf
 			}(),
 		},
-
+		"default AWS config is not valid": {
+			cnf: func() *Config {
+				cnf := Default()
+				cnf.RemoveProviderAndAttestationExcept(cloudprovider.AWS)
+				return cnf
+			}(),
+			wantErr:      true,
+			wantErrCount: awsErrCount,
+		},
+		"AWS config with correct region and zone format": {
+			cnf: func() *Config {
+				cnf := Default()
+				cnf.Provider.AWS.Region = "us-east-2"
+				cnf.Provider.AWS.Zone = "us-east-2a"
+				cnf.RemoveProviderAndAttestationExcept(cloudprovider.AWS)
+				return cnf
+			}(),
+			wantErr:      true,
+			wantErrCount: awsErrCount - 2,
+		},
+		"AWS config with wrong region and zone format": {
+			cnf: func() *Config {
+				cnf := Default()
+				cnf.Provider.AWS.Region = "us-west2"
+				cnf.Provider.AWS.Zone = "a"
+				cnf.RemoveProviderAndAttestationExcept(cloudprovider.AWS)
+				return cnf
+			}(),
+			wantErr:      true,
+			wantErrCount: awsErrCount,
+		},
 		"default GCP config is not valid": {
 			cnf: func() *Config {
 				cnf := Default()

internal/config/validation.go

@@ -11,6 +11,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -113,6 +114,26 @@ func validateGCPInstanceType(fl validator.FieldLevel) bool {
 	return validInstanceTypeForProvider(fl.Field().String(), false, cloudprovider.GCP)
 }
 
+func validateAWSRegionField(fl validator.FieldLevel) bool {
+	return ValidateAWSRegion(fl.Field().String())
+}
+
+func validateAWSZoneField(fl validator.FieldLevel) bool {
+	return ValidateAWSZone(fl.Field().String())
+}
+
+// ValidateAWSZone validates that the zone is in the correct format.
+func ValidateAWSZone(zone string) bool {
+	awsZoneRegex := regexp.MustCompile(`^\w+-\w+-[1-9][abc]$`)
+	return awsZoneRegex.MatchString(zone)
+}
+
+// ValidateAWSRegion validates that the region is in the correct format.
+func ValidateAWSRegion(region string) bool {
+	awsRegionRegex := regexp.MustCompile(`^\w+-\w+-[1-9]$`)
+	return awsRegionRegex.MatchString(region)
+}
+
 // validateProvider checks if zero or more than one providers are defined in the config.
 func validateProvider(sl validator.StructLevel) {
 	provider := sl.Current().Interface().(ProviderConfig)
@@ -181,6 +202,26 @@ func registerNoAttestationError(ut ut.Translator) error {
 	return ut.Add("no_attestation", "{0}: No attestation has been defined (requires either awsSEVSNP, awsNitroTPM, azureSEVSNP, azureTrustedLaunch, gcpSEVES, or qemuVTPM)", true)
 }
 
+func registerAWSRegionError(ut ut.Translator) error {
+	return ut.Add("aws_region", "{0}: has invalid format: {1}", true)
+}
+
+func translateAWSRegionError(ut ut.Translator, fe validator.FieldError) string {
+	t, _ := ut.T("aws_region", fe.Field(), "field must be of format eu-central-1")
+
+	return t
+}
+
+func translateAWSZoneError(ut ut.Translator, fe validator.FieldError) string {
+	t, _ := ut.T("aws_zone", fe.Field(), "field must be of format eu-central-1a")
+
+	return t
+}
+
+func registerAWSZoneError(ut ut.Translator) error {
+	return ut.Add("aws_zone", "{0}: has invalid format: {1}", true)
+}
+
 func registerMoreThanOneAttestationError(ut ut.Translator) error {
 	return ut.Add("more_than_one_attestation", "{0}: Only one attestation can be defined ({1} are defined)", true)
 }