diff --git a/controllers/bsl.go b/controllers/bsl.go index 938014a9b1..29f864ff63 100644 --- a/controllers/bsl.go +++ b/controllers/bsl.go @@ -284,7 +284,7 @@ func (r *DPAReconciler) updateBSLFromSpec(bsl *velerov1.BackupStorageLocation, b if bslSpec.Provider == "aws" && bslSpec.Config != nil { s3Url := bslSpec.Config["s3Url"] if len(s3Url) > 0 { - if s3Url, err = common.StripDefaultPorts(s3Url); err == nil { + if s3Url, err = aws.StripDefaultPorts(s3Url); err == nil { bslSpec.Config["s3Url"] = s3Url } } diff --git a/pkg/common/common.go b/pkg/common/common.go index d031b125ec..dc5a72aa07 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -2,14 +2,11 @@ package common import ( "fmt" - "net/http" - "net/url" "os" "regexp" "sort" "strings" - "github.com/aws/aws-sdk-go/aws/request" "github.com/vmware-tanzu/velero/pkg/types" corev1 "k8s.io/api/core/v1" ) @@ -210,23 +207,6 @@ func CCOWorkflow() bool { return false } -// StripDefaultPorts removes port 80 from HTTP URLs and 443 from HTTPS URLs. -// Defer to the actual AWS SDK implementation to match its behavior exactly. -func StripDefaultPorts(fromUrl string) (string, error) { - u, err := url.Parse(fromUrl) - if err != nil { - return "", err - } - r := http.Request{ - URL: u, - } - request.SanitizeHostForHeader(&r) - if r.Host != "" { - r.URL.Host = r.Host - } - return r.URL.String(), nil -} - // GetImagePullPolicy get imagePullPolicy for a container, based on its image, if an override is not provided. // If override is provided, use the override imagePullPolicy. // If image contains a sha256 or sha512 digest, use IfNotPresent; otherwise, Always. diff --git a/pkg/common/common_test.go b/pkg/common/common_test.go index a5d8972a3c..b5c5868e82 100644 --- a/pkg/common/common_test.go +++ b/pkg/common/common_test.go @@ -107,51 +107,6 @@ func TestAppendTTMapAsCopy(t *testing.T) { }) } -func TestStripDefaultPorts(t *testing.T) { - tests := []struct { - name string - base string - want string - }{ - { - name: "port-free URL is returned unchanged", - base: "https://s3.region.cloud-object-storage.appdomain.cloud/bucket-name", - want: "https://s3.region.cloud-object-storage.appdomain.cloud/bucket-name", - }, - { - name: "HTTPS port is removed from URL", - base: "https://s3.region.cloud-object-storage.appdomain.cloud:443/bucket-name", - want: "https://s3.region.cloud-object-storage.appdomain.cloud/bucket-name", - }, - { - name: "HTTP port is removed from URL", - base: "http://s3.region.cloud-object-storage.appdomain.cloud:80/bucket-name", - want: "http://s3.region.cloud-object-storage.appdomain.cloud/bucket-name", - }, - { - name: "alternate HTTP port is preserved", - base: "http://10.0.188.30:9000", - want: "http://10.0.188.30:9000", - }, - { - name: "alternate HTTPS port is preserved", - base: "https://10.0.188.30:9000", - want: "https://10.0.188.30:9000", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := StripDefaultPorts(tt.base) - if err != nil { - t.Errorf("An error occurred: %v", err) - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("StripDefaultPorts() = %v, want %v", got, tt.want) - } - }) - } -} - func TestGetImagePullPolicy(t *testing.T) { tests := []struct { name string diff --git a/pkg/storage/aws/s3.go b/pkg/storage/aws/s3.go index d34c77ea8c..3a7d25b417 100644 --- a/pkg/storage/aws/s3.go +++ b/pkg/storage/aws/s3.go @@ -3,11 +3,14 @@ package aws import ( "context" "errors" + "net/http" + "net/url" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/feature/s3/manager" "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/aws-sdk-go/aws/request" ) func BucketRegionIsDiscoverable(bucket string) bool { @@ -40,3 +43,20 @@ func GetBucketRegion(bucket string) (string, error) { } return "", errors.New("unable to determine bucket's region: " + err.Error()) } + +// StripDefaultPorts removes port 80 from HTTP URLs and 443 from HTTPS URLs. +// Defer to the actual AWS SDK implementation to match its behavior exactly. +func StripDefaultPorts(fromUrl string) (string, error) { + u, err := url.Parse(fromUrl) + if err != nil { + return "", err + } + r := http.Request{ + URL: u, + } + request.SanitizeHostForHeader(&r) + if r.Host != "" { + r.URL.Host = r.Host + } + return r.URL.String(), nil +} diff --git a/pkg/storage/aws/s3_test.go b/pkg/storage/aws/s3_test.go index aeb493e0be..4843808034 100644 --- a/pkg/storage/aws/s3_test.go +++ b/pkg/storage/aws/s3_test.go @@ -1,6 +1,7 @@ package aws import ( + "reflect" "testing" ) @@ -82,3 +83,48 @@ func TestBucketRegionIsDiscoverable(t *testing.T) { }) } } + +func TestStripDefaultPorts(t *testing.T) { + tests := []struct { + name string + base string + want string + }{ + { + name: "port-free URL is returned unchanged", + base: "https://s3.region.cloud-object-storage.appdomain.cloud/bucket-name", + want: "https://s3.region.cloud-object-storage.appdomain.cloud/bucket-name", + }, + { + name: "HTTPS port is removed from URL", + base: "https://s3.region.cloud-object-storage.appdomain.cloud:443/bucket-name", + want: "https://s3.region.cloud-object-storage.appdomain.cloud/bucket-name", + }, + { + name: "HTTP port is removed from URL", + base: "http://s3.region.cloud-object-storage.appdomain.cloud:80/bucket-name", + want: "http://s3.region.cloud-object-storage.appdomain.cloud/bucket-name", + }, + { + name: "alternate HTTP port is preserved", + base: "http://10.0.188.30:9000", + want: "http://10.0.188.30:9000", + }, + { + name: "alternate HTTPS port is preserved", + base: "https://10.0.188.30:9000", + want: "https://10.0.188.30:9000", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := StripDefaultPorts(tt.base) + if err != nil { + t.Errorf("An error occurred: %v", err) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("StripDefaultPorts() = %v, want %v", got, tt.want) + } + }) + } +}