diff --git a/.github/workflows/update-deps.yaml b/.github/workflows/update-deps.yaml index 6c7ee1553c..266b4aa244 100644 --- a/.github/workflows/update-deps.yaml +++ b/.github/workflows/update-deps.yaml @@ -18,7 +18,7 @@ jobs: pull-requests: write steps: - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 - - uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 + - uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed with: go-version-file: go.mod - name: Update Dependencies diff --git a/cmd/ecr-credential-provider/main.go b/cmd/ecr-credential-provider/main.go index d6b745e572..b26076a086 100644 --- a/cmd/ecr-credential-provider/main.go +++ b/cmd/ecr-credential-provider/main.go @@ -27,10 +27,10 @@ import ( "strings" "time" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/ecr" - "github.com/aws/aws-sdk-go/service/ecrpublic" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/ecr" + "github.com/aws/aws-sdk-go-v2/service/ecrpublic" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,12 +46,12 @@ var ecrPrivateHostPattern = regexp.MustCompile(`^(\d{12})\.dkr\.ecr(\-fips)?\.([ // ECR abstracts the calls we make to aws-sdk for testing purposes type ECR interface { - GetAuthorizationToken(input *ecr.GetAuthorizationTokenInput) (*ecr.GetAuthorizationTokenOutput, error) + GetAuthorizationToken(ctx context.Context, params *ecr.GetAuthorizationTokenInput, optFns ...func(*ecr.Options)) (*ecr.GetAuthorizationTokenOutput, error) } // ECRPublic abstracts the calls we make to aws-sdk for testing purposes type ECRPublic interface { - GetAuthorizationToken(input *ecrpublic.GetAuthorizationTokenInput) (*ecrpublic.GetAuthorizationTokenOutput, error) + GetAuthorizationToken(ctx context.Context, params *ecrpublic.GetAuthorizationTokenInput, optFns ...func(*ecrpublic.Options)) (*ecrpublic.GetAuthorizationTokenOutput, error) } type ecrPlugin struct { @@ -59,35 +59,36 @@ type ecrPlugin struct { ecrPublic ECRPublic } -func defaultECRProvider(region string) (*ecr.ECR, error) { - cfg := aws.Config{} +func defaultECRProvider(ctx context.Context, region string) (ECR, error) { + var cfg aws.Config + var err error if region != "" { klog.Warningf("No region found in the image reference, the default region will be used. Please refer to AWS SDK documentation for configuration purpose.") - cfg.Region = aws.String(region) + cfg, err = config.LoadDefaultConfig(ctx, + config.WithRegion(region), + ) + } else { + cfg, err = config.LoadDefaultConfig(ctx) } - sess, err := session.NewSessionWithOptions(session.Options{ - Config: cfg, - SharedConfigState: session.SharedConfigEnable, - }) + if err != nil { return nil, err } - return ecr.New(sess), nil + return ecr.NewFromConfig(cfg), nil } -func publicECRProvider() (*ecrpublic.ECRPublic, error) { +func publicECRProvider(ctx context.Context) (ECRPublic, error) { // ECR public registries are only in one region and only accessible from regions // in the "aws" partition. - sess, err := session.NewSessionWithOptions(session.Options{ - Config: aws.Config{Region: aws.String(ecrPublicRegion)}, - SharedConfigState: session.SharedConfigEnable, - }) + cfg, err := config.LoadDefaultConfig(ctx, + config.WithRegion(ecrPublicRegion), + ) if err != nil { return nil, err } - return ecrpublic.New(sess), nil + return ecrpublic.NewFromConfig(cfg), nil } type credsData struct { @@ -95,18 +96,18 @@ type credsData struct { expiresAt *time.Time } -func (e *ecrPlugin) getPublicCredsData() (*credsData, error) { +func (e *ecrPlugin) getPublicCredsData(ctx context.Context) (*credsData, error) { klog.Infof("Getting creds for public registry") var err error if e.ecrPublic == nil { - e.ecrPublic, err = publicECRProvider() + e.ecrPublic, err = publicECRProvider(ctx) } if err != nil { return nil, err } - output, err := e.ecrPublic.GetAuthorizationToken(&ecrpublic.GetAuthorizationTokenInput{}) + output, err := e.ecrPublic.GetAuthorizationToken(ctx, &ecrpublic.GetAuthorizationTokenInput{}) if err != nil { return nil, err } @@ -125,18 +126,18 @@ func (e *ecrPlugin) getPublicCredsData() (*credsData, error) { }, nil } -func (e *ecrPlugin) getPrivateCredsData(imageHost string, image string) (*credsData, error) { +func (e *ecrPlugin) getPrivateCredsData(ctx context.Context, imageHost string, image string) (*credsData, error) { klog.Infof("Getting creds for private image %s", image) var err error if e.ecr == nil { region := parseRegionFromECRPrivateHost(imageHost) - e.ecr, err = defaultECRProvider(region) + e.ecr, err = defaultECRProvider(ctx, region) if err != nil { return nil, err } } - output, err := e.ecr.GetAuthorizationToken(&ecr.GetAuthorizationTokenInput{}) + output, err := e.ecr.GetAuthorizationToken(ctx, &ecr.GetAuthorizationTokenInput{}) if err != nil { return nil, err } @@ -162,9 +163,9 @@ func (e *ecrPlugin) GetCredentials(ctx context.Context, image string, args []str } if imageHost == ecrPublicHost { - creds, err = e.getPublicCredsData() + creds, err = e.getPublicCredsData(ctx) } else { - creds, err = e.getPrivateCredsData(imageHost, image) + creds, err = e.getPrivateCredsData(ctx, imageHost, image) } if err != nil { @@ -175,7 +176,7 @@ func (e *ecrPlugin) GetCredentials(ctx context.Context, image string, args []str return nil, errors.New("authorization token in response was nil") } - decodedToken, err := base64.StdEncoding.DecodeString(aws.StringValue(creds.authToken)) + decodedToken, err := base64.StdEncoding.DecodeString(aws.ToString(creds.authToken)) if err != nil { return nil, err } diff --git a/cmd/ecr-credential-provider/main_test.go b/cmd/ecr-credential-provider/main_test.go index 51f4d5b0cb..c2c1f6bd11 100644 --- a/cmd/ecr-credential-provider/main_test.go +++ b/cmd/ecr-credential-provider/main_test.go @@ -24,24 +24,50 @@ import ( "testing" "time" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ecr" - "github.com/aws/aws-sdk-go/service/ecrpublic" - "github.com/golang/mock/gomock" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/ecr" + "github.com/aws/aws-sdk-go-v2/service/ecr/types" + "github.com/aws/aws-sdk-go-v2/service/ecrpublic" + publictypes "github.com/aws/aws-sdk-go-v2/service/ecrpublic/types" + "github.com/stretchr/testify/mock" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/cloud-provider-aws/pkg/mocks" v1 "k8s.io/kubelet/pkg/apis/credentialprovider/v1" ) +type MockedECR struct { + mock.Mock +} + +func (m *MockedECR) GetAuthorizationToken(ctx context.Context, params *ecr.GetAuthorizationTokenInput, optFns ...func(*ecr.Options)) (*ecr.GetAuthorizationTokenOutput, error) { + args := m.Called(ctx, params) + if args.Get(1) != nil { + return args.Get(0).(*ecr.GetAuthorizationTokenOutput), args.Get(1).(error) + } + return args.Get(0).(*ecr.GetAuthorizationTokenOutput), nil +} + +// ECRPublic abstracts the calls we make to aws-sdk for testing purposes +type MockedECRPublic struct { + mock.Mock +} + +func (m *MockedECRPublic) GetAuthorizationToken(ctx context.Context, params *ecrpublic.GetAuthorizationTokenInput, optFns ...func(*ecrpublic.Options)) (*ecrpublic.GetAuthorizationTokenOutput, error) { + args := m.Called(ctx, params) + if args.Get(1) != nil { + return args.Get(0).(*ecrpublic.GetAuthorizationTokenOutput), args.Get(1).(error) + } + return args.Get(0).(*ecrpublic.GetAuthorizationTokenOutput), nil +} + func generatePrivateGetAuthorizationTokenOutput(user string, password string, proxy string, expiration *time.Time) *ecr.GetAuthorizationTokenOutput { creds := []byte(fmt.Sprintf("%s:%s", user, password)) - data := &ecr.AuthorizationData{ + data := types.AuthorizationData{ AuthorizationToken: aws.String(base64.StdEncoding.EncodeToString(creds)), ExpiresAt: expiration, ProxyEndpoint: aws.String(proxy), } output := &ecr.GetAuthorizationTokenOutput{ - AuthorizationData: []*ecr.AuthorizationData{data}, + AuthorizationData: []types.AuthorizationData{data}, } return output } @@ -60,11 +86,6 @@ func generateResponse(registry string, username string, password string) *v1.Cre } func Test_GetCredentials_Private(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - mockECR := mocks.NewMockECR(ctrl) - testcases := []struct { name string image string @@ -109,7 +130,7 @@ func Test_GetCredentials_Private(t *testing.T) { { name: "empty authorization token", image: "123456789123.dkr.ecr.us-west-2.amazonaws.com", - getAuthorizationTokenOutput: &ecr.GetAuthorizationTokenOutput{AuthorizationData: []*ecr.AuthorizationData{{}}}, + getAuthorizationTokenOutput: &ecr.GetAuthorizationTokenOutput{AuthorizationData: []types.AuthorizationData{{}}}, getAuthorizationTokenError: nil, expectedError: errors.New("authorization token in response was nil"), }, @@ -124,19 +145,19 @@ func Test_GetCredentials_Private(t *testing.T) { name: "invalid authorization token", image: "123456789123.dkr.ecr.us-west-2.amazonaws.com", getAuthorizationTokenOutput: &ecr.GetAuthorizationTokenOutput{ - AuthorizationData: []*ecr.AuthorizationData{ - {AuthorizationToken: aws.String(base64.StdEncoding.EncodeToString([]byte(fmt.Sprint("foo"))))}, + AuthorizationData: []types.AuthorizationData{ + {AuthorizationToken: aws.String(base64.StdEncoding.EncodeToString([]byte("foo")))}, }, }, getAuthorizationTokenError: nil, expectedError: errors.New("error parsing username and password from authorization token"), }, } - for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - p := &ecrPlugin{ecr: mockECR} - mockECR.EXPECT().GetAuthorizationToken(gomock.Any()).Return(testcase.getAuthorizationTokenOutput, testcase.getAuthorizationTokenError) + mockECR := MockedECR{} + p := &ecrPlugin{ecr: &mockECR} + mockECR.On("GetAuthorizationToken", mock.Anything, mock.Anything).Return(testcase.getAuthorizationTokenOutput, testcase.getAuthorizationTokenError) creds, err := p.GetCredentials(context.TODO(), testcase.image, testcase.args) @@ -163,7 +184,7 @@ func Test_GetCredentials_Private(t *testing.T) { func generatePublicGetAuthorizationTokenOutput(user string, password string, proxy string, expiration *time.Time) *ecrpublic.GetAuthorizationTokenOutput { creds := []byte(fmt.Sprintf("%s:%s", user, password)) - data := &ecrpublic.AuthorizationData{ + data := &publictypes.AuthorizationData{ AuthorizationToken: aws.String(base64.StdEncoding.EncodeToString(creds)), ExpiresAt: expiration, } @@ -174,11 +195,6 @@ func generatePublicGetAuthorizationTokenOutput(user string, password string, pro } func Test_GetCredentials_Public(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - mockECRPublic := mocks.NewMockECRPublic(ctrl) - testcases := []struct { name string image string @@ -211,7 +227,7 @@ func Test_GetCredentials_Public(t *testing.T) { { name: "empty authorization token", image: "public.ecr.aws", - getAuthorizationTokenOutput: &ecrpublic.GetAuthorizationTokenOutput{AuthorizationData: &ecrpublic.AuthorizationData{}}, + getAuthorizationTokenOutput: &ecrpublic.GetAuthorizationTokenOutput{AuthorizationData: &publictypes.AuthorizationData{}}, getAuthorizationTokenError: nil, expectedError: errors.New("authorization token in response was nil"), }, @@ -226,8 +242,8 @@ func Test_GetCredentials_Public(t *testing.T) { name: "invalid authorization token", image: "public.ecr.aws", getAuthorizationTokenOutput: &ecrpublic.GetAuthorizationTokenOutput{ - AuthorizationData: &ecrpublic.AuthorizationData{ - AuthorizationToken: aws.String(base64.StdEncoding.EncodeToString([]byte(fmt.Sprint("foo")))), + AuthorizationData: &publictypes.AuthorizationData{ + AuthorizationToken: aws.String(base64.StdEncoding.EncodeToString([]byte("foo"))), }, }, getAuthorizationTokenError: nil, @@ -237,8 +253,9 @@ func Test_GetCredentials_Public(t *testing.T) { for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - p := &ecrPlugin{ecrPublic: mockECRPublic} - mockECRPublic.EXPECT().GetAuthorizationToken(gomock.Any()).Return(testcase.getAuthorizationTokenOutput, testcase.getAuthorizationTokenError) + mockECRPublic := MockedECRPublic{} + p := &ecrPlugin{ecrPublic: &mockECRPublic} + mockECRPublic.On("GetAuthorizationToken", mock.Anything, mock.Anything).Return(testcase.getAuthorizationTokenOutput, testcase.getAuthorizationTokenError) creds, err := p.GetCredentials(context.TODO(), testcase.image, testcase.args) diff --git a/docs/prerequisites.md b/docs/prerequisites.md index d7f0422189..2d48c407f6 100644 --- a/docs/prerequisites.md +++ b/docs/prerequisites.md @@ -44,6 +44,7 @@ For the `aws-cloud-controller-manager` to be able to communicate to AWS APIs, yo "ec2:DetachVolume", "ec2:RevokeSecurityGroupIngress", "ec2:DescribeVpcs", + "ec2:DescribeInstanceTopology", "elasticloadbalancing:AddTags", "elasticloadbalancing:AttachLoadBalancerToSubnets", "elasticloadbalancing:ApplySecurityGroupsToLoadBalancer", diff --git a/go.mod b/go.mod index 940eef0987..880ea93de8 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,10 @@ go 1.22.7 require ( github.com/aws/aws-sdk-go v1.55.5 - github.com/golang/mock v1.6.0 + github.com/aws/aws-sdk-go-v2 v1.32.2 + github.com/aws/aws-sdk-go-v2/config v1.28.0 + github.com/aws/aws-sdk-go-v2/service/ecr v1.36.2 + github.com/aws/aws-sdk-go-v2/service/ecrpublic v1.27.2 github.com/spf13/cobra v1.8.1 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.9.0 @@ -27,6 +30,18 @@ require ( github.com/NYTimes/gziphandler v1.1.1 // indirect github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect + github.com/aws/aws-sdk-go-v2/credentials v1.17.41 // indirect + github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.17 // indirect + github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.21 // indirect + github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.21 // indirect + github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 // indirect + github.com/aws/aws-sdk-go-v2/service/ec2 v1.186.0 + github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.0 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.2 // indirect + github.com/aws/aws-sdk-go-v2/service/sso v1.24.2 // indirect + github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.2 // indirect + github.com/aws/aws-sdk-go-v2/service/sts v1.32.2 // indirect + github.com/aws/smithy-go v1.22.0 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect diff --git a/go.sum b/go.sum index b1afd57096..decaa59330 100644 --- a/go.sum +++ b/go.sum @@ -8,6 +8,38 @@ github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3d github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= github.com/aws/aws-sdk-go v1.55.5 h1:KKUZBfBoyqy5d3swXyiC7Q76ic40rYcbqH7qjh59kzU= github.com/aws/aws-sdk-go v1.55.5/go.mod h1:eRwEWoyTWFMVYVQzKMNHWP5/RV4xIUGMQfXQHfHkpNU= +github.com/aws/aws-sdk-go-v2 v1.32.2 h1:AkNLZEyYMLnx/Q/mSKkcMqwNFXMAvFto9bNsHqcTduI= +github.com/aws/aws-sdk-go-v2 v1.32.2/go.mod h1:2SK5n0a2karNTv5tbP1SjsX0uhttou00v/HpXKM1ZUo= +github.com/aws/aws-sdk-go-v2/config v1.28.0 h1:FosVYWcqEtWNxHn8gB/Vs6jOlNwSoyOCA/g/sxyySOQ= +github.com/aws/aws-sdk-go-v2/config v1.28.0/go.mod h1:pYhbtvg1siOOg8h5an77rXle9tVG8T+BWLWAo7cOukc= +github.com/aws/aws-sdk-go-v2/credentials v1.17.41 h1:7gXo+Axmp+R4Z+AK8YFQO0ZV3L0gizGINCOWxSLY9W8= +github.com/aws/aws-sdk-go-v2/credentials v1.17.41/go.mod h1:u4Eb8d3394YLubphT4jLEwN1rLNq2wFOlT6OuxFwPzU= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.17 h1:TMH3f/SCAWdNtXXVPPu5D6wrr4G5hI1rAxbcocKfC7Q= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.17/go.mod h1:1ZRXLdTpzdJb9fwTMXiLipENRxkGMTn1sfKexGllQCw= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.21 h1:UAsR3xA31QGf79WzpG/ixT9FZvQlh5HY1NRqSHBNOCk= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.21/go.mod h1:JNr43NFf5L9YaG3eKTm7HQzls9J+A9YYcGI5Quh1r2Y= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.21 h1:6jZVETqmYCadGFvrYEQfC5fAQmlo80CeL5psbno6r0s= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.21/go.mod h1:1SR0GbLlnN3QUmYaflZNiH1ql+1qrSiB2vwcJ+4UM60= +github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 h1:VaRN3TlFdd6KxX1x3ILT5ynH6HvKgqdiXoTxAF4HQcQ= +github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1/go.mod h1:FbtygfRFze9usAadmnGJNc8KsP346kEe+y2/oyhGAGc= +github.com/aws/aws-sdk-go-v2/service/ec2 v1.186.0 h1:n2l2WeV+lEABrGwG/4MsE0WFEbd3j7yKsmZzbnEm5CY= +github.com/aws/aws-sdk-go-v2/service/ec2 v1.186.0/go.mod h1:kYXaB4FzyhEJjvrJ84oPnMElLiEAjGxxUunVW2tBSng= +github.com/aws/aws-sdk-go-v2/service/ecr v1.36.2 h1:VDQaVwGOokbd3VUbHF+wupiffdrbAZPdQnr5XZMJqrs= +github.com/aws/aws-sdk-go-v2/service/ecr v1.36.2/go.mod h1:lvUlMghKYmSxSfv0vU7pdU/8jSY+s0zpG8xXhaGKCw0= +github.com/aws/aws-sdk-go-v2/service/ecrpublic v1.27.2 h1:Zru9Iy2JPM5+uRnFnoqeOZzi8JIVIHJ0ua6JdeDHcyg= +github.com/aws/aws-sdk-go-v2/service/ecrpublic v1.27.2/go.mod h1:PtQC3XjutCYFCn1+i8+wtpDaXvEK+vXF2gyLIKAmh4A= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.0 h1:TToQNkvGguu209puTojY/ozlqy2d/SFNcoLIqTFi42g= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.0/go.mod h1:0jp+ltwkf+SwG2fm/PKo8t4y8pJSgOCO4D8Lz3k0aHQ= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.2 h1:s7NA1SOw8q/5c0wr8477yOPp0z+uBaXBnLE0XYb0POA= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.2/go.mod h1:fnjjWyAW/Pj5HYOxl9LJqWtEwS7W2qgcRLWP+uWbss0= +github.com/aws/aws-sdk-go-v2/service/sso v1.24.2 h1:bSYXVyUzoTHoKalBmwaZxs97HU9DWWI3ehHSAMa7xOk= +github.com/aws/aws-sdk-go-v2/service/sso v1.24.2/go.mod h1:skMqY7JElusiOUjMJMOv1jJsP7YUg7DrhgqZZWuzu1U= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.2 h1:AhmO1fHINP9vFYUE0LHzCWg/LfUWUF+zFPEcY9QXb7o= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.2/go.mod h1:o8aQygT2+MVP0NaV6kbdE1YnnIM8RRVQzoeUH45GOdI= +github.com/aws/aws-sdk-go-v2/service/sts v1.32.2 h1:CiS7i0+FUe+/YY1GvIBLLrR/XNGZ4CtM1Ll0XavNuVo= +github.com/aws/aws-sdk-go-v2/service/sts v1.32.2/go.mod h1:HtaiBI8CjYoNVde8arShXb94UbQQi9L4EMr6D+xGBwo= +github.com/aws/smithy-go v1.22.0 h1:uunKnWlcoL3zO7q+gG2Pk53joueEOsnNB28QdMsmiMM= +github.com/aws/smithy-go v1.22.0/go.mod h1:irrKGvNn1InZwb2d7fkIRNucdfwR8R+Ts3wxYa/cJHg= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= @@ -61,8 +93,6 @@ github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOW github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da h1:oI5xCqsCo564l8iNU+DwB5epxmsaqB+rhGL0m5jtYqE= github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= -github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= -github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/btree v1.0.1 h1:gK4Kx5IaGY9CD5sPJ36FHiBJ6ZXl0kilRiiCj+jdYp4= @@ -174,7 +204,6 @@ github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 h1:eY9dn8+vbi4tKz5 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= go.etcd.io/bbolt v1.3.9 h1:8x7aARPEXiXbHmtUwAIv7eV2fQFHrLLavdiJ3uzJXoI= go.etcd.io/bbolt v1.3.9/go.mod h1:zaO32+Ti0PK1ivdPtgMESzuzL2VPoIG1PCQNvOdo/dE= @@ -222,7 +251,6 @@ golang.org/x/exp v0.0.0-20230515195305-f3d0a9c9a5cc h1:mCRnTeVUjcrhlRmO0VK8a6k6R golang.org/x/exp v0.0.0-20230515195305-f3d0a9c9a5cc/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.17.0 h1:zY54UmvipHiNd+pm+m0x9KhZ9hl1/7QNMyxXbc6ICqA= @@ -230,7 +258,6 @@ golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= @@ -241,16 +268,12 @@ golang.org/x/oauth2 v0.21.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbht golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M= golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -260,7 +283,6 @@ golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws= golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= @@ -281,7 +303,6 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= -golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d h1:vU5i/LfpvrRCpgM/VPfJLg5KjxD3E+hfT1SH+d9zLwg= diff --git a/pkg/mocks/mock_ecr.go b/pkg/mocks/mock_ecr.go deleted file mode 100644 index 8a5b7445ab..0000000000 --- a/pkg/mocks/mock_ecr.go +++ /dev/null @@ -1,89 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: cmd/ecr-credential-provider/main.go - -// Package mocks is a generated GoMock package. -package mocks - -import ( - reflect "reflect" - - ecr "github.com/aws/aws-sdk-go/service/ecr" - ecrpublic "github.com/aws/aws-sdk-go/service/ecrpublic" - gomock "github.com/golang/mock/gomock" -) - -// MockECR is a mock of ECR interface. -type MockECR struct { - ctrl *gomock.Controller - recorder *MockECRMockRecorder -} - -// MockECRMockRecorder is the mock recorder for MockECR. -type MockECRMockRecorder struct { - mock *MockECR -} - -// NewMockECR creates a new mock instance. -func NewMockECR(ctrl *gomock.Controller) *MockECR { - mock := &MockECR{ctrl: ctrl} - mock.recorder = &MockECRMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockECR) EXPECT() *MockECRMockRecorder { - return m.recorder -} - -// GetAuthorizationToken mocks base method. -func (m *MockECR) GetAuthorizationToken(input *ecr.GetAuthorizationTokenInput) (*ecr.GetAuthorizationTokenOutput, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetAuthorizationToken", input) - ret0, _ := ret[0].(*ecr.GetAuthorizationTokenOutput) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetAuthorizationToken indicates an expected call of GetAuthorizationToken. -func (mr *MockECRMockRecorder) GetAuthorizationToken(input interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAuthorizationToken", reflect.TypeOf((*MockECR)(nil).GetAuthorizationToken), input) -} - -// MockECRPublic is a mock of ECRPublic interface. -type MockECRPublic struct { - ctrl *gomock.Controller - recorder *MockECRPublicMockRecorder -} - -// MockECRPublicMockRecorder is the mock recorder for MockECRPublic. -type MockECRPublicMockRecorder struct { - mock *MockECRPublic -} - -// NewMockECRPublic creates a new mock instance. -func NewMockECRPublic(ctrl *gomock.Controller) *MockECRPublic { - mock := &MockECRPublic{ctrl: ctrl} - mock.recorder = &MockECRPublicMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockECRPublic) EXPECT() *MockECRPublicMockRecorder { - return m.recorder -} - -// GetAuthorizationToken mocks base method. -func (m *MockECRPublic) GetAuthorizationToken(input *ecrpublic.GetAuthorizationTokenInput) (*ecrpublic.GetAuthorizationTokenOutput, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetAuthorizationToken", input) - ret0, _ := ret[0].(*ecrpublic.GetAuthorizationTokenOutput) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetAuthorizationToken indicates an expected call of GetAuthorizationToken. -func (mr *MockECRPublicMockRecorder) GetAuthorizationToken(input interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAuthorizationToken", reflect.TypeOf((*MockECRPublic)(nil).GetAuthorizationToken), input) -} diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index 8977e3bf7e..7a37f35d81 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -61,6 +61,8 @@ import ( "k8s.io/cloud-provider-aws/pkg/providers/v1/iface" "k8s.io/cloud-provider-aws/pkg/providers/v1/variant" _ "k8s.io/cloud-provider-aws/pkg/providers/v1/variant/fargate" // ensure the fargate variant gets registered + "k8s.io/cloud-provider-aws/pkg/resourcemanagers" + "k8s.io/cloud-provider-aws/pkg/services" ) // NLBHealthCheckRuleDescription is the comment used on a security group rule to @@ -389,8 +391,9 @@ type Cloud struct { // Note that we cache some state in awsInstance (mountpoints), so we must preserve the instance selfAWSInstance *awsInstance - instanceCache instanceCache - zoneCache zoneCache + instanceCache instanceCache + zoneCache zoneCache + instanceTopologyManager resourcemanagers.InstanceTopologyManager clientBuilder cloudprovider.ControllerClientBuilder kubeClient clientset.Interface @@ -586,6 +589,11 @@ func newAWSCloud2(cfg config.CloudConfig, awsServices Services, provider config. return nil, fmt.Errorf("error creating AWS EC2 client: %v", err) } + ec2v2, err := services.NewEc2SdkV2(regionName) + if err != nil { + return nil, fmt.Errorf("error creating AWS EC2v2 client: %v", err) + } + elb, err := awsServices.LoadBalancing(regionName) if err != nil { return nil, fmt.Errorf("error creating AWS ELB client: %v", err) @@ -618,6 +626,7 @@ func newAWSCloud2(cfg config.CloudConfig, awsServices Services, provider config. } awsCloud.instanceCache.cloud = awsCloud awsCloud.zoneCache.cloud = awsCloud + awsCloud.instanceTopologyManager = resourcemanagers.NewInstanceTopologyManager(ec2v2) tagged := cfg.Global.KubernetesClusterTag != "" || cfg.Global.KubernetesClusterID != "" if cfg.Global.VPC != "" && (cfg.Global.SubnetID != "" || cfg.Global.RoleARN != "") && tagged { diff --git a/pkg/providers/v1/iface/types.go b/pkg/providers/v1/iface/types.go index d70d13e235..451ffecc36 100644 --- a/pkg/providers/v1/iface/types.go +++ b/pkg/providers/v1/iface/types.go @@ -1,6 +1,8 @@ package iface -import "github.com/aws/aws-sdk-go/service/ec2" +import ( + "github.com/aws/aws-sdk-go/service/ec2" +) // EC2 is an abstraction over AWS', to allow mocking/other implementations // Note that the DescribeX functions return a list, so callers don't need to deal with paging diff --git a/pkg/providers/v1/instances_v2.go b/pkg/providers/v1/instances_v2.go index a08f4dd45d..f7a08a2d16 100644 --- a/pkg/providers/v1/instances_v2.go +++ b/pkg/providers/v1/instances_v2.go @@ -22,6 +22,7 @@ package aws import ( "context" + "strconv" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -63,16 +64,34 @@ func (c *Cloud) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, erro return c.InstanceShutdownByProviderID(ctx, providerID) } -func (c *Cloud) getAdditionalLabels(zoneName string) (map[string]string, error) { +func (c *Cloud) getAdditionalLabels(ctx context.Context, zoneName string, instanceID string, instanceType string, + region string, existingLabels map[string]string) (map[string]string, error) { additionalLabels := map[string]string{} - // Add the zone ID to the additional labels - zoneID, err := c.zoneCache.getZoneIDByZoneName(zoneName) - if err != nil { - return nil, err + // If zone ID label is already set, skip. + if _, ok := existingLabels[LabelZoneID]; !ok { + // Add the zone ID to the additional labels + zoneID, err := c.zoneCache.getZoneIDByZoneName(zoneName) + if err != nil { + return nil, err + } + + additionalLabels[LabelZoneID] = zoneID } - additionalLabels[LabelZoneID] = zoneID + // If topology labels are already set, skip. + if _, ok := existingLabels[LabelNetworkNodePrefix+"1"]; !ok { + nodeTopology, err := c.instanceTopologyManager.GetNodeTopology(ctx, instanceType, region, instanceID) + if err != nil { + return nil, err + } else if nodeTopology != nil { + for index, networkNode := range nodeTopology.NetworkNodes { + layer := index + 1 + label := LabelNetworkNodePrefix + strconv.Itoa(layer) + additionalLabels[label] = networkNode + } + } + } return additionalLabels, nil } @@ -103,7 +122,12 @@ func (c *Cloud) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprov return nil, err } - additionalLabels, err := c.getAdditionalLabels(zone.FailureDomain) + instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID() + if err != nil { + return nil, err + } + + additionalLabels, err := c.getAdditionalLabels(ctx, zone.FailureDomain, string(instanceID), instanceType, zone.Region, node.Labels) if err != nil { return nil, err } diff --git a/pkg/providers/v1/instances_v2_test.go b/pkg/providers/v1/instances_v2_test.go index fb52438178..ecbf1b8bbd 100644 --- a/pkg/providers/v1/instances_v2_test.go +++ b/pkg/providers/v1/instances_v2_test.go @@ -19,13 +19,17 @@ package aws import ( "context" "fmt" + "testing" + + awsv2 "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/ec2/types" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" v1 "k8s.io/api/core/v1" - "testing" + "k8s.io/cloud-provider-aws/pkg/resourcemanagers" ) func TestGetProviderId(t *testing.T) { @@ -164,6 +168,16 @@ func TestInstanceMetadata(t *testing.T) { t.Run("Should return populated InstanceMetadata", func(t *testing.T) { instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true) c, _ := mockInstancesResp(&instance, []*ec2.Instance{&instance}) + var mockedTopologyManager resourcemanagers.MockedInstanceTopologyManager + c.instanceTopologyManager = &mockedTopologyManager + mockedTopologyManager.On("GetNodeTopology", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&types.InstanceTopology{ + AvailabilityZone: awsv2.String("us-west-2b"), + GroupName: new(string), + InstanceId: awsv2.String("i-123456789"), + InstanceType: new(string), + NetworkNodes: []string{"nn-123456789", "nn-234567890", "nn-345678901"}, + ZoneId: awsv2.String("az2"), + }, nil) node := &v1.Node{ Spec: v1.NodeSpec{ ProviderID: fmt.Sprintf("aws:///us-west-2c/1abc-2def/%s", *instance.InstanceId), @@ -175,6 +189,7 @@ func TestInstanceMetadata(t *testing.T) { t.Errorf("Should not error getting InstanceMetadata: %s", err) } + mockedTopologyManager.AssertNumberOfCalls(t, "GetNodeTopology", 1) assert.Equal(t, "aws:///us-west-2c/1abc-2def/i-00000000000000000", result.ProviderID) assert.Equal(t, "c3.large", result.InstanceType) assert.Equal(t, []v1.NodeAddress{ @@ -187,9 +202,40 @@ func TestInstanceMetadata(t *testing.T) { assert.Equal(t, "us-west-2a", result.Zone) assert.Equal(t, "us-west-2", result.Region) assert.Equal(t, map[string]string{ - LabelZoneID: "az1", + LabelZoneID: "az1", + LabelNetworkNodePrefix + "1": "nn-123456789", + LabelNetworkNodePrefix + "2": "nn-234567890", + LabelNetworkNodePrefix + "3": "nn-345678901", }, result.AdditionalLabels) }) + + t.Run("Should skip additional labels if already set", func(t *testing.T) { + instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true) + c, _ := mockInstancesResp(&instance, []*ec2.Instance{&instance}) + var mockedTopologyManager resourcemanagers.MockedInstanceTopologyManager + c.instanceTopologyManager = &mockedTopologyManager + node := &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: fmt.Sprintf("aws:///us-west-2c/1abc-2def/%s", *instance.InstanceId), + }, + } + // Set labels to skip attempts to update them + node.Labels = map[string]string{ + LabelZoneID: "az1", + LabelNetworkNodePrefix + "1": "nn-123456789", + LabelNetworkNodePrefix + "2": "nn-234567890", + LabelNetworkNodePrefix + "3": "nn-345678901", + } + + result, err := c.InstanceMetadata(context.TODO(), node) + if err != nil { + t.Errorf("Should not error getting InstanceMetadata: %s", err) + } + + mockedTopologyManager.AssertNumberOfCalls(t, "GetNodeTopology", 0) + // Validate that labels are unchanged. + assert.Equal(t, map[string]string{}, result.AdditionalLabels) + }) } func getCloudWithMockedDescribeInstances(instanceExists bool, instanceState string) *Cloud { diff --git a/pkg/providers/v1/well_known_labels.go b/pkg/providers/v1/well_known_labels.go index b567e91666..be9179c957 100644 --- a/pkg/providers/v1/well_known_labels.go +++ b/pkg/providers/v1/well_known_labels.go @@ -20,4 +20,8 @@ const ( // LabelZoneID is a topology label that can be applied to any resource // but will be initially applied to nodes. LabelZoneID = "topology.k8s.aws/zone-id" + // LabelNetworkNodePrefix is a topology label that can be applied to any resource, + // but will be initially applied to nodes. The suffix should be an incremented + // integer starting at 1. + LabelNetworkNodePrefix = "topology.k8s.aws/network-node-layer-" ) diff --git a/pkg/resourcemanagers/topology.go b/pkg/resourcemanagers/topology.go new file mode 100644 index 0000000000..ea99dbefd2 --- /dev/null +++ b/pkg/resourcemanagers/topology.go @@ -0,0 +1,128 @@ +/* +Copyright 2024 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 resourcemanagers + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/aws/aws-sdk-go-v2/service/ec2" + "github.com/aws/aws-sdk-go-v2/service/ec2/types" + "github.com/aws/smithy-go" + "k8s.io/client-go/tools/cache" + "k8s.io/cloud-provider-aws/pkg/services" + "k8s.io/klog/v2" +) + +const instanceTopologyManagerCacheTimeout = 24 * time.Hour + +// stringKeyFunc is a string as cache key function +func topStringKeyFunc(obj interface{}) (string, error) { + // Type should already be a string, so just return as is. + s, ok := obj.(string) + if !ok { + return "", fmt.Errorf("failed to cast to string: %+v", obj) + } + + return s, nil +} + +// InstanceTopologyManager enables mocking the InstanceTopologyManager. +type InstanceTopologyManager interface { + GetNodeTopology(ctx context.Context, instanceType string, region string, instanceID string) (*types.InstanceTopology, error) +} + +// instanceTopologyManager manages getting instance topology for nodes. +type instanceTopologyManager struct { + ec2 services.Ec2SdkV2 + unsupportedKeyStore cache.Store +} + +// NewInstanceTopologyManager generates a new InstanceTopologyManager. +func NewInstanceTopologyManager(ec2 services.Ec2SdkV2) InstanceTopologyManager { + return &instanceTopologyManager{ + ec2: ec2, + // These should change very infrequently, if ever, so checking once a day sounds fair. + unsupportedKeyStore: cache.NewTTLStore(topStringKeyFunc, instanceTopologyManagerCacheTimeout), + } +} + +// GetNodeTopology gets the instance topology for a node. +func (t *instanceTopologyManager) GetNodeTopology(ctx context.Context, instanceType string, region string, instanceID string) (*types.InstanceTopology, error) { + if t.mightSupportTopology(instanceType, region) { + request := &ec2.DescribeInstanceTopologyInput{InstanceIds: []string{instanceID}} + topologies, err := t.ec2.DescribeInstanceTopology(ctx, request) + if err != nil { + var apiErr smithy.APIError + if errors.As(err, &apiErr) { + code := apiErr.ErrorCode() + switch code { + case "UnsupportedOperation": + klog.Infof("ec2:DescribeInstanceTopology is not available in %s: %q", region, err) + // If region is unsupported, track it to avoid making the call in the future. + t.addUnsupported(region) + return nil, nil + case "UnauthorizedOperation": + // Gracefully handle the DecribeInstanceTopology access missing error + klog.Warningf("Not authorized to perform: ec2:DescribeInstanceTopology, permission missing: %q", err) + return nil, nil + case "RequestLimitExceeded": + // Gracefully handle request throttling + klog.Warningf("Exceeded ec2:DescribeInstanceTopology request limits. Try again later: %q", err) + return nil, nil + } + } + + // Unhandled error + klog.Errorf("Error describing instance topology: %q", err) + return nil, err + } else if len(topologies) == 0 { + // If no topology is returned, track the instance type as unsupported + klog.Infof("Instance type %s unsupported for getting instance topology", instanceType) + t.addUnsupported(instanceType) + return nil, nil + } + + return &topologies[0], nil + } + return nil, nil +} + +func (t *instanceTopologyManager) addUnsupported(key string) { + err := t.unsupportedKeyStore.Add(key) + if err != nil { + klog.Errorf("Failed to cache unsupported key %s: %q", key, err) + } +} + +func (t *instanceTopologyManager) mightSupportTopology(instanceType string, region string) bool { + if _, exists, err := t.unsupportedKeyStore.GetByKey(region); exists { + return false + } else if err != nil { + klog.Errorf("Failed to get cached unsupported region: %q:", err) + } + + if _, exists, err := t.unsupportedKeyStore.GetByKey(instanceType); exists { + return false + } else if err != nil { + klog.Errorf("Failed to get cached unsupported instance type: %q:", err) + } + + return true +} diff --git a/pkg/resourcemanagers/topology_mock.go b/pkg/resourcemanagers/topology_mock.go new file mode 100644 index 0000000000..ed5835e5c7 --- /dev/null +++ b/pkg/resourcemanagers/topology_mock.go @@ -0,0 +1,39 @@ +/* +Copyright 2024 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 resourcemanagers + +import ( + "context" + + "github.com/aws/aws-sdk-go-v2/service/ec2/types" + "github.com/stretchr/testify/mock" +) + +// MockedInstanceTopologyManager creates an InstanceTopologyManager mock. +type MockedInstanceTopologyManager struct { + InstanceTopologyManager + mock.Mock +} + +// GetNodeTopology mocks InstanceTopologyManager.GetNodeTopology. +func (m *MockedInstanceTopologyManager) GetNodeTopology(ctx context.Context, instanceType string, region string, instanceID string) (*types.InstanceTopology, error) { + args := m.Called(ctx, instanceType, region, instanceID) + if args.Get(1) != nil { + return nil, args.Get(1).(error) + } + return args.Get(0).(*types.InstanceTopology), nil +} diff --git a/pkg/resourcemanagers/topology_test.go b/pkg/resourcemanagers/topology_test.go new file mode 100644 index 0000000000..1eae0304ad --- /dev/null +++ b/pkg/resourcemanagers/topology_test.go @@ -0,0 +1,126 @@ +/* +Copyright 2024 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 resourcemanagers + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go-v2/service/ec2/types" + "github.com/stretchr/testify/mock" + "k8s.io/cloud-provider-aws/pkg/services" +) + +func TestGetNodeTopology(t *testing.T) { + t.Run("Should handle unsupported regions and utilize cache", func(t *testing.T) { + mockedEc2SdkV2 := services.MockedEc2SdkV2{} + topologyManager := NewInstanceTopologyManager(&mockedEc2SdkV2) + + mockedEc2SdkV2.On("DescribeInstanceTopology", mock.Anything, mock.Anything).Return(nil, + services.NewMockAPIError("UnsupportedOperation", "Not supported in region")) + + // Loop multiple times to check cache use + for i := 0; i < 2; i++ { + topology, err := topologyManager.GetNodeTopology(context.TODO(), "some-type", "some-region", "some-id") + if err != nil { + t.Errorf("Should not error getting node topology: %s", err) + } + if topology != nil { + t.Errorf("Should not be returning a topology: %v", topology) + } + } + + mockedEc2SdkV2.AssertNumberOfCalls(t, "DescribeInstanceTopology", 1) + }) + + t.Run("Should handle unsupported instance types and utilize cache", func(t *testing.T) { + mockedEc2SdkV2 := services.MockedEc2SdkV2{} + topologyManager := NewInstanceTopologyManager(&mockedEc2SdkV2) + + mockedEc2SdkV2.On("DescribeInstanceTopology", mock.Anything, mock.Anything).Return([]types.InstanceTopology{}, nil) + + // Loop multiple times to check cache use + for i := 0; i < 2; i++ { + topology, err := topologyManager.GetNodeTopology(context.TODO(), "some-type", "some-region", "some-id") + if err != nil { + t.Errorf("Should not error getting node topology: %s", err) + } + if topology != nil { + t.Errorf("Should not be returning a topology: %v", topology) + } + } + + mockedEc2SdkV2.AssertNumberOfCalls(t, "DescribeInstanceTopology", 1) + }) + + t.Run("Should handle missing permissions to call DescribeInstanceTopology", func(t *testing.T) { + mockedEc2SdkV2 := services.MockedEc2SdkV2{} + topologyManager := NewInstanceTopologyManager(&mockedEc2SdkV2) + + mockedEc2SdkV2.On("DescribeInstanceTopology", mock.Anything, mock.Anything).Return(nil, + services.NewMockAPIError("UnauthorizedOperation", "Update your perms")) + + // Loop multiple times to check cache use + for i := 0; i < 2; i++ { + topology, err := topologyManager.GetNodeTopology(context.TODO(), "some-type", "some-region", "some-id") + if err != nil { + t.Errorf("Should not error getting node topology: %s", err) + } + if topology != nil { + t.Errorf("Should not be returning a topology: %v", topology) + } + } + + mockedEc2SdkV2.AssertNumberOfCalls(t, "DescribeInstanceTopology", 2) + }) + + t.Run("Should handle exceeding request limits for DescribeInstanceTopology", func(t *testing.T) { + mockedEc2SdkV2 := services.MockedEc2SdkV2{} + topologyManager := NewInstanceTopologyManager(&mockedEc2SdkV2) + + mockedEc2SdkV2.On("DescribeInstanceTopology", mock.Anything, mock.Anything).Return(nil, + services.NewMockAPIError("RequestLimitExceeded", "Slow down!")) + + // Loop multiple times to check cache use + for i := 0; i < 2; i++ { + topology, err := topologyManager.GetNodeTopology(context.TODO(), "some-type", "some-region", "some-id") + if err != nil { + t.Errorf("Should not error getting node topology: %s", err) + } + if topology != nil { + t.Errorf("Should not be returning a topology: %v", topology) + } + } + + mockedEc2SdkV2.AssertNumberOfCalls(t, "DescribeInstanceTopology", 2) + }) + + t.Run("Should return unhandled errors", func(t *testing.T) { + mockedEc2SdkV2 := services.MockedEc2SdkV2{} + topologyManager := NewInstanceTopologyManager(&mockedEc2SdkV2) + + mockedEc2SdkV2.On("DescribeInstanceTopology", mock.Anything, mock.Anything).Return(nil, + services.NewMockAPIError("NOPE", "Nice try.")) + + _, err := topologyManager.GetNodeTopology(context.TODO(), "some-type", "some-region", "some-id") + if err == nil { + t.Errorf("Should have gotten an error") + } + + mockedEc2SdkV2.AssertNumberOfCalls(t, "DescribeInstanceTopology", 1) + }) +} diff --git a/pkg/services/aws_ec2.go b/pkg/services/aws_ec2.go new file mode 100644 index 0000000000..5d0d2981c9 --- /dev/null +++ b/pkg/services/aws_ec2.go @@ -0,0 +1,70 @@ +/* +Copyright 2024 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 services + +import ( + "context" + + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/ec2" + "github.com/aws/aws-sdk-go-v2/service/ec2/types" +) + +// EC2ClientV2 is an interface to allow it to be mocked. +type EC2ClientV2 interface { + DescribeInstanceTopology(ctx context.Context, params *ec2.DescribeInstanceTopologyInput, optFns ...func(*ec2.Options)) (*ec2.DescribeInstanceTopologyOutput, error) +} + +// Ec2SdkV2 is an implementation of the EC2 v2 interface, backed by aws-sdk-go-v2 +type Ec2SdkV2 interface { + DescribeInstanceTopology(ctx context.Context, request *ec2.DescribeInstanceTopologyInput) ([]types.InstanceTopology, error) +} + +// ec2SdkV2 is an implementation of the EC2 v2 interface, backed by aws-sdk-go-v2 +type ec2SdkV2 struct { + Ec2 EC2ClientV2 +} + +// NewEc2SdkV2 is a constructor for Ec2SdkV2 that creates a default EC2 client. +func NewEc2SdkV2(region string) (Ec2SdkV2, error) { + cfg, err := config.LoadDefaultConfig(context.TODO()) + if err != nil { + return nil, err + } + + client := ec2.NewFromConfig(cfg, func(o *ec2.Options) { + o.Region = region + }) + + return &ec2SdkV2{Ec2: client}, nil +} + +// DescribeInstanceTopology paginates calls to EC2 DescribeInstanceTopology API. +func (s *ec2SdkV2) DescribeInstanceTopology(ctx context.Context, request *ec2.DescribeInstanceTopologyInput) ([]types.InstanceTopology, error) { + var topologies []types.InstanceTopology + + paginator := ec2.NewDescribeInstanceTopologyPaginator(s.Ec2, request) + for paginator.HasMorePages() { + output, err := paginator.NextPage(ctx) + if err != nil { + return nil, err + } + topologies = append(topologies, output.Instances...) + } + + return topologies, nil +} diff --git a/pkg/services/aws_ec2_mock.go b/pkg/services/aws_ec2_mock.go new file mode 100644 index 0000000000..a037561cf9 --- /dev/null +++ b/pkg/services/aws_ec2_mock.go @@ -0,0 +1,55 @@ +/* +Copyright 2024 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 services + +import ( + "context" + + "github.com/aws/aws-sdk-go-v2/service/ec2" + "github.com/aws/aws-sdk-go-v2/service/ec2/types" + "github.com/stretchr/testify/mock" +) + +// MockedEC2ClientV2 mocks EC2ClientV2. +type MockedEC2ClientV2 struct { + EC2ClientV2 + mock.Mock +} + +// DescribeInstanceTopology mocks EC2ClientV2.DescribeInstanceTopology. +func (m *MockedEC2ClientV2) DescribeInstanceTopology(ctx context.Context, params *ec2.DescribeInstanceTopologyInput, optFns ...func(*ec2.Options)) (*ec2.DescribeInstanceTopologyOutput, error) { + args := m.Called(ctx, params) + if args.Get(1) != nil { + return nil, args.Get(1).(error) + } + return args.Get(0).(*ec2.DescribeInstanceTopologyOutput), nil +} + +// MockedEc2SdkV2 is an implementation of the EC2 v2 interface, backed by aws-sdk-go-v2 +type MockedEc2SdkV2 struct { + Ec2SdkV2 + mock.Mock +} + +// DescribeInstanceTopology mocks EC2ClientV2.DescribeInstanceTopology. +func (m *MockedEc2SdkV2) DescribeInstanceTopology(ctx context.Context, request *ec2.DescribeInstanceTopologyInput) ([]types.InstanceTopology, error) { + args := m.Called(ctx, request) + if args.Get(1) != nil { + return nil, args.Get(1).(error) + } + return args.Get(0).([]types.InstanceTopology), nil +} diff --git a/pkg/services/errors_mock.go b/pkg/services/errors_mock.go new file mode 100644 index 0000000000..ac680b9bca --- /dev/null +++ b/pkg/services/errors_mock.go @@ -0,0 +1,35 @@ +package services + +import ( + "github.com/aws/smithy-go" +) + +// MockAPIError mocks smithy.APIError +type MockAPIError struct { + error + code string + message string +} + +// NewMockAPIError returns a new APIError +func NewMockAPIError(code string, message string) smithy.APIError { + return &MockAPIError{ + code: code, + message: message, + } +} + +// ErrorCode returns the error code +func (e *MockAPIError) ErrorCode() string { + return e.code +} + +// ErrorMessage returns the error message +func (e *MockAPIError) ErrorMessage() string { + return e.message +} + +// ErrorFault isn't really implemented. +func (e *MockAPIError) ErrorFault() smithy.ErrorFault { + return 1 +} diff --git a/tests/e2e/nodes.go b/tests/e2e/nodes.go index d08bbac488..ba6adba462 100644 --- a/tests/e2e/nodes.go +++ b/tests/e2e/nodes.go @@ -22,6 +22,9 @@ import ( . "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" + authv1 "k8s.io/api/authorization/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/test/e2e/framework" e2enode "k8s.io/kubernetes/test/e2e/framework/node" ) @@ -44,4 +47,60 @@ var _ = Describe("[cloud-provider-aws-e2e] nodes", func() { gomega.Expect(node.Labels).To(gomega.HaveKey("topology.k8s.aws/zone-id")) } }) + + It("should label nodes with topology network info if instance is supported", func(ctx context.Context) { + framework.ExpectNoError(e2enode.WaitForAllNodesSchedulable(ctx, f.ClientSet, 10*time.Minute)) + nodeList, err := e2enode.GetReadySchedulableNodes(ctx, f.ClientSet) + framework.ExpectNoError(err) + + if len(nodeList.Items) < 2 { + framework.Failf("Conformance requires at least two nodes") + } + clientConfig, err := framework.LoadConfig() + framework.ExpectNoError(err) + client, err := kubernetes.NewForConfig(clientConfig) + framework.ExpectNoError(err) + + ssar := &authv1.SelfSubjectAccessReview{ + Spec: authv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authv1.ResourceAttributes{ + Group: "ec2.amazonaws.com", + Resource: "describeInstanceTopology", + Verb: "get", + }, + }, + } + result, err := client.AuthorizationV1().SelfSubjectAccessReviews().Create(context.TODO(), ssar, metav1.CreateOptions{}) + if err != nil { + framework.Failf("Error checking EC2 describeInstanceTopology access: %v", err) + } + allowed := result.Status.Allowed + supportedInstanceType := "p4d.24xlarge" + topologyNetworkLabel1 := "topology.k8s.aws/network-node-layer-1" + topologyNetworkLabel2 := "topology.k8s.aws/network-node-layer-2" + topologyNetworkLabel3 := "topology.k8s.aws/network-node-layer-3" + + for _, node := range nodeList.Items { + instanceType, hasInstanceType := node.Labels["node.kubernetes.io/instance-type"] + if !hasInstanceType { + framework.Failf("Node %s does not have instance-type label", node.Name) + } + + if instanceType == supportedInstanceType && allowed { + gomega.Expect(node.Labels).To(gomega.HaveKey(topologyNetworkLabel1), + "Node with instance type %s should have label %s", supportedInstanceType, topologyNetworkLabel1) + gomega.Expect(node.Labels).To(gomega.HaveKey(topologyNetworkLabel2), + "Node with instance type %s should have label %s", supportedInstanceType, topologyNetworkLabel2) + gomega.Expect(node.Labels).To(gomega.HaveKey(topologyNetworkLabel3), + "Node with instance type %s should have label %s", supportedInstanceType, topologyNetworkLabel3) + } else { + gomega.Expect(node.Labels).NotTo(gomega.HaveKey(topologyNetworkLabel1), + "Node with instance type %s should not have label %s", instanceType, topologyNetworkLabel1) + gomega.Expect(node.Labels).NotTo(gomega.HaveKey(topologyNetworkLabel2), + "Node with instance type %s should not have label %s", instanceType, topologyNetworkLabel2) + gomega.Expect(node.Labels).NotTo(gomega.HaveKey(topologyNetworkLabel3), + "Node with instance type %s should not have label %s", instanceType, topologyNetworkLabel3) + } + } + }) })