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