fix issue with runtime label (#87)

* update the runtime test to check for the watchBundle func

update adapter to deal with old and new runtime labels

add some parsing tests

* changes based on PR feedback

registries/adapters/adapter.go

@@ -25,6 +25,8 @@ import (
 	"net/url"
 	"strconv"
 
+	"fmt"
+
 	"github.com/automationbroker/bundle-lib/bundle"
 	log "github.com/sirupsen/logrus"
 	yaml "gopkg.in/yaml.v1"
@@ -57,21 +59,39 @@ type Configuration struct {
 	Tag        string
 }
 
-// Retrieve the spec from a registry manifest request
-func imageToSpec(req *http.Request, image string) (*bundle.Spec, error) {
-	log.Debug("Registry::imageToSpec")
-	spec := &bundle.Spec{}
-	req.Header.Add("Accept", "application/json")
+type registryResponseError struct {
+	code    int
+	message string
+}
+
+func (rre *registryResponseError) Error() string {
+	return fmt.Sprintf("unexpected registry response code: %v message: %v", rre.code, rre.message)
+}
 
-	resp, err := http.DefaultClient.Do(req)
+func registryResponseHandler(resp *http.Response) ([]byte, error) {
+	defer resp.Body.Close()
+	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
 		return nil, err
 	}
-	defer resp.Body.Close()
+	if resp.StatusCode == http.StatusUnauthorized {
+		return nil, &registryResponseError{code: resp.StatusCode, message: "Unable to authenticate to the registry, registry credentials could be invalid"}
+	}
+
+	if resp.StatusCode != http.StatusOK {
+		return nil, &registryResponseError{code: resp.StatusCode, message: fmt.Sprintf("unexpected response code %v body %v", resp.StatusCode, string(body))}
+	}
+	return body, nil
+}
 
+// Retrieve the spec from a registry manifest request
+func imageToSpec(imageDetails []byte, image string) (*bundle.Spec, error) {
+	log.Debug("Registry::imageToSpec")
+	spec := &bundle.Spec{}
 	type label struct {
-		Spec    string `json:"com.redhat.apb.spec"`
-		Runtime string `json:"com.redhat.bundle.runtime"`
+		Spec             string `json:"com.redhat.apb.spec"`
+		LegacyAPBRuntime string `json:"com.redhat.apb.runtime"`
+		BundleRuntime    string `json:"com.redhat.bundle.runtime"`
 	}
 
 	type config struct {
@@ -86,24 +106,8 @@ func imageToSpec(req *http.Request, image string) (*bundle.Spec, error) {
 		Config *config `json:"config"`
 	}{}
 
-	if resp.StatusCode == http.StatusUnauthorized {
-		log.Errorf("Unable to authenticate to the registry, registry credentials could be invalid.")
-		return nil, nil
-	}
-
-	// resp.Body is an io.Reader, which are a one time use.  Save the
-	// contents to a byte[] for debugging, then remake the io.Reader for the
-	// JSON decoder.
-	debug, _ := ioutil.ReadAll(resp.Body)
-	if resp.StatusCode != http.StatusOK {
-		log.Errorf("Image '%s' may not exist in registry.", image)
-		log.Error(string(debug))
-		return nil, nil
-	}
-
-	r := bytes.NewReader(debug)
-	err = json.NewDecoder(r).Decode(&hist)
-	if err != nil {
+	r := bytes.NewReader(imageDetails)
+	if err := json.NewDecoder(r).Decode(&hist); err != nil {
 		log.Errorf("Error grabbing JSON body from response: %s", err)
 		return nil, err
 	}
@@ -113,8 +117,7 @@ func imageToSpec(req *http.Request, image string) (*bundle.Spec, error) {
 		return nil, nil
 	}
 
-	err = json.Unmarshal([]byte(hist.History[0]["v1Compatibility"]), &conf)
-	if err != nil {
+	if err := json.Unmarshal([]byte(hist.History[0]["v1Compatibility"]), &conf); err != nil {
 		log.Errorf("Error unmarshalling intermediary JSON response: %s", err)
 		return nil, err
 	}
@@ -139,7 +142,14 @@ func imageToSpec(req *http.Request, image string) (*bundle.Spec, error) {
 		return nil, err
 	}
 
-	spec.Runtime, err = getAPBRuntimeVersion(conf.Config.Label.Runtime)
+	version := conf.Config.Label.LegacyAPBRuntime
+
+	// prefer bundle runtime
+	if conf.Config.Label.BundleRuntime != "" {
+		log.Debugf("bundle runtime present using this over apb runtime")
+		version = conf.Config.Label.BundleRuntime
+	}
+	spec.Runtime, err = getAPBRuntimeVersion(version)
 	if err != nil {
 		return nil, err
 	}
@@ -148,6 +158,7 @@ func imageToSpec(req *http.Request, image string) (*bundle.Spec, error) {
 
 	log.Debugf("adapter::imageToSpec -> Got plans %+v", spec.Plans)
 	log.Debugf("Successfully converted Image %s into Spec", spec.Image)
+	log.Infof("adapter::imageToSpec -> Image %s runtime is %d", spec.Image, spec.Runtime)
 
 	return spec, nil
 }

registries/adapters/adapter_test.go

@@ -17,11 +17,92 @@
 package adapters
 
 import (
+	"encoding/json"
 	"testing"
 
+	"fmt"
+
+	"github.com/automationbroker/bundle-lib/bundle"
 	ft "github.com/stretchr/testify/assert"
 )
 
 func TestSpecLabel(t *testing.T) {
 	ft.Equal(t, BundleSpecLabel, "com.redhat.apb.spec", "spec label does not match dockerhub")
 }
+
+func TestImageToSpec(t *testing.T) {
+	var testApbSpec = "dmVyc2lvbjogMS4wCm5hbWU6IG1lZGlhd2lraS1hcGIKZGVzY3JpcHRpb246IE1lZGlhd2lraSBhcGIgaW1wbGVtZW50YXRpb24KYmluZGFibGU6IEZhbHNlCmFzeW5jOiBvcHRpb25hbAptZXRhZGF0YToKICBkb2N1bWVudGF0aW9uVXJsOiBodHRwczovL3d3dy5tZWRpYXdpa2kub3JnL3dpa2kvRG9jdW1lbnRhdGlvbgogIGxvbmdEZXNjcmlwdGlvbjogQW4gYXBiIHRoYXQgZGVwbG95cyBNZWRpYXdpa2kgMS4yMwogIGRlcGVuZGVuY2llczogWydkb2NrZXIuaW8vYW5zaWJsZXBsYXlib29rYnVuZGxlL21lZGlhd2lraTEyMzpsYXRlc3QnXQogIGRpc3BsYXlOYW1lOiBNZWRpYXdpa2kgKEFQQikKICBjb25zb2xlLm9wZW5zaGlmdC5pby9pY29uQ2xhc3M6IGljb24tbWVkaWF3aWtpCiAgcHJvdmlkZXJEaXNwbGF5TmFtZTogIlJlZCBIYXQsIEluYy4iCnBsYW5zOgogIC0gbmFtZTogZGVmYXVsdAogICAgZGVzY3JpcHRpb246IEFuIEFQQiB0aGF0IGRlcGxveXMgTWVkaWFXaWtpCiAgICBmcmVlOiBUcnVlCiAgICBtZXRhZGF0YToKICAgICAgZGlzcGxheU5hbWU6IERlZmF1bHQKICAgICAgbG9uZ0Rlc2NyaXB0aW9uOiBUaGlzIHBsYW4gZGVwbG95cyBhIHNpbmdsZSBtZWRpYXdpa2kgaW5zdGFuY2Ugd2l0aG91dCBhIERCCiAgICAgIGNvc3Q6ICQwLjAwCiAgICBwYXJhbWV0ZXJzOgogICAgICAtIG5hbWU6IG1lZGlhd2lraV9kYl9zY2hlbWEKICAgICAgICBkZWZhdWx0OiBtZWRpYXdpa2kKICAgICAgICB0eXBlOiBzdHJpbmcKICAgICAgICB0aXRsZTogTWVkaWF3aWtpIERCIFNjaGVtYQogICAgICAgIHJlcXVpcmVkOiBUcnVlCiAgICAgIC0gbmFtZTogbWVkaWF3aWtpX3NpdGVfbmFtZQogICAgICAgIGRlZmF1bHQ6IE1lZGlhV2lraQogICAgICAgIHR5cGU6IHN0cmluZwogICAgICAgIHRpdGxlOiBNZWRpYXdpa2kgU2l0ZSBOYW1lCiAgICAgICAgcmVxdWlyZWQ6IFRydWUKICAgICAgICB1cGRhdGFibGU6IFRydWUKICAgICAgLSBuYW1lOiBtZWRpYXdpa2lfc2l0ZV9sYW5nCiAgICAgICAgZGVmYXVsdDogZW4KICAgICAgICB0eXBlOiBzdHJpbmcKICAgICAgICB0aXRsZTogTWVkaWF3aWtpIFNpdGUgTGFuZ3VhZ2UKICAgICAgICByZXF1aXJlZDogVHJ1ZQogICAgICAtIG5hbWU6IG1lZGlhd2lraV9hZG1pbl91c2VyCiAgICAgICAgZGVmYXVsdDogYWRtaW4KICAgICAgICB0eXBlOiBzdHJpbmcKICAgICAgICB0aXRsZTogTWVkaWF3aWtpIEFkbWluIFVzZXIKICAgICAgICByZXF1aXJlZDogVHJ1ZQogICAgICAtIG5hbWU6IG1lZGlhd2lraV9hZG1pbl9wYXNzCiAgICAgICAgdHlwZTogc3RyaW5nCiAgICAgICAgdGl0bGU6IE1lZGlhd2lraSBBZG1pbiBVc2VyIFBhc3N3b3JkCiAgICAgICAgcmVxdWlyZWQ6IFRydWUKICAgICAgICBkaXNwbGF5X3R5cGU6IHBhc3N3b3JkCg=="
+	type history struct {
+		History []map[string]string `json:"history"`
+	}
+	cases := []struct {
+		Name     string
+		Response history
+		Validate func(t *testing.T, spec *bundle.Spec)
+	}{
+		{
+			Name: "test spec parsed correctly and runtime version 1 and spec version 1.0 when no Label present",
+			Response: history{History: []map[string]string{
+				{
+					"v1Compatibility": fmt.Sprintf("{\"config\":{\"Labels\":{\"build-date\":\"20170801\",\"com.redhat.apb.spec\":\"%s\",\"com.redhat.apb.version\":\"0.1.0\"}}}", testApbSpec),
+				},
+			}},
+			Validate: func(t *testing.T, spec *bundle.Spec) {
+				if spec.Runtime != 1 {
+					t.Fatalf("Expected the runtime to be %v but it was %v", 1, spec.Runtime)
+				}
+				if spec.Version != "1.0" {
+					t.Fatalf("Expected the version to be %v but it was %v", "1.0", spec.Version)
+				}
+			},
+		},
+		{
+			Name: "test spec parsed correctly and runtime version 2 and spec version 1.0 when apb Label present",
+			Response: history{History: []map[string]string{
+				{
+					"v1Compatibility": fmt.Sprintf("{\"config\":{\"Labels\":{\"build-date\":\"20170801\",\"com.redhat.apb.spec\":\"%s\",\"com.redhat.apb.version\":\"0.1.0\",\"com.redhat.apb.runtime\":\"2\"}}}", testApbSpec),
+				},
+			}},
+			Validate: func(t *testing.T, spec *bundle.Spec) {
+				if spec.Runtime != 2 {
+					t.Fatalf("Expected the runtime to be %v but it was %v", 2, spec.Runtime)
+				}
+				if spec.Version != "1.0" {
+					t.Fatalf("Expected the version to be %v but it was %v", "1.0", spec.Version)
+				}
+			},
+		},
+		{
+			Name: "test spec parsed correctly and runtime version 2 and spec version 1.0 when bundle Label present",
+			Response: history{History: []map[string]string{
+				{
+					"v1Compatibility": fmt.Sprintf("{\"config\":{\"Labels\":{\"build-date\":\"20170801\",\"com.redhat.apb.spec\":\"%s\",\"com.redhat.apb.version\":\"0.1.0\",\"com.redhat.bundle.runtime\":\"2\"}}}", testApbSpec),
+				},
+			}},
+			Validate: func(t *testing.T, spec *bundle.Spec) {
+				if spec.Runtime != 2 {
+					t.Fatalf("Expected the runtime to be %v but it was %v", 2, spec.Runtime)
+				}
+				if spec.Version != "1.0" {
+					t.Fatalf("Expected the version to be %v but it was %v", "1.0", spec.Version)
+				}
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.Name, func(t *testing.T) {
+			b, err := json.Marshal(tc.Response)
+			if err != nil {
+				t.Fatalf("failed to marshal response from test case %v", err)
+			}
+			spec, err := imageToSpec(b, "maleck13/3scale-apb")
+			if err != nil {
+				t.Fatal(err)
+			}
+			if tc.Validate != nil {
+				tc.Validate(t, spec)
+			}
+		})
+	}
+}

registries/adapters/dockerhub_adapter.go

@@ -231,7 +231,16 @@ func (r DockerHubAdapter) loadSpec(imageName string) (*bundle.Spec, error) {
 		return nil, err
 	}
 	req.Header.Set("Authorization", fmt.Sprintf("Bearer %v", token))
-	return imageToSpec(req, fmt.Sprintf("%s/%s:%s", r.RegistryName(), imageName, r.Config.Tag))
+	req.Header.Add("Accept", "application/json")
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return nil, err
+	}
+	body, err := registryResponseHandler(resp)
+	if err != nil {
+		return nil, fmt.Errorf("DockerHubAdapter::error handling dockerhub registery response %s", err)
+	}
+	return imageToSpec(body, fmt.Sprintf("%s/%s:%s", r.RegistryName(), imageName, r.Config.Tag))
 }
 
 func (r DockerHubAdapter) getBearerToken(imageName string) (string, error) {

registries/adapters/openshift_adapter.go

@@ -144,5 +144,14 @@ func (r OpenShiftAdapter) loadSpec(imageName string) (*bundle.Spec, error) {
 		return nil, err
 	}
 	req.Header.Set("Authorization", fmt.Sprintf("Bearer %v", token))
-	return imageToSpec(req, fmt.Sprintf("%s/%s:%s", r.RegistryName(), imageName, r.Config.Tag))
+	req.Header.Add("Accept", "application/json")
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return nil, err
+	}
+	body, err := registryResponseHandler(resp)
+	if err != nil {
+		return nil, fmt.Errorf("OpenShiftAdapter::error handling openshift registery response %s", err)
+	}
+	return imageToSpec(body, fmt.Sprintf("%s/%s:%s", r.RegistryName(), imageName, r.Config.Tag))
 }

registries/adapters/rhcc_adapter.go

@@ -128,6 +128,15 @@ func (r RHCCAdapter) loadSpec(imageName string) (*bundle.Spec, error) {
 	if err != nil {
 		return nil, err
 	}
+	req.Header.Add("Accept", "application/json")
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return nil, err
+	}
+	body, err := registryResponseHandler(resp)
+	if err != nil {
+		return nil, fmt.Errorf("RHCCAdapter::error handling openshift registery response %s", err)
+	}
 
-	return imageToSpec(req, fmt.Sprintf("%s/%s:%s", r.RegistryName(), imageName, r.Config.Tag))
+	return imageToSpec(body, fmt.Sprintf("%s/%s:%s", r.RegistryName(), imageName, r.Config.Tag))
 }

runtime/runtime_test.go

@@ -300,6 +300,9 @@ func TestNewRuntime(t *testing.T) {
 			}
 			NewRuntime(tc.config)
 			p := Provider.(*provider)
+			if p.watchBundle == nil {
+				t.Fatalf("expected a watchBundle function to be defined but it was nil ")
+			}
 			if len(p.preSandboxCreate) != len(tc.expectedProvider.preSandboxCreate) {
 				t.Fatalf("invalid provider for configuration: %#+v \n\n got: %#+v \n\n exp: %#+v", tc.config, Provider, tc.expectedProvider)
 			}
@@ -318,6 +321,7 @@ func TestNewRuntime(t *testing.T) {
 			if !reflect.DeepEqual(tc.expectedProvider.ExtractedCredential, p.ExtractedCredential) {
 				t.Fatalf("invalid provider for configuration: %#+v \n\n got: %#+v \n\n exp: %#+v", tc.config, Provider, tc.expectedProvider)
 			}
+
 		})
 	}
 }