Fix URL building

Previous to this commit the URL built to query the SUSE images could contain a consecutive double slash (e.g.: https://susepubliccloudinfo.suse.com/v1//amazon/eu-west-1/images/active.json) This lead to a 400 response from the remote server. This commit makes URL building reliable.
SUSE · Jul 6, 2021 · d0d99cd · d0d99cd
1 parent a7ac17a
commit d0d99cd
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 26 deletions.
diff --git a/pkg/info-service/images.go b/pkg/info-service/images.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+	"path/filepath"
 	"regexp"
 	"sort"
 	"time"
@@ -46,6 +47,7 @@ type imagesReply struct {
 // images
 type SearchParams struct {
 	APIEndpoint   string
+	APIVersion    string
 	Cloud         string
 	NameRegex     string
 	Region        string
@@ -55,7 +57,10 @@ type SearchParams struct {
 
 // APIEndpoint is the endoint of the public instance of
 // https://github.com/SUSE-Enceladus/public-cloud-info-service
-const APIEndpoint = "https://susepubliccloudinfo.suse.com/v1/"
+const APIEndpoint = "https://susepubliccloudinfo.suse.com"
+
+// APIVersion is the version of the enceladus API to be queried
+const APIVersion = "v1"
 
 // ValidImageStates holds the valid states of public cloud images as documented here:
 // https://github.com/SUSE-Enceladus/public-cloud-info-service#server-design
@@ -78,34 +83,44 @@ func GetImages(params SearchParams) ([]Image, error) {
 		params.APIEndpoint = APIEndpoint
 	}
 
-	u, err := url.Parse(fmt.Sprintf(
-		"%s/%s/%s/images/%s.json",
-		params.APIEndpoint,
+	if params.APIVersion == "" {
+		params.APIVersion = APIVersion
+	}
+
+	baseURL, err := url.Parse(params.APIEndpoint)
+	if err != nil {
+		return images, err
+	}
+
+	urlPath := filepath.Join(
+		params.APIVersion,
 		params.Cloud,
 		params.Region,
-		params.State))
+		"images",
+		fmt.Sprintf("%s.json", params.State))
+	relURL, err := baseURL.Parse(urlPath)
 	if err != nil {
 		return images, err
 	}
 
-	resp, err := http.Get(u.String())
+	resp, err := http.Get(relURL.String())
 	if err != nil {
 		return images,
-			fmt.Errorf("Error while accessing %v: %v", u, err)
+			fmt.Errorf("Error while accessing %v: %v", relURL, err)
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode != 200 {
 		return images,
 			fmt.Errorf("Unexpected HTTP status %d while accessing %v",
-				resp.StatusCode, u)
+				resp.StatusCode, relURL)
 	}
 
 	var reply imagesReply
 	if err = json.NewDecoder(resp.Body).Decode(&reply); err != nil {
 		return images,
 			fmt.Errorf("Error while decoding remote response from %s: %v",
-				u, err)
+				relURL, err)
 	}
 
 	if params.NameRegex != "" {

diff --git a/pkg/info-service/images_test.go b/pkg/info-service/images_test.go
@@ -11,12 +11,14 @@ import (
 
 func TestValidRequestForActiveImages(t *testing.T) {
 	params := SearchParams{
-		Cloud:  "amazon",
-		Region: "eu-central-1",
-		State:  "active",
+		APIVersion: "v1",
+		Cloud:      "amazon",
+		Region:     "eu-central-1",
+		State:      "active",
 	}
 	expectedRequest := fmt.Sprintf(
-		"/%s/%s/images/%s.json",
+		"/%s/%s/%s/images/%s.json",
+		params.APIVersion,
 		params.Cloud,
 		params.Region,
 		params.State)
@@ -50,13 +52,15 @@ func TestValidRequestForActiveImages(t *testing.T) {
 
 func TestFilterImages(t *testing.T) {
 	params := SearchParams{
-		Cloud:     "amazon",
-		Region:    "eu-central-1",
-		State:     "active",
-		NameRegex: "suse-sles-15-sp1-byos.*-hvm-ssd-x86_64",
+		APIVersion: "v1",
+		Cloud:      "amazon",
+		Region:     "eu-central-1",
+		State:      "active",
+		NameRegex:  "suse-sles-15-sp1-byos.*-hvm-ssd-x86_64",
 	}
 	expectedRequest := fmt.Sprintf(
-		"/%s/%s/images/%s.json",
+		"/%s/%s/%s/images/%s.json",
+		params.APIVersion,
 		params.Cloud,
 		params.Region,
 		params.State)
@@ -90,14 +94,16 @@ func TestFilterImages(t *testing.T) {
 
 func TestSortAscendingImages(t *testing.T) {
 	params := SearchParams{
+		APIVersion:    "v1",
 		Cloud:         "amazon",
 		Region:        "eu-central-1",
 		State:         "active",
 		SortAscending: true,
 		NameRegex:     "suse-sles-.*-sapcal.*-hvm-ssd-x86_64",
 	}
 	expectedRequest := fmt.Sprintf(
-		"/%s/%s/images/%s.json",
+		"/%s/%s/%s/images/%s.json",
+		params.APIVersion,
 		params.Cloud,
 		params.Region,
 		params.State)
@@ -125,7 +131,7 @@ func TestSortAscendingImages(t *testing.T) {
 		t.Fatal("It should've run just fine...")
 	}
 	if len(images) != 3 {
-		t.Fatalf("Unexpected number of images found. Got %d, expected %d", len(images), 1)
+		t.Fatalf("Unexpected number of images found. Got %d, expected %d", len(images), 3)
 	}
 
 	expectedIDs := []string{
@@ -143,13 +149,15 @@ func TestSortAscendingImages(t *testing.T) {
 func TestSortDescendingImages(t *testing.T) {
 	// descending order is the default one
 	params := SearchParams{
-		Cloud:     "amazon",
-		Region:    "eu-central-1",
-		State:     "active",
-		NameRegex: "suse-sles-.*-sapcal.*-hvm-ssd-x86_64",
+		APIVersion: "v1",
+		Cloud:      "amazon",
+		Region:     "eu-central-1",
+		State:      "active",
+		NameRegex:  "suse-sles-.*-sapcal.*-hvm-ssd-x86_64",
 	}
 	expectedRequest := fmt.Sprintf(
-		"/%s/%s/images/%s.json",
+		"/%s/%s/%s/images/%s.json",
+		params.APIVersion,
 		params.Cloud,
 		params.Region,
 		params.State)
@@ -177,7 +185,7 @@ func TestSortDescendingImages(t *testing.T) {
 		t.Fatal("It should've run just fine...")
 	}
 	if len(images) != 3 {
-		t.Fatalf("Unexpected number of images found. Got %d, expected %d", len(images), 1)
+		t.Fatalf("Unexpected number of images found. Got %d, expected %d", len(images), 3)
 	}
 
 	expectedIDs := []string{