Repository: osbuild/osbuild-composer
Pull request #1592 (2021-07-26 14:18:27)

Reflect changes made to some osbuild stages before the v30 release
This Pull Request reflects changes made to some osbuild stages before the v30 release (https://github.com/osbuild/osbuild/pull/739):
- change the following stages from creating multiple configuration files to just a single file:
  - `cloud-init`
  - `modprobe`
  - `dracut.conf`
  - `systemd-logind`
- the functionality to create Systemd unit drop-ins was extracted from the `systemd` stage to a new `systemd.unit` stage

This pull request includes:

- [x] adequate testing for the new functionality or fixed issue
- [ ] adequate documentation informing people about the change such as
  - [ ] create a file in [news/unreleased](https://github.com/osbuild/osbuild-composer/tree/main/docs/news/unreleased) directory if this change should be mentioned in the release news
  - [ ] submit a PR for the [guides](https://github.com/osbuild/guides) repository if this PR changed any behavior described there: https://www.osbuild.org/guides/

<!--
Thanks for proposing a change to osbuild-composer!

Please don't remove the above check list. These are things that each pull
request must have before it is merged. It helps maintainers to not forget
anything.

If the reason for ticking any of the boxes is ambiguous, please add a short
note explaining why.

For user-visible changes, "adequate documentation" is an entry describing the
change for users in docs/news. Please refer to docs/news/README.md for details.

In addition, if this pull request fixes a downstream issue, please refer to
test/README.md and add these additional items:

- [ ] 1st commit of any `rhbz#` related PR contains bug reproducer; CI reports FAIL or
- [ ] PR contains automated tests for new functionality and
- [ ] QE has approved reproducer/new tests and
- [ ] Subsequent commits provide bug fixes without modifying the reproducer; CI reports PASS and
- [ ] QE approves this PR; RHBZ status is set to `MODIFIED + Verified=Tested`
-->



Patch:
diff --git a/internal/osbuild2/cloud_init_stage.go b/internal/osbuild2/cloud_init_stage.go
index 0d6ca9cfc..c12e4a015 100644
--- a/internal/osbuild2/cloud_init_stage.go
+++ b/internal/osbuild2/cloud_init_stage.go
@@ -6,7 +6,8 @@ import (
 )
 
 type CloudInitStageOptions struct {
-	ConfigFiles map[string]CloudInitConfigFile `json:"configuration_files,omitempty"`
+	Filename string              `json:"filename"`
+	Config   CloudInitConfigFile `json:"config"`
 }
 
 func (CloudInitStageOptions) isStageOptions() {}
diff --git a/internal/osbuild2/dracut_conf_stage.go b/internal/osbuild2/dracut_conf_stage.go
index 776d9cfb8..9131d9390 100644
--- a/internal/osbuild2/dracut_conf_stage.go
+++ b/internal/osbuild2/dracut_conf_stage.go
@@ -6,13 +6,14 @@ import (
 )
 
 type DracutConfStageOptions struct {
-	ConfigFiles map[string]DracutConfigFile `json:"configuration_files,omitempty"`
+	Filename string           `json:"filename"`
+	Config   DracutConfigFile `json:"config"`
 }
 
 func (DracutConfStageOptions) isStageOptions() {}
 
 // Dracut.conf stage creates dracut configuration files under /usr/lib/dracut/dracut.conf.d/
-func NewDracutConfStageOptions(options *DracutConfStageOptions) *Stage {
+func NewDracutConfStage(options *DracutConfStageOptions) *Stage {
 	return &Stage{
 		Type:    "org.osbuild.dracut.conf",
 		Options: options,
diff --git a/internal/osbuild2/modprobe_stage.go b/internal/osbuild2/modprobe_stage.go
index 04d349b28..66cd39675 100644
--- a/internal/osbuild2/modprobe_stage.go
+++ b/internal/osbuild2/modprobe_stage.go
@@ -6,7 +6,8 @@ import (
 )
 
 type ModprobeStageOptions struct {
-	ConfigFiles map[string]ModprobeConfigCmdList `json:"configuration_files,omitempty"`
+	Filename string                `json:"filename"`
+	Commands ModprobeConfigCmdList `json:"commands"`
 }
 
 func (ModprobeStageOptions) isStageOptions() {}
diff --git a/internal/osbuild2/stage.go b/internal/osbuild2/stage.go
index fa2701d19..87ab877a5 100644
--- a/internal/osbuild2/stage.go
+++ b/internal/osbuild2/stage.go
@@ -81,6 +81,8 @@ func (stage *Stage) UnmarshalJSON(data []byte) error {
 		options = new(RHSMStageOptions)
 	case "org.osbuild.systemd":
 		options = new(SystemdStageOptions)
+	case "org.osbuild.systemd.unit":
+		options = new(SystemdUnitStageOptions)
 	case "org.osbuild.systemd-logind":
 		options = new(SystemdLogindStageOptions)
 	case "org.osbuild.script":
diff --git a/internal/osbuild2/systemd_logind_stage.go b/internal/osbuild2/systemd_logind_stage.go
index 6e13cdae3..a69759fa9 100644
--- a/internal/osbuild2/systemd_logind_stage.go
+++ b/internal/osbuild2/systemd_logind_stage.go
@@ -6,7 +6,8 @@ import (
 )
 
 type SystemdLogindStageOptions struct {
-	ConfigDropins map[string]SystemdLogindConfigDropin `json:"configuration_dropins,omitempty"`
+	Filename string                    `json:"filename"`
+	Config   SystemdLogindConfigDropin `json:"config"`
 }
 
 func (SystemdLogindStageOptions) isStageOptions() {}
diff --git a/internal/osbuild2/systemd_stage.go b/internal/osbuild2/systemd_stage.go
index 35f7100b9..ca0b80a64 100644
--- a/internal/osbuild2/systemd_stage.go
+++ b/internal/osbuild2/systemd_stage.go
@@ -4,9 +4,6 @@ type SystemdStageOptions struct {
 	EnabledServices  []string `json:"enabled_services,omitempty"`
 	DisabledServices []string `json:"disabled_services,omitempty"`
 	DefaultTarget    string   `json:"default_target,omitempty"`
-
-	// For now we support only .service drop-ins, but this may change in the future
-	UnitDropins map[string]SystemdServiceUnitDropins `json:"unit_dropins,omitempty"`
 }
 
 func (SystemdStageOptions) isStageOptions() {}
@@ -17,17 +14,3 @@ func NewSystemdStage(options *SystemdStageOptions) *Stage {
 		Options: options,
 	}
 }
-
-// Drop-in configurations for a '.service' unit
-type SystemdServiceUnitDropins map[string]SystemdServiceUnitDropin
-
-// Drop-in configuration for a '.service' unit
-type SystemdServiceUnitDropin struct {
-	Service *SystemdUnitServiceSection `json:"Service,omitempty"`
-}
-
-// 'Service' configuration section of a unit file
-type SystemdUnitServiceSection struct {
-	// Sets environment variables for executed process
-	Environment string `json:"Environment,omitempty"`
-}
diff --git a/internal/osbuild2/systemd_unit_stage.go b/internal/osbuild2/systemd_unit_stage.go
new file mode 100644
index 000000000..708ced8cc
--- /dev/null
+++ b/internal/osbuild2/systemd_unit_stage.go
@@ -0,0 +1,27 @@
+package osbuild2
+
+type SystemdUnitStageOptions struct {
+	Unit   string                   `json:"unit"`
+	Dropin string                   `json:"dropin"`
+	Config SystemdServiceUnitDropin `json:"config"`
+}
+
+func (SystemdUnitStageOptions) isStageOptions() {}
+
+func NewSystemdUnitStage(options *SystemdUnitStageOptions) *Stage {
+	return &Stage{
+		Type:    "org.osbuild.systemd.unit",
+		Options: options,
+	}
+}
+
+// Drop-in configuration for a '.service' unit
+type SystemdServiceUnitDropin struct {
+	Service *SystemdUnitServiceSection `json:"Service,omitempty"`
+}
+
+// 'Service' configuration section of a unit file
+type SystemdUnitServiceSection struct {
+	// Sets environment variables for executed process
+	Environment string `json:"Environment,omitempty"`
+}


Test patch:
diff --git a/internal/osbuild2/cloud_init_stage_test.go b/internal/osbuild2/cloud_init_stage_test.go
index 3db7a8a34..93d2e438f 100644
--- a/internal/osbuild2/cloud_init_stage_test.go
+++ b/internal/osbuild2/cloud_init_stage_test.go
@@ -21,32 +21,33 @@ func TestCloudInitStage_MarshalJSON_Invalid(t *testing.T) {
 		name    string
 		options CloudInitStageOptions
 	}{
+		{
+			name:    "empty-options",
+			options: CloudInitStageOptions{},
+		},
 		{
 			name: "no-config-file-section",
 			options: CloudInitStageOptions{
-				ConfigFiles: map[string]CloudInitConfigFile{
-					"00-default_user.cfg": {},
-				},
+				Filename: "00-default_user.cfg",
+				Config:   CloudInitConfigFile{},
 			},
 		},
 		{
 			name: "no-system-info-section-option",
 			options: CloudInitStageOptions{
-				ConfigFiles: map[string]CloudInitConfigFile{
-					"00-default_user.cfg": {
-						SystemInfo: &CloudInitConfigSystemInfo{},
-					},
+				Filename: "00-default_user.cfg",
+				Config: CloudInitConfigFile{
+					SystemInfo: &CloudInitConfigSystemInfo{},
 				},
 			},
 		},
 		{
 			name: "no-default-user-section-option",
 			options: CloudInitStageOptions{
-				ConfigFiles: map[string]CloudInitConfigFile{
-					"00-default_user.cfg": {
-						SystemInfo: &CloudInitConfigSystemInfo{
-							DefaultUser: &CloudInitConfigDefaultUser{},
-						},
+				Filename: "00-default_user.cfg",
+				Config: CloudInitConfigFile{
+					SystemInfo: &CloudInitConfigSystemInfo{
+						DefaultUser: &CloudInitConfigDefaultUser{},
 					},
 				},
 			},
diff --git a/internal/osbuild2/dracut_conf_stage_test.go b/internal/osbuild2/dracut_conf_stage_test.go
index faedea3ce..b68b7c326 100644
--- a/internal/osbuild2/dracut_conf_stage_test.go
+++ b/internal/osbuild2/dracut_conf_stage_test.go
@@ -12,7 +12,7 @@ func TestNewDracutConfStage(t *testing.T) {
 		Type:    "org.osbuild.dracut.conf",
 		Options: &DracutConfStageOptions{},
 	}
-	actualStage := NewDracutConfStageOptions(&DracutConfStageOptions{})
+	actualStage := NewDracutConfStage(&DracutConfStageOptions{})
 	assert.Equal(t, expectedStage, actualStage)
 }
 
@@ -21,12 +21,15 @@ func TestDracutConfStage_MarshalJSON_Invalid(t *testing.T) {
 		name    string
 		options DracutConfStageOptions
 	}{
+		{
+			name:    "empty-options",
+			options: DracutConfStageOptions{},
+		},
 		{
 			name: "no-options-in-config",
 			options: DracutConfStageOptions{
-				ConfigFiles: map[string]DracutConfigFile{
-					"testing.conf": {},
-				},
+				Filename: "testing.conf",
+				Config:   DracutConfigFile{},
 			},
 		},
 	}
diff --git a/internal/osbuild2/modprobe_stage_test.go b/internal/osbuild2/modprobe_stage_test.go
index fd5ee43ad..18ca6aa27 100644
--- a/internal/osbuild2/modprobe_stage_test.go
+++ b/internal/osbuild2/modprobe_stage_test.go
@@ -21,12 +21,15 @@ func TestModprobeStage_MarshalJSON_Invalid(t *testing.T) {
 		name    string
 		options ModprobeStageOptions
 	}{
+		{
+			name:    "empty-options",
+			options: ModprobeStageOptions{},
+		},
 		{
 			name: "no-commands",
 			options: ModprobeStageOptions{
-				ConfigFiles: map[string]ModprobeConfigCmdList{
-					"disallow-modules.conf": {},
-				},
+				Filename: "disallow-modules.conf",
+				Commands: ModprobeConfigCmdList{},
 			},
 		},
 	}
diff --git a/internal/osbuild2/stage_test.go b/internal/osbuild2/stage_test.go
index 4b4492260..2caff731f 100644
--- a/internal/osbuild2/stage_test.go
+++ b/internal/osbuild2/stage_test.go
@@ -80,32 +80,21 @@ func TestStage_UnmarshalJSON(t *testing.T) {
 		},
 		{
 			name: "cloud-init",
-			fields: fields{
-				Type:    "org.osbuild.cloud-init",
-				Options: &CloudInitStageOptions{},
-			},
-			args: args{
-				data: []byte(`{"type":"org.osbuild.cloud-init","options":{}}`),
-			},
-		},
-		{
-			name: "cloud-init-data",
 			fields: fields{
 				Type: "org.osbuild.cloud-init",
 				Options: &CloudInitStageOptions{
-					ConfigFiles: map[string]CloudInitConfigFile{
-						"00-default_user.cfg": {
-							SystemInfo: &CloudInitConfigSystemInfo{
-								DefaultUser: &CloudInitConfigDefaultUser{
-									Name: "ec2-user",
-								},
+					Filename: "00-default_user.cfg",
+					Config: CloudInitConfigFile{
+						SystemInfo: &CloudInitConfigSystemInfo{
+							DefaultUser: &CloudInitConfigDefaultUser{
+								Name: "ec2-user",
 							},
 						},
 					},
 				},
 			},
 			args: args{
-				data: []byte(`{"type":"org.osbuild.cloud-init","options":{"configuration_files":{"00-default_user.cfg":{"system_info":{"default_user":{"name":"ec2-user"}}}}}}`),
+				data: []byte(`{"type":"org.osbuild.cloud-init","options":{"filename":"00-default_user.cfg","config":{"system_info":{"default_user":{"name":"ec2-user"}}}}}`),
 			},
 		},
 		{
@@ -158,39 +147,25 @@ func TestStage_UnmarshalJSON(t *testing.T) {
 		},
 		{
 			name: "dracut.conf",
-			fields: fields{
-				Type:    "org.osbuild.dracut.conf",
-				Options: &DracutConfStageOptions{},
-			},
-			args: args{
-				data: []byte(`{"type":"org.osbuild.dracut.conf","options":{}}`),
-			},
-		},
-		{
-			name: "dracut.conf-data",
 			fields: fields{
 				Type: "org.osbuild.dracut.conf",
 				Options: &DracutConfStageOptions{
-					ConfigFiles: map[string]DracutConfigFile{
-						"sgdisk.conf": {
-							Install: []string{"sgdisk"},
-						},
-						"testing.conf": {
-							Compress:       "xz",
-							AddModules:     []string{"floppy"},
-							OmitModules:    []string{"nouveau"},
-							AddDrivers:     []string{"driver1"},
-							ForceDrivers:   []string{"driver2"},
-							Filesystems:    []string{"ext4"},
-							Install:        []string{"file1"},
-							EarlyMicrocode: common.BoolToPtr(false),
-							Reproducible:   common.BoolToPtr(false),
-						},
+					Filename: "testing.conf",
+					Config: DracutConfigFile{
+						Compress:       "xz",
+						AddModules:     []string{"floppy"},
+						OmitModules:    []string{"nouveau"},
+						AddDrivers:     []string{"driver1"},
+						ForceDrivers:   []string{"driver2"},
+						Filesystems:    []string{"ext4"},
+						Install:        []string{"file1"},
+						EarlyMicrocode: common.BoolToPtr(false),
+						Reproducible:   common.BoolToPtr(false),
 					},
 				},
 			},
 			args: args{
-				data: []byte(`{"type":"org.osbuild.dracut.conf","options":{"configuration_files":{"sgdisk.conf":{"install_items":["sgdisk"]},"testing.conf":{"compress":"xz","add_dracutmodules":["floppy"],"omit_dracutmodules":["nouveau"],"add_drivers":["driver1"],"force_drivers":["driver2"],"filesystems":["ext4"],"install_items":["file1"],"early_microcode":false,"reproducible":false}}}}`),
+				data: []byte(`{"type":"org.osbuild.dracut.conf","options":{"filename":"testing.conf","config":{"compress":"xz","add_dracutmodules":["floppy"],"omit_dracutmodules":["nouveau"],"add_drivers":["driver1"],"force_drivers":["driver2"],"filesystems":["ext4"],"install_items":["file1"],"early_microcode":false,"reproducible":false}}}`),
 			},
 		},
 		{
@@ -310,41 +285,24 @@ func TestStage_UnmarshalJSON(t *testing.T) {
 		},
 		{
 			name: "modprobe",
-			fields: fields{
-				Type:    "org.osbuild.modprobe",
-				Options: &ModprobeStageOptions{},
-			},
-			args: args{
-				data: []byte(`{"type":"org.osbuild.modprobe","options":{}}`),
-			},
-		},
-		{
-			name: "modprobe-data",
 			fields: fields{
 				Type: "org.osbuild.modprobe",
 				Options: &ModprobeStageOptions{
-					ConfigFiles: map[string]ModprobeConfigCmdList{
-						"disallow-modules.conf": {
-							&ModprobeConfigCmdBlacklist{
-								Command:    "blacklist",
-								Modulename: "nouveau",
-							},
-							&ModprobeConfigCmdBlacklist{
-								Command:    "blacklist",
-								Modulename: "floppy",
-							},
+					Filename: "disallow-modules.conf",
+					Commands: ModprobeConfigCmdList{
+						&ModprobeConfigCmdBlacklist{
+							Command:    "blacklist",
+							Modulename: "nouveau",
 						},
-						"disallow-additional-modules.conf": {
-							&ModprobeConfigCmdBlacklist{
-								Command:    "blacklist",
-								Modulename: "my-module",
-							},
+						&ModprobeConfigCmdBlacklist{
+							Command:    "blacklist",
+							Modulename: "floppy",
 						},
 					},
 				},
 			},
 			args: args{
-				data: []byte(`{"type":"org.osbuild.modprobe","options":{"configuration_files":{"disallow-additional-modules.conf":[{"command":"blacklist","modulename":"my-module"}],"disallow-modules.conf":[{"command":"blacklist","modulename":"nouveau"},{"command":"blacklist","modulename":"floppy"}]}}}`),
+				data: []byte(`{"type":"org.osbuild.modprobe","options":{"filename":"disallow-modules.conf","commands":[{"command":"blacklist","modulename":"nouveau"},{"command":"blacklist","modulename":"floppy"}]}}`),
 			},
 		},
 		{
@@ -490,49 +448,36 @@ func TestStage_UnmarshalJSON(t *testing.T) {
 		{
 			name: "systemd-unit-dropins",
 			fields: fields{
-				Type: "org.osbuild.systemd",
-				Options: &SystemdStageOptions{
-					UnitDropins: map[string]SystemdServiceUnitDropins{
-						"nm-cloud-setup.service": {
-							"10-rh-enable-for-ec2.conf": {
-								Service: &SystemdUnitServiceSection{
-									Environment: "NM_CLOUD_SETUP_EC2=yes",
-								},
-							},
+				Type: "org.osbuild.systemd.unit",
+				Options: &SystemdUnitStageOptions{
+					Unit:   "nm-cloud-setup.service",
+					Dropin: "10-rh-enable-for-ec2.conf",
+					Config: SystemdServiceUnitDropin{
+						Service: &SystemdUnitServiceSection{
+							Environment: "NM_CLOUD_SETUP_EC2=yes",
 						},
 					},
 				},
 			},
 			args: args{
-				data: []byte(`{"type":"org.osbuild.systemd","options":{"unit_dropins":{"nm-cloud-setup.service":{"10-rh-enable-for-ec2.conf":{"Service":{"Environment":"NM_CLOUD_SETUP_EC2=yes"}}}}}}`),
+				data: []byte(`{"type":"org.osbuild.systemd.unit","options":{"unit":"nm-cloud-setup.service","dropin":"10-rh-enable-for-ec2.conf","config":{"Service":{"Environment":"NM_CLOUD_SETUP_EC2=yes"}}}}`),
 			},
 		},
 		{
 			name: "systemd-logind",
-			fields: fields{
-				Type:    "org.osbuild.systemd-logind",
-				Options: &SystemdLogindStageOptions{},
-			},
-			args: args{
-				data: []byte(`{"type":"org.osbuild.systemd-logind","options":{}}`),
-			},
-		},
-		{
-			name: "systemd-logind-data",
 			fields: fields{
 				Type: "org.osbuild.systemd-logind",
 				Options: &SystemdLogindStageOptions{
-					ConfigDropins: map[string]SystemdLogindConfigDropin{
-						"10-ec2-getty-fix.conf": {
-							Login: SystemdLogindConfigLoginSection{
-								NAutoVT: common.IntToPtr(0),
-							},
+					Filename: "10-ec2-getty-fix.conf",
+					Config: SystemdLogindConfigDropin{
+						Login: SystemdLogindConfigLoginSection{
+							NAutoVT: common.IntToPtr(0),
 						},
 					},
 				},
 			},
 			args: args{
-				data: []byte(`{"type":"org.osbuild.systemd-logind","options":{"configuration_dropins":{"10-ec2-getty-fix.conf":{"Login":{"NAutoVT":0}}}}}`),
+				data: []byte(`{"type":"org.osbuild.systemd-logind","options":{"filename":"10-ec2-getty-fix.conf","config":{"Login":{"NAutoVT":0}}}}`),
 			},
 		},
 		{
diff --git a/internal/osbuild2/systemd_logind_stage_test.go b/internal/osbuild2/systemd_logind_stage_test.go
index a3be6fb38..f64fbf1c0 100644
--- a/internal/osbuild2/systemd_logind_stage_test.go
+++ b/internal/osbuild2/systemd_logind_stage_test.go
@@ -21,13 +21,16 @@ func TestSystemdLogindStage_MarshalJSON_Invalid(t *testing.T) {
 		name    string
 		options SystemdLogindStageOptions
 	}{
+		{
+			name:    "empty-options",
+			options: SystemdLogindStageOptions{},
+		},
 		{
 			name: "no-section-options",
 			options: SystemdLogindStageOptions{
-				ConfigDropins: map[string]SystemdLogindConfigDropin{
-					"10-ec2-getty-fix.conf": {
-						Login: SystemdLogindConfigLoginSection{},
-					},
+				Filename: "10-ec2-getty-fix.conf",
+				Config: SystemdLogindConfigDropin{
+					Login: SystemdLogindConfigLoginSection{},
 				},
 			},
 		},
diff --git a/internal/osbuild2/systemd_unit_stage_test.go b/internal/osbuild2/systemd_unit_stage_test.go
new file mode 100644
index 000000000..9d9fa3ce3
--- /dev/null
+++ b/internal/osbuild2/systemd_unit_stage_test.go
@@ -0,0 +1,16 @@
+package osbuild2
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewSystemdUnitStage(t *testing.T) {
+	expectedStage := &Stage{
+		Type:    "org.osbuild.systemd.unit",
+		Options: &SystemdUnitStageOptions{},
+	}
+	actualStage := NewSystemdUnitStage(&SystemdUnitStageOptions{})
+	assert.Equal(t, expectedStage, actualStage)
+}