Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate ECS client to aws-sdk-go-v2 #4447

Open
wants to merge 18 commits into
base: dev
Choose a base branch
from
Open

Migrate ECS client to aws-sdk-go-v2 #4447

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d763eee
migrate ecs-agent/api/ecs
tinnywang Oct 23, 2024
d7f7a27
migrate ecs-agent/utils
tinnywang Oct 27, 2024
e64f752
migrate ecs-agent/tmds/handlers/taskprotection/v1
tinnywang Oct 27, 2024
99a016e
migrate agent module
tinnywang Oct 27, 2024
98226e0
migrate ECS client credential provider
tinnywang Oct 28, 2024
bdce259
ecs-agent/api/errors package handles SDK v1 and v2 errors
tinnywang Oct 31, 2024
ac956e9
regenerate code
tinnywang Nov 12, 2024
c3cc11b
handle SDK v2 errors in task protection handlers
tinnywang Nov 14, 2024
bea4e89
fix tests
tinnywang Nov 15, 2024
5124bd9
migrate remaining packages
tinnywang Nov 16, 2024
b7ab4bb
remove SDK v1 ECS client
tinnywang Nov 16, 2024
76f9073
cleanup
tinnywang Nov 17, 2024
50f1c9a
update more error handling
tinnywang Dec 4, 2024
c5bf057
fix windows test
tinnywang Dec 5, 2024
447b0eb
fix after rebase
tinnywang Dec 10, 2024
443e1d5
Merge branch 'dev' into aws-sdk-go-v2/ecs
Yiyuanzzz Jan 23, 2025
029b479
Merge branch 'dev' into aws-sdk-go-v2/ecs
Yiyuanzzz Jan 29, 2025
6b5700a
Merge branch 'dev' into aws-sdk-go-v2/ecs
Yiyuanzzz Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions agent/app/agent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import (
ecstypes "github.com/aws/aws-sdk-go-v2/service/ecs/types"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
smithy "github.com/aws/smithy-go"
"github.com/docker/docker/api/types"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -1059,8 +1060,9 @@ func TestReregisterContainerInstanceInstanceTypeChanged(t *testing.T) {
mockDockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(),
gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil),
client.EXPECT().RegisterContainerInstance(containerInstanceARN, gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any(), gomock.Any()).Return("", "", awserr.New("",
apierrors.InstanceTypeChangedErrorMessage, errors.New(""))),
gomock.Any(), gomock.Any()).Return("", "", &smithy.GenericAPIError{
Message: apierrors.InstanceTypeChangedErrorMessage,
}),
)

