Merge pull request #9328 from justinsb/vfs_readable_refactor

IAM: Refactor vfs-access logic so we can see the required readable paths
kubernetes · Jun 11, 2020 · d6929b6 · d6929b6
2 parents 98d1f7b + 0351590
commit d6929b6
Showing 1 changed file with 66 additions and 74 deletions.
diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go
@@ -335,78 +335,23 @@ func (b *PolicyBuilder) AddS3Permissions(p *Policy) (*Policy, error) {
 					),
 				})
 			} else {
-				if b.Role == kops.InstanceGroupRoleMaster {
-					p.Statement = append(p.Statement, &Statement{
-						Effect: StatementEffectAllow,
-						Action: stringorslice.Slice([]string{"s3:Get*"}),
-						Resource: stringorslice.Of(
-							strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/*"}, ""),
-						),
-					})
-				} else if b.Role == kops.InstanceGroupRoleNode {
-					resources := []string{
-						strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/addons/*"}, ""),
-						strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/cluster.spec"}, ""),
-						strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/config"}, ""),
-						strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/instancegroup/*"}, ""),
-						strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/pki/issued/*"}, ""),
-						strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/pki/private/kube-proxy/*"}, ""),
-						strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/pki/ssh/*"}, ""),
-						strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/secrets/dockerconfig"}, ""),
-					}
-
-					// @check if bootstrap tokens are enabled and if so enable access to client certificate
-					if b.UseBootstrapTokens() {
-						resources = append(resources, strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/pki/private/node-authorizer-client/*"}, ""))
-					} else {
-						resources = append(resources, strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/pki/private/kubelet/*"}, ""))
-					}
-
-					sort.Strings(resources)
-
-					p.Statement = append(p.Statement, &Statement{
-						Effect:   StatementEffectAllow,
-						Action:   stringorslice.Slice([]string{"s3:Get*"}),
-						Resource: stringorslice.Of(resources...),
-					})
-
-					networkingSpec := b.Cluster.Spec.Networking
-
-					if networkingSpec != nil {
-						// @check if kuberoute is enabled and permit access to the private key
-						if networkingSpec.Kuberouter != nil {
-							p.Statement = append(p.Statement, &Statement{
-								Effect: StatementEffectAllow,
-								Action: stringorslice.Slice([]string{"s3:Get*"}),
-								Resource: stringorslice.Of(
-									strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/pki/private/kube-router/*"}, ""),
-								),
-							})
-						}
-
-						// @check if calico is enabled as the CNI provider and permit access to the client TLS certificate by default
-						if networkingSpec.Calico != nil {
-							p.Statement = append(p.Statement, &Statement{
-								Effect: StatementEffectAllow,
-								Action: stringorslice.Slice([]string{"s3:Get*"}),
-								Resource: stringorslice.Of(
-									strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/pki/private/calico-client/*"}, ""),
-								),
-							})
-						}
-
-						// @check if cilium is enabled as the CNI provider and permit access to the cilium etc client TLS certificate by default
-						if networkingSpec.Cilium != nil && networkingSpec.Cilium.EtcdManaged {
-							p.Statement = append(p.Statement, &Statement{
-								Effect: StatementEffectAllow,
-								Action: stringorslice.Slice([]string{"s3:Get*"}),
-								Resource: stringorslice.Of(
-									strings.Join([]string{b.IAMPrefix(), ":s3:::", iamS3Path, "/pki/private/etcd-clients-ca-cilium/*"}, ""),
-								),
-							})
-						}
-					}
+				resources, err := ReadableStatePaths(b.Cluster, b.Role)
+				if err != nil {
+					return nil, err
 				}
+
+				sort.Strings(resources)
+
+				// Add the prefix for IAM
+				for i, r := range resources {
+					resources[i] = b.IAMPrefix() + ":s3:::" + iamS3Path + r
+				}
+
+				p.Statement = append(p.Statement, &Statement{
+					Effect:   StatementEffectAllow,
+					Action:   stringorslice.Slice([]string{"s3:Get*"}),
+					Resource: stringorslice.Of(resources...),
+				})
 			}
 		} else if _, ok := vfsPath.(*vfs.MemFSPath); ok {
 			// Tests -ignore - nothing we can do in terms of IAM policy
@@ -491,6 +436,53 @@ func WriteableVFSPaths(cluster *kops.Cluster, role kops.InstanceGroupRole) ([]vf
 	return paths, nil
 }
 
+// ReadableStatePaths returns the file paths that should be readable in the cluster's state store "directory"
+func ReadableStatePaths(cluster *kops.Cluster, role kops.InstanceGroupRole) ([]string, error) {
+	var paths []string
+
+	if role == kops.InstanceGroupRoleMaster {
+		paths = append(paths, "/*")
+	} else if role == kops.InstanceGroupRoleNode {
+		paths = append(paths,
+			"/addons/*",
+			"/cluster.spec",
+			"/config",
+			"/instancegroup/*",
+			"/pki/issued/*",
+			"/pki/private/kube-proxy/*",
+			"/pki/ssh/*",
+			"/secrets/dockerconfig",
+		)
+
+		// @check if bootstrap tokens are enabled and if so enable access to client certificate
+		if UseBootstrapTokens(cluster) {
+			paths = append(paths, "/pki/private/node-authorizer-client/*")
+		} else {
+			paths = append(paths, "/pki/private/kubelet/*")
+		}
+
+		networkingSpec := cluster.Spec.Networking
+
+		if networkingSpec != nil {
+			// @check if kuberoute is enabled and permit access to the private key
+			if networkingSpec.Kuberouter != nil {
+				paths = append(paths, "/pki/private/kube-router/*")
+			}
+
+			// @check if calico is enabled as the CNI provider and permit access to the client TLS certificate by default
+			if networkingSpec.Calico != nil {
+				paths = append(paths, "/pki/private/calico-client/*")
+			}
+
+			// @check if cilium is enabled as the CNI provider and permit access to the cilium etc client TLS certificate by default
+			if networkingSpec.Cilium != nil && networkingSpec.Cilium.EtcdManaged {
+				paths = append(paths, "/pki/private/etcd-clients-ca-cilium/*")
+			}
+		}
+	}
+	return paths, nil
+}
+
 // PolicyResource defines the PolicyBuilder and DNSZone to use when building the
 // IAM policy document for a given instance group role
 type PolicyResource struct {
@@ -537,12 +529,12 @@ func (b *PolicyResource) Open() (io.Reader, error) {
 
 // UseBootstrapTokens check if we are using bootstrap tokens - @TODO, i don't like this we should probably pass in
 // the kops model into the builder rather than duplicating the code. I'll leave for another PR
-func (b *PolicyBuilder) UseBootstrapTokens() bool {
-	if b.Cluster.Spec.KubeAPIServer == nil {
+func UseBootstrapTokens(cluster *kops.Cluster) bool {
+	if cluster.Spec.KubeAPIServer == nil {
 		return false
 	}
 
-	return fi.BoolValue(b.Cluster.Spec.KubeAPIServer.EnableBootstrapAuthToken)
+	return fi.BoolValue(cluster.Spec.KubeAPIServer.EnableBootstrapAuthToken)
 }
 
 func addECRPermissions(p *Policy) {