feat: implement remote signing transport for S3 requests and add tests #458
Conversation
io/s3.go
Outdated
| host == "s3.amazonaws.com" || | ||
| // Regional S3 endpoints | ||
| (len(host) > 12 && host[len(host)-12:] == ".amazonaws.com" && (host[:3] == "s3." || host[len(host)-17:len(host)-12] == ".s3")) || | ||
| // Virtual hosted-style bucket access | ||
| (len(host) > 17 && host[len(host)-17:] == ".s3.amazonaws.com") || | ||
| // Path-style access to S3 | ||
| (len(host) > 3 && host[:3] == "s3.") || | ||
| // Cloudflare R2 endpoints | ||
| (len(host) > 20 && host[len(host)-20:] == ".r2.cloudflarestorage.com") || | ||
| // MinIO or other custom S3-compatible endpoints (be more conservative) | ||
| (len(host) > 0 && (host == "localhost:9000" || host == "127.0.0.1:9000" || | ||
| // Only sign if it looks like an S3 request pattern (has bucket-like structure) | ||
| // and is NOT a catalog service (which typically has /catalog/ in the path) | ||
| (req.URL.Path != "" && !strings.Contains(req.URL.Path, "/catalog/") && | ||
| !strings.Contains(host, "catalog") && | ||
| // Exclude common non-S3 service patterns | ||
| !strings.Contains(host, "glue.") && | ||
| !strings.Contains(host, "api.") && | ||
| !strings.Contains(host, "catalog."))))) |
There was a problem hiding this comment.
Personally, I would put this under a separate helper function. wdyt?
There was a problem hiding this comment.
Sure, it could be put in the utils folder? Although I don't think this logic will be used elsewhere, so not sure that there is much benefit breaking the function into pieces.
There was a problem hiding this comment.
I suggest we put it in io/utils.go. Wdyt @zeroshade ?
There was a problem hiding this comment.
utils folder are generally bad practice, i would even keep this function in this file, or as @dttung2905 proposed in io/utils.go file.
There was a problem hiding this comment.
I vote to keep it in this file since there isn't any reusability. But I'm fine moving it as well. Who makes the final call ? 😄
There was a problem hiding this comment.
@flarco it's up to you, same file - it's good enough.
… timeout for remote signing transport
… issues and improve metadata handling
… encoding handling
… and refactor transport initialization
S3 tables and Lakekeeper
Hello, these changes allowed to use iceberg-go with the Cloudflare R2 Data Catalog and AWS S3 Tables REST
Let me know if any formatting or changes/tests are needed.