Skip to content

Commit

Permalink
fix: Remove indirect import of AWS package (#1585)
Browse files Browse the repository at this point in the history
Signed-off-by: Mateus Oliveira <[email protected]>
  • Loading branch information
mateusoliveira43 authored Nov 11, 2024
1 parent dd2ddc9 commit 738a1a0
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 66 deletions.
2 changes: 1 addition & 1 deletion controllers/bsl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
20 changes: 0 additions & 20 deletions pkg/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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.
Expand Down
45 changes: 0 additions & 45 deletions pkg/common/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions pkg/storage/aws/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
46 changes: 46 additions & 0 deletions pkg/storage/aws/s3_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package aws

import (
"reflect"
"testing"
)

Expand Down Expand Up @@ -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)
}
})
}
}

0 comments on commit 738a1a0

Please sign in to comment.