diff --git a/pkg/services/cloudmigration/gmsclient/gms_client.go b/pkg/services/cloudmigration/gmsclient/gms_client.go index 441e8b1d9e6..18f4c0539a7 100644 --- a/pkg/services/cloudmigration/gmsclient/gms_client.go +++ b/pkg/services/cloudmigration/gmsclient/gms_client.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "net/http" + "net/url" "strings" "sync" "time" @@ -40,7 +41,10 @@ type gmsClientImpl struct { func (c *gmsClientImpl) ValidateKey(ctx context.Context, cm cloudmigration.CloudMigrationSession) (err error) { // TODO: there is a lot of boilerplate code in these methods, we should consolidate them when we have a gardening period - path := fmt.Sprintf("%s/api/v1/validate-key", c.buildBasePath(cm.ClusterSlug)) + path, err := c.buildURL(cm.ClusterSlug, "/api/v1/validate-key") + if err != nil { + return err + } ctx, cancel := context.WithTimeout(ctx, c.cfg.CloudMigration.GMSValidateKeyTimeout) defer cancel() @@ -78,7 +82,10 @@ func (c *gmsClientImpl) ValidateKey(ctx context.Context, cm cloudmigration.Cloud } func (c *gmsClientImpl) StartSnapshot(ctx context.Context, session cloudmigration.CloudMigrationSession) (out *cloudmigration.StartSnapshotResponse, err error) { - path := fmt.Sprintf("%s/api/v1/start-snapshot", c.buildBasePath(session.ClusterSlug)) + path, err := c.buildURL(session.ClusterSlug, "/api/v1/start-snapshot") + if err != nil { + return nil, err + } ctx, cancel := context.WithTimeout(ctx, c.cfg.CloudMigration.GMSStartSnapshotTimeout) defer cancel() @@ -123,7 +130,11 @@ func (c *gmsClientImpl) GetSnapshotStatus(ctx context.Context, session cloudmigr c.getStatusMux.Lock() defer c.getStatusMux.Unlock() - path := fmt.Sprintf("%s/api/v1/snapshots/%s/status?offset=%d", c.buildBasePath(session.ClusterSlug), snapshot.GMSSnapshotUID, offset) + path, err := c.buildURL(session.ClusterSlug, fmt.Sprintf("/api/v1/snapshots/%s/status?offset=%d", snapshot.GMSSnapshotUID, offset)) + if err != nil { + c.log.Error("error parsing snapshot status url", "err", err.Error()) + return nil, err + } ctx, cancel := context.WithTimeout(ctx, c.cfg.CloudMigration.GMSGetSnapshotStatusTimeout) defer cancel() @@ -167,7 +178,11 @@ func (c *gmsClientImpl) GetSnapshotStatus(ctx context.Context, session cloudmigr } func (c *gmsClientImpl) CreatePresignedUploadUrl(ctx context.Context, session cloudmigration.CloudMigrationSession, snapshot cloudmigration.CloudMigrationSnapshot) (string, error) { - path := fmt.Sprintf("%s/api/v1/snapshots/%s/create-upload-url", c.buildBasePath(session.ClusterSlug), snapshot.GMSSnapshotUID) + path, err := c.buildURL(session.ClusterSlug, fmt.Sprintf("/api/v1/snapshots/%s/create-upload-url", snapshot.GMSSnapshotUID)) + if err != nil { + c.log.Error("error parsing upload url", "err", err.Error()) + return "", err + } ctx, cancel := context.WithTimeout(ctx, c.cfg.CloudMigration.GMSCreateUploadUrlTimeout) defer cancel() @@ -217,7 +232,11 @@ func (c *gmsClientImpl) ReportEvent(ctx context.Context, session cloudmigration. ctx, cancel := context.WithTimeout(ctx, c.cfg.CloudMigration.GMSReportEventTimeout) defer cancel() - path := fmt.Sprintf("%s/api/v1/events", c.buildBasePath(session.ClusterSlug)) + path, err := c.buildURL(session.ClusterSlug, "/api/v1/events") + if err != nil { + c.log.Error("parsing events url", "err", err.Error()) + return + } var buf bytes.Buffer if err := json.NewEncoder(&buf).Encode(event); err != nil { @@ -255,12 +274,21 @@ func (c *gmsClientImpl) ReportEvent(ctx context.Context, session cloudmigration. }() } -func (c *gmsClientImpl) buildBasePath(clusterSlug string) string { +func (c *gmsClientImpl) buildURL(clusterSlug, path string) (string, error) { domain := c.cfg.CloudMigration.GMSDomain + baseURL := fmt.Sprintf("https://cms-%s.%s/cloud-migrations", clusterSlug, domain) + + // Override the host if we are configuring it with a scheme prefix. if strings.HasPrefix(domain, "http://") || strings.HasPrefix(domain, "https://") { - return domain + baseURL = domain } - return fmt.Sprintf("https://cms-%s.%s/cloud-migrations", clusterSlug, domain) + + parsed, err := url.Parse(baseURL + path) + if err != nil { + return "", fmt.Errorf("building url: %w", err) + } + + return parsed.String(), nil } // handleGMSErrors parses the error message from GMS and translates it to an appropriate error message diff --git a/pkg/services/cloudmigration/gmsclient/gms_client_test.go b/pkg/services/cloudmigration/gmsclient/gms_client_test.go index d89d8d2acfb..449c64decf7 100644 --- a/pkg/services/cloudmigration/gmsclient/gms_client_test.go +++ b/pkg/services/cloudmigration/gmsclient/gms_client_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/require" ) -func Test_buildBasePath(t *testing.T) { +func Test_buildURL(t *testing.T) { t.Parallel() // Domain is required @@ -38,6 +38,7 @@ func Test_buildBasePath(t *testing.T) { description string domain string clusterSlug string + path string expected string }{ { @@ -52,17 +53,34 @@ func Test_buildBasePath(t *testing.T) { clusterSlug: "anything", expected: "https://some-domain:8080", }, + { + description: "domain starts with https://, should return domain", + domain: "https://some-domain:8080", + clusterSlug: "anything", + path: "/test?foo=bar&baz=qax#fragment", + expected: "https://some-domain:8080/test?foo=bar&baz=qax#fragment", + }, { description: "domain doesn't start with http or https, should build a string using the domain and clusterSlug", domain: "gms-dev", clusterSlug: "us-east-1", expected: "https://cms-us-east-1.gms-dev/cloud-migrations", }, + { + description: "it parses and escapes the path when building the URL", + domain: "gms-dev", + clusterSlug: "use-east-1", + path: `/this//is//a/\very-Nice_páTh?x=/çç&y=/éé#aaaa`, + expected: "https://cms-use-east-1.gms-dev/cloud-migrations/this//is//a/%5Cvery-Nice_p%C3%A1Th?x=/çç&y=/éé#aaaa", + }, } for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { client.cfg.CloudMigration.GMSDomain = tt.domain - assert.Equal(t, tt.expected, client.buildBasePath(tt.clusterSlug)) + + url, err := client.buildURL(tt.clusterSlug, tt.path) + assert.NoError(t, err) + assert.Equal(t, tt.expected, url) }) } }