Parcourir la source

Remove the IMGPROXY_S3_MULTI_REGION config. imgproxy now always work in multi-regional S3 mode

DarthSim il y a 1 mois
Parent
commit
10285240a9
4 fichiers modifiés avec 45 ajouts et 39 suppressions
  1. 3 0
      CHANGELOG.md
  2. 0 3
      config/config.go
  3. 41 25
      transport/s3/s3.go
  4. 1 11
      transport/s3/s3_test.go

+ 3 - 0
CHANGELOG.md

@@ -22,6 +22,9 @@
 - (pro) Fix flattening behavior when chained pipelines are used and the resulting format doesn't support transparency.
 - (pro) Fix advanced smart crop when the most feature points are located close to the right or the bottom edge.
 
+### Removed
+- Remove the `IMGPROXY_S3_MULTI_REGION` config. imgproxy now always work in multi-regional S3 mode.
+
 ## [3.27.2] - 2025-01-27
 ### Fixed
 - Fix preventing requests to `0.0.0.0` when imgproxy is configured to deny loopback addresses.

+ 0 - 3
config/config.go

@@ -116,7 +116,6 @@ var (
 	S3EndpointUsePathStyle    bool
 	S3AssumeRoleArn           string
 	S3AssumeRoleExternalID    string
-	S3MultiRegion             bool
 	S3DecryptionClientEnabled bool
 
 	GCSEnabled  bool
@@ -328,7 +327,6 @@ func Reset() {
 	S3EndpointUsePathStyle = true
 	S3AssumeRoleArn = ""
 	S3AssumeRoleExternalID = ""
-	S3MultiRegion = false
 	S3DecryptionClientEnabled = false
 	GCSEnabled = false
 	GCSKey = ""
@@ -579,7 +577,6 @@ func Configure() error {
 	configurators.Bool(&S3EndpointUsePathStyle, "IMGPROXY_S3_ENDPOINT_USE_PATH_STYLE")
 	configurators.String(&S3AssumeRoleArn, "IMGPROXY_S3_ASSUME_ROLE_ARN")
 	configurators.String(&S3AssumeRoleExternalID, "IMGPROXY_S3_ASSUME_ROLE_EXTERNAL_ID")
-	configurators.Bool(&S3MultiRegion, "IMGPROXY_S3_MULTI_REGION")
 	configurators.Bool(&S3DecryptionClientEnabled, "IMGPROXY_S3_USE_DECRYPTION_CLIENT")
 
 	configurators.Bool(&GCSEnabled, "IMGPROXY_USE_GCS")

+ 41 - 25
transport/s3/s3.go

@@ -16,7 +16,6 @@ import (
 	awsHttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
 	awsConfig "github.com/aws/aws-sdk-go-v2/config"
 	"github.com/aws/aws-sdk-go-v2/credentials/stscreds"
-	s3Manager "github.com/aws/aws-sdk-go-v2/feature/s3/manager"
 	"github.com/aws/aws-sdk-go-v2/service/kms"
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"github.com/aws/aws-sdk-go-v2/service/sts"
@@ -149,17 +148,30 @@ func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) {
 		}
 	}
 
-	client, err := t.getClient(req.Context(), *input.Bucket)
-	if err != nil {
-		return handleError(req, err)
-	}
+	client := t.getBucketClient(bucket)
 
 	output, err := client.GetObject(req.Context(), input)
-	if err != nil {
-		if output != nil && output.Body != nil {
+
+	defer func() {
+		if err != nil && output != nil && output.Body != nil {
 			output.Body.Close()
 		}
+	}()
+
+	if err != nil {
+		// Check if the error is the region mismatch error.
+		// If so, create a new client with the correct region and retry the request.
+		if region := regionFromError(err); len(region) != 0 {
+			client, err = t.createBucketClient(req.Context(), bucket, region)
+			if err != nil {
+				return handleError(req, err)
+			}
+
+			output, err = client.GetObject(req.Context(), input)
+		}
+	}
 
+	if err != nil {
 		return handleError(req, err)
 	}
 
@@ -221,11 +233,7 @@ func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) {
 	}, nil
 }
 
-func (t *transport) getClient(ctx context.Context, bucket string) (s3Client, error) {
-	if !config.S3MultiRegion {
-		return t.defaultClient, nil
-	}
-
+func (t *transport) getBucketClient(bucket string) s3Client {
 	var client s3Client
 
 	func() {
@@ -235,27 +243,22 @@ func (t *transport) getClient(ctx context.Context, bucket string) (s3Client, err
 	}()
 
 	if client != nil {
-		return client, nil
+		return client
 	}
 
+	return t.defaultClient
+}
+
+func (t *transport) createBucketClient(ctx context.Context, bucket, region string) (s3Client, error) {
 	t.mu.Lock()
 	defer t.mu.Unlock()
 
 	// Check again if someone did this before us
-	if client = t.clientsByBucket[bucket]; client != nil {
+	if client := t.clientsByBucket[bucket]; client != nil {
 		return client, nil
 	}
 
-	region, err := s3Manager.GetBucketRegion(ctx, t.defaultClient, bucket)
-	if err != nil {
-		return nil, ierrors.Wrap(err, 0, ierrors.WithPrefix("can't get bucket region"))
-	}
-
-	if len(region) == 0 {
-		region = t.defaultConfig.Region
-	}
-
-	if client = t.clientsByRegion[region]; client != nil {
+	if client := t.clientsByRegion[region]; client != nil {
 		t.clientsByBucket[bucket] = client
 		return client, nil
 	}
@@ -263,7 +266,7 @@ func (t *transport) getClient(ctx context.Context, bucket string) (s3Client, err
 	conf := t.defaultConfig.Copy()
 	conf.Region = region
 
-	client, err = createClient(conf, t.clientOptions)
+	client, err := createClient(conf, t.clientOptions)
 	if err != nil {
 		return nil, ierrors.Wrap(err, 0, ierrors.WithPrefix("can't create regional S3 client"))
 	}
@@ -292,6 +295,19 @@ func createClient(conf aws.Config, opts []func(*s3.Options)) (s3Client, error) {
 	}
 }
 
+func regionFromError(err error) string {
+	var rerr *awsHttp.ResponseError
+	if !errors.As(err, &rerr) {
+		return ""
+	}
+
+	if rerr.Response == nil || rerr.Response.StatusCode != 301 {
+		return ""
+	}
+
+	return rerr.Response.Header.Get("X-Amz-Bucket-Region")
+}
+
 func handleError(req *http.Request, err error) (*http.Response, error) {
 	var rerr *awsHttp.ResponseError
 	if !errors.As(err, &rerr) {

+ 1 - 11
transport/s3/s3_test.go

@@ -46,8 +46,7 @@ func (s *S3TestSuite) SetupSuite() {
 	err = backend.CreateBucket("test")
 	s.Require().NoError(err)
 
-	svc, err := s.transport.(*transport).getClient(context.Background(), "test")
-	s.Require().NoError(err)
+	svc := s.transport.(*transport).defaultClient
 	s.Require().NotNil(svc)
 	s.Require().IsType(&s3.Client{}, svc)
 
@@ -158,15 +157,6 @@ func (s *S3TestSuite) TestRoundTripWithUpdatedLastModifiedReturns200() {
 	s.Require().Equal(http.StatusOK, response.StatusCode)
 }
 
-func (s *S3TestSuite) TestRoundTripWithMultiregionEnabledReturns200() {
-	config.S3MultiRegion = true
-	request, _ := http.NewRequest("GET", "s3://test/foo/test.png", nil)
-
-	response, err := s.transport.RoundTrip(request)
-	s.Require().NoError(err)
-	s.Require().Equal(200, response.StatusCode)
-}
-
 func TestS3Transport(t *testing.T) {
 	suite.Run(t, new(S3TestSuite))
 }