Add irsa support for awslbcontroller

This commit also introduces support for adding token projection volumes for well-known SAs.
Slightly less complicated than explicitly parsing the objects for a manifest

Author: Ole Markus With

cmd/kops/integration_test.go

@@ -25,6 +25,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"encoding/pem"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -58,10 +59,10 @@ type integrationTest struct {
 	private        bool
 	zones          int
 	expectPolicies bool
-	// expectServiceAccountRoles is true if we expect to assign per-ServiceAccount IAM roles (instead of just using the node roles)
-	expectServiceAccountRoles bool
-	lifecycleOverrides        []string
-	sshKey                    bool
+	// expectServiceAccountRolePolicies is a list of per-ServiceAccount IAM roles (instead of just using the node roles)
+	expectServiceAccountRolePolicies []string
+	lifecycleOverrides               []string
+	sshKey                           bool
 	// caKey is true if we should use a provided ca.crt & ca.key as our CA
 	caKey           bool
 	jsonOutput      bool
@@ -120,9 +121,12 @@ func (i *integrationTest) withPrivate() *integrationTest {
 	return i
 }
 
-// withServiceAccountRoles indicates we expect to assign per-ServiceAccount IAM roles (instead of just using the node roles)
-func (i *integrationTest) withServiceAccountRoles() *integrationTest {
-	i.expectServiceAccountRoles = true
+// withServiceAccountRoles indicates we expect to assign an IAM role for a ServiceAccount (instead of just using the node roles)
+func (i *integrationTest) withServiceAccountRole(sa string, inlinePolicy bool) *integrationTest {
+	i.expectServiceAccountRolePolicies = append(i.expectServiceAccountRolePolicies, fmt.Sprintf("aws_iam_role_%s.sa.%s_policy", sa, i.clusterName))
+	if inlinePolicy {
+		i.expectServiceAccountRolePolicies = append(i.expectServiceAccountRolePolicies, fmt.Sprintf("aws_iam_role_policy_%s.sa.%s_policy", sa, i.clusterName))
+	}
 	return i
 }
 
@@ -283,7 +287,11 @@ func TestPublicJWKS(t *testing.T) {
 	defer unsetFeatureFlags()
 
 	// We have to use a fixed CA because the fingerprint is inserted into the AWS WebIdentity configuration.
-	newIntegrationTest("minimal.example.com", "public-jwks").withCAKey().withServiceAccountRoles().runTestTerraformAWS(t)
+	newIntegrationTest("minimal.example.com", "public-jwks").
+		withCAKey().
+		withServiceAccountRole("dns-controller.kube-system", true).
+		runTestTerraformAWS(t)
+
 }
 
 // TestAWSLBController runs a simple configuration, but with AWS LB controller, UseServiceAccountIAM and PublicJWKS enabled
@@ -295,7 +303,11 @@ func TestAWSLBController(t *testing.T) {
 	defer unsetFeatureFlags()
 
 	// We have to use a fixed CA because the fingerprint is inserted into the AWS WebIdentity configuration.
-	newIntegrationTest("minimal.example.com", "aws-lb-controller").withCAKey().withServiceAccountRoles().runTestTerraformAWS(t)
+	newIntegrationTest("minimal.example.com", "aws-lb-controller").
+		withCAKey().
+		withServiceAccountRole("dns-controller.kube-system", true).
+		withServiceAccountRole("aws-load-balancer-controller.kube-system", true).
+		runTestTerraformAWS(t)
 }
 
 // TestSharedSubnet runs the test on a configuration with a shared subnet (and VPC)
@@ -568,12 +580,9 @@ func (i *integrationTest) runTestTerraformAWS(t *testing.T) {
 			}
 		}
 	}
-	if i.expectServiceAccountRoles {
-		expectedFilenames = append(expectedFilenames, []string{
-			"aws_iam_role_dns-controller.kube-system.sa." + i.clusterName + "_policy",
-			"aws_iam_role_policy_dns-controller.kube-system.sa." + i.clusterName + "_policy",
-		}...)
-	}
+
+	expectedFilenames = append(expectedFilenames, i.expectServiceAccountRolePolicies...)
+
 	i.runTest(t, h, expectedFilenames, tfFileName, tfFileName, nil)
 }
 

pkg/model/components/addonmanifests/BUILD.bazel

