diff --git a/cfnresource/naming.go b/cfnresource/naming.go new file mode 100644 index 000000000..a546ef6f8 --- /dev/null +++ b/cfnresource/naming.go @@ -0,0 +1,14 @@ +package cfnresource + +import ( + "fmt" +) + +func ValidateRoleNameLength(clusterName string, nestedStackLogicalName string, managedIAMRoleName string, region string) error { + name := fmt.Sprintf("%s-%s-PRK1CVQNY7XZ-%s-%s", clusterName, nestedStackLogicalName, region, managedIAMRoleName) + if len(name) > 64 { + limit := 64 - len(name) + len(clusterName) + len(nestedStackLogicalName) + len(managedIAMRoleName) + return fmt.Errorf("IAM role name(=%s) will be %d characters long. It exceeds the AWS limit of 64 characters: cluster name(=%s) + nested stack name(=%s) + managed iam role name(=%s) should be less than or equal to %d", name, len(name), clusterName, nestedStackLogicalName, managedIAMRoleName, limit) + } + return nil +} diff --git a/cfnresource/naming_test.go b/cfnresource/naming_test.go new file mode 100644 index 000000000..997b3d493 --- /dev/null +++ b/cfnresource/naming_test.go @@ -0,0 +1,16 @@ +package cfnresource + +import "testing" + +func TestValidateRoleNameLength(t *testing.T) { + t.Run("WhenMax", func(t *testing.T) { + if e := ValidateRoleNameLength("my-firstcluster", "prodWorkerks", "prod-workers", "us-east-1"); e != nil { + t.Errorf("expected validation to succeed but failed: %v", e) + } + }) + t.Run("WhenTooLong", func(t *testing.T) { + if e := ValidateRoleNameLength("my-secondcluster", "prodWorkerks", "prod-workers", "us-east-1"); e == nil { + t.Error("expected validation to fail but succeeded") + } + }) +} diff --git a/core/controlplane/config/config.go b/core/controlplane/config/config.go index 4018a6bed..abde6700c 100644 --- a/core/controlplane/config/config.go +++ b/core/controlplane/config/config.go @@ -12,6 +12,7 @@ import ( "unicode/utf8" "github.com/coreos/go-semver/semver" + "github.com/coreos/kube-aws/cfnresource" "github.com/coreos/kube-aws/coreos/amiregistry" "github.com/coreos/kube-aws/filereader/userdatatemplate" "github.com/coreos/kube-aws/model" @@ -846,6 +847,13 @@ func (c Config) InternetGatewayRef() string { } } +// NestedStackName returns a sanitized name of this control-plane which is usable as a valid cloudformation nested stack name +func (c Cluster) NestedStackName() string { + // Convert stack name into something valid as a cfn resource name or + // we'll end up with cfn errors like "Template format error: Resource name test5-controlplane is non alphanumeric" + return strings.Title(strings.Replace(c.StackName(), "-", "", -1)) +} + func (c Cluster) valid() error { validClusterNaming := regexp.MustCompile("^[a-zA-Z0-9-:]+$") if !validClusterNaming.MatchString(c.ClusterName) { @@ -951,6 +959,10 @@ func (c Cluster) valid() error { fmt.Println(`WARNING: instance types "t2.nano" and "t2.micro" are not recommended. See https://github.com/coreos/kube-aws/issues/258 for more information`) } + if e := cfnresource.ValidateRoleNameLength(c.ClusterName, c.NestedStackName(), c.Controller.ManagedIamRoleName, c.Region.String()); e != nil { + return e + } + return nil } diff --git a/core/controlplane/config/templates/cluster.yaml b/core/controlplane/config/templates/cluster.yaml index 4d3b5edf8..e6f0feec5 100644 --- a/core/controlplane/config/templates/cluster.yaml +++ b/core/controlplane/config/templates/cluster.yaml @@ -63,6 +63,10 @@ kmsKeyArn: "{{.KMSKeyARN}}" # # Role will be created with "Ref": {"AWS::StackName"}-{"AWS::Region"}-yourManagedRole # # to follow the recommendation in AWS documentation http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-iam-role.html # # This is also intended to be used in combination with .experimental.kube2IamSupport. See #297 for more information. +# # +# # ATTENTION: Consider limiting number of characters in clusterName and managedIamRoleName to avoid the resulting IAM +# # role name's length from exceeding the AWS limit: 64 +# # See https://github.com/kubernetes-incubator/kube-aws/issues/347 # managedIamRoleName: "yourManagedRole" # # If omitted, public subnets are created by kube-aws and used for controller nodes # subnets: @@ -143,6 +147,10 @@ worker: # # Role will be created with "Ref": {"AWS::StackName"}-{"AWS::Region"}-yourManagedRole # # to follow the recommendation in AWS documentation http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-iam-role.html # # This is also intended to be used in combination with kube2IamSupport. See #297 for more information. +# # +# # ATTENTION: Consider limiting number of characters in clusterName, nodePools[].name and managedIamRoleName to +# # avoid the resulting IAM role name's length from exceeding the AWS limit: 64 +# # See https://github.com/kubernetes-incubator/kube-aws/issues/347 # managedIamRoleName: "yourManagedRole" # # # Additional EBS volumes mounted on the worker diff --git a/core/nodepool/config/config.go b/core/nodepool/config/config.go index 3e6bc5eff..3ead4d2fc 100644 --- a/core/nodepool/config/config.go +++ b/core/nodepool/config/config.go @@ -8,6 +8,7 @@ import ( "strings" "errors" + "github.com/coreos/kube-aws/cfnresource" cfg "github.com/coreos/kube-aws/core/controlplane/config" "github.com/coreos/kube-aws/coreos/amiregistry" "github.com/coreos/kube-aws/filereader/userdatatemplate" @@ -56,6 +57,13 @@ type StackTemplateOptions struct { SkipWait bool } +// NestedStackName returns a sanitized name of this node pool which is usable as a valid cloudformation nested stack name +func (c ProvidedConfig) NestedStackName() string { + // Convert stack name into something valid as a cfn resource name or + // we'll end up with cfn errors like "Template format error: Resource name test5-controlplane is non alphanumeric" + return strings.Title(strings.Replace(c.StackName(), "-", "", -1)) +} + func (c ProvidedConfig) StackConfig(opts StackTemplateOptions) (*StackConfig, error) { var err error stackConfig := StackConfig{} @@ -261,6 +269,10 @@ func (c ProvidedConfig) valid() error { return fmt.Errorf("awsNodeLabels can't be enabled for node pool because the total number of characters in clusterName(=\"%s\") + node pool's name(=\"%s\") exceeds the limit of %d", c.ClusterName, c.NodePoolName, limit) } + if e := cfnresource.ValidateRoleNameLength(c.ClusterName, c.NestedStackName(), c.ManagedIamRoleName, c.Region.String()); e != nil { + return e + } + return nil } diff --git a/core/root/template_params.go b/core/root/template_params.go index 7d27e69ba..a7bde451f 100644 --- a/core/root/template_params.go +++ b/core/root/template_params.go @@ -4,7 +4,6 @@ import ( "fmt" controlplane "github.com/coreos/kube-aws/core/controlplane/cluster" nodepool "github.com/coreos/kube-aws/core/nodepool/cluster" - "strings" ) type TemplateParams struct { @@ -32,8 +31,7 @@ type controlPlane struct { } func (p controlPlane) Name() string { - stackName := p.controlPlane.StackName() - return strings.Title(strings.Replace(stackName, "-", "", -1)) + return p.controlPlane.NestedStackName() } func (p controlPlane) Tags() map[string]string { @@ -55,10 +53,7 @@ type nodePool struct { } func (p nodePool) Name() string { - // Convert stack name into something valid as a cfn resource name or - // we'll end up with cfn errors like "Template format error: Resource name test5-controlplane is non alphanumeric" - stackName := p.nodePool.StackName() - return strings.Title(strings.Replace(stackName, "-", "", -1)) + return p.nodePool.NestedStackName() } func (p nodePool) Tags() map[string]string { diff --git a/test/integration/aws_test.go b/test/integration/aws_test.go index b8ad1371e..8fdefaf13 100644 --- a/test/integration/aws_test.go +++ b/test/integration/aws_test.go @@ -99,3 +99,8 @@ func (s kubeAwsSettings) withClusterName(n string) kubeAwsSettings { s.clusterName = n return s } + +func (s kubeAwsSettings) withRegion(r string) kubeAwsSettings { + s.region = r + return s +} diff --git a/test/integration/maincluster_test.go b/test/integration/maincluster_test.go index 52e8721c1..2f9a12e97 100644 --- a/test/integration/maincluster_test.go +++ b/test/integration/maincluster_test.go @@ -1110,14 +1110,14 @@ worker: context: "WithKube2IamSupport", configYaml: minimalValidConfigYaml + ` controller: - managedIamRoleName: mycontrollerrole + managedIamRoleName: myrole1 experimental: kube2IamSupport: enabled: true worker: nodePools: - name: pool1 - managedIamRoleName: myworkerrole + managedIamRoleName: myrole2 kube2IamSupport: enabled: true `, @@ -1125,8 +1125,8 @@ worker: hasDefaultEtcdSettings, asgBasedNodePoolHasWaitSignalEnabled, func(c *config.Config, t *testing.T) { - expectedControllerRoleName := "mycontrollerrole" - expectedWorkerRoleName := "myworkerrole" + expectedControllerRoleName := "myrole1" + expectedWorkerRoleName := "myrole2" if expectedControllerRoleName != c.Controller.ManagedIamRoleName { t.Errorf("controller's managedIamRoleName didn't match : expected=%v actual=%v", expectedControllerRoleName, c.Controller.ManagedIamRoleName) @@ -2333,14 +2333,14 @@ routeTableId: rtb-1a2b3c4d worker: nodePools: - name: pool1 - managedIamRoleName: "yourManagedRole" + managedIamRoleName: "myManagedRole" `, assertConfig: []ConfigTester{ hasDefaultEtcdSettings, hasDefaultExperimentalFeatures, func(c *config.Config, t *testing.T) { - if c.NodePools[0].ManagedIamRoleName != "yourManagedRole" { - t.Errorf("managedIamRoleName: expected=yourManagedRole actual=%s", c.NodePools[0].ManagedIamRoleName) + if c.NodePools[0].ManagedIamRoleName != "myManagedRole" { + t.Errorf("managedIamRoleName: expected=myManagedRole actual=%s", c.NodePools[0].ManagedIamRoleName) } }, }, @@ -2818,6 +2818,24 @@ worker: `, expectedErrorMessage: "unknown keys found in worker.nodePools[0].clusterAutoscaler: baz", }, + { + context: "WithTooLongControllerIAMRoleName", + configYaml: kubeAwsSettings.withClusterName("kubeaws-it-main").withRegion("ap-northeast-1").minimumValidClusterYaml() + ` +controller: + managedIamRoleName: foobarba +`, + expectedErrorMessage: "IAM role name(=kubeaws-it-main-Controlplane-PRK1CVQNY7XZ-ap-northeast-1-foobarba) will be 65 characters long. It exceeds the AWS limit of 64 characters: cluster name(=kubeaws-it-main) + nested stack name(=Controlplane) + managed iam role name(=foobarba) should be less than or equal to 34", + }, + { + context: "WithTooLongWorkerIAMRoleName", + configYaml: kubeAwsSettings.withClusterName("kubeaws-it-main").withRegion("ap-northeast-1").minimumValidClusterYaml() + ` +worker: + nodePools: + - name: pool1 + managedIamRoleName: foobarbazbaraaa +`, + expectedErrorMessage: "IAM role name(=kubeaws-it-main-Pool1-PRK1CVQNY7XZ-ap-northeast-1-foobarbazbaraaa) will be 65 characters long. It exceeds the AWS limit of 64 characters: cluster name(=kubeaws-it-main) + nested stack name(=Pool1) + managed iam role name(=foobarbazbaraaa) should be less than or equal to 34", + }, } for _, invalidCase := range parseErrorCases {