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

Add wsclient library to ecs-agent module #3690

Merged
merged 2 commits into from
May 23, 2023
Merged

Add wsclient library to ecs-agent module #3690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14407ee
Add wsclient library to ecs-agent module
May 10, 2023
26670d5
Merge branch 'dev' into ecs-agent-wsclient-lib
RichaGangwar May 19, 2023
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
3 changes: 1 addition & 2 deletions agent/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ require (
github.com/stretchr/testify v1.7.0
github.com/vishvananda/netlink v1.1.1-0.20201029203352-d40f9887b852
go.etcd.io/bbolt v1.3.5
golang.org/x/net v0.8.0
golang.org/x/sys v0.6.0
golang.org/x/tools v0.6.0
google.golang.org/grpc v1.52.0
Expand Down Expand Up @@ -66,10 +65,10 @@ require (
github.com/sirupsen/logrus v1.8.1 // indirect
github.com/vishvananda/netns v0.0.0-20200728191858-db3c7e526aae // indirect
golang.org/x/mod v0.8.0 // indirect
golang.org/x/net v0.8.0 // indirect
golang.org/x/text v0.8.0 // indirect
golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e // indirect
google.golang.org/genproto v0.0.0-20221118155620-16455021b5e6 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

Expand Down
1 change: 0 additions & 1 deletion agent/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,6 @@ gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
Expand Down
6 changes: 3 additions & 3 deletions agent/httpclient/httpclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ import (
"time"

"github.com/aws/amazon-ecs-agent/agent/config"
"github.com/aws/amazon-ecs-agent/agent/utils"
"github.com/aws/amazon-ecs-agent/agent/utils/cipher"
"github.com/aws/amazon-ecs-agent/agent/version"
"github.com/aws/amazon-ecs-agent/ecs-agent/utils/cipher"
"github.com/aws/amazon-ecs-agent/ecs-agent/utils/httpproxy"
Copy link
Member

Choose a reason for hiding this comment

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

will we be able to remove https://github.com/aws/amazon-ecs-agent/blob/master/agent/utils/httpproxy in a future pr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, agent/utils/httpproxy is already moved to ecs-agent/utils/httpproxy as part of this PR. Is that what you are referring to Ray?

)

// Taken from the default http.Client behavior
Expand Down Expand Up @@ -60,7 +60,7 @@ func New(timeout time.Duration, insecureSkipVerify bool) *http.Client {
// Note, these defaults are taken from the golang http library. We do not
// explicitly do not use theirs to avoid changing their behavior.
transport := &http.Transport{
Proxy: utils.Proxy,
Proxy: httpproxy.Proxy,
Dial: (&net.Dialer{
Timeout: defaultDialTimeout,
KeepAlive: defaultDialKeepalive,
Expand Down
2 changes: 1 addition & 1 deletion agent/httpclient/httpclient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"testing"
"time"

"github.com/aws/amazon-ecs-agent/agent/utils/cipher"
"github.com/aws/amazon-ecs-agent/ecs-agent/utils/cipher"
"github.com/stretchr/testify/assert"
)

Expand Down
2 changes: 1 addition & 1 deletion agent/tcs/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ import (
"github.com/aws/amazon-ecs-agent/agent/config"
"github.com/aws/amazon-ecs-agent/agent/stats"
"github.com/aws/amazon-ecs-agent/agent/tcs/model/ecstcs"
"github.com/aws/amazon-ecs-agent/agent/utils"
"github.com/aws/amazon-ecs-agent/agent/wsclient"
"github.com/aws/amazon-ecs-agent/ecs-agent/doctor"
"github.com/aws/amazon-ecs-agent/ecs-agent/utils"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/private/protocol/json/jsonutil"
Expand Down
8 changes: 0 additions & 8 deletions agent/utils/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,12 @@ import (
"io/ioutil"
"math"
"math/big"
"net/http"
"net/url"
"path/filepath"
"reflect"
"strconv"
"strings"

"github.com/aws/amazon-ecs-agent/agent/ecs_client/model/ecs"
"github.com/aws/amazon-ecs-agent/agent/utils/httpproxy"
commonutils "github.com/aws/amazon-ecs-agent/ecs-agent/utils"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
Expand Down Expand Up @@ -243,8 +240,3 @@ func GetENIAttachmentId(eniAttachmentArn string) (string, error) {
}
return fields[len(fields)-1], nil
}

// Proxy is an uncached version of http.ProxyFromEnvironment.
func Proxy(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
}
0 agent/utils/cipher/cipher.go → ...cs-agent/ecs-agent/utils/cipher/cipher.go
View file
Open in desktop
File renamed without changes.
6 changes: 6 additions & 0 deletions agent/utils/httpproxy/proxy.go → ...-agent/ecs-agent/utils/httpproxy/proxy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions agent/utils/sign_http_request.go → ...gent/ecs-agent/utils/sign_http_request.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions agent/vendor/modules.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ github.com/aws/amazon-ecs-agent/ecs-agent/tmds/logging
github.com/aws/amazon-ecs-agent/ecs-agent/tmds/utils/mux
github.com/aws/amazon-ecs-agent/ecs-agent/utils
github.com/aws/amazon-ecs-agent/ecs-agent/utils/arn
github.com/aws/amazon-ecs-agent/ecs-agent/utils/cipher
github.com/aws/amazon-ecs-agent/ecs-agent/utils/httpproxy
github.com/aws/amazon-ecs-agent/ecs-agent/utils/ttime
github.com/aws/amazon-ecs-agent/ecs-agent/utils/ttime/mocks
# github.com/aws/aws-sdk-go v1.36.0
Expand Down Expand Up @@ -434,8 +436,6 @@ google.golang.org/protobuf/types/descriptorpb
google.golang.org/protobuf/types/known/anypb
google.golang.org/protobuf/types/known/durationpb
google.golang.org/protobuf/types/known/timestamppb
# gopkg.in/yaml.v2 v2.4.0
## explicit; go 1.15
# gopkg.in/yaml.v3 v3.0.1
## explicit
gopkg.in/yaml.v3
Expand Down
7 changes: 4 additions & 3 deletions agent/wsclient/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ import (
"time"

"github.com/aws/amazon-ecs-agent/agent/config"
"github.com/aws/amazon-ecs-agent/agent/utils"
"github.com/aws/amazon-ecs-agent/agent/utils/cipher"
"github.com/aws/amazon-ecs-agent/agent/wsclient/wsconn"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't imports of "github.com/aws/amazon-ecs-agent/agent/wsclient/wsconn" be updated to "github.com/aws/amazon-ecs-agent/ecs-agent/wsclient/wsconn" then we can remove the unused files in agent/wsclient/wsconn, right? (i.e., like what we've done with agent/utils/cipher and agent/utils/httpproxy)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not think it makes sense to switch just wsconn imports to ecs-agent/wsclient. The plan is to plug in the ecs-agent/wsclient with ecs-agent/tcs and ecs-agent/acs, and once we have the confidence in testing, we would completely remove the agent/wsclient package. It made sense to move the utils to avoid the cyclic dependency issue between agent and ecs-agent.

Copy link
Contributor

@danehlim danehlim May 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I was just thinking since the files in agent/wsclient/wsconn and ecs-agent/wsclient/wsconn are identical, we could just have a single one living in ecs-agent/to refer to as a source of truth since we will be removing agent/wsclient/wsconn in the future anyway.

Copy link
Member

@fierlion fierlion May 22, 2023 •
edited by RichaGangwar
Loading

Choose a reason for hiding this comment

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

once we have the confidence in testing, we would completely remove the agent/wsclient package

do we have a TODO for this anywhere (in code or in a tracking ticket)?

Reply from RichaGangwar:
Yes Ray, Thats the plan. As part of tcs and acs shared lib, we will integrate this new wsclient lib in the clients. And once well tested, we will remove the agent/wsclient. There isn't any TODO right now. Will add it in description to keep track of it. Thank you :)

"github.com/aws/amazon-ecs-agent/ecs-agent/logger"
"github.com/aws/amazon-ecs-agent/ecs-agent/utils"
"github.com/aws/amazon-ecs-agent/ecs-agent/utils/cipher"
"github.com/aws/amazon-ecs-agent/ecs-agent/utils/httpproxy"

"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/private/protocol/json/jsonutil"
Expand Down Expand Up @@ -192,7 +193,7 @@ func (cs *ClientServerImpl) Connect() error {
ReadBufferSize: readBufSize,
WriteBufferSize: writeBufSize,
TLSClientConfig: tlsConfig,
Proxy: utils.Proxy,
Proxy: httpproxy.Proxy,
NetDial: timeoutDialer.Dial,
HandshakeTimeout: wsHandshakeTimeout,
}
Expand Down
5 changes: 4 additions & 1 deletion ecs-agent/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ require (
github.com/didip/tollbooth v4.0.2+incompatible
github.com/golang/mock v1.4.1
github.com/gorilla/mux v1.8.0
github.com/gorilla/websocket v1.5.0
github.com/pkg/errors v0.9.1
github.com/stretchr/testify v1.7.0
golang.org/x/net v0.8.0
golang.org/x/sys v0.6.0
golang.org/x/tools v0.6.0
)
Expand All @@ -20,7 +22,8 @@ require (
github.com/patrickmn/go-cache v2.1.0+incompatible // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
golang.org/x/mod v0.8.0 // indirect
golang.org/x/net v0.8.0 // indirect
golang.org/x/text v0.8.0 // indirect
golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
7 changes: 6 additions & 1 deletion ecs-agent/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ github.com/golang/mock v1.4.1 h1:ocYkMQY5RrXTYgXl7ICpV0IXwlEQGwKIsery4gyXa1U=
github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI=
github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=
github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg=
github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo=
github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8=
Expand Down Expand Up @@ -42,6 +44,8 @@ golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.8.0 h1:57P1ETyNKtuIjB4SRd15iJxuhj8Gc416Y78H3qgMh68=
golang.org/x/text v0.8.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e h1:EHBhcS0mlXEAVwNyO2dLfjToGsyY4j24pTs2ScHnX7s=
golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
Expand All @@ -50,8 +54,9 @@ golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM=
golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
Expand Down
37 changes: 37 additions & 0 deletions ecs-agent/utils/cipher/cipher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License"). You may
// not use this file except in compliance with the License. A copy of the
// License is located at
//
// http://aws.amazon.com/apache2.0/
//
// or in the "license" file accompanying this file. This file 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 cipher provides customized cipher configuration for agent client
package cipher

import (
"crypto/tls"
)

// Only support a subset of ciphers, corresponding cipher suite names can be found here: https://golang.org/pkg/crypto/tls/#Config
var SupportedCipherSuites = []uint16{
tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
tls.TLS_RSA_WITH_AES_128_CBC_SHA256,
tls.TLS_RSA_WITH_AES_128_CBC_SHA,
tls.TLS_RSA_WITH_AES_256_GCM_SHA384,
tls.TLS_RSA_WITH_AES_256_CBC_SHA,
}

func WithSupportedCipherSuites(config *tls.Config) {
config.CipherSuites = SupportedCipherSuites
}
Loading