@@ -0,0 +1,50 @@
+/*
+Copyright 2020 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 awsloadbalancercontroller
+
+import (
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/kops/pkg/model/iam"
+	"k8s.io/kops/pkg/util/stringorslice"
+)
+
+// ServiceAccount represents the service-account used by the dns-controller.
+// It implements iam.Subject to get AWS IAM permissions.
+type ServiceAccount struct {
+}
+
+var _ iam.Subject = &ServiceAccount{}
+
+// BuildAWSPolicy generates a custom policy for a ServiceAccount IAM role.
+func (r *ServiceAccount) BuildAWSPolicy(b *iam.PolicyBuilder) (*iam.Policy, error) {
+	p := &iam.Policy{
+		Version: iam.PolicyDefaultVersion,
+	}
+
+	resource := stringorslice.Slice([]string{"*"})
+	iam.AddAWSLoadbalancerControllerPermissions(p, resource, b.Cluster.ObjectMeta.Name)
+
+	return p, nil
+}
+
+// ServiceAccount returns the kubernetes service account used.
+func (r *ServiceAccount) ServiceAccount() (types.NamespacedName, bool) {
+	return types.NamespacedName{
+		Namespace: "kube-system",
+		Name:      "aws-load-balancer-controller",
+	}, true
+}

pkg/model/components/addonmanifests/awsloadbalancercontroller/BUILD.bazel

@@ -19,13 +19,17 @@ package addonmanifests
 import (
 	"fmt"
 
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"k8s.io/klog/v2"
 	addonsapi "k8s.io/kops/channels/pkg/api"
 	"k8s.io/kops/pkg/assets"
 	"k8s.io/kops/pkg/kubemanifest"
 	"k8s.io/kops/pkg/model"
+	"k8s.io/kops/pkg/model/components/addonmanifests/awsloadbalancercontroller"
 	"k8s.io/kops/pkg/model/components/addonmanifests/dnscontroller"
+	"k8s.io/kops/pkg/model/iam"
 	"k8s.io/kops/upup/pkg/fi"
 )
 
@@ -49,6 +53,11 @@ func RemapAddonManifest(addon *addonsapi.AddonSpec, context *model.KopsModelCont
 			return nil, fmt.Errorf("failed to annotate %q: %w", name, err)
 		}
 
+		err = addServiceAccountRole(context, objects)
+		if err != nil {
+			return nil, fmt.Errorf("failed to add service account for %q: %w", name, err)
+		}
+
 		b, err := objects.ToYAML()
 		if err != nil {
 			return nil, err
@@ -68,6 +77,48 @@ func RemapAddonManifest(addon *addonsapi.AddonSpec, context *model.KopsModelCont
 	return manifest, nil
 }
 
+func addServiceAccountRole(context *model.KopsModelContext, objects kubemanifest.ObjectList) error {
+	for _, object := range objects {
+		if object.Kind() != "Deployment" {
+			continue
+		}
+		if object.APIVersion() != "apps/v1" {
+			continue
+		}
+		podSpec := &corev1.PodSpec{}
+
+		if err := object.Reparse(podSpec, "spec", "template", "spec"); err != nil {
+			return fmt.Errorf("failed to parse spec.template.spec from Deployment: %v", err)
+		}
+		containers := podSpec.Containers
+		sa := podSpec.ServiceAccountName
+		subject := getWellknownServiceAccount(sa)
+		if subject == nil {
+			continue
+		}
+		for k, container := range containers {
+			if err := iam.AddServiceAccountRole(&context.IAMModelContext, podSpec, &container, subject); err != nil {
+				return err
+			}
+			containers[k] = container
+		}
+		if err := object.Set(podSpec, "spec", "template", "spec"); err != nil {
+			return fmt.Errorf("failed to set object: %w", err)
+		}
+
+	}
+	return nil
+}
+
+func getWellknownServiceAccount(name string) iam.Subject {
+	switch name {
+	case "aws-load-balancer-controller":
+		return &awsloadbalancercontroller.ServiceAccount{}
+	default:
+		return nil
+	}
+}
+
 func addLabels(addon *addonsapi.AddonSpec, objects kubemanifest.ObjectList) error {
 
 	for _, object := range objects {

pkg/model/components/addonmanifests/awsloadbalancercontroller/iam.go

@@ -259,7 +259,6 @@ func (r *NodeRoleAPIServer) BuildAWSPolicy(b *PolicyBuilder) (*Policy, error) {
 	if b.Cluster.Spec.IAM.Legacy {
 		addLegacyDNSControllerPermissions(b, p)
 	}
-	AddDNSControllerPermissions(b, p)
 
 	if b.Cluster.Spec.IAM.Legacy || b.Cluster.Spec.IAM.AllowContainerRegistry {
 		addECRPermissions(p)
@@ -311,6 +310,10 @@ func (r *NodeRoleMaster) BuildAWSPolicy(b *PolicyBuilder) (*Policy, error) {
 			addLegacyDNSControllerPermissions(b, p)
 		}
 		AddDNSControllerPermissions(b, p)
+
+		if b.Cluster.Spec.AWSLoadBalancerController != nil && fi.BoolValue(b.Cluster.Spec.AWSLoadBalancerController.Enabled) {
+			AddAWSLoadbalancerControllerPermissions(p, resource, b.Cluster.GetName())
+		}
 	}
 
 	if b.Cluster.Spec.IAM.Legacy || b.Cluster.Spec.IAM.AllowContainerRegistry {
@@ -333,10 +336,6 @@ func (r *NodeRoleMaster) BuildAWSPolicy(b *PolicyBuilder) (*Policy, error) {
 		addCalicoSrcDstCheckPermissions(p)
 	}
 
-	if b.Cluster.Spec.AWSLoadBalancerController != nil && fi.BoolValue(b.Cluster.Spec.AWSLoadBalancerController.Enabled) {
-		addAWSLoadbalancerControllerPermissions(p, b.Cluster.GetName())
-	}
-
 	return p, nil
 }
 
@@ -722,15 +721,18 @@ func addCalicoSrcDstCheckPermissions(p *Policy) {
 	})
 }
 
-func addAWSLoadbalancerControllerPermissions(p *Policy, clusterName string) {
+// AddAWSLoadbalancerControllerPermissions adds the permissions needed for the aws load balancer controller to the givnen policy
+func AddAWSLoadbalancerControllerPermissions(p *Policy, resource stringorslice.StringOrSlice, clusterName string) {
+	addMasterEC2Policies(p, resource, false, clusterName)
+	addMasterELBPolicies(p, resource, false)
 	p.Statement = append(p.Statement, &Statement{
 		Effect: StatementEffectAllow,
 		Action: stringorslice.Of(
 			"ec2:AuthorizeSecurityGroupIngress", // aws.go
 			"ec2:DeleteSecurityGroup",           // aws.go
 			"ec2:RevokeSecurityGroupIngress",    // aws.go
 		),
-		Resource: stringorslice.Slice([]string{"*"}),
+		Resource: resource,
 		Condition: Condition{
 			"StringEquals": map[string]string{
 				"ec2:ResourceTag/elbv2.k8s.aws/cluster": clusterName,
@@ -748,7 +750,7 @@ func addAWSLoadbalancerControllerPermissions(p *Policy, clusterName string) {
 				"elasticloadbalancing:DescribeListenerCertificates",
 				"elasticloadbalancing:CreateRule",
 			),
-			Resource: stringorslice.Slice([]string{"*"}),
+			Resource: resource,
 		})
 }
 

pkg/model/components/addonmanifests/remap.go

@@ -1169,35 +1169,6 @@
               "Resource": [
                 "*"
               ]
-            },
-            {
-              "Action": [
-                "route53:ChangeResourceRecordSets",
-                "route53:ListResourceRecordSets",
-                "route53:GetHostedZone"
-              ],
-              "Effect": "Allow",
-              "Resource": [
-                "arn:aws:route53:::hostedzone/Z1AFAKE1ZON3YO"
-              ]
-            },
-            {
-              "Action": [
-                "route53:GetChange"
-              ],
-              "Effect": "Allow",
-              "Resource": [
-                "arn:aws:route53:::change/*"
-              ]
-            },
-            {
-              "Action": [
-                "route53:ListHostedZones"
-              ],
-              "Effect": "Allow",
-              "Resource": [
-                "*"
-              ]
             }
           ],
           "Version": "2012-10-17"

pkg/model/iam/iam_builder.go

@@ -0,0 +1,17 @@
+{
+  "Statement": [
+    {
+      "Action": "sts:AssumeRoleWithWebIdentity",
+      "Condition": {
+        "StringEquals": {
+          "discovery.example.com/minimal.example.com/oidc:sub": "system:serviceaccount:kube-system:aws-load-balancer-controller"
+        }
+      },
+      "Effect": "Allow",
+      "Principal": {
+        "Federated": "arn:aws:iam::123456789012:oidc-provider/discovery.example.com/minimal.example.com/oidc"
+      }
+    }
+  ],
+  "Version": "2012-10-17"
+}