diff --git a/pkg/apis/kops/validation/instancegroup.go b/pkg/apis/kops/validation/instancegroup.go index 148cc96770..ecdf506298 100644 --- a/pkg/apis/kops/validation/instancegroup.go +++ b/pkg/apis/kops/validation/instancegroup.go @@ -248,7 +248,7 @@ func validateInstanceProfile(v *kops.IAMProfileSpec, fldPath *field.Path) field. if v != nil && v.Profile != nil { instanceProfileARN := *v.Profile parsedARN, err := arn.Parse(instanceProfileARN) - if err != nil || !strings.HasPrefix(parsedARN.Resource, "instance-profile") { + if err != nil || !strings.HasPrefix(parsedARN.Resource, "instance-profile/") { allErrs = append(allErrs, field.Invalid(fldPath.Child("profile"), instanceProfileARN, "Instance Group IAM Instance Profile must be a valid aws arn such as arn:aws:iam::123456789012:instance-profile/KopsExampleRole")) } diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index 353ce098b1..2c13bde20a 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -23,21 +23,21 @@ import ( "regexp" "strings" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/blang/semver/v4" "golang.org/x/net/ipv4" "golang.org/x/net/ipv6" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/kops/pkg/featureflag" - "k8s.io/kops/upup/pkg/fi" - "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/util/intstr" utilnet "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/sets" utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kops/pkg/apis/kops" + "k8s.io/kops/pkg/featureflag" "k8s.io/kops/pkg/model/components" "k8s.io/kops/pkg/model/iam" + "k8s.io/kops/upup/pkg/fi" ) func newValidateCluster(cluster *kops.Cluster) field.ErrorList { @@ -158,12 +158,18 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie allErrs = append(allErrs, validateNodeTerminationHandler(c, spec.NodeTerminationHandler, fieldPath.Child("nodeTerminationHandler"))...) } - // IAM additionalPolicies + // IAM additional policies if spec.AdditionalPolicies != nil { for k, v := range *spec.AdditionalPolicies { allErrs = append(allErrs, validateAdditionalPolicy(k, v, fieldPath.Child("additionalPolicies"))...) } } + // IAM external policies + if spec.ExternalPolicies != nil { + for k, v := range *spec.ExternalPolicies { + allErrs = append(allErrs, validateExternalPolicies(k, v, fieldPath.Child("externalPolicies"))...) + } + } // EtcdClusters { @@ -798,31 +804,51 @@ func validateNetworkingGCE(c *kops.ClusterSpec, v *kops.GCENetworkingSpec, fldPa } func validateAdditionalPolicy(role string, policy string, fldPath *field.Path) field.ErrorList { - errs := field.ErrorList{} + allErrs := field.ErrorList{} var valid []string for _, r := range kops.AllInstanceGroupRoles { valid = append(valid, strings.ToLower(string(r))) } - errs = append(errs, IsValidValue(fldPath, &role, valid)...) + allErrs = append(allErrs, IsValidValue(fldPath, &role, valid)...) statements, err := iam.ParseStatements(policy) if err != nil { - errs = append(errs, field.Invalid(fldPath.Key(role), policy, "policy was not valid JSON: "+err.Error())) + allErrs = append(allErrs, field.Invalid(fldPath.Key(role), policy, "policy was not valid JSON: "+err.Error())) } // Trivial validation of policy, mostly to make sure it isn't some other random object for i, statement := range statements { fldEffect := fldPath.Key(role).Index(i).Child("Effect") if statement.Effect == "" { - errs = append(errs, field.Required(fldEffect, "Effect must be specified for IAM policy")) + allErrs = append(allErrs, field.Required(fldEffect, "Effect must be specified for IAM policy")) } else { value := string(statement.Effect) - errs = append(errs, IsValidValue(fldEffect, &value, []string{"Allow", "Deny"})...) + allErrs = append(allErrs, IsValidValue(fldEffect, &value, []string{"Allow", "Deny"})...) } } - return errs + return allErrs +} + +func validateExternalPolicies(role string, policies []string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + var valid []string + for _, r := range kops.AllInstanceGroupRoles { + valid = append(valid, strings.ToLower(string(r))) + } + allErrs = append(allErrs, IsValidValue(fldPath, &role, valid)...) + + for _, policy := range policies { + parsedARN, err := arn.Parse(policy) + if err != nil || !strings.HasPrefix(parsedARN.Resource, "policy/") { + allErrs = append(allErrs, field.Invalid(fldPath.Child(role), policy, + "Policy must be a valid AWS ARN such as arn:aws:iam::123456789012:policy/KopsExamplePolicy")) + } + } + + return allErrs } func validateEtcdClusterSpec(spec kops.EtcdClusterSpec, c *kops.Cluster, fieldPath *field.Path) field.ErrorList { diff --git a/tests/integration/update_cluster/externalpolicies/in-v1alpha2.yaml b/tests/integration/update_cluster/externalpolicies/in-v1alpha2.yaml index 915feceec9..fda0e8351f 100644 --- a/tests/integration/update_cluster/externalpolicies/in-v1alpha2.yaml +++ b/tests/integration/update_cluster/externalpolicies/in-v1alpha2.yaml @@ -50,11 +50,11 @@ spec: nodes: public externalPolicies: node: - - aws:arn:iam:123456789000:policy:test-policy + - arn:aws:iam::123456789000:policy/test-policy master: - - aws:arn:iam:123456789000:policy:test-policy + - arn:aws:iam::123456789000:policy/test-policy bastion: - - aws:arn:iam:123456789000:policy:test-policy + - arn:aws:iam::123456789000:policy/test-policy subnets: - cidr: 172.20.32.0/19 name: us-test-1a diff --git a/tests/integration/update_cluster/externalpolicies/kubernetes.tf b/tests/integration/update_cluster/externalpolicies/kubernetes.tf index 03ddf6067d..720e2a26af 100644 --- a/tests/integration/update_cluster/externalpolicies/kubernetes.tf +++ b/tests/integration/update_cluster/externalpolicies/kubernetes.tf @@ -268,13 +268,13 @@ resource "aws_iam_instance_profile" "nodes-externalpolicies-example-com" { role = aws_iam_role.nodes-externalpolicies-example-com.name } -resource "aws_iam_role_policy_attachment" "master-policyoverride-1178482798" { - policy_arn = "aws:arn:iam:123456789000:policy:test-policy" +resource "aws_iam_role_policy_attachment" "master-policyoverride-1242070525" { + policy_arn = "arn:aws:iam::123456789000:policy/test-policy" role = aws_iam_role.masters-externalpolicies-example-com.name } -resource "aws_iam_role_policy_attachment" "node-policyoverride-1178482798" { - policy_arn = "aws:arn:iam:123456789000:policy:test-policy" +resource "aws_iam_role_policy_attachment" "node-policyoverride-1242070525" { + policy_arn = "arn:aws:iam::123456789000:policy/test-policy" role = aws_iam_role.nodes-externalpolicies-example-com.name }