Repository: osbuild/osbuild-composer
Pull request #3612 (2023-08-10 12:34:24)

COMPOSER-2016: blueprint: make Convert respect nils
Previously, nil values in the conversion source were in some cases converted to empty arrays or empty objects. This is undesirable, because it can be in certain cases changing the semantics of the blueprint. See e.g. https://github.com/osbuild/images/blob/f317064da574a8475d438972e9c54bc67ffa1dfb/pkg/distro/rhel7/imagetype.go#L239C7-L239C7

This commit modifies the conversion process so nil values are converted without any changes. Also, the `Convert` function was covered with a unit test.


This pull request includes:

- [x] adequate testing for the new functionality or fixed issue


Patch:
diff --git a/internal/blueprint/blueprint.go b/internal/blueprint/blueprint.go
index 6ca5c6a3b..460220dd9 100644
--- a/internal/blueprint/blueprint.go
+++ b/internal/blueprint/blueprint.go
@@ -185,29 +185,42 @@ func (b *Blueprint) CryptPasswords() error {
 }
 
 func Convert(bp Blueprint) iblueprint.Blueprint {
-	pkgs := make([]iblueprint.Package, len(bp.Packages))
-	for idx := range bp.Packages {
-		pkgs[idx] = iblueprint.Package(bp.Packages[idx])
+	var pkgs []iblueprint.Package
+	if len(bp.Packages) > 0 {
+		pkgs = make([]iblueprint.Package, len(bp.Packages))
+		for idx := range bp.Packages {
+			pkgs[idx] = iblueprint.Package(bp.Packages[idx])
+		}
 	}
 
-	modules := make([]iblueprint.Package, len(bp.Modules))
-	for idx := range bp.Modules {
-		modules[idx] = iblueprint.Package(bp.Modules[idx])
+	var modules []iblueprint.Package
+	if len(bp.Modules) > 0 {
+		modules = make([]iblueprint.Package, len(bp.Modules))
+		for idx := range bp.Modules {
+			modules[idx] = iblueprint.Package(bp.Modules[idx])
+		}
 	}
 
-	groups := make([]iblueprint.Group, len(bp.Groups))
-	for idx := range bp.Groups {
-		groups[idx] = iblueprint.Group(bp.Groups[idx])
+	var groups []iblueprint.Group
+	if len(bp.Groups) > 0 {
+		groups = make([]iblueprint.Group, len(bp.Groups))
+		for idx := range bp.Groups {
+			groups[idx] = iblueprint.Group(bp.Groups[idx])
+		}
 	}
 
-	containers := make([]iblueprint.Container, len(bp.Containers))
-	for idx := range bp.Containers {
-		containers[idx] = iblueprint.Container(bp.Containers[idx])
+	var containers []iblueprint.Container
+
+	if len(bp.Containers) > 0 {
+		containers = make([]iblueprint.Container, len(bp.Containers))
+		for idx := range bp.Containers {
+			containers[idx] = iblueprint.Container(bp.Containers[idx])
+		}
 	}
 
-	customizations := iblueprint.Customizations{}
+	var customizations *iblueprint.Customizations
 	if c := bp.Customizations; c != nil {
-		customizations = iblueprint.Customizations{
+		customizations = &iblueprint.Customizations{
 			Hostname:           c.Hostname,
 			InstallationDevice: c.InstallationDevice,
 		}
@@ -331,7 +344,7 @@ func Convert(bp Blueprint) iblueprint.Blueprint {
 		Modules:        modules,
 		Groups:         groups,
 		Containers:     containers,
-		Customizations: &customizations,
+		Customizations: customizations,
 		Distro:         bp.Distro,
 	}
 


Test patch:
diff --git a/internal/blueprint/blueprint_convert_test.go b/internal/blueprint/blueprint_convert_test.go
new file mode 100644
index 000000000..e767d83c7
--- /dev/null
+++ b/internal/blueprint/blueprint_convert_test.go
@@ -0,0 +1,332 @@
+package blueprint
+
+import (
+	"testing"
+
+	iblueprint "github.com/osbuild/images/pkg/blueprint"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/osbuild/osbuild-composer/internal/common"
+)
+
+func TestConvert(t *testing.T) {
+	tests := []struct {
+		name     string
+		src      Blueprint
+		expected iblueprint.Blueprint
+	}{
+		{
+			name:     "empty",
+			src:      Blueprint{},
+			expected: iblueprint.Blueprint{},
+		},
+		{
+			name: "everything",
+			src: Blueprint{
+				Name:        "name",
+				Description: "desc",
+				Version:     "version",
+				Packages: []Package{
+					{
+						Name:    "package-name",
+						Version: "package-version",
+					},
+				},
+				Modules: []Package{
+					{
+						Name:    "module-name",
+						Version: "module-version",
+					},
+				},
+				Groups: []Group{
+					{
+						Name: "group-name",
+					},
+				},
+				Containers: []Container{
+					{
+						Source:    "source",
+						Name:      "name",
+						TLSVerify: common.ToPtr(true),
+					},
+				},
+				Customizations: &Customizations{
+					Hostname: common.ToPtr("hostname"),
+					Kernel: &KernelCustomization{
+						Name:   "kernel-name",
+						Append: "kernel-append",
+					},
+					SSHKey: []SSHKeyCustomization{
+						{
+							User: "ssh-user",
+							Key:  "ssh-key",
+						},
+					},
+					User: []UserCustomization{
+						{
+							Name:        "user-name",
+							Description: common.ToPtr("user-desc"),
+							Password:    common.ToPtr("user-password"),
+							Key:         common.ToPtr("user-key"),
+							Home:        common.ToPtr("/home/user"),
+							Shell:       common.ToPtr("fish"),
+							Groups:      []string{"wheel"},
+							UID:         common.ToPtr(42),
+							GID:         common.ToPtr(2023),
+						},
+					},
+					Group: []GroupCustomization{
+						{
+							Name: "group",
+							GID:  common.ToPtr(7),
+						},
+					},
+					Timezone: &TimezoneCustomization{
+						Timezone:   common.ToPtr("timezone"),
+						NTPServers: []string{"ntp-server"},
+					},
+					Locale: &LocaleCustomization{
+						Languages: []string{"language"},
+						Keyboard:  common.ToPtr("keyboard"),
+					},
+					Firewall: &FirewallCustomization{
+						Ports: []string{"80"},
+						Services: &FirewallServicesCustomization{
+							Enabled:  []string{"ssh"},
+							Disabled: []string{"ntp"},
+						},
+						Zones: []FirewallZoneCustomization{
+							{
+								Name:    common.ToPtr("name"),
+								Sources: []string{"src"},
+							},
+						},
+					},
+					Services: &ServicesCustomization{
+						Enabled:  []string{"osbuild-composer.service"},
+						Disabled: []string{"lorax-composer.service"},
+					},
+					Filesystem: []FilesystemCustomization{
+						{
+							Mountpoint: "/usr",
+							MinSize:    1024,
+						},
+					},
+					InstallationDevice: "/dev/sda",
+					FDO: &FDOCustomization{
+						ManufacturingServerURL: "http://manufacturing.fdo",
+						DiunPubKeyInsecure:     "insecure-pubkey",
+						DiunPubKeyHash:         "hash-pubkey",
+						DiunPubKeyRootCerts:    "root-certs",
+					},
+					OpenSCAP: &OpenSCAPCustomization{
+						DataStream: "stream",
+						ProfileID:  "profile",
+						Tailoring: &OpenSCAPTailoringCustomizations{
+							Selected:   []string{"cloth"},
+							Unselected: []string{"leather"},
+						},
+					},
+					Ignition: &IgnitionCustomization{
+						Embedded: &EmbeddedIgnitionCustomization{
+							Config: "ignition-config",
+						},
+						FirstBoot: &FirstBootIgnitionCustomization{
+							ProvisioningURL: "http://provisioning.edge",
+						},
+					},
+					Directories: []DirectoryCustomization{
+						{
+							Path:          "/dir",
+							User:          common.ToPtr("dir-user"),
+							Group:         common.ToPtr("dir-group"),
+							Mode:          "0777",
+							EnsureParents: true,
+						},
+					},
+					Files: []FileCustomization{
+						{
+							Path:  "/file",
+							User:  common.ToPtr("file-user`"),
+							Group: common.ToPtr("file-group"),
+							Mode:  "0755",
+							Data:  "literal easter egg",
+						},
+					},
+					Repositories: []RepositoryCustomization{
+						{
+							Id:           "repoid",
+							BaseURLs:     []string{"http://baseurl"},
+							GPGKeys:      []string{"repo-gpgkey"},
+							Metalink:     "http://metalink",
+							Mirrorlist:   "http://mirrorlist",
+							Name:         "reponame",
+							Priority:     common.ToPtr(987),
+							Enabled:      common.ToPtr(true),
+							GPGCheck:     common.ToPtr(true),
+							RepoGPGCheck: common.ToPtr(true),
+							SSLVerify:    common.ToPtr(true),
+							Filename:     "repofile",
+						},
+					},
+				},
+				Distro: "distro",
+			},
+			expected: iblueprint.Blueprint{
+				Name:        "name",
+				Description: "desc",
+				Version:     "version",
+				Packages: []iblueprint.Package{
+					{
+						Name:    "package-name",
+						Version: "package-version",
+					},
+				},
+				Modules: []iblueprint.Package{
+					{
+						Name:    "module-name",
+						Version: "module-version",
+					},
+				},
+				Groups: []iblueprint.Group{
+					{
+						Name: "group-name",
+					},
+				},
+				Containers: []iblueprint.Container{
+					{
+						Source:    "source",
+						Name:      "name",
+						TLSVerify: common.ToPtr(true),
+					},
+				},
+				Customizations: &iblueprint.Customizations{
+					Hostname: common.ToPtr("hostname"),
+					Kernel: &iblueprint.KernelCustomization{
+						Name:   "kernel-name",
+						Append: "kernel-append",
+					},
+					SSHKey: []iblueprint.SSHKeyCustomization{
+						{
+							User: "ssh-user",
+							Key:  "ssh-key",
+						},
+					},
+					User: []iblueprint.UserCustomization{
+						{
+							Name:        "user-name",
+							Description: common.ToPtr("user-desc"),
+							Password:    common.ToPtr("user-password"),
+							Key:         common.ToPtr("user-key"),
+							Home:        common.ToPtr("/home/user"),
+							Shell:       common.ToPtr("fish"),
+							Groups:      []string{"wheel"},
+							UID:         common.ToPtr(42),
+							GID:         common.ToPtr(2023),
+						},
+					},
+					Group: []iblueprint.GroupCustomization{
+						{
+							Name: "group",
+							GID:  common.ToPtr(7),
+						},
+					},
+					Timezone: &iblueprint.TimezoneCustomization{
+						Timezone:   common.ToPtr("timezone"),
+						NTPServers: []string{"ntp-server"},
+					},
+					Locale: &iblueprint.LocaleCustomization{
+						Languages: []string{"language"},
+						Keyboard:  common.ToPtr("keyboard"),
+					},
+					Firewall: &iblueprint.FirewallCustomization{
+						Ports: []string{"80"},
+						Services: &iblueprint.FirewallServicesCustomization{
+							Enabled:  []string{"ssh"},
+							Disabled: []string{"ntp"},
+						},
+						Zones: []iblueprint.FirewallZoneCustomization{
+							{
+								Name:    common.ToPtr("name"),
+								Sources: []string{"src"},
+							},
+						},
+					},
+					Services: &iblueprint.ServicesCustomization{
+						Enabled:  []string{"osbuild-composer.service"},
+						Disabled: []string{"lorax-composer.service"},
+					},
+					Filesystem: []iblueprint.FilesystemCustomization{
+						{
+							Mountpoint: "/usr",
+							MinSize:    1024,
+						},
+					},
+					InstallationDevice: "/dev/sda",
+					FDO: &iblueprint.FDOCustomization{
+						ManufacturingServerURL: "http://manufacturing.fdo",
+						DiunPubKeyInsecure:     "insecure-pubkey",
+						DiunPubKeyHash:         "hash-pubkey",
+						DiunPubKeyRootCerts:    "root-certs",
+					},
+					OpenSCAP: &iblueprint.OpenSCAPCustomization{
+						DataStream: "stream",
+						ProfileID:  "profile",
+						Tailoring: &iblueprint.OpenSCAPTailoringCustomizations{
+							Selected:   []string{"cloth"},
+							Unselected: []string{"leather"},
+						},
+					},
+					Ignition: &iblueprint.IgnitionCustomization{
+						Embedded: &iblueprint.EmbeddedIgnitionCustomization{
+							Config: "ignition-config",
+						},
+						FirstBoot: &iblueprint.FirstBootIgnitionCustomization{
+							ProvisioningURL: "http://provisioning.edge",
+						},
+					},
+					Directories: []iblueprint.DirectoryCustomization{
+						{
+							Path:          "/dir",
+							User:          common.ToPtr("dir-user"),
+							Group:         common.ToPtr("dir-group"),
+							Mode:          "0777",
+							EnsureParents: true,
+						},
+					},
+					Files: []iblueprint.FileCustomization{
+						{
+							Path:  "/file",
+							User:  common.ToPtr("file-user`"),
+							Group: common.ToPtr("file-group"),
+							Mode:  "0755",
+							Data:  "literal easter egg",
+						},
+					},
+					Repositories: []iblueprint.RepositoryCustomization{
+						{
+							Id:           "repoid",
+							BaseURLs:     []string{"http://baseurl"},
+							GPGKeys:      []string{"repo-gpgkey"},
+							Metalink:     "http://metalink",
+							Mirrorlist:   "http://mirrorlist",
+							Name:         "reponame",
+							Priority:     common.ToPtr(987),
+							Enabled:      common.ToPtr(true),
+							GPGCheck:     common.ToPtr(true),
+							RepoGPGCheck: common.ToPtr(true),
+							SSLVerify:    common.ToPtr(true),
+							Filename:     "repofile",
+						},
+					},
+				},
+				Distro: "distro",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.expected, Convert(tt.src))
+		})
+	}
+}