cfg := getTestConfig()
Expand Down
2 changes: 1 addition & 1 deletion agent/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ require (
github.com/aws/aws-sdk-go-v2/credentials v1.17.42
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.18
github.com/aws/aws-sdk-go-v2/service/ecs v1.47.3
github.com/aws/smithy-go v1.22.1
github.com/awslabs/go-config-generator-for-fluentd-and-fluentbit v0.0.0-20210308162251-8959c62cb8f9
github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575
github.com/container-storage-interface/spec v1.8.0
Expand Down Expand Up @@ -53,7 +54,6 @@ require (
github.com/aws/aws-sdk-go-v2/service/sso v1.24.3 // indirect
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.3 // indirect
github.com/aws/aws-sdk-go-v2/service/sts v1.32.3 // indirect
github.com/aws/smithy-go v1.22.1 // indirect
github.com/cilium/ebpf v0.16.0 // indirect
github.com/containerd/containerd v1.7.24 // indirect
github.com/containerd/log v0.1.0 // indirect
Expand Down
76 changes: 51 additions & 25 deletions agent/handlers/task_server_setup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,14 @@ import (
v2 "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v2"
v4 "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v4/state"
mock_execwrapper "github.com/aws/amazon-ecs-agent/ecs-agent/utils/execwrapper/mocks"
smithy "github.com/aws/smithy-go"

awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
"github.com/aws/aws-sdk-go-v2/service/ecs"
ecstypes "github.com/aws/aws-sdk-go-v2/service/ecs/types"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/request"
smithyhttp "github.com/aws/smithy-go/transport/http"
"github.com/docker/docker/api/types"
"github.com/golang/mock/gomock"
"github.com/gorilla/mux"
Expand Down Expand Up @@ -3287,11 +3289,17 @@ func TestGetTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.NewRequestFailure(
awserr.New(apierrors.ErrCodeServerException, ecsErrMessage, nil),
http.StatusInternalServerError,
ecsRequestID,
),
&awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: http.StatusInternalServerError,
},
},
Err: &ecstypes.ServerException{Message: &ecsErrMessage},
},
RequestID: ecsRequestID,
},
),
expectedStatusCode: http.StatusInternalServerError,
expectedResponseBody: tptypes.TaskProtectionResponse{
Expand All @@ -3311,11 +3319,17 @@ func TestGetTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.NewRequestFailure(
awserr.New(apierrors.ErrCodeAccessDeniedException, ecsErrMessage, nil),
http.StatusBadRequest,
ecsRequestID,
),
&awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: http.StatusBadRequest,
},
},
Err: &ecstypes.AccessDeniedException{Message: &ecsErrMessage},
},
RequestID: ecsRequestID,
},
),
expectedStatusCode: http.StatusBadRequest,
expectedResponseBody: tptypes.TaskProtectionResponse{
Expand All @@ -3335,7 +3349,7 @@ func TestGetTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.New(apierrors.ErrCodeInvalidParameterException, ecsErrMessage, nil)),
&ecstypes.InvalidParameterException{Message: &ecsErrMessage}),
expectedStatusCode: http.StatusInternalServerError,
expectedResponseBody: tptypes.TaskProtectionResponse{
Error: &tptypes.ErrorResponse{
Expand All @@ -3352,7 +3366,7 @@ func TestGetTaskProtection(t *testing.T) {
setStateExpectations: happyStateExpectations,
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil, awserr.New(request.CanceledErrorCode, "request cancelled", nil)),
nil, &smithy.CanceledError{}),
expectedStatusCode: http.StatusGatewayTimeout,
expectedResponseBody: tptypes.TaskProtectionResponse{
Error: &tptypes.ErrorResponse{
Expand Down Expand Up @@ -3561,11 +3575,17 @@ func TestUpdateTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.NewRequestFailure(
awserr.New(apierrors.ErrCodeServerException, ecsErrMessage, nil),
http.StatusInternalServerError,
ecsRequestID,
),
&awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: http.StatusInternalServerError,
},
},
Err: &ecstypes.ServerException{Message: &ecsErrMessage},
},
RequestID: ecsRequestID,
},
),
expectedStatusCode: http.StatusInternalServerError,
expectedResponseBody: tptypes.TaskProtectionResponse{
Expand All @@ -3583,11 +3603,17 @@ func TestUpdateTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.NewRequestFailure(
awserr.New(apierrors.ErrCodeAccessDeniedException, ecsErrMessage, nil),
http.StatusBadRequest,
ecsRequestID,
),
&awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: http.StatusBadRequest,
},
},
Err: &ecstypes.AccessDeniedException{Message: &ecsErrMessage},
},
RequestID: ecsRequestID,
},
),
expectedStatusCode: http.StatusBadRequest,
expectedResponseBody: tptypes.TaskProtectionResponse{
Expand All @@ -3605,7 +3631,7 @@ func TestUpdateTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.New(apierrors.ErrCodeInvalidParameterException, ecsErrMessage, nil)),
&ecstypes.InvalidParameterException{Message: &ecsErrMessage}),
expectedStatusCode: http.StatusInternalServerError,
expectedResponseBody: tptypes.TaskProtectionResponse{
Error: &tptypes.ErrorResponse{
Expand All @@ -3620,7 +3646,7 @@ func TestUpdateTaskProtection(t *testing.T) {
setStateExpectations: happyStateExpectations,
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil, awserr.New(request.CanceledErrorCode, "request cancelled", nil)),
nil, &smithy.CanceledError{}),
expectedStatusCode: http.StatusGatewayTimeout,
expectedResponseBody: tptypes.TaskProtectionResponse{
Error: &tptypes.ErrorResponse{
Expand Down
20 changes: 17 additions & 3 deletions agent/handlers/v2/response.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
package v2

import (
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/smithy-go"
"github.com/cihub/seelog"
"github.com/pkg/errors"
Comment on lines +17 to +21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this import block should go below with other external imports. import order -

  1. standard imports, 2. ecs agent imports, 3. external imports


apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container"
"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
Expand All @@ -25,9 +29,7 @@ import (
tmdsresponse "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/response"
"github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/utils"
tmdsv2 "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v2"
"github.com/aws/aws-sdk-go/aws"
"github.com/cihub/seelog"
"github.com/pkg/errors"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
)

// Agent versions >= 1.2.0: Null, zero, and CPU values of 1
Expand Down Expand Up @@ -280,5 +282,17 @@ func newErrorResponse(err error, field, resourceARN string) *tmdsv2.ErrorRespons
}
}

var apiErr smithy.APIError
if errors.As(err, &apiErr) {
errResp.ErrorCode = apiErr.ErrorCode()
errResp.ErrorMessage = apiErr.ErrorMessage()
}

var re *awshttp.ResponseError
if errors.As(err, &re) {
errResp.StatusCode = re.HTTPStatusCode()
errResp.RequestId = re.RequestID
}

return errResp
}
38 changes: 29 additions & 9 deletions agent/handlers/v2/response_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package v2
import (
"encoding/json"
"fmt"
"net/http"
"testing"
"time"

Expand All @@ -30,12 +31,12 @@ import (
apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status"
ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface"
tmdsv2 "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v2"
"github.com/aws/aws-sdk-go-v2/aws"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
ecstypes "github.com/aws/aws-sdk-go-v2/service/ecs/types"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
smithyhttp "github.com/aws/smithy-go/transport/http"
"github.com/docker/docker/api/types"
"github.com/golang/mock/gomock"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -679,33 +680,52 @@ func TestTaskResponseWithV4TagsError(t *testing.T) {
},
}

errCode := "ThrottlingException"
errMessage := "Rate exceeded"
errStatusCode := 400
containerTagsRequestId := "cef9da77-aee7-431d-84d5-f92b2d342c51"
taskTagsRequestId := "45dbbc67-0c60-4248-855e-14fdf4c11870"
containerTagsErr := awserr.NewRequestFailure(awserr.Error(awserr.New(errCode, errMessage, errors.New(""))), errStatusCode, containerTagsRequestId)
taskTagsError := awserr.NewRequestFailure(awserr.Error(awserr.New(errCode, errMessage, errors.New(""))), errStatusCode, taskTagsRequestId)
containerTagsErr := &awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: errStatusCode,
},
},
Err: &ecstypes.LimitExceededException{Message: &errMessage},
},
RequestID: containerTagsRequestId,
}
taskTagsErr := &awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: errStatusCode,
},
},
Err: &ecstypes.LimitExceededException{Message: &errMessage},
},
RequestID: taskTagsRequestId,
}

