Merge pull request #5541 from justinsb/validate_additional_policies

Validate IAM additionalPolicies

pkg/apis/kops/instancegroup.go

@@ -60,6 +60,7 @@ const (
 	InstanceGroupRoleBastion InstanceGroupRole = "Bastion"
 )
 
+// AllInstanceGroupRoles is a slice of all valid InstanceGroupRole values
 var AllInstanceGroupRoles = []InstanceGroupRole{
 	InstanceGroupRoleNode,
 	InstanceGroupRoleMaster,

pkg/apis/kops/validation/BUILD.bazel

@@ -18,6 +18,7 @@ go_library(
         "//pkg/apis/kops/util:go_default_library",
         "//pkg/featureflag:go_default_library",
         "//pkg/model/components:go_default_library",
+        "//pkg/model/iam:go_default_library",
         "//upup/pkg/fi:go_default_library",
         "//upup/pkg/fi/cloudup/awsup:go_default_library",
         "//vendor/github.com/blang/semver:go_default_library",

pkg/apis/kops/validation/validation.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/pkg/model/iam"
 )
 
 var validDockerConfigStorageValues = []string{"aufs", "btrfs", "devicemapper", "overlay", "overlay2", "zfs"}
@@ -54,7 +55,7 @@ func newValidateCluster(cluster *kops.Cluster) field.ErrorList {
 func validateClusterSpec(spec *kops.ClusterSpec, fieldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	allErrs = append(allErrs, validateSubnets(spec.Subnets, field.NewPath("spec"))...)
+	allErrs = append(allErrs, validateSubnets(spec.Subnets, fieldPath.Child("subnets"))...)
 
 	// SSHAccess
 	for i, cidr := range spec.SSHAccess {
@@ -95,6 +96,13 @@ func validateClusterSpec(spec *kops.ClusterSpec, fieldPath *field.Path) field.Er
 		allErrs = append(allErrs, validateNetworking(spec.Networking, fieldPath.Child("networking"))...)
 	}
 
+	// IAM additionalPolicies
+	if spec.AdditionalPolicies != nil {
+		for k, v := range *spec.AdditionalPolicies {
+			allErrs = append(allErrs, validateAdditionalPolicy(k, v, fieldPath.Child("additionalPolicies"))...)
+		}
+	}
+
 	return allErrs
 }
 
@@ -250,3 +258,39 @@ func validateNetworkingFlannel(v *kops.FlannelNetworkingSpec, fldPath *field.Pat
 
 	return allErrs
 }
+
+func validateAdditionalPolicy(role string, policy string, fldPath *field.Path) field.ErrorList {
+	errs := field.ErrorList{}
+
+	valid := sets.NewString()
+	for _, r := range kops.AllInstanceGroupRoles {
+		k := strings.ToLower(string(r))
+		valid.Insert(k)
+	}
+	if !valid.Has(role) {
+		message := fmt.Sprintf("role is not known (valid values: %s)", strings.Join(valid.List(), ","))
+		errs = append(errs, field.Invalid(fldPath, role, message))
+	}
+
+	statements, err := iam.ParseStatements(policy)
+	if err != nil {
+		errs = append(errs, 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")
+		switch statement.Effect {
+		case "Allow", "Deny":
+			//valid
+
+		case "":
+			errs = append(errs, field.Required(fldEffect, "Effect must be specified for IAM policy"))
+
+		default:
+			errs = append(errs, field.Invalid(fldEffect, statement.Effect, "Effect must be 'Allow' or 'Deny'"))
+		}
+	}
+
+	return errs
+}

pkg/apis/kops/validation/validation_test.go

@@ -241,3 +241,53 @@ func Test_Validate_Networking_Flannel(t *testing.T) {
 		testErrors(t, g.Input, errs, g.ExpectedErrors)
 	}
 }
+
+func Test_Validate_AdditionalPolicies(t *testing.T) {
+	grid := []struct {
+		Input          map[string]string
+		ExpectedErrors []string
+	}{
+		{
+			Input: map[string]string{},
+		},
+		{
+			Input: map[string]string{
+				"master": `[ { "Action": [ "s3:GetObject" ], "Resource": [ "*" ], "Effect": "Allow" } ]`,
+			},
+		},
+		{
+			Input: map[string]string{
+				"notarole": `[ { "Action": [ "s3:GetObject" ], "Resource": [ "*" ], "Effect": "Allow" } ]`,
+			},
+			ExpectedErrors: []string{"Invalid value::spec.additionalPolicies"},
+		},
+		{
+			Input: map[string]string{
+				"master": `badjson`,
+			},
+			ExpectedErrors: []string{"Invalid value::spec.additionalPolicies[master]"},
+		},
+		{
+			Input: map[string]string{
+				"master": `[ { "Action": [ "s3:GetObject" ], "Resource": [ "*" ] } ]`,
+			},
+			ExpectedErrors: []string{"Required value::spec.additionalPolicies[master][0].Effect"},
+		},
+		{
+			Input: map[string]string{
+				"master": `[ { "Action": [ "s3:GetObject" ], "Resource": [ "*" ], "Effect": "allow" } ]`,
+			},
+			ExpectedErrors: []string{"Invalid value::spec.additionalPolicies[master][0].Effect"},
+		},
+	}
+	for _, g := range grid {
+		clusterSpec := &kops.ClusterSpec{
+			AdditionalPolicies: &g.Input,
+			Subnets: []kops.ClusterSubnetSpec{
+				{Name: "subnet1"},
+			},
+		}
+		errs := validateClusterSpec(clusterSpec, field.NewPath("spec"))
+		testErrors(t, g.Input, errs, g.ExpectedErrors)
+	}
+}

pkg/model/iam.go

@@ -17,7 +17,6 @@ limitations under the License.
 package model
 
 import (
-	"encoding/json"
 	"fmt"
 	"strings"
 	"text/template"
@@ -186,8 +185,8 @@ func (b *IAMModelBuilder) buildIAMTasks(igRole kops.InstanceGroupRole, iamName s
 					Version: iam.PolicyDefaultVersion,
 				}
 
-				statements := make([]*iam.Statement, 0)
+				statements, err := iam.ParseStatements(additionalPolicy)
-				if err := json.Unmarshal([]byte(additionalPolicy), &statements); err != nil {
+				if err != nil {
 					return fmt.Errorf("additionalPolicy %q is invalid: %v", strings.ToLower(string(igRole)), err)
 				}
 

pkg/model/iam/BUILD.bazel

@@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
-    srcs = ["iam_builder.go"],
+    srcs = [
+        "iam_builder.go",
+        "types.go",
+    ],
     importpath = "k8s.io/kops/pkg/model/iam",
     visibility = ["//visibility:public"],
     deps = [

pkg/model/iam/types.go

@@ -0,0 +1,31 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package iam
+
+import (
+	"encoding/json"
+	"fmt"
+)
+
+// ParseStatements parses JSON into a list of Statements
+func ParseStatements(policy string) ([]*Statement, error) {
+	statements := make([]*Statement, 0)
+	if err := json.Unmarshal([]byte(policy), &statements); err != nil {
+		return nil, fmt.Errorf("error parsing IAM statements: %v", err)
+	}
+	return statements, nil
+}