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.

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

Update AWS SDK to latest #128

Merged
merged 1 commit into from
Dec 2, 2019
Merged

Update AWS SDK to latest #128

merged 1 commit into from
Dec 2, 2019

Conversation

notnoop
Copy link
Contributor

@notnoop notnoop commented Nov 25, 2019

Update to the latest AWS SDK to pick up EC2 Instance Metadata Service v2
support, announced in Nov 19, 2019. Updating it here is necessary for
downstream projects that haven't adopted go mod (e.g. Nomad), and rely
on vendored provider packages here.

For more info on IMDSv2:
https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-metadata.html
.

github.com/davecgh/go-spew v1.1.1 // indirect
github.com/denverdino/aliyungo v0.0.0-20170926055100-d3308649c661
github.com/dgrijalva/jwt-go v3.2.0+incompatible // indirect
github.com/digitalocean/godo v1.1.1
github.com/dimchansky/utfbom v1.1.0 // indirect
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unrelated change but it looks like it's a dependency of github.com/Azure/go-autorest v.10.7.0 [1] but somehow we missed declaring it here.

[1] https://github.com/Azure/go-autorest/blob/v10.7.0/autorest/azure/auth/auth.go#L34

Copy link
Member

Choose a reason for hiding this comment

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

Is this the most conservative update that includes the things we need?

FWIW Consul now pulls in 1.25.42 anyway thanks to recent AWS CA provider work and we tested go-discover integration afterwards and it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops - I had updated it to v1.25.41 the most recent release with ec2metadata changes[1]. I accidentally updated it with v1.15.41 earlier.

[1] https://github.com/aws/aws-sdk-go/releases/

Copy link
Member

Choose a reason for hiding this comment

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

Correction: Consul pulls in v1.25.32 which is what I tested with go-discover stuff.

Let me sanity check Consul AWS cloud auto join with 1.25.41 and then we can call this good.

Copy link
Member

Choose a reason for hiding this comment

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

Yep works with 1.25.41:

$ go get github.com/aws/[email protected]
...
$ make dev
$ envchain csl consul agent -retry-join "provider=aws tag_key=Role tag_value=consul-server region=us-west-1" -data-dir "/tmp/consul"
...
2019/12/02 15:26:47 [INFO] agent: Discovered LAN servers: 10.0.1.241 10.0.1.174 10.0.1.12 10.0.1.33 10.0.1.237 10.0.1.44 10.0.1.11 10.0.1.156 10.0.1.28
...

(We have dev servers running in that region so this discovered those and could join them.

@notnoop notnoop requested review from alvin-huang and banks November 26, 2019 16:18
Update to the latest AWS SDK to pick up EC2 Instance Metadata Service v2
support, announced in Nov 19, 2019.  Updating it here is necessary for
downstream projects that haven't adopted go mod (e.g. Nomad), and rely
on vendored provider packages here.

For more info on IMDSv2:
https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-metadata.html
.

This updates version to v1.25.41, the latest version with ec2metadata
changes.
@notnoop notnoop force-pushed the c-vendor-aws-v1.15.41 branch from 0ad4019 to b18ea84 Compare December 2, 2019 01:39
Copy link
Member

@banks banks left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

github.com/davecgh/go-spew v1.1.1 // indirect
github.com/denverdino/aliyungo v0.0.0-20170926055100-d3308649c661
github.com/dgrijalva/jwt-go v3.2.0+incompatible // indirect
github.com/digitalocean/godo v1.1.1
github.com/dimchansky/utfbom v1.1.0 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

Yep works with 1.25.41:

$ go get github.com/aws/[email protected]
...
$ make dev
$ envchain csl consul agent -retry-join "provider=aws tag_key=Role tag_value=consul-server region=us-west-1" -data-dir "/tmp/consul"
...
2019/12/02 15:26:47 [INFO] agent: Discovered LAN servers: 10.0.1.241 10.0.1.174 10.0.1.12 10.0.1.33 10.0.1.237 10.0.1.44 10.0.1.11 10.0.1.156 10.0.1.28
...

(We have dev servers running in that region so this discovered those and could join them.

@banks
Copy link
Member

banks commented Dec 2, 2019

I also ran our integration tests for the new CA using this version for completeness and got a pass:

$ envchain csl go test -v github.com/hashicorp/consul/agent/connect/ca -count=1 -run='^TestAWS'
...
PASS
ok  	github.com/hashicorp/consul/agent/connect/ca	37.372s

@notnoop
Copy link
Contributor Author

notnoop commented Dec 2, 2019

Thanks!

notnoop pushed a commit to hashicorp/nomad that referenced this pull request Dec 2, 2019
Update github.com/aws/aws-sdk-go and github.com/hashicorp/go-discover to
pick up support for EC2 Metadata Instance Service v2 changes.

Follow up to hashicorp/go-discover#128 .
mikemorris added a commit to hashicorp/consul that referenced this pull request Dec 2, 2019
@mikemorris mikemorris deleted the c-vendor-aws-v1.15.41 branch December 2, 2019 22:30
notnoop pushed a commit to hashicorp/nomad that referenced this pull request Dec 3, 2019
Update github.com/aws/aws-sdk-go and github.com/hashicorp/go-discover to
pick up support for EC2 Metadata Instance Service v2 changes.

Follow up to hashicorp/go-discover#128 .
mikemorris added a commit to hashicorp/consul that referenced this pull request Dec 4, 2019
Refs hashicorp/go-discover#128

* deps: add replace directive for gocheck

Transitive dep, source at https://launchpad.net/gocheck indicates
project moved. This also avoids a dependency on bzr when fetching
modules. Refs #6818

* deps: make update-vendor

* test: update retry-join expected names from go-discover
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants