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

Add S3 IPv6 "dualstack" endpoint support . Option = "use_dual_stack" [bool] #7482

Merged
merged 1 commit into from Dec 8, 2023

Conversation

tonymet
Copy link
Contributor

@tonymet tonymet commented Dec 2, 2023

What is the purpose of this change?

Currently rclone hangs when IPv4 route to S3 is unavailable (e.g. no IPV4 interface or no IPV4 route to internet ). This change adds S3 IPv6 "dualstack" endpoint support . Option = use_dual_stack [bool] . Default S3 endpoints are IPv4 NOT IPv6

See AWS docs on IPv6:
"To make a request to an S3 bucket over IPv6, you need to use a dual-stack endpoint."

Why Now?

Starting Feb, EC2 will charge for public IPv4 addresses & NAT traffic. More AWS users will be using rclone & S3 on IPV6

Steps to Reproduce

  1. Run test from a host with IPv6 address.
  2. If the host has IPv4 address, use security-group, routing table or firewall to block IPv4 traffic (e.g. outbound destination 0.0.0.0 = block)
  3. configure new rclone endpoint "test-s3"
  4. run rclone ls test-s3:

ACTUAL RESULTS
rclone hangs

EXPECTED RESULTS
Files listed are visible

go run . ls s3-test:|head -1
502256548864 tonym-backup/2021-12-29/archive2.tar

Testing This Change

  1. Checkout this commit
  2. set use_dual_stack=true in rclone.conf (see below)
  3. Set up your rclone machine to block IPv4 (see STEPS TO REPRODUCE)
  4. go run . ls test-s3:
grep dual_stack~/.config/rclone/rclone.conf 
use_dual_stack= true

Testing S3 Default & DualStack endpoints

Example S3 queries to default & dual-stack endpoints for example

$ dig +short AAA s3.us-west-2.amazonaws.com|head -1
52.92.234.56
$ dig +short AAAA s3.us-west-2.amazonaws.com|head -1
# no answer

Dualstack (both IPv4 & IPv6 lookup work)

# 
 dig +short AAAA s3.dualstack.us-west-2.amazonaws.com |head -1
2600:1fa0:403f:aa89:345c:c3b8::
$ dig +short A s3.dualstack.us-west-2.amazonaws.com |head -1
52.92.243.184

Was the change discussed in an issue or in the forum before?

https://forum.rclone.org/t/ipv6-guide-review-with-google-drive-ec2-s3/43218

IPv6 Support Could Use Attention...

  1. ✅--bind to IPv6
  2. ❌oauth using IPv6 (::1)
  3. (this PR) ❌Connect to S3 endpoint on IPv6
  4. ✅ Connect to Google Drive API endpoint on IPv6 (once authorized )
  5. ❓Connect to other backend endpoints on IPv6

I recommend dumping all rclone endpoints and testing dig AAAA to see which ones currently support IPv6.

Unit Testing

I've added test coverage s3.go covering enable/disable dualstack config

 cd backend/s3 ; go test .
ok      github.com/rclone/rclone/backend/s3     (cached)

no broken tests

 go test ./...

ok      github.com/rclone/rclone/backend/alias  0.023s
ok      github.com/rclone/rclone/backend/azureblob      0.024s
ok      github.com/rclone/rclone/backend/azurefiles     0.032s
# truncated

Checklist

  • [✅ ] I have read the contribution guidelines.
  • [✅ ] I have added tests for all changes in this PR if appropriate.
  • [✅ ] I have added documentation for the changes if appropriate.
  • [✅ ] All commit messages are in house style.
  • [✅ ] I'm done, this Pull Request is ready for review :-)

@tonymet tonymet force-pushed the s3-ipv6-dualstack branch 4 times, most recently from 088e7d4 to cf0ec77 Compare December 3, 2023 00:26
@miyurusankalpa
Copy link

Why not set this option enabled by default? Endpoint is dual stacked, so it should work on IPv4 only setups too.

@tonymet
Copy link
Contributor Author

tonymet commented Dec 3, 2023

Why not set this option enabled by default? Endpoint is dual stacked, so it should work on IPv4 only setups too.

I'm following AWS cli pattern here. aws cli requires use_dualstack_endpoint = true to activate dualstack endpoints.

Copy link
Member

@ncw ncw left a comment

Choose a reason for hiding this comment

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

Thanks for doing this.

