Skip to content

Commit

Permalink
PCP-618: capa fix hardcoded role arn for aws iam authenticator (#519)
Browse files Browse the repository at this point in the history
  • Loading branch information
AmitSahastra committed Jan 14, 2023
1 parent d15060e commit 9bf07cd
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 21 deletions.
2 changes: 2 additions & 0 deletions cmd/clusterawsadm/api/bootstrap/v1beta1/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ const (
DefaultStackName = "cluster-api-provider-aws-sigs-k8s-io"
// DefaultPartitionName is the default security partition for AWS ARNs.
DefaultPartitionName = "aws"
// DefaultPartitionNameUSGov is the default security partition for AWS ARNs.
DefaultPartitionNameUSGov = "us-gov"
// DefaultKMSAliasPattern is the default KMS alias.
DefaultKMSAliasPattern = "cluster-api-provider-aws-*"
// DefaultS3BucketPrefix is the default S3 bucket prefix.
Expand Down
9 changes: 7 additions & 2 deletions cmd/clusterawsadm/cloudformation/bootstrap/fargate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,13 @@ import (
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/eks"
)

func fargateProfilePolicies(roleSpec *bootstrapv1.AWSIAMRoleSpec) []string {
policies := eks.FargateRolePolicies()
func (t Template) fargateProfilePolicies(roleSpec *bootstrapv1.AWSIAMRoleSpec) []string {
var policies []string
if t.Spec.Partition == bootstrapv1.DefaultPartitionNameUSGov {
policies = eks.FargateRolePoliciesAWSUSGov()
} else {
policies = eks.FargateRolePolicies()
}
if roleSpec.ExtraPolicyAttachments != nil {
policies = append(policies, roleSpec.ExtraPolicyAttachments...)
}
Expand Down
13 changes: 11 additions & 2 deletions cmd/clusterawsadm/cloudformation/bootstrap/managed_nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,19 @@ limitations under the License.

package bootstrap

import "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/eks"
import (
"sigs.k8s.io/cluster-api-provider-aws/cmd/clusterawsadm/api/bootstrap/v1beta1"
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/eks"
)

func (t Template) eksMachinePoolPolicies() []string {
policies := eks.NodegroupRolePolicies()

var policies []string
if t.Spec.Partition == v1beta1.DefaultPartitionNameUSGov {
policies = eks.NodegroupRolePoliciesAWSUSGov()
} else {
policies = eks.NodegroupRolePolicies()
}
if t.Spec.EKS.ManagedMachinePool.ExtraPolicyAttachments != nil {
policies = append(policies, t.Spec.EKS.ManagedMachinePool.ExtraPolicyAttachments...)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/clusterawsadm/cloudformation/bootstrap/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func (t Template) RenderCloudFormation() *cloudformation.Template {
template.Resources[AWSIAMRoleEKSFargate] = &cfn_iam.Role{
RoleName: expinfrav1.DefaultEKSFargateRole,
AssumeRolePolicyDocument: AssumeRolePolicy(iamv1.PrincipalService, []string{eksiam.EKSFargateService}),
ManagedPolicyArns: fargateProfilePolicies(t.Spec.EKS.Fargate),
ManagedPolicyArns: t.fargateProfilePolicies(t.Spec.EKS.Fargate),
Tags: converters.MapToCloudFormationTags(t.Spec.EKS.Fargate.Tags),
}
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ require (
k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed
sigs.k8s.io/aws-iam-authenticator v0.5.10
sigs.k8s.io/cluster-api v1.2.7
sigs.k8s.io/cluster-api-provider-aws v1.5.2
sigs.k8s.io/cluster-api/test v1.2.7
sigs.k8s.io/controller-runtime v0.12.3
sigs.k8s.io/kustomize/api v0.12.1
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1268,6 +1268,8 @@ sigs.k8s.io/aws-iam-authenticator v0.5.10 h1:YGPh/SpRxNkWXfGkURKGsgWvz70x41SB4Qa
sigs.k8s.io/aws-iam-authenticator v0.5.10/go.mod h1:3oJI6vy91KxVSb9g+v9gtJsFnFDJoq+ySJhGDBKpKFk=
sigs.k8s.io/cluster-api v1.2.7 h1:LjdToomKcAaOh71b2TSKbOucZgkw6NQhwiVaGuidI6E=
sigs.k8s.io/cluster-api v1.2.7/go.mod h1:qLMSP/QUb0zwBXoXo2MmzV+YLyNQBkK7OwYEUDOQyQA=
sigs.k8s.io/cluster-api-provider-aws v1.5.2 h1:0UkRCL9Yq/YAFBbl6Jlm0XE8nqxB+DAMkEBMUrpRGKE=
sigs.k8s.io/cluster-api-provider-aws v1.5.2/go.mod h1:67wutYFBvXPdpBOs0JJpmLUMbf+1ay5IZ5m+Hdey3cA=
sigs.k8s.io/cluster-api/test v1.2.7 h1:bHn9e2cOeX4tQcCPUQgw2oA8iFC6eZgRUs4+UUwCyZY=
sigs.k8s.io/cluster-api/test v1.2.7/go.mod h1:j7i8GyZTzGA6aLZXIAF94Cnixuy5ziXrn3z8z/vZ91E=
sigs.k8s.io/controller-runtime v0.12.3 h1:FCM8xeY/FI8hoAfh/V4XbbYMY20gElh9yh+A98usMio=
Expand Down
23 changes: 18 additions & 5 deletions pkg/cloud/services/ec2/ami.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ const (
// https://ubuntu.com/server/docs/cloud-images/amazon-ec2
ubuntuOwnerID = "099720109477"

ubuntuOwnerIDUsGov = "513442679011"

// Description regex for fetching Ubuntu AMIs for bastion host.
ubuntuImageDescription = "Canonical??Ubuntu??20.04?LTS??amd64?focal?image*"

Expand Down Expand Up @@ -195,13 +197,9 @@ func GetLatestImage(imgs []*ec2.Image) (*ec2.Image, error) {
return imgs[len(imgs)-1], nil
}

func (s *Service) defaultBastionAMILookup() (string, error) {
func (s *Service) defaultBastionAMILookup(region string) (string, error) {
describeImageInput := &ec2.DescribeImagesInput{
Filters: []*ec2.Filter{
{
Name: aws.String("owner-id"),
Values: []*string{aws.String(ubuntuOwnerID)},
},
{
Name: aws.String("architecture"),
Values: []*string{aws.String("x86_64")},
Expand All @@ -220,6 +218,21 @@ func (s *Service) defaultBastionAMILookup() (string, error) {
},
},
}

if strings.Contains(region, defaultUsGovPartitionName) {
filter := &ec2.Filter{
Name: aws.String("owner-id"),
Values: []*string{aws.String(ubuntuOwnerIDUsGov)},
}
describeImageInput.Filters = append(describeImageInput.Filters, filter)
} else {
filter := &ec2.Filter{
Name: aws.String("owner-id"),
Values: []*string{aws.String(ubuntuOwnerID)},
}
describeImageInput.Filters = append(describeImageInput.Filters, filter)
}

out, err := s.EC2Client.DescribeImages(describeImageInput)
if err != nil {
return "", errors.Wrapf(err, "failed to describe images within region: %q", s.scope.Region())
Expand Down
8 changes: 5 additions & 3 deletions pkg/cloud/services/ec2/bastion.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (

const (
defaultSSHKeyName = "default"
defaultUsGovPartitionName = "us-gov"
)

var (
Expand Down Expand Up @@ -76,7 +77,7 @@ func (s *Service) ReconcileBastion() error {
return errors.Wrap(err, "failed to patch conditions")
}
}
defaultBastion, err := s.getDefaultBastion(s.scope.Bastion().InstanceType, s.scope.Bastion().AMI)
defaultBastion, err := s.getDefaultBastion(s.scope.Bastion().InstanceType, s.scope.Bastion().AMI, instance)
if err != nil {
record.Warnf(s.scope.InfraCluster(), "FailedFetchingBastion", "Failed to fetch default bastion instance: %v", err)
return err
Expand Down Expand Up @@ -166,7 +167,7 @@ func (s *Service) describeBastionInstance() (*infrav1.Instance, error) {
return nil, awserrors.NewNotFound("bastion host not found")
}

func (s *Service) getDefaultBastion(instanceType, ami string) (*infrav1.Instance, error) {
func (s *Service) getDefaultBastion(instanceType, ami string, instance *infrav1.Instance) (*infrav1.Instance, error) {
name := fmt.Sprintf("%s-bastion", s.scope.Name())
userData, _ := userdata.NewBastion(&userdata.BastionInput{})

Expand All @@ -186,9 +187,10 @@ func (s *Service) getDefaultBastion(instanceType, ami string) (*infrav1.Instance
}
}

region := s.scope.Region()
if ami == "" {
var err error
ami, err = s.defaultBastionAMILookup()
ami, err = s.defaultBastionAMILookup(region)
if err != nil {
return nil, err
}
Expand Down
34 changes: 31 additions & 3 deletions pkg/cloud/services/eks/roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package eks

import (
"fmt"
"sigs.k8s.io/cluster-api-provider-aws/cmd/clusterawsadm/api/bootstrap/v1beta1"
"strings"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
Expand Down Expand Up @@ -52,6 +54,22 @@ func FargateRolePolicies() []string {
}
}

// NodegroupRolePolicies gives the policies required for a nodegroup role.
func NodegroupRolePoliciesAWSUSGov() []string {
return []string{
"arn:aws-us-gov:iam::aws:policy/AmazonEKSWorkerNodePolicy",
"arn:aws-us-gov:iam::aws:policy/AmazonEKS_CNI_Policy", //TODO: Can remove when CAPA supports provisioning of OIDC web identity federation with service account token volume projection
"arn:aws-us-gov:iam::aws:policy/AmazonEC2ContainerRegistryReadOnly",
}
}

// FargateRolePolicies gives the policies required for a fargate role.
func FargateRolePoliciesAWSUSGov() []string {
return []string{
"arn:aws-us-gov:iam::aws:policy/AmazonEKSFargatePodExecutionRolePolicy",
}
}

func (s *Service) reconcileControlPlaneIAMRole() error {
s.scope.Debug("Reconciling EKS Control Plane IAM Role")

Expand Down Expand Up @@ -94,7 +112,7 @@ func (s *Service) reconcileControlPlaneIAMRole() error {
//TODO: check tags and trust relationship to see if they need updating

policies := []*string{
aws.String("arn:aws:iam::aws:policy/AmazonEKSClusterPolicy"),
aws.String("arn:*:iam::aws:policy/AmazonEKSClusterPolicy"),
}
if s.scope.ControlPlane.Spec.RoleAdditionalPolicies != nil {
if !s.scope.AllowAdditionalRoles() && len(*s.scope.ControlPlane.Spec.RoleAdditionalPolicies) > 0 {
Expand Down Expand Up @@ -203,7 +221,12 @@ func (s *NodegroupService) reconcileNodegroupIAMRole() error {
return errors.Wrapf(err, "error ensuring tags and policy document are set on node role")
}

policies := NodegroupRolePolicies()
var policies []string
if strings.Contains(s.scope.ControlPlane.Spec.Region, v1beta1.DefaultPartitionNameUSGov) {
policies = NodegroupRolePoliciesAWSUSGov()
} else {
policies = NodegroupRolePolicies()
}
if len(s.scope.ManagedMachinePool.Spec.RoleAdditionalPolicies) > 0 {
if !s.scope.AllowAdditionalRoles() {
return ErrCannotUseAdditionalRoles
Expand Down Expand Up @@ -319,7 +342,12 @@ func (s *FargateService) reconcileFargateIAMRole() (requeue bool, err error) {
return updatedRole, errors.Wrapf(err, "error ensuring tags and policy document are set on fargate role")
}

policies := FargateRolePolicies()
var policies []string
if strings.Contains(s.scope.ControlPlane.Spec.Region, v1beta1.DefaultPartitionNameUSGov) {
policies = FargateRolePoliciesAWSUSGov()
} else {
policies = FargateRolePolicies()
}
updatedPolicies, err := s.EnsurePoliciesAttached(role, aws.StringSlice(policies))
if err != nil {
return updatedRole, errors.Wrapf(err, "error ensuring policies are attached: %v", policies)
Expand Down
2 changes: 1 addition & 1 deletion pkg/cloud/services/iamauth/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (s *Service) ReconcileIAMAuthenticator(ctx context.Context) error {
return fmt.Errorf("getting aws-iam-authenticator backend: %w", err)
}

roleARN := fmt.Sprintf("arn:aws:iam::%s:role/nodes%s", accountID, iamv1.DefaultNameSuffix)
roleARN := fmt.Sprintf("arn:*:iam::%s:role/nodes%s", accountID, iamv1.DefaultNameSuffix)
nodesRoleMapping := ekscontrolplanev1.RoleMapping{
RoleARN: roleARN,
KubernetesMapping: ekscontrolplanev1.KubernetesMapping{
Expand Down
8 changes: 4 additions & 4 deletions pkg/cloud/services/s3/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,10 @@ func (s *Service) bucketPolicy(bucketName string) (string, error) {
Sid: "control-plane",
Effect: iam.EffectAllow,
Principal: map[iam.PrincipalType]iam.PrincipalID{
iam.PrincipalAWS: []string{fmt.Sprintf("arn:aws:iam::%s:role/%s", *accountID.Account, bucket.ControlPlaneIAMInstanceProfile)},
iam.PrincipalAWS: []string{fmt.Sprintf("arn:*:iam::%s:role/%s", *accountID.Account, bucket.ControlPlaneIAMInstanceProfile)},
},
Action: []string{"s3:GetObject"},
Resource: []string{fmt.Sprintf("arn:aws:s3:::%s/control-plane/*", bucketName)},
Resource: []string{fmt.Sprintf("arn:*:s3:::%s/control-plane/*", bucketName)},
},
}

Expand All @@ -254,10 +254,10 @@ func (s *Service) bucketPolicy(bucketName string) (string, error) {
Sid: iamInstanceProfile,
Effect: iam.EffectAllow,
Principal: map[iam.PrincipalType]iam.PrincipalID{
iam.PrincipalAWS: []string{fmt.Sprintf("arn:aws:iam::%s:role/%s", *accountID.Account, iamInstanceProfile)},
iam.PrincipalAWS: []string{fmt.Sprintf("arn:*:iam::%s:role/%s", *accountID.Account, iamInstanceProfile)},
},
Action: []string{"s3:GetObject"},
Resource: []string{fmt.Sprintf("arn:aws:s3:::%s/node/*", bucketName)},
Resource: []string{fmt.Sprintf("arn:*:s3:::%s/node/*", bucketName)},
})
}

Expand Down

0 comments on commit 9bf07cd

Please sign in to comment.