Use a struct for GenericMaps

Switch GenericMap from a named `map[string]interface{}` type
to a struct with a Data field. This allows us to override the openapi
spec once, at the type level, instead of retroactively at runtime for
each new field of type GenericMap added to our API structs.

This prevents the "managedFields" log that occurs when we forget to add
new fields to the OpenAPI modification logic.

Signed-off-by: Nick Hale <4175918+njhale@users.noreply.github.com>

integration/client/apps/apps_test.go

@@ -302,10 +302,12 @@ func TestAppUpdate(t *testing.T) {
 		},
 	}, thirdApp.Spec.Links)
 
-	assert.Equal(t, v1.GenericMap{
-		"param1": "val1",
-		"param2": "val3",
-		"param3": "val3",
+	assert.Equal(t, &v1.GenericMap{
+		Data: map[string]any{
+			"param1": "val1",
+			"param2": "val3",
+			"param3": "val3",
+		},
 	}, thirdApp.Spec.DeployArgs)
 
 	assert.Equal(t, imageID2, thirdApp.Spec.Image)
@@ -461,7 +463,7 @@ func TestAppRun(t *testing.T) {
 	assert.Equal(t, v1.PublishModeAll, app.Spec.PublishMode)
 	assert.Equal(t, "volume", app.Spec.Volumes[0].Volume)
 	assert.Equal(t, "secret", app.Spec.Secrets[0].Secret)
-	assert.Equal(t, "value", app.Spec.DeployArgs["key"])
+	assert.Equal(t, "value", app.Spec.DeployArgs.Data["key"])
 }
 
 func TestAppRunImageVariations(t *testing.T) {

integration/run/run_test.go

@@ -916,8 +916,10 @@ func TestDeployParam(t *testing.T) {
 		},
 		Spec: v1.AppInstanceSpec{
 			Image: image.ID,
-			DeployArgs: map[string]any{
-				"someInt": 5,
+			DeployArgs: &v1.GenericMap{
+				Data: map[string]any{
+					"someInt": 5,
+				},
 			},
 		},
 	})

integration/services/services_test.go

@@ -1,13 +1,16 @@
 package services
 
 import (
+	"encoding/json"
+	"fmt"
 	"testing"
 
 	"github.com/acorn-io/runtime/integration/helper"
 	apiv1 "github.com/acorn-io/runtime/pkg/apis/api.acorn.io/v1"
 	v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
 	"github.com/acorn-io/runtime/pkg/client"
 	"github.com/acorn-io/runtime/pkg/labels"
+	"github.com/acorn-io/z"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -25,6 +28,10 @@ func TestServiceInfo(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	imageJSON := string(z.MustBe(json.MarshalIndent(image, "", "    ")))
+
+	fmt.Println("imageJSON:", imageJSON)
+
 	app, err := c.AppRun(ctx, image.ID, nil)
 	if err != nil {
 		t.Fatal(err)

pkg/apis/api.acorn.io/v1/event.go

@@ -42,6 +42,6 @@ const (
 	EventSeverityError = internalv1.EventSeverityError
 )
 
-func Mapify(v any) (internalv1.GenericMap, error) {
-	return internalv1.Mapify(v)
+func Mapify(data any) (internalv1.GenericMap, error) {
+	return internalv1.Mapify(data)
 }

pkg/apis/api.acorn.io/v1/types.go

@@ -257,12 +257,12 @@ type ImageDetails struct {
 	metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
 
 	// Input Params
-	ImageName     string        `json:"imageName,omitempty"`
-	NestedDigest  string        `json:"nestedDigest,omitempty"`
-	DeployArgs    v1.GenericMap `json:"deployArgs,omitempty"`
-	Profiles      []string      `json:"profiles,omitempty"`
-	Auth          *RegistryAuth `json:"auth,omitempty"`
-	IncludeNested bool          `json:"includeNested,omitempty"`
+	ImageName     string         `json:"imageName,omitempty"`
+	NestedDigest  string         `json:"nestedDigest,omitempty"`
+	DeployArgs    *v1.GenericMap `json:"deployArgs,omitempty"`
+	Profiles      []string       `json:"profiles,omitempty"`
+	Auth          *RegistryAuth  `json:"auth,omitempty"`
+	IncludeNested bool           `json:"includeNested,omitempty"`
 	// NoDefaultRegistry - if true, do not assume a default registry on the image if none is specified
 	NoDefaultRegistry bool `json:"noDefaultRegistry,omitempty"`
 

pkg/apis/internal.acorn.io/v1/appimage.go

@@ -5,12 +5,12 @@ type AppImage struct {
 	// ImageInstance.Name
 	ID string `json:"id,omitempty"`
 	// Name is the image name requested by the user of any format
-	Name      string     `json:"name,omitempty"`
-	Digest    string     `json:"digest,omitempty"`
-	Acornfile string     `json:"acornfile,omitempty"`
-	ImageData ImagesData `json:"imageData,omitempty"`
-	BuildArgs GenericMap `json:"buildArgs,omitempty"`
-	VCS       VCS        `json:"vcs,omitempty"`
+	Name      string      `json:"name,omitempty"`
+	Digest    string      `json:"digest,omitempty"`
+	Acornfile string      `json:"acornfile,omitempty"`
+	ImageData ImagesData  `json:"imageData,omitempty"`
+	BuildArgs *GenericMap `json:"buildArgs,omitempty"`
+	VCS       VCS         `json:"vcs,omitempty"`
 }
 
 type VCS struct {

pkg/apis/internal.acorn.io/v1/appinstance.go

@@ -99,7 +99,7 @@ type AppInstanceSpec struct {
 	PublishMode             PublishMode      `json:"publishMode,omitempty"`
 	Links                   []ServiceBinding `json:"services,omitempty"`
 	Publish                 []PortBinding    `json:"ports,omitempty"`
-	DeployArgs              GenericMap       `json:"deployArgs,omitempty"`
+	DeployArgs              *GenericMap      `json:"deployArgs,omitempty"`
 	Permissions             []Permissions    `json:"permissions,omitempty"`
 	ImageGrantedPermissions []Permissions    `json:"imageGrantedPermissions,omitempty"`
 	AutoUpgrade             *bool            `json:"autoUpgrade,omitempty"`

pkg/apis/internal.acorn.io/v1/appspec.go

@@ -34,10 +34,10 @@ const (
 type ChangeType string
 
 type AcornBuild struct {
-	OriginalImage string     `json:"originalImage,omitempty"`
-	Context       string     `json:"context,omitempty"`
-	Acornfile     string     `json:"acornfile,omitempty"`
-	BuildArgs     GenericMap `json:"buildArgs,omitempty"`
+	OriginalImage string      `json:"originalImage,omitempty"`
+	Context       string      `json:"context,omitempty"`
+	Acornfile     string      `json:"acornfile,omitempty"`
+	BuildArgs     *GenericMap `json:"buildArgs,omitempty"`
 }
 
 type Build struct {
@@ -726,7 +726,7 @@ type Acorn struct {
 	Image               string                 `json:"image,omitempty"`
 	Build               *AcornBuild            `json:"build,omitempty"`
 	Profiles            []string               `json:"profiles,omitempty"`
-	DeployArgs          GenericMap             `json:"deployArgs,omitempty"`
+	DeployArgs          *GenericMap            `json:"deployArgs,omitempty"`
 	Publish             PortBindings           `json:"publish,omitempty"`
 	PublishMode         PublishMode            `json:"publishMode,omitempty"`
 	Environment         NameValues             `json:"environment,omitempty"`
@@ -749,7 +749,7 @@ type Secret struct {
 	Name        string            `json:"name,omitempty"`
 	Description string            `json:"description,omitempty"`
 	Type        string            `json:"type,omitempty"`
-	Params      GenericMap        `json:"params,omitempty"`
+	Params      *GenericMap       `json:"params,omitempty"`
 	Data        map[string]string `json:"data,omitempty"`
 }
 
@@ -791,11 +791,11 @@ type Service struct {
 	Address             string                 `json:"address,omitempty"`
 	Ports               Ports                  `json:"ports,omitempty"`
 	Container           string                 `json:"container,omitempty"`
-	Data                GenericMap             `json:"data,omitempty"`
+	Data                *GenericMap            `json:"data,omitempty"`
 	Generated           *GeneratedService      `json:"generated,omitempty"`
 	Image               string                 `json:"image,omitempty"`
 	Build               *AcornBuild            `json:"build,omitempty"`
-	ServiceArgs         GenericMap             `json:"serviceArgs,omitempty"`
+	ServiceArgs         *GenericMap            `json:"serviceArgs,omitempty"`
 	Environment         NameValues             `json:"environment,omitempty"`
 	Secrets             SecretBindings         `json:"secrets,omitempty"`
 	Links               ServiceBindings        `json:"links,omitempty"`

pkg/apis/internal.acorn.io/v1/appstatus.go

@@ -101,7 +101,7 @@ type ServiceStatus struct {
 	CommonStatus               `json:",inline"`
 	Default                    bool              `json:"default,omitempty"`
 	Ports                      Ports             `json:"ports,omitempty"`
-	Data                       GenericMap        `json:"data,omitempty"`
+	Data                       *GenericMap       `json:"data,omitempty"`
 	Consumer                   *ServiceConsumer  `json:"consumer,omitempty"`
 	Secrets                    []string          `json:"secrets,omitempty"`
 	Address                    string            `json:"address,omitempty"`

pkg/apis/internal.acorn.io/v1/build.go

@@ -96,13 +96,13 @@ type AcornImageBuildInstance struct {
 }
 
 type AcornImageBuildInstanceSpec struct {
-	ContextCacheKey string     `json:"contextCacheKey,omitempty"`
-	BuilderName     string     `json:"builderName,omitempty" wrangler:"required"`
-	Acornfile       string     `json:"acornfile,omitempty"`
-	Profiles        []string   `json:"profiles,omitempty"`
-	Platforms       []Platform `json:"platforms,omitempty"`
-	Args            GenericMap `json:"args,omitempty"`
-	VCS             VCS        `json:"vcs,omitempty"`
+	ContextCacheKey string      `json:"contextCacheKey,omitempty"`
+	BuilderName     string      `json:"builderName,omitempty" wrangler:"required"`
+	Acornfile       string      `json:"acornfile,omitempty"`
+	Profiles        []string    `json:"profiles,omitempty"`
+	Platforms       []Platform  `json:"platforms,omitempty"`
+	Args            *GenericMap `json:"args,omitempty"`
+	VCS             VCS         `json:"vcs,omitempty"`
 }
 
 type AcornImageBuildInstanceStatus struct {

pkg/apis/internal.acorn.io/v1/event.go

@@ -62,7 +62,7 @@ type EventInstance struct {
 	// but can be used to hold any data related to the event.
 	//
 	// +optional
-	Details GenericMap `json:"details,omitempty"`
+	Details *GenericMap `json:"details,omitempty"`
 }
 
 // GetObserved returns the time that the Event was first observed.

pkg/apis/internal.acorn.io/v1/map.go

@@ -3,25 +3,94 @@ package v1
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 
+	"github.com/acorn-io/baaah/pkg/typed"
 	"github.com/sirupsen/logrus"
+	"k8s.io/kube-openapi/pkg/common"
+	"k8s.io/kube-openapi/pkg/validation/spec"
 )
 
-type GenericMap map[string]interface{}
+type GenericMap struct {
+	// +optional
+	Data map[string]any `json:"-"`
+}
 
-func (in GenericMap) MarshalJSON() ([]byte, error) {
-	return json.Marshal((map[string]interface{})(in))
+func (GenericMap) OpenAPIDefinition() common.OpenAPIDefinition {
+	return common.OpenAPIDefinition{
+		Schema: spec.Schema{
+			VendorExtensible: spec.VendorExtensible{
+				Extensions: spec.Extensions{
+					"x-kubernetes-preserve-unknown-fields": "true",
+				},
+			},
+			SchemaProps: spec.SchemaProps{
+				Type: []string{"object"},
+			},
+		},
+	}
 }
 
-func translateObject(data interface{}) (ret interface{}, err error) {
+func (g *GenericMap) UnmarshalJSON(data []byte) error {
+	if g == nil {
+		return fmt.Errorf("%T: UnmarshalJSON on nil pointer", g)
+	}
+
+	dec := json.NewDecoder(bytes.NewBuffer(data))
+	dec.UseNumber()
+
+	d := map[string]any{}
+	if err := dec.Decode(&d); err != nil {
+		return fmt.Errorf("%T: Failed to decode data: %w", g, err)
+	}
+
+	if _, err := translateObject(d); err != nil {
+		return fmt.Errorf("%T: Failed to translate object: %w", g, err)
+	}
+
+	if len(d) > 0 {
+		// Consumers expect empty generic maps to have a nil Data field
+		g.Data = d
+	}
+
+	return nil
+}
+
+// GetData returns the underlying map[string]any and nil if the GenericMap is nil.
+func (g *GenericMap) GetData() map[string]any {
+	if g == nil {
+		return nil
+	}
+
+	return g.Data
+}
+
+// Merge merges the given map into this map, returning a new map, leaving the original unchanged.
+func (g *GenericMap) Merge(from *GenericMap) *GenericMap {
+	merged := typed.Concat(g.GetData(), from.GetData())
+	if merged == nil {
+		return nil
+	}
+
+	return &GenericMap{
+		Data: merged,
+	}
+}
+
+// MarshalJSON may get called on pointers or values, so implement MarshalJSON on value.
+func (g GenericMap) MarshalJSON() ([]byte, error) {
+	return json.Marshal(g.GetData())
+}
+
+func translateObject(data any) (ret any, err error) {
 	switch t := data.(type) {
-	case map[string]interface{}:
+	case map[string]any:
 		for k, v := range t {
 			if t[k], err = translateObject(v); err != nil {
 				return nil, err
 			}
 		}
-	case []interface{}:
+	case []any:
 		for i, v := range t {
 			if t[i], err = translateObject(v); err != nil {
 				return nil, err
@@ -37,35 +106,33 @@ func translateObject(data interface{}) (ret interface{}, err error) {
 	return data, nil
 }
 
-func (in *GenericMap) UnmarshalJSON(data []byte) error {
-	dec := json.NewDecoder(bytes.NewBuffer(data))
-	dec.UseNumber()
-	if err := dec.Decode((*map[string]interface{})(in)); err != nil {
-		return err
+func (in *GenericMap) DeepCopyInto(out *GenericMap) {
+	var err error
+	if *out, err = Mapify(in.Data); err != nil {
+		logrus.WithError(err).Errorf("failed to deep copy into [%T]", out)
 	}
-	_, err := translateObject(*((*map[string]interface{})(in)))
-	return err
 }
 
-func (in *GenericMap) DeepCopyInto(out *GenericMap) {
-	*out = map[string]interface{}{}
-	data, err := in.MarshalJSON()
-	if err != nil {
-		logrus.Errorf("failed to marshal [%T] during deep copy: [%s]", out, err)
-		return
+func NewGenericMap(data map[string]any) *GenericMap {
+	if data == nil {
+		return nil
 	}
 
-	if err := out.UnmarshalJSON(data); err != nil {
-		logrus.Errorf("failed to unmarshal [%T] during deep copy: [%s]", out, err)
+	return &GenericMap{
+		Data: data,
 	}
 }
 
-func Mapify(obj any) (GenericMap, error) {
-	data, err := json.Marshal(obj)
+func Mapify(data any) (GenericMap, error) {
+	marshaled, err := json.Marshal(data)
 	if err != nil {
-		return nil, err
+		return GenericMap{}, err
+	}
+
+	gm := &GenericMap{}
+	if err := gm.UnmarshalJSON(marshaled); err != nil {
+		return GenericMap{}, err
 	}
 
-	gm := make(GenericMap)
-	return gm, gm.UnmarshalJSON(data)
+	return *gm, nil
 }