I had no idea that S3 endpoints wouldn't be dual stack by default. Seems rather quaint that they aren't but I guess AWS have their reasons.

I wrote some notes inline for some improvements.

Thank you :-)

backend/s3/s3.go Outdated
@@ -2628,6 +2628,7 @@ type Options struct {
Region string `config:"region"`
Endpoint string `config:"endpoint"`
STSEndpoint string `config:"sts_endpoint"`
DualstackEndpoint bool `config:"dualstack_endpoint"`
Copy link
Member

Choose a reason for hiding this comment

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

This needs a config setting like this one

rclone/backend/s3/s3.go

Lines 2212 to 2221 in 298c13e

}, {
Name: "v2_auth",
Help: `If true use v2 authentication.
If this is false (the default) then rclone will use v4 authentication.
If it is set then rclone will use v2 authentication.
Use this only if v4 signatures don't work, e.g. pre Jewel/v10 CEPH.`,
Default: false,
Advanced: true,

@@ -1126,6 +1126,19 @@ Properties:
- Owner gets FULL_CONTROL.
- The AuthenticatedUsers group gets READ access.

#### --s3-dualstack-endpoint
Copy link
Member

Choose a reason for hiding this comment

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

This should be auto generated from the config you need to add above :-)

backend/s3/s3.go Outdated
@@ -2956,6 +2957,9 @@ func s3Connection(ctx context.Context, opt *Options, client *http.Client) (*s3.S
r.addService("sts", opt.STSEndpoint)
awsConfig.WithEndpointResolver(r)
}
if opt.DualstackEndpoint {
awsConfig.WithUseDualStack(true)
Copy link
Member

Choose a reason for hiding this comment

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

The AWS docs say

	// Deprecated: This option will continue to function for S3 and S3 Control for backwards compatibility.
	// UseDualStackEndpoint should be used to enable usage of a service's dual-stack endpoint for all service clients
	// moving forward. For S3 and S3 Control, when UseDualStackEndpoint is set to a non-zero value it takes higher
	// precedence then this option.
	UseDualStack *bool

About this option.

backend/s3/s3.go Outdated
@@ -2628,6 +2628,7 @@ type Options struct {
Region string `config:"region"`
Endpoint string `config:"endpoint"`
STSEndpoint string `config:"sts_endpoint"`
DualstackEndpoint bool `config:"dualstack_endpoint"`
Copy link
Member

Choose a reason for hiding this comment

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

Can we call this use_dual_stack since it is a bool? That would fit in better with the other options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we call this use_dual_stack since it is a bool? That would fit in better with the other options.

Thanks I’ll update with the feedback and thanks for catching the deprecation. There’s a v1 & v2 AWS api for GO and a couple other deprecations so this is one i missed. I’ll have the updates shortly.

Copy link
Member

Choose a reason for hiding this comment

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

Migrate to the new AWS SDK is probably something we should bite the bullet and do at some point, however that is out of scope for this PR :-)

@tonymet tonymet force-pushed the s3-ipv6-dualstack branch 2 times, most recently from 15e9e76 to d45e387 Compare December 4, 2023 19:15
@tonymet
Copy link
Contributor Author

tonymet commented Dec 4, 2023

@ncw

  • ✅ change to use_dual_stack
  • ✅ use newer UseDualstackEndpoint API
  • ✅ clean up error message in s3_test.go
  • ✅ add doc config and cleanup .md file
    Ready to merge

dualstack_endpoint=true enables IPv6 DNS lookup for S3 endpoints
in s3.go, add Options.DualstackEndpoint to support IPv6 on S3
@tonymet tonymet changed the title Add S3 IPv6 "dualstack" endpoint support . Option = "dualstack_endpoint [bool]" Add S3 IPv6 "dualstack" endpoint support . Option = "use_dual_stack" [bool] Dec 4, 2023
Copy link
Member

@ncw ncw left a comment

Choose a reason for hiding this comment

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

This looks great now - thanks!

I'll just run the CI and if it passed (modulo the thing I know will fail!) I'll merge - thank you :-)

@ncw ncw merged commit 9fe343b into rclone:master Dec 8, 2023
9 of 10 checks passed
@ncw
Copy link
Member

ncw commented Dec 8, 2023

Thank you :-)

@tonymet
Copy link
Contributor Author

tonymet commented Dec 8, 2023

many thanks for all the help and guidance. I'm really happy to contribute to rclone it's been a great tool

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.

None yet

3 participants