Implement default values

[lzap]

The schema defines some default values which are not dependent on image type, arch or other fields.

Initially, I implemented them via custom JSON marshalers but turns out this is very unreadable and problematic for nested structures. #14

E.g. DNFRepo has one default value so it looked like this:

// UnmarshalJSON handles default values.
func (dr *DNFRepository) UnmarshalJSON(data []byte) error {
	type tmpType DNFRepository
	tmp := tmpType(*dr)

	if err := json.Unmarshal(data, &tmp); err != nil {
		return err
	}

	if tmp.TLSVerify == nil {
		tmp.TLSVerify = ptr.To(true)
	}

	if tmp.GPGCheck == nil {
		tmp.GPGCheck = ptr.To(true)
	}

	if tmp.GPGCheckRepo == nil {
		tmp.GPGCheckRepo = ptr.To(true)
	}

	*dr = DNFRepository(tmp)
	return nil
}

// MarshalJSON handles default values.
func (dr DNFRepository) MarshalJSON() ([]byte, error) {
	type tmpType DNFRepository
	tmp := tmpType(dr)

	if tmp.TLSVerify != nil && *tmp.TLSVerify {
		tmp.TLSVerify = nil
	}

	if tmp.GPGCheck != nil && *tmp.GPGCheck {
		tmp.GPGCheck = nil
	}

	if tmp.GPGCheckRepo != nil && *tmp.GPGCheckRepo {
		tmp.GPGCheckRepo = nil
	}

	return json.Marshal(tmp)
}

But there is a nested structure that also has some default values. The generic approach will not work:

// UnmarshalJSON handles default values.
func (dr *DNFRepoUsage) UnmarshalJSON(data []byte) error {
	type tmpType DNFRepoUsage
	tmp := tmpType(*dr)

	if err := json.Unmarshal(data, &tmp); err != nil {
		return err
	}

	if tmp.Configure == nil {
		tmp.Configure = ptr.To(true)
	}

	if tmp.Install == nil {
		tmp.Install = ptr.To(true)
	}

	*dr = DNFRepoUsage(tmp)
	return nil
}

// MarshalJSON handles default values.
func (dr DNFRepoUsage) MarshalJSON() ([]byte, error) {
	type tmpType DNFRepoUsage
	tmp := tmpType(dr)

	if tmp.Configure != nil && *tmp.Configure {
		tmp.Configure = nil
	}

	if tmp.Install != nil && *tmp.Install {
		tmp.Install = nil
	}

	return json.Marshal(tmp)
}

The marshaling method must be called explicitly because it is not defined on the temporary DNFRepo type, this is too clunky and I do not think it is a good solution.

This ticket is to track possible implementation of other solution, perhaps by manual assignment of default values or some struct merging. For now, here is the test for all default values defined in the schema so when the time comes, this test should pass:

package ubp

import (
	"encoding/json"
	"testing"

	"github.com/google/go-cmp/cmp"
	"github.com/google/go-cmp/cmp/cmpopts"
)

