From f99b2183d13efb3bf16dc944aee485f9ca35f827 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Tue, 6 Oct 2026 12:17:40 +0200 Subject: [PATCH] fix: convert non-string endpoint attributes to v1alpha1 Endpoint attributes are free-form (`map[string]apiext.JSON`) in v1alpha2 but string-based (`map[string]string`) in v1alpha1. Conversion from v1alpha2 is implemented as a JSON round-trip, so an endpoint holding a non-string attribute such as `discoverable: true` failed to convert with: cannot unmarshal bool into Go struct field Endpoint.container.endpoints.attributes of type string Rewrite the endpoint attributes of the marshalled component into their string representation before decoding into v1alpha1. Values keep their verbatim JSON text rather than going through `GetString`, so no precision is lost: `1048576` becomes "1048576" and not "1.048576e+06". The three conversion paths that decode a component from v1alpha2 are covered: plain components, plugin component overrides and parent component overrides, across the container, kubernetes and openshift component types. Co-Authored-By: Claude Opus 5 Signed-off-by: Anatolii Bazko --- .../v1alpha1/component_plugin_conversion.go | 4 + .../v1alpha1/components_conversion.go | 7 + .../v1alpha1/endpoint_conversion.go | 151 ++++++++++++ .../v1alpha1/endpoint_conversion_test.go | 230 ++++++++++++++++++ .../workspaces/v1alpha1/parent_conversion.go | 4 + 5 files changed, 396 insertions(+) create mode 100644 pkg/apis/workspaces/v1alpha1/endpoint_conversion.go create mode 100644 pkg/apis/workspaces/v1alpha1/endpoint_conversion_test.go diff --git a/pkg/apis/workspaces/v1alpha1/component_plugin_conversion.go b/pkg/apis/workspaces/v1alpha1/component_plugin_conversion.go index 22faea499..501dd141b 100644 --- a/pkg/apis/workspaces/v1alpha1/component_plugin_conversion.go +++ b/pkg/apis/workspaces/v1alpha1/component_plugin_conversion.go @@ -168,6 +168,10 @@ func convertPluginComponentSubComponentFrom_v1alpha2(src *v1alpha2.ComponentPlug if err != nil { return err } + jsonComponent, err = stringifyComponentEndpointAttributes(jsonComponent) + if err != nil { + return err + } err = json.Unmarshal(jsonComponent, &dest) if err != nil { return err diff --git a/pkg/apis/workspaces/v1alpha1/components_conversion.go b/pkg/apis/workspaces/v1alpha1/components_conversion.go index 3b5947b9b..776bb8dfa 100644 --- a/pkg/apis/workspaces/v1alpha1/components_conversion.go +++ b/pkg/apis/workspaces/v1alpha1/components_conversion.go @@ -56,7 +56,14 @@ func convertComponentFrom_v1alpha2(src *v1alpha2.Component, dest *Component) err if err != nil { return err } + jsonComponent, err = stringifyComponentEndpointAttributes(jsonComponent) + if err != nil { + return err + } err = json.Unmarshal(jsonComponent, dest) + if err != nil { + return err + } switch { case dest.Container != nil: dest.Container.Name = name diff --git a/pkg/apis/workspaces/v1alpha1/endpoint_conversion.go b/pkg/apis/workspaces/v1alpha1/endpoint_conversion.go new file mode 100644 index 000000000..1dec9e1ed --- /dev/null +++ b/pkg/apis/workspaces/v1alpha1/endpoint_conversion.go @@ -0,0 +1,151 @@ +// +// +// 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" +) + +// componentsWithEndpoints lists the keys of the component types that declare endpoints. +var componentsWithEndpoints = []string{"container", "kubernetes", "openshift"} + +// stringifyComponentEndpointAttributes rewrites the endpoint attributes of a marshalled v1alpha2 +// component so that it can be decoded into its v1alpha1 counterpart. +// +// 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 a component holding an endpoint with a non-string +// attribute would fail with "cannot unmarshal bool into Go struct field +// Endpoint.container.endpoints.attributes of type string". +// +// All the v1alpha2 component flavours (`Component`, `ComponentPluginOverride` and +// `ComponentParentOverride`) share the same JSON representation, so this operates on the +// marshalled bytes rather than on the Go types. +func stringifyComponentEndpointAttributes(data []byte) ([]byte, error) { + component := map[string]json.RawMessage{} + if err := json.Unmarshal(data, &component); err != nil { + return nil, err + } + + rewritten := false + for _, componentType := range componentsWithEndpoints { + body, found := component[componentType] + if !found { + continue + } + updatedBody, updated, err := stringifyEndpointAttributes(body) + if err != nil { + return nil, err + } + if updated { + component[componentType] = updatedBody + rewritten = true + } + } + + // Leave the document untouched when there is nothing to convert. + if !rewritten { + return data, nil + } + return json.Marshal(component) +} + +// stringifyEndpointAttributes rewrites the attributes of the endpoints declared by a marshalled +// component body, and reports whether anything was rewritten. +func stringifyEndpointAttributes(body json.RawMessage) (json.RawMessage, bool, error) { + componentBody := map[string]json.RawMessage{} + if err := json.Unmarshal(body, &componentBody); err != nil { + return nil, false, err + } + rawEndpoints, found := componentBody["endpoints"] + if !found { + return nil, false, nil + } + + var endpoints []map[string]json.RawMessage + if err := json.Unmarshal(rawEndpoints, &endpoints); err != nil { + return nil, false, err + } + + rewritten := false + for _, endpoint := range endpoints { + rawAttributes, found := endpoint["attributes"] + if !found { + continue + } + freeFormAttributes := attributes.Attributes{} + if err := json.Unmarshal(rawAttributes, &freeFormAttributes); err != nil { + return nil, false, err + } + if len(freeFormAttributes) == 0 { + continue + } + stringAttributes, err := json.Marshal(stringifyAttributes(freeFormAttributes)) + if err != nil { + return nil, false, err + } + endpoint["attributes"] = stringAttributes + rewritten = true + } + + if !rewritten { + return nil, false, nil + } + + updatedEndpoints, err := json.Marshal(endpoints) + if err != nil { + return nil, false, err + } + componentBody["endpoints"] = updatedEndpoints + + updatedBody, err := json.Marshal(componentBody) + if err != nil { + return nil, false, err + } + return updatedBody, true, nil +} + +// stringifyAttributes converts free-form attributes into the string-based map v1alpha1 expects. +// A JSON string is unquoted; every other value (boolean, number, object, array) keeps its verbatim +// JSON text, so that no precision is lost: `1048576` becomes "1048576", and not the "1.048576e+06" +// that `attributes.Attributes.GetString` would produce. +// +// The conversion is one way. Nothing parses these strings back when converting to v1alpha2 again, +// so `discoverable: true` comes out of a v1alpha2 -> v1alpha1 -> v1alpha2 round-trip as the string +// "true". Scalars stay usable, because `GetBoolean` and `GetNumber` fall back to strconv when the +// attribute holds a string, but an object or an array comes back as a string and no longer decodes +// with `GetInto`. Parsing the strings back is not an option: a string attribute the user actually +// authored as "true" cannot be told apart from a stringified boolean. +func stringifyAttributes(attrs attributes.Attributes) map[string]string { + stringAttributes := make(map[string]string, len(attrs)) + for key, value := range attrs { + // A JSON `null` is decoded into an empty Raw by apiext.JSON. + if len(value.Raw) == 0 { + stringAttributes[key] = "null" + continue + } + var stringValue string + if err := json.Unmarshal(value.Raw, &stringValue); err != nil { + stringValue = string(value.Raw) + } + stringAttributes[key] = stringValue + } + return stringAttributes +} diff --git a/pkg/apis/workspaces/v1alpha1/endpoint_conversion_test.go b/pkg/apis/workspaces/v1alpha1/endpoint_conversion_test.go new file mode 100644 index 000000000..c36e1171f --- /dev/null +++ b/pkg/apis/workspaces/v1alpha1/endpoint_conversion_test.go @@ -0,0 +1,230 @@ +// +// +// 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 ( + "testing" + + "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/api/v2/pkg/attributes" + "github.com/stretchr/testify/assert" +) + +// 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_EndpointAttributes(t *testing.T) { + for _, tt := range getEndpointAttributeConversionTestCases() { + 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_EndpointAttributes(t *testing.T) { + for _, tt := range getEndpointAttributeConversionTestCases() { + t.Run(tt.name, func(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: 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.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, tt.expected, overriddenContainer.Endpoints[0].Attributes, + "Endpoint attributes should be converted to their string representation") + }) + } +} + +// Endpoint attributes of a component overridden by a parent go through their own JSON round-trip, +// and so have to handle non-string attributes as well. Kubernetes and Openshift components declare +// endpoints too. +func TestParentComponentConversionFrom_v1alpha2_EndpointAttributes(t *testing.T) { + for _, tt := range getEndpointAttributeConversionTestCases() { + t.Run(tt.name, func(t *testing.T) { + src := &v1alpha2.ComponentParentOverride{ + Name: "postgresql", + ComponentUnionParentOverride: v1alpha2.ComponentUnionParentOverride{ + Kubernetes: &v1alpha2.KubernetesComponentParentOverride{ + K8sLikeComponentParentOverride: v1alpha2.K8sLikeComponentParentOverride{ + K8sLikeComponentLocationParentOverride: v1alpha2.K8sLikeComponentLocationParentOverride{ + Inlined: "kubernetes-resource", + }, + Endpoints: []v1alpha2.EndpointParentOverride{ + { + Name: "postgresql", + TargetPort: 5432, + Attributes: tt.attributes, + }, + }, + }, + }, + }, + } + output := &Component{} + + err := convertParentComponentFrom_v1alpha2(src, output) + if !assert.NoError(t, err, "Should not return error when converting from v1alpha2") { + return + } + + if !assert.NotNil(t, output.Kubernetes, "Kubernetes component should be converted") { + return + } + if !assert.Len(t, output.Kubernetes.Endpoints, 1, "Endpoint should be converted") { + return + } + assert.Equal(t, tt.expected, output.Kubernetes.Endpoints[0].Attributes, + "Endpoint attributes should be converted to their string representation") + }) + } +} + +func getEndpointAttributeConversionTestCases() []struct { + name string + attributes attributes.Attributes + expected map[string]string +} { + return []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"}, + }, + { + name: "large number attribute value", + attributes: attributes.Attributes{}.PutInteger("size", 1048576), + expected: map[string]string{"size": "1048576"}, + }, + { + name: "object attribute value", + attributes: attributes.Attributes{}.Put("meta", map[string]interface{}{"a": 1}, nil), + expected: map[string]string{"meta": `{"a":1}`}, + }, + { + name: "array attribute value", + attributes: attributes.Attributes{}.Put("ports", []int{1, 2}, nil), + expected: map[string]string{"ports": "[1,2]"}, + }, + { + name: "null attribute value", + attributes: attributes.Attributes{}.Put("discoverable", nil, nil), + expected: map[string]string{"discoverable": "null"}, + }, + { + name: "empty attributes", + attributes: attributes.Attributes{}, + expected: nil, + }, + } +} diff --git a/pkg/apis/workspaces/v1alpha1/parent_conversion.go b/pkg/apis/workspaces/v1alpha1/parent_conversion.go index e6e4468ca..e2538e0ab 100644 --- a/pkg/apis/workspaces/v1alpha1/parent_conversion.go +++ b/pkg/apis/workspaces/v1alpha1/parent_conversion.go @@ -273,6 +273,10 @@ func convertParentComponentFrom_v1alpha2(src *v1alpha2.ComponentParentOverride, if err != nil { return err } + jsonComponent, err = stringifyComponentEndpointAttributes(jsonComponent) + if err != nil { + return err + } err = json.Unmarshal(jsonComponent, &dest) if err != nil { return err