gomock.InOrder(
state.EXPECT().TaskByArn(taskARN).Return(task, true),
state.EXPECT().ContainerMapByArn(taskARN).Return(containerNameToDockerContainer, true),
ecsClient.EXPECT().GetResourceTags(containerInstanceArn).Return(nil, containerTagsErr),
ecsClient.EXPECT().GetResourceTags(taskARN).Return(nil, taskTagsError),
ecsClient.EXPECT().GetResourceTags(taskARN).Return(nil, taskTagsErr),
)

taskWithTagsResponse, err := NewTaskResponse(taskARN, state, ecsClient, cluster, availabilityZone, containerInstanceArn, true, true)
assert.NoError(t, err)
_, err = json.Marshal(taskWithTagsResponse)
assert.NoError(t, err)
assert.Equal(t, taskWithTagsResponse.Errors[0].ErrorField, "ContainerInstanceTags")
assert.Equal(t, taskWithTagsResponse.Errors[0].ErrorCode, errCode)
assert.Equal(t, taskWithTagsResponse.Errors[0].ErrorCode, (&ecstypes.LimitExceededException{}).ErrorCode())
assert.Equal(t, taskWithTagsResponse.Errors[0].ErrorMessage, errMessage)
assert.Equal(t, taskWithTagsResponse.Errors[0].StatusCode, errStatusCode)
assert.Equal(t, taskWithTagsResponse.Errors[0].RequestId, containerTagsRequestId)
assert.Equal(t, taskWithTagsResponse.Errors[0].ResourceARN, containerInstanceArn)
assert.Equal(t, taskWithTagsResponse.Errors[1].ErrorField, "TaskTags")
assert.Equal(t, taskWithTagsResponse.Errors[1].ErrorCode, errCode)
assert.Equal(t, taskWithTagsResponse.Errors[1].ErrorCode, (&ecstypes.LimitExceededException{}).ErrorCode())
assert.Equal(t, taskWithTagsResponse.Errors[1].ErrorMessage, errMessage)
assert.Equal(t, taskWithTagsResponse.Errors[1].StatusCode, errStatusCode)
assert.Equal(t, taskWithTagsResponse.Errors[1].RequestId, taskTagsRequestId)
Expand Down
21 changes: 19 additions & 2 deletions agent/utils/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ import (
"strings"

commonutils "github.com/aws/amazon-ecs-agent/ecs-agent/utils"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
"github.com/aws/aws-sdk-go-v2/service/ecs/types"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/smithy-go"

"github.com/pkg/errors"
)
Expand Down Expand Up @@ -140,16 +142,31 @@ func Remove(slice []string, s int) []string {
// the passed in error code.
func IsAWSErrorCodeEqual(err error, code string) bool {
awsErr, ok := err.(awserr.Error)
return ok && awsErr.Code() == code
if ok {
return awsErr.Code() == code
}

var apiErr smithy.APIError
if errors.As(err, &apiErr) {
return apiErr.ErrorCode() == code
}

return false
}

// GetRequestFailureStatusCode returns the status code from a
// RequestFailure error, or 0 if the error is not of that type
func GetRequestFailureStatusCode(err error) int {
var statusCode int
if reqErr, ok := err.(awserr.RequestFailure); ok {
statusCode = reqErr.StatusCode()
return reqErr.StatusCode()
}

var re *awshttp.ResponseError
if errors.As(err, &re) {
return re.HTTPStatusCode()
}

return statusCode
}

Expand Down
Loading