func TestPopulateDefaults(t *testing.T) {
	ubpDefaults := &Blueprint{
		Containers: []Container{
			{
				Name:   "container",
				Source: "source",
			},
		},
		DNF: DNF{
			Repositories: []DNFRepository{{ID: "repo"}},
		},
		FSNodes: []FSNode{
			{
				Path: "file",
			},
			{
				Path: "dir",
				Type: "dir",
			},
		},
		Network: Network{
			Firewall: NetworkFirewall{
				Services: []NetworkService{
					{
						union: []byte(`{"name": "ssh"}`),
					},
					{
						union: []byte(`{"port": 22}`),
					},
					{
						union: []byte(`{"from": 200, "to": 300}`),
					},
				},
			},
		},
		Storage: Storage{
			Type: StorageTypeGPT,
			Partitions: []StoragePartition{
				{
					union: []byte(`{}`),
				},
				{
					union: []byte(`{"type": "lvm"}`),
				},
				{
					union: []byte(`{"type": "btrfs"}`),
				},
			},
		},
		Timedate: TimeDate{
			Timezone: "UTC",
		},
	}

	tests := []struct {
		name string
		val  func(*Blueprint) any
		want any
	}{
		{
			name: "container-tls-verify",
			val: func(ubp *Blueprint) any {
				return *ubp.Containers[0].TLSVerify
			},
			want: true,
		},
		{
			name: "dnf-repo-tls-verify",
			val: func(ubp *Blueprint) any {
				return *ubp.DNF.Repositories[0].TLSVerify
			},
			want: true,
		},
		{
			name: "dnf-repo-usage-configure",
			val: func(ubp *Blueprint) any {
				return *ubp.DNF.Repositories[0].Usage.Configure
			},
			want: true,
		},
		{
			name: "dnf-repo-usage-install",
			val: func(ubp *Blueprint) any {
				return *ubp.DNF.Repositories[0].Usage.Install
			},
			want: true,
		},
		{
			name: "fsnode-mode-file",
			val: func(ubp *Blueprint) any {
				return ubp.FSNodes[0].Mode
			},
			want: DefaultFileFSNodeMode,
		},
		{
			name: "fsnode-mode-dir",
			val: func(ubp *Blueprint) any {
				return ubp.FSNodes[1].Mode
			},
			want: DefaultDirFSNodeMode,
		},
		{
			name: "network-firewall-port-protocol",
			val: func(ubp *Blueprint) any {
				s, err := ubp.Network.Firewall.Services[1].AsFirewallPort()
				if err != nil {
					t.Fatalf("firewall as call failed: %v", err)
				}
				return s.Protocol
			},
			want: ProtocolAny,
		},
		{
			name: "network-firewall-service-protocol",
			val: func(ubp *Blueprint) any {
				s, err := ubp.Network.Firewall.Services[0].AsFirewallService()
				if err != nil {
					t.Fatalf("firewall as call failed: %v", err)
				}
				return s.Protocol
			},
			want: ProtocolAny,
		},
		{
			name: "network-firewall-from-to-protocol",
			val: func(ubp *Blueprint) any {
				s, err := ubp.Network.Firewall.Services[2].AsFirewallFromTo()
				if err != nil {
					t.Fatalf("firewall as call failed: %v", err)
				}
				return s.Protocol
			},
			want: ProtocolAny,
		},
		{
			name: "network-firewall-port-enabled",
			val: func(ubp *Blueprint) any {
				s, err := ubp.Network.Firewall.Services[1].AsFirewallPort()
				if err != nil {
					t.Fatalf("firewall as call failed: %v", err)
				}
				return *s.Enabled
			},
			want: true,
		},
		{
			name: "network-firewall-service-enabled",
			val: func(ubp *Blueprint) any {
				s, err := ubp.Network.Firewall.Services[0].AsFirewallService()
				if err != nil {
					t.Fatalf("firewall as call failed: %v", err)
				}
				return *s.Enabled
			},
			want: true,
		},
		{
			name: "network-firewall-from-to-enabled",
			val: func(ubp *Blueprint) any {
				s, err := ubp.Network.Firewall.Services[2].AsFirewallFromTo()
				if err != nil {
					t.Fatalf("firewall as call failed: %v", err)
				}
				return *s.Enabled
			},
			want: true,
		},
	}

	if err := PopulateDefaults(nil); err != nil {
		t.Errorf("PopulateDefaults should not return an error for nil input: %v", err)
	}

	if err := PopulateDefaults(ubpDefaults); err != nil {
		t.Errorf("PopulateDefaults returned an error: %v", err)
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			if got := tt.val(ubpDefaults); got != tt.want {
				t.Errorf("PopulateDefaults() = %v, want %v", got, tt.want)
			}
		})
	}

	buf, err := json.MarshalIndent(ubpDefaults, "", "  ")
	if err != nil {
		t.Fatalf("MarshalJSON defaults failed: %v", err)
	}

	want := `{
	"containers": [
		{
			"name": "container",
			"source": "source"
		}
	],
	"dnf": {
		"repositories": [
			{
				"id": "repo",
				"usage": {}
			}
		]
	},
	"fsnodes": [
		{
			"path": "file"
		},
		{
			"path": "dir",
			"type": "dir"
		}
	],
	"network": {
		"firewall": {
			"services": [
				{
					"name": "ssh"
				},
				{
					"port": 22
				},
				{
					"from": 200,
					"to": 300
				}
			]
		}
	},
	"storage": {
		"partitions": [
			{},
			{
				"type": "lvm"
			},
			{
				"type": "btrfs"
			}
		],
		"type": "gpt"
	}
}`

	if diff := cmp.Diff(want, string(buf), cmpopts.EquateEmpty()); diff != "" {
		t.Errorf("MarshalJSON defaults mismatch (-want +got):\n%s", diff)
	}
}

We might also decide to ignore default values in UBP completely and leave this on the images library.

[lzap]

Few ideas:

• During load/save, explicitly set the default values. We would manually maintain a function that does that + the test above.
• Default values are part of the type via "getters", e.g. for fsnode.Type we would have fsnode.DefaultType(). The only drawback is that people must know not to access the Type field directly (because it will be empty or nil) which could lead to errors.