diff --git a/pkg/apis/workspaces/v1alpha1/components_conversion_test.go b/pkg/apis/workspaces/v1alpha1/components_conversion_test.go index 7ef53ed0c..4f5d6cbab 100644 --- a/pkg/apis/workspaces/v1alpha1/components_conversion_test.go +++ b/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") +} diff --git a/pkg/apis/workspaces/v1alpha1/endpoint_conversion.go b/pkg/apis/workspaces/v1alpha1/endpoint_conversion.go new file mode 100644 index 000000000..a49f69723 --- /dev/null +++ b/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 +}