fix: Endpoint attributes conversion

pkg/apis/workspaces/v1alpha1/components_conversion_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
+	"github.com/devfile/api/v2/pkg/attributes"
 	"github.com/google/go-cmp/cmp"
 	fuzz "github.com/google/gofuzz"
 	"github.com/stretchr/testify/assert"
@@ -74,3 +75,127 @@ func TestComponentConversionFrom_v1alpha2(t *testing.T) {
 
 	assert.Equal(t, &Component{}, output, "Conversion from v1alpha2 should be skipped for Image Component")
 }
+
+// Endpoint attributes are typed `map[string]apiext.JSON` in v1alpha2 but `map[string]string` in
+// v1alpha1, so any non-string attribute value (e.g. `discoverable: true`) breaks the JSON round-trip
+// the conversion relies on. Non-string values must be converted to their string representation
+// instead of being dropped or failing the conversion.
+func TestComponentConversionFrom_v1alpha2_NonStringEndpointAttributes(t *testing.T) {
+	tests := []struct {
+		name       string
+		attributes attributes.Attributes
+		expected   map[string]string
+	}{
+		{
+			name:       "boolean attribute value",
+			attributes: attributes.Attributes{}.PutBoolean("discoverable", true),
+			expected:   map[string]string{"discoverable": "true"},
+		},
+		{
+			name:       "number attribute value",
+			attributes: attributes.Attributes{}.PutInteger("weight", 10),
+			expected:   map[string]string{"weight": "10"},
+		},
+		{
+			name:       "string attribute value",
+			attributes: attributes.Attributes{}.PutString("type", "terminal"),
+			expected:   map[string]string{"type": "terminal"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			src := &v1alpha2.Component{
+				Name: "postgresql",
+				ComponentUnion: v1alpha2.ComponentUnion{
+					Container: &v1alpha2.ContainerComponent{
+						Container: v1alpha2.Container{
+							Image: "postgres:latest",
+						},
+						Endpoints: []v1alpha2.Endpoint{
+							{
+								Name:       "postgresql",
+								TargetPort: 5432,
+								Exposure:   v1alpha2.InternalEndpointExposure,
+								Attributes: tt.attributes,
+							},
+						},
+					},
+				},
+			}
+			output := &Component{}
+
+			err := convertComponentFrom_v1alpha2(src, output)
+			if !assert.NoError(t, err, "Should not return error when converting from v1alpha2") {
+				return
+			}
+
+			if !assert.NotNil(t, output.Container, "Container component should be converted") {
+				return
+			}
+			if !assert.Len(t, output.Container.Endpoints, 1, "Endpoint should be converted") {
+				return
+			}
+			assert.Equal(t, tt.expected, output.Container.Endpoints[0].Attributes,
+				"Endpoint attributes should be converted to their string representation")
+		})
+	}
+}
+
+// Endpoint attributes of a container overridden by a plugin component go through their own JSON
+// round-trip, which checks the unmarshalling error and so fails the whole conversion with
+// "cannot unmarshal bool into Go struct field Endpoint.container.endpoints.attributes of type string".
+func TestPluginComponentConversionFrom_v1alpha2_NonStringEndpointAttributes(t *testing.T) {
+	src := &v1alpha2.Component{
+		Name: "my-plugin",
+		ComponentUnion: v1alpha2.ComponentUnion{
+			Plugin: &v1alpha2.PluginComponent{
+				ImportReference: v1alpha2.ImportReference{
+					ImportReferenceUnion: v1alpha2.ImportReferenceUnion{
+						Uri: "https://example.com/plugin.yaml",
+					},
+				},
+				PluginOverrides: v1alpha2.PluginOverrides{
+					Components: []v1alpha2.ComponentPluginOverride{
+						{
+							Name: "postgresql",
+							ComponentUnionPluginOverride: v1alpha2.ComponentUnionPluginOverride{
+								Container: &v1alpha2.ContainerComponentPluginOverride{
+									ContainerPluginOverride: v1alpha2.ContainerPluginOverride{
+										Image: "postgres:latest",
+									},
+									Endpoints: []v1alpha2.EndpointPluginOverride{
+										{
+											Name:       "postgresql",
+											TargetPort: 5432,
+											Attributes: attributes.Attributes{}.PutBoolean("discoverable", true),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	output := &Component{}
+
+	err := convertComponentFrom_v1alpha2(src, output)
+	if !assert.NoError(t, err, "Should not return error when converting from v1alpha2") {
+		return
+	}
+
+	if !assert.Len(t, output.Plugin.Components, 1, "Plugin component override should be converted") {
+		return
+	}
+	overriddenContainer := output.Plugin.Components[0].Container
+	if !assert.NotNil(t, overriddenContainer, "Container override should be converted") {
+		return
+	}
+	if !assert.Len(t, overriddenContainer.Endpoints, 1, "Endpoint should be converted") {
+		return
+	}
+	assert.Equal(t, map[string]string{"discoverable": "true"}, overriddenContainer.Endpoints[0].Attributes,
+		"Endpoint attributes should be converted to their string representation")
+}

pkg/apis/workspaces/v1alpha1/endpoint_conversion.go

@@ -0,0 +1,69 @@
+//
+//
+// Copyright Red Hat
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package v1alpha1
+
+import (
+	"encoding/json"
+
+	"github.com/devfile/api/v2/pkg/attributes"
+)
+
+// UnmarshalJSON decodes an Endpoint, converting attribute values that are not JSON strings into
+// their string representation.
+//
+// Endpoint attributes are free-form (`map[string]apiext.JSON`) in v1alpha2 but string-based
+// (`map[string]string`) in v1alpha1, so an attribute such as `discoverable: true` is valid in
+// v1alpha2 yet cannot be decoded as-is here. Conversion from v1alpha2 is implemented as a JSON
+// round-trip, so without this the conversion of an endpoint holding a non-string attribute would
+// fail with "cannot unmarshal bool into Go struct field Endpoint.container.endpoints.attributes of
+// type string".
+func (endpoint *Endpoint) UnmarshalJSON(data []byte) error {
+	// The alias prevents json.Unmarshal from recursing into this method, and the shadowing
+	// `Attributes` field captures the free-form attributes before they reach the string-based one.
+	type endpointAlias Endpoint
+
+	decoded := &struct {
+		Attributes attributes.Attributes `json:"attributes,omitempty"`
+		*endpointAlias
+	}{
+		endpointAlias: (*endpointAlias)(endpoint),
+	}
+	if err := json.Unmarshal(data, decoded); err != nil {
+		return err
+	}
+	endpoint.Attributes = stringifyAttributes(decoded.Attributes)
+	return nil
+}
+
+// stringifyAttributes converts free-form attributes into the string-based map v1alpha1 expects.
+// Strings, booleans and numbers are converted to their string representation; any other value
+// (an object or an array) keeps its raw JSON representation so that nothing is silently dropped.
+func stringifyAttributes(attrs attributes.Attributes) map[string]string {
+	if attrs == nil {
+		return nil
+	}
+	stringAttributes := make(map[string]string, len(attrs))
+	for key, value := range attrs {
+		var err error
+		stringValue := attrs.GetString(key, &err)
+		if err != nil {
+			stringValue = string(value.Raw)
+		}
+		stringAttributes[key] = stringValue
+	}
+	return stringAttributes
+}