From 332d7e9a6000596b394f63f0ad85f5d75a21dc23 Mon Sep 17 00:00:00 2001 From: Keerthan Reddy Mala Date: Fri, 25 Aug 2023 15:57:28 -0700 Subject: [PATCH 1/5] Add cluster details to the sts through headers --- pkg/providers/v1/aws.go | 19 ++++++- pkg/providers/v1/aws_utils.go | 26 ++++++++++ pkg/providers/v1/aws_utils_test.go | 83 ++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 pkg/providers/v1/aws_utils_test.go diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index 805314ed6f..1b4f885f18 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -249,6 +249,9 @@ const volumeAttachmentStuck = "VolumeAttachmentStuck" // Indicates that a node has volumes stuck in attaching state and hence it is not fit for scheduling more pods const nodeWithImpairedVolumes = "NodeWithImpairedVolumes" +const SourceKey = "x-amz-source-arn" +const AccountKey = "x-amz-source-account" + const ( // volumeAttachmentConsecutiveErrorLimit is the number of consecutive errors we will ignore when waiting for a volume to attach/detach volumeAttachmentStatusConsecutiveErrorLimit = 10 @@ -1261,11 +1264,25 @@ func init() { var creds *credentials.Credentials if cfg.Global.RoleARN != "" { klog.Infof("Using AWS assumed role %v", cfg.Global.RoleARN) + stsClient := sts.New(sess) + if cfg.Global.KubernetesClusterID != "" { + sourceAcct, sourceArn, err := GetSourceAcctAndArn(cfg.Global.RoleARN, regionName, cfg.Global.KubernetesClusterID) + if err != nil { + return nil, err + } + stsClient.Handlers.Sign.PushFront(func(s *request.Request) { + s.ApplyOptions(request.WithSetRequestHeaders(map[string]string{ + SourceKey: sourceArn, + AccountKey: sourceAcct, + })) + }) + klog.Infof("configuring STS client with extra headers") + } creds = credentials.NewChainCredentials( []credentials.Provider{ &credentials.EnvProvider{}, assumeRoleProvider(&stscreds.AssumeRoleProvider{ - Client: sts.New(sess), + Client: stsClient, RoleARN: cfg.Global.RoleARN, }), }) diff --git a/pkg/providers/v1/aws_utils.go b/pkg/providers/v1/aws_utils.go index bd373d64e5..424a4c37b6 100644 --- a/pkg/providers/v1/aws_utils.go +++ b/pkg/providers/v1/aws_utils.go @@ -17,7 +17,11 @@ limitations under the License. package aws import ( + "errors" + "fmt" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/arn" "k8s.io/apimachinery/pkg/util/sets" ) @@ -43,3 +47,25 @@ func stringSetFromPointers(in []*string) sets.String { } return out } + +// GetSourceAcctAndArn constructs source acct and arn and return them for use +func GetSourceAcctAndArn(roleARN, region, clusterName string) (string, string, error) { + // ARN format (https://docs.aws.amazon.com/IAM/latest/UserGuide/reference-arns.html) + // arn:partition:service:region:account-id:resource-type/resource-id + // IAM format, region is always blank + // arn:aws:iam::account:role/role-name-with-path + if !arn.IsARN(roleARN) { + return "", "", fmt.Errorf("incorrect ARN format for role %s", roleARN) + } + if region == "" { + return "", "", errors.New("region can't be empty") + } + + parsedArn, err := arn.Parse(roleARN) + if err != nil { + return "", "", err + } + + sourceArn := fmt.Sprintf("arn:%s:eks:%s:%s:cluster/%s", parsedArn.Partition, region, parsedArn.AccountID, clusterName) + return parsedArn.AccountID, sourceArn, nil +} diff --git a/pkg/providers/v1/aws_utils_test.go b/pkg/providers/v1/aws_utils_test.go new file mode 100644 index 0000000000..a02dbcd076 --- /dev/null +++ b/pkg/providers/v1/aws_utils_test.go @@ -0,0 +1,83 @@ +/* +Copyright 2014 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 aws + +import "testing" + +func TestGetSourceAcctAndArn(t *testing.T) { + type args struct { + roleARN string + region string + clusterName string + } + tests := []struct { + name string + args args + want string + want1 string + wantErr bool + }{ + { + name: "corect role arn", + args: args{ + roleARN: "arn:aws:iam::123456789876:role/test-cluster", + region: "us-west-2", + clusterName: "test-cluster", + }, + want: "123456789876", + want1: "arn:aws:eks:us-west-2:123456789876:cluster/test-cluster", + wantErr: false, + }, + { + name: "incorect role arn", + args: args{ + roleARN: "arn:aws:iam::123456789876", + region: "us-west-2", + clusterName: "test-cluster", + }, + want: "", + want1: "", + wantErr: true, + }, + { + name: "empty region", + args: args{ + roleARN: "arn:aws:iam::123456789876:role/test-cluster", + region: "", + clusterName: "test-cluster", + }, + want: "", + want1: "", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, got1, err := GetSourceAcctAndArn(tt.args.roleARN, tt.args.region, tt.args.clusterName) + if (err != nil) != tt.wantErr { + t.Errorf("GetSourceAcctAndArn() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("GetSourceAcctAndArn() got = %v, want %v", got, tt.want) + } + if got1 != tt.want1 { + t.Errorf("GetSourceAcctAndArn() got1 = %v, want %v", got1, tt.want1) + } + }) + } +} From 11f07651c8701a3d9cd4f533b14af74a38c3fd8d Mon Sep 17 00:00:00 2001 From: Keerthan Reddy Mala Date: Mon, 28 Aug 2023 10:29:36 -0700 Subject: [PATCH 2/5] make constants internal --- pkg/providers/v1/aws.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index 1b4f885f18..c0719e38a1 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -249,8 +249,8 @@ const volumeAttachmentStuck = "VolumeAttachmentStuck" // Indicates that a node has volumes stuck in attaching state and hence it is not fit for scheduling more pods const nodeWithImpairedVolumes = "NodeWithImpairedVolumes" -const SourceKey = "x-amz-source-arn" -const AccountKey = "x-amz-source-account" +const sourceKey = "x-amz-source-arn" +const accountKey = "x-amz-source-account" const ( // volumeAttachmentConsecutiveErrorLimit is the number of consecutive errors we will ignore when waiting for a volume to attach/detach @@ -1272,8 +1272,8 @@ func init() { } stsClient.Handlers.Sign.PushFront(func(s *request.Request) { s.ApplyOptions(request.WithSetRequestHeaders(map[string]string{ - SourceKey: sourceArn, - AccountKey: sourceAcct, + sourceKey: sourceArn, + accountKey: sourceAcct, })) }) klog.Infof("configuring STS client with extra headers") From edde9c2d486962d850ca703f9449b02604ba0321 Mon Sep 17 00:00:00 2001 From: Keerthan Reddy Mala Date: Wed, 6 Sep 2023 16:52:27 -0700 Subject: [PATCH 3/5] get source arn from the config --- pkg/providers/v1/aws.go | 29 +++++++++++++++---------- pkg/providers/v1/aws_utils.go | 15 +++++-------- pkg/providers/v1/aws_utils_test.go | 35 +++++------------------------- 3 files changed, 28 insertions(+), 51 deletions(-) diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index c0719e38a1..e5894f82a6 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -617,6 +617,9 @@ type CloudConfig struct { // RoleARN is the IAM role to assume when interaction with AWS APIs. RoleARN string + // SourceARN is value which is passed while assuming role using RoleARN and used for + // restricting the access. https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html + SourceARN string // KubernetesClusterTag is the legacy cluster id we'll use to identify our cluster resources KubernetesClusterTag string @@ -1265,19 +1268,21 @@ func init() { if cfg.Global.RoleARN != "" { klog.Infof("Using AWS assumed role %v", cfg.Global.RoleARN) stsClient := sts.New(sess) - if cfg.Global.KubernetesClusterID != "" { - sourceAcct, sourceArn, err := GetSourceAcctAndArn(cfg.Global.RoleARN, regionName, cfg.Global.KubernetesClusterID) - if err != nil { - return nil, err - } - stsClient.Handlers.Sign.PushFront(func(s *request.Request) { - s.ApplyOptions(request.WithSetRequestHeaders(map[string]string{ - sourceKey: sourceArn, - accountKey: sourceAcct, - })) - }) - klog.Infof("configuring STS client with extra headers") + sourceAcct, err := GetSourceAcct(cfg.Global.RoleARN) + if err != nil { + return nil, err + } + reqHeaders := map[string]string{ + accountKey: sourceAcct, } + if cfg.Global.SourceARN != "" { + reqHeaders[sourceKey] = cfg.Global.SourceARN + } + stsClient.Handlers.Sign.PushFront(func(s *request.Request) { + s.ApplyOptions(request.WithSetRequestHeaders(reqHeaders)) + }) + klog.Infof("configuring STS client with extra headers") + creds = credentials.NewChainCredentials( []credentials.Provider{ &credentials.EnvProvider{}, diff --git a/pkg/providers/v1/aws_utils.go b/pkg/providers/v1/aws_utils.go index 424a4c37b6..0b2b83b7ad 100644 --- a/pkg/providers/v1/aws_utils.go +++ b/pkg/providers/v1/aws_utils.go @@ -17,7 +17,6 @@ limitations under the License. package aws import ( - "errors" "fmt" "github.com/aws/aws-sdk-go/aws" @@ -48,24 +47,20 @@ func stringSetFromPointers(in []*string) sets.String { return out } -// GetSourceAcctAndArn constructs source acct and arn and return them for use -func GetSourceAcctAndArn(roleARN, region, clusterName string) (string, string, error) { +// GetSourceAcct constructs source acct and return them for use +func GetSourceAcct(roleARN string) (string, error) { // ARN format (https://docs.aws.amazon.com/IAM/latest/UserGuide/reference-arns.html) // arn:partition:service:region:account-id:resource-type/resource-id // IAM format, region is always blank // arn:aws:iam::account:role/role-name-with-path if !arn.IsARN(roleARN) { - return "", "", fmt.Errorf("incorrect ARN format for role %s", roleARN) - } - if region == "" { - return "", "", errors.New("region can't be empty") + return "", fmt.Errorf("incorrect ARN format for role %s", roleARN) } parsedArn, err := arn.Parse(roleARN) if err != nil { - return "", "", err + return "", err } - sourceArn := fmt.Sprintf("arn:%s:eks:%s:%s:cluster/%s", parsedArn.Partition, region, parsedArn.AccountID, clusterName) - return parsedArn.AccountID, sourceArn, nil + return parsedArn.AccountID, nil } diff --git a/pkg/providers/v1/aws_utils_test.go b/pkg/providers/v1/aws_utils_test.go index a02dbcd076..3a3925b076 100644 --- a/pkg/providers/v1/aws_utils_test.go +++ b/pkg/providers/v1/aws_utils_test.go @@ -20,63 +20,40 @@ import "testing" func TestGetSourceAcctAndArn(t *testing.T) { type args struct { - roleARN string - region string - clusterName string + roleARN string } tests := []struct { name string args args want string - want1 string wantErr bool }{ { name: "corect role arn", args: args{ - roleARN: "arn:aws:iam::123456789876:role/test-cluster", - region: "us-west-2", - clusterName: "test-cluster", + roleARN: "arn:aws:iam::123456789876:role/test-cluster", }, want: "123456789876", - want1: "arn:aws:eks:us-west-2:123456789876:cluster/test-cluster", wantErr: false, }, { name: "incorect role arn", args: args{ - roleARN: "arn:aws:iam::123456789876", - region: "us-west-2", - clusterName: "test-cluster", + roleARN: "arn:aws:iam::123456789876", }, want: "", - want1: "", - wantErr: true, - }, - { - name: "empty region", - args: args{ - roleARN: "arn:aws:iam::123456789876:role/test-cluster", - region: "", - clusterName: "test-cluster", - }, - want: "", - want1: "", wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, got1, err := GetSourceAcctAndArn(tt.args.roleARN, tt.args.region, tt.args.clusterName) + got, err := GetSourceAcct(tt.args.roleARN) if (err != nil) != tt.wantErr { - t.Errorf("GetSourceAcctAndArn() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("GetSourceAcct() error = %v, wantErr %v", err, tt.wantErr) return } if got != tt.want { - t.Errorf("GetSourceAcctAndArn() got = %v, want %v", got, tt.want) - } - if got1 != tt.want1 { - t.Errorf("GetSourceAcctAndArn() got1 = %v, want %v", got1, tt.want1) + t.Errorf("GetSourceAcct() got = %v, want %v", got, tt.want) } }) } From 45f693cf6eff925c1fad5efbb417a24550a5ca96 Mon Sep 17 00:00:00 2001 From: Keerthan Reddy Mala Date: Wed, 6 Sep 2023 19:58:41 -0700 Subject: [PATCH 4/5] ignore warnings while parsing the config --- pkg/providers/v1/aws.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index e5894f82a6..e647c1ccb9 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -1304,7 +1304,7 @@ func readAWSCloudConfig(config io.Reader) (*CloudConfig, error) { var err error if config != nil { - err = gcfg.ReadInto(&cfg, config) + err = gcfg.FatalOnly(gcfg.ReadInto(&cfg, config)) if err != nil { return nil, err } From bc0eede2300a7fc87af36d4a3d0096cb3d443a50 Mon Sep 17 00:00:00 2001 From: Keerthan Reddy Mala Date: Tue, 12 Sep 2023 10:26:59 -0700 Subject: [PATCH 5/5] address review comments --- pkg/providers/v1/aws.go | 47 ++++++++++++++++++------------ pkg/providers/v1/aws_utils.go | 4 +-- pkg/providers/v1/aws_utils_test.go | 6 ++-- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index e647c1ccb9..e3a4b90b6e 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -249,8 +249,8 @@ const volumeAttachmentStuck = "VolumeAttachmentStuck" // Indicates that a node has volumes stuck in attaching state and hence it is not fit for scheduling more pods const nodeWithImpairedVolumes = "NodeWithImpairedVolumes" -const sourceKey = "x-amz-source-arn" -const accountKey = "x-amz-source-account" +const headerSourceArn = "x-amz-source-arn" +const headerSourceAccount = "x-amz-source-account" const ( // volumeAttachmentConsecutiveErrorLimit is the number of consecutive errors we will ignore when waiting for a volume to attach/detach @@ -617,8 +617,10 @@ type CloudConfig struct { // RoleARN is the IAM role to assume when interaction with AWS APIs. RoleARN string - // SourceARN is value which is passed while assuming role using RoleARN and used for - // restricting the access. https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html + // SourceARN is value which is passed while assuming role specified by RoleARN. When a service + // assumes a role in your account, you can include the aws:SourceAccount and aws:SourceArn global + // condition context keys in your role trust policy to limit access to the role to only requests that are generated + // by expected resources. https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html SourceARN string // KubernetesClusterTag is the legacy cluster id we'll use to identify our cluster resources @@ -1266,23 +1268,10 @@ func init() { var creds *credentials.Credentials if cfg.Global.RoleARN != "" { - klog.Infof("Using AWS assumed role %v", cfg.Global.RoleARN) - stsClient := sts.New(sess) - sourceAcct, err := GetSourceAcct(cfg.Global.RoleARN) + stsClient, err := getSTSClient(sess, cfg.Global.RoleARN, cfg.Global.SourceARN) if err != nil { - return nil, err - } - reqHeaders := map[string]string{ - accountKey: sourceAcct, - } - if cfg.Global.SourceARN != "" { - reqHeaders[sourceKey] = cfg.Global.SourceARN + return nil, fmt.Errorf("unable to create sts client, %v", err) } - stsClient.Handlers.Sign.PushFront(func(s *request.Request) { - s.ApplyOptions(request.WithSetRequestHeaders(reqHeaders)) - }) - klog.Infof("configuring STS client with extra headers") - creds = credentials.NewChainCredentials( []credentials.Provider{ &credentials.EnvProvider{}, @@ -1298,6 +1287,26 @@ func init() { }) } +func getSTSClient(sess *session.Session, roleARN, sourceARN string) (*sts.STS, error) { + klog.Infof("Using AWS assumed role %v", roleARN) + stsClient := sts.New(sess) + sourceAcct, err := GetSourceAccount(roleARN) + if err != nil { + return nil, err + } + reqHeaders := map[string]string{ + headerSourceAccount: sourceAcct, + } + if sourceARN != "" { + reqHeaders[headerSourceArn] = sourceARN + } + stsClient.Handlers.Sign.PushFront(func(s *request.Request) { + s.ApplyOptions(request.WithSetRequestHeaders(reqHeaders)) + }) + klog.V(4).Infof("configuring STS client with extra headers, %v", reqHeaders) + return stsClient, nil +} + // readAWSCloudConfig reads an instance of AWSCloudConfig from config reader. func readAWSCloudConfig(config io.Reader) (*CloudConfig, error) { var cfg CloudConfig diff --git a/pkg/providers/v1/aws_utils.go b/pkg/providers/v1/aws_utils.go index 0b2b83b7ad..621731ed1c 100644 --- a/pkg/providers/v1/aws_utils.go +++ b/pkg/providers/v1/aws_utils.go @@ -47,8 +47,8 @@ func stringSetFromPointers(in []*string) sets.String { return out } -// GetSourceAcct constructs source acct and return them for use -func GetSourceAcct(roleARN string) (string, error) { +// GetSourceAccount constructs source acct and return them for use +func GetSourceAccount(roleARN string) (string, error) { // ARN format (https://docs.aws.amazon.com/IAM/latest/UserGuide/reference-arns.html) // arn:partition:service:region:account-id:resource-type/resource-id // IAM format, region is always blank diff --git a/pkg/providers/v1/aws_utils_test.go b/pkg/providers/v1/aws_utils_test.go index 3a3925b076..5dfe9460d6 100644 --- a/pkg/providers/v1/aws_utils_test.go +++ b/pkg/providers/v1/aws_utils_test.go @@ -47,13 +47,13 @@ func TestGetSourceAcctAndArn(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GetSourceAcct(tt.args.roleARN) + got, err := GetSourceAccount(tt.args.roleARN) if (err != nil) != tt.wantErr { - t.Errorf("GetSourceAcct() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("GetSourceAccount() error = %v, wantErr %v", err, tt.wantErr) return } if got != tt.want { - t.Errorf("GetSourceAcct() got = %v, want %v", got, tt.want) + t.Errorf("GetSourceAccount() got = %v, want %v", got, tt.want) } }) }