Remove RHC credentials from the system after registration (HMS-3814)

Schutzfile

@@ -8,21 +8,21 @@
   "centos-8": {
     "dependencies": {
       "osbuild": {
-        "commit": "5b75592fefc4ee2d9e240a15b7002f2537de31b0"
+        "commit": "f255fba09f2f0c879d92df37e3d9f8e31903720d"
       }
     }
   },
   "centos-9": {
     "dependencies": {
       "osbuild": {
-        "commit": "5b75592fefc4ee2d9e240a15b7002f2537de31b0"
+        "commit": "f255fba09f2f0c879d92df37e3d9f8e31903720d"
       }
     }
   },
   "fedora-39": {
     "dependencies": {
       "osbuild": {
-        "commit": "5b75592fefc4ee2d9e240a15b7002f2537de31b0"
+        "commit": "f255fba09f2f0c879d92df37e3d9f8e31903720d"
       }
     },
     "repos": [

cmd/build/main.go

@@ -37,8 +37,8 @@ func check(err error) {
 
 type BuildConfig struct {
 	Name      string               `json:"name"`
-	OSTree    *ostree.ImageOptions `json:"ostree,omitempty"`
 	Blueprint *blueprint.Blueprint `json:"blueprint,omitempty"`
+	Options   distro.ImageOptions  `json:"options"`
 	Depends   interface{}          `json:"depends,omitempty"` // ignored
 }
 
@@ -61,8 +61,7 @@ func loadConfig(path string) BuildConfig {
 func makeManifest(imgType distro.ImageType, config BuildConfig, distribution distro.Distro, repos []rpmmd.RepoConfig, archName string, seedArg int64, cacheRoot string) (manifest.OSBuildManifest, error) {
 	cacheDir := filepath.Join(cacheRoot, archName+distribution.Name())
 
-	options := distro.ImageOptions{Size: 0}
-	options.OSTree = config.OSTree
+	options := config.Options
 
 	// add RHSM fact to detect changes
 	options.Facts = &facts.ImageOptions{

cmd/gen-manifests/main.go

@@ -52,9 +52,9 @@ type buildRequest struct {
 
 type BuildConfig struct {
 	Name      string               `json:"name"`
-	OSTree    *ostree.ImageOptions `json:"ostree,omitempty"`
 	Blueprint *blueprint.Blueprint `json:"blueprint,omitempty"`
-	Depends   BuildDependency      `json:"depends,omitempty"`
+	Options   distro.ImageOptions  `json:"options"`
+	Depends   interface{}          `json:"depends,omitempty"` // ignored
 }
 
 type BuildDependency struct {
@@ -210,8 +210,7 @@ func makeManifestJob(
 	filename := fmt.Sprintf("%s-%s-%s-%s.json", u(distroName), u(archName), u(imgType.Name()), u(name))
 	cacheDir := filepath.Join(cacheRoot, archName+distribution.Name())
 
-	options := distro.ImageOptions{Size: 0}
-	options.OSTree = bc.OSTree
+	options := bc.Options
 
 	var bp blueprint.Blueprint
 	if bc.Blueprint != nil {

pkg/distro/distro.go

@@ -148,11 +148,11 @@ type ImageType interface {
 
 // The ImageOptions specify options for a specific image build
 type ImageOptions struct {
-	Size             uint64
-	OSTree           *ostree.ImageOptions
-	Subscription     *subscription.ImageOptions
-	Facts            *facts.ImageOptions
-	PartitioningMode disk.PartitioningMode
+	Size             uint64                     `json:"size"`
+	OSTree           *ostree.ImageOptions       `json:"ostree,omitempty"`
+	Subscription     *subscription.ImageOptions `json:"subscription,omitempty"`
+	Facts            *facts.ImageOptions        `json:"facts,omitempty"`
+	PartitioningMode disk.PartitioningMode      `json:"partitioning-mode,omitempty"`
 }
 
 type BasePartitionTableMap map[string]disk.PartitionTable

pkg/manifest/os.go

@@ -558,16 +558,27 @@ func (p *OS) serialize() osbuild.Pipeline {
 	// - Register with subscription-manager, no Insights or rhc
 	// - Register with subscription-manager and enable Insights, no rhc
 	if p.Subscription != nil {
+		// Write a key file that will contain the org ID and activation key to be sourced in the systemd service.
+		// The file will also act as the ConditionFirstBoot file.
+		subkeyFilepath := "/etc/osbuild-subscription-register.env"
+		subkeyContent := fmt.Sprintf("ORG_ID=%s\nACTIVATION_KEY=%s", p.Subscription.Organization, p.Subscription.ActivationKey)
+		if subkeyFile, err := fsnode.NewFile(subkeyFilepath, nil, "root", "root", []byte(subkeyContent)); err == nil {
+			p.Files = append(p.Files, subkeyFile)
+		} else {
+			panic(err)
+		}
+
 		var commands []string
 		if p.Subscription.Rhc {
+			// TODO: replace org ID and activation key with env vars
 			// Use rhc for registration instead of subscription manager
-			commands = []string{fmt.Sprintf("/usr/bin/rhc connect -o=%s -a=%s --server %s", p.Subscription.Organization, p.Subscription.ActivationKey, p.Subscription.ServerUrl)}
+			commands = []string{fmt.Sprintf("/usr/bin/rhc connect --organization=${ORG_ID} --activation-key=${ACTIVATION_KEY} --server %s", p.Subscription.ServerUrl)}
 			// insights-client creates the .gnupg directory during boot process, and is labeled incorrectly
 			commands = append(commands, "restorecon -R /root/.gnupg")
 			// execute the rhc post install script as the selinuxenabled check doesn't work in the buildroot container
 			commands = append(commands, "/usr/sbin/semanage permissive --add rhcd_t")
 		} else {
-			commands = []string{fmt.Sprintf("/usr/sbin/subscription-manager register --org=%s --activationkey=%s --serverurl %s --baseurl %s", p.Subscription.Organization, p.Subscription.ActivationKey, p.Subscription.ServerUrl, p.Subscription.BaseUrl)}
+			commands = []string{fmt.Sprintf("/usr/sbin/subscription-manager register --org=${ORG_ID} --activationkey=${ACTIVATION_KEY} --serverurl %s --baseurl %s", p.Subscription.ServerUrl, p.Subscription.BaseUrl)}
 
 			// Insights is optional when using subscription-manager
 			if p.Subscription.Insights {
@@ -577,10 +588,33 @@ func (p *OS) serialize() osbuild.Pipeline {
 			}
 		}
 
-		pipeline.AddStage(osbuild.NewFirstBootStage(&osbuild.FirstBootStageOptions{
-			Commands:       commands,
-			WaitForNetwork: true,
-		}))
+		commands = append(commands, fmt.Sprintf("/usr/bin/rm %s", subkeyFilepath))
+
+		subscribeServiceFile := "osbuild-subscription-register.service"
+		regServiceStageOptions := &osbuild.SystemdUnitCreateStageOptions{
+			Filename: subscribeServiceFile,
+			UnitType: "system",
+			UnitPath: osbuild.Usr,
+			Config: osbuild.SystemdServiceUnit{
+				Unit: &osbuild.Unit{
+					Description:         "First-boot service for registering with Red Hat subscription manager and/or insights",
+					ConditionPathExists: []string{subkeyFilepath},
+					Wants:               []string{"network-online.target"},
+					After:               []string{"network-online.target"},
+				},
+				Service: &osbuild.Service{
+					Type:            osbuild.Oneshot,
+					RemainAfterExit: false,
+					ExecStart:       commands,
+					EnvironmentFile: []string{subkeyFilepath},
+				},
+				Install: &osbuild.Install{
+					WantedBy: []string{"default.target"},
+				},
+			},
+		}
+		pipeline.AddStage(osbuild.NewSystemdUnitCreateStage(regServiceStageOptions))
+		p.EnabledServices = append(p.EnabledServices, subscribeServiceFile)
 
 		if rhsmConfig, exists := p.RHSMConfig[subscription.RHSMConfigWithSubscription]; exists {
 			pipeline.AddStage(osbuild.NewRHSMStage(rhsmConfig))

pkg/manifest/os_test.go

@@ -1,6 +1,7 @@
 package manifest
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/osbuild/images/pkg/osbuild"
@@ -44,20 +45,31 @@ func findStage(name string, stages []*osbuild.Stage) *osbuild.Stage {
 	return nil
 }
 
-// CheckFirstBootStageOptions checks the Command strings
-func CheckFirstBootStageOptions(t *testing.T, stages []*osbuild.Stage, commands []string) {
-	// Find the FirstBootStage
-	s := findStage("org.osbuild.first-boot", stages)
+// CheckSystemdStageOptions checks the Command strings
+func CheckSystemdStageOptions(t *testing.T, stages []*osbuild.Stage, commands []string) {
+	// Find the systemd.unit.create stage
+	s := findStage("org.osbuild.systemd.unit.create", stages)
 	require.NotNil(t, s)
 
 	require.NotNil(t, s.Options)
-	options, ok := s.Options.(*osbuild.FirstBootStageOptions)
+	options, ok := s.Options.(*osbuild.SystemdUnitCreateStageOptions)
 	require.True(t, ok)
-	require.Equal(t, len(options.Commands), len(commands))
+
+	// unit must be conditioned on the keyfile
+	require.Len(t, options.Config.Unit.ConditionPathExists, 1)
+	keyfile := options.Config.Unit.ConditionPathExists[0]
+	// keyfile is also the EnvironmentFile
+	require.Len(t, options.Config.Service.EnvironmentFile, 1)
+	assert.Equal(t, keyfile, options.Config.Service.EnvironmentFile[0])
+
+	execStart := options.Config.Service.ExecStart
+	// the rm command gets prepended in every case
+	commands = append(commands, fmt.Sprintf("/usr/bin/rm %s", keyfile))
+	require.Equal(t, len(execStart), len(commands))
 
 	// Make sure the commands are the same
 	for idx, cmd := range commands {
-		assert.Equal(t, cmd, options.Commands[idx])
+		assert.Equal(t, cmd, options.Config.Service.ExecStart[idx])
 	}
 }
 
@@ -84,8 +96,8 @@ func TestSubscriptionManagerCommands(t *testing.T) {
 		BaseUrl:       "http://cdn.redhat.com/",
 	}
 	pipeline := os.serialize()
-	CheckFirstBootStageOptions(t, pipeline.Stages, []string{
-		"/usr/sbin/subscription-manager register --org=2040324 --activationkey=my-secret-key --serverurl subscription.rhsm.redhat.com --baseurl http://cdn.redhat.com/",
+	CheckSystemdStageOptions(t, pipeline.Stages, []string{
+		"/usr/sbin/subscription-manager register --org=${ORG_ID} --activationkey=${ACTIVATION_KEY} --serverurl subscription.rhsm.redhat.com --baseurl http://cdn.redhat.com/",
 	})
 }
 
@@ -99,8 +111,8 @@ func TestSubscriptionManagerInsightsCommands(t *testing.T) {
 		Insights:      true,
 	}
 	pipeline := os.serialize()
-	CheckFirstBootStageOptions(t, pipeline.Stages, []string{
-		"/usr/sbin/subscription-manager register --org=2040324 --activationkey=my-secret-key --serverurl subscription.rhsm.redhat.com --baseurl http://cdn.redhat.com/",
+	CheckSystemdStageOptions(t, pipeline.Stages, []string{
+		"/usr/sbin/subscription-manager register --org=${ORG_ID} --activationkey=${ACTIVATION_KEY} --serverurl subscription.rhsm.redhat.com --baseurl http://cdn.redhat.com/",
 		"/usr/bin/insights-client --register",
 		"restorecon -R /root/.gnupg",
 	})
@@ -117,8 +129,8 @@ func TestRhcInsightsCommands(t *testing.T) {
 		Rhc:           true,
 	}
 	pipeline := os.serialize()
-	CheckFirstBootStageOptions(t, pipeline.Stages, []string{
-		"/usr/bin/rhc connect -o=2040324 -a=my-secret-key --server subscription.rhsm.redhat.com",
+	CheckSystemdStageOptions(t, pipeline.Stages, []string{
+		"/usr/bin/rhc connect --organization=${ORG_ID} --activation-key=${ACTIVATION_KEY} --server subscription.rhsm.redhat.com",
 		"restorecon -R /root/.gnupg",
 		"/usr/sbin/semanage permissive --add rhcd_t",
 	})

pkg/manifest/ostree_deployment.go

@@ -370,7 +370,7 @@ func (p *OSTreeDeployment) serialize() osbuild.Pipeline {
 		// issue # https://github.com/osbuild/images/issues/352
 		if len(p.CustomFileSystems) != 0 {
 			serviceName := "osbuild-ostree-mountpoints.service"
-			stageOption := osbuild.NewSystemdUnitCreateStageOptions(createMountpointService(serviceName, p.CustomFileSystems))
+			stageOption := osbuild.NewSystemdUnitCreateStage(createMountpointService(serviceName, p.CustomFileSystems))
 			stageOption.MountOSTree(p.osName, ref, 0)
 			pipeline.AddStage(stageOption)
 			p.EnabledServices = append(p.EnabledServices, serviceName)

pkg/osbuild/systemd_unit_create_stage.go

@@ -28,6 +28,7 @@ type Unit struct {
 	ConditionPathIsDirectory []string `json:"ConditionPathIsDirectory,omitempty"`
 	Requires                 []string `json:"Requires,omitempty"`
 	Wants                    []string `json:"Wants,omitempty"`
+	After                    []string `json:"After,omitempty"`
 }
 
 type Service struct {
@@ -61,6 +62,15 @@ type SystemdUnitCreateStageOptions struct {
 func (SystemdUnitCreateStageOptions) isStageOptions() {}
 
 func (o *SystemdUnitCreateStageOptions) validate() error {
+	fre := regexp.MustCompile(filenameRegex)
+	if !fre.MatchString(o.Filename) {
+		return fmt.Errorf("filename %q doesn't conform to schema (%s)", o.Filename, filenameRegex)
+	}
+
+	if o.Config.Install == nil {
+		return fmt.Errorf("Install section of systemd unit is required")
+	}
+
 	vre := regexp.MustCompile(envVarRegex)
 	if service := o.Config.Service; service != nil {
 		for _, envVar := range service.Environment {
@@ -72,7 +82,7 @@ func (o *SystemdUnitCreateStageOptions) validate() error {
 	return nil
 }
 
-func NewSystemdUnitCreateStageOptions(options *SystemdUnitCreateStageOptions) *Stage {
+func NewSystemdUnitCreateStage(options *SystemdUnitCreateStageOptions) *Stage {
 	if err := options.validate(); err != nil {
 		panic(err)
 	}

pkg/osbuild/systemd_unit_create_stage_test.go

@@ -48,7 +48,7 @@ func TestNewSystemdUnitCreateStage(t *testing.T) {
 		Options: &options,
 	}
 
-	actualStage := NewSystemdUnitCreateStageOptions(&options)
+	actualStage := NewSystemdUnitCreateStage(&options)
 	assert.Equal(t, expectedStage, actualStage)
 }
 
@@ -65,6 +65,6 @@ func TestNewSystemdUnitCreateStageInEtc(t *testing.T) {
 		Options: &options,
 	}
 
-	actualStage := NewSystemdUnitCreateStageOptions(&options)
+	actualStage := NewSystemdUnitCreateStage(&options)
 	assert.Equal(t, expectedStage, actualStage)
 }

pkg/subscription/subscription.go

@@ -4,12 +4,12 @@ package subscription
 // ServerUrl denotes the host to register the system with
 // BaseUrl specifies the repository URL for DNF
 type ImageOptions struct {
-	Organization  string
-	ActivationKey string
-	ServerUrl     string
-	BaseUrl       string
-	Insights      bool
-	Rhc           bool
+	Organization  string `json:"organization"`
+	ActivationKey string `json:"activation_key"`
+	ServerUrl     string `json:"server_url"`
+	BaseUrl       string `json:"base_url"`
+	Insights      bool   `json:"insights"`
+	Rhc           bool   `json:"rhc"`
 }
 
 type RHSMStatus string

test/configs/edge-ostree-pull-device-fips.json

@@ -6,8 +6,10 @@
       "installation_device": "/dev/vda"
     }
   },
-  "ostree": {
-    "url": "http://example.com/repo"
+  "options": {
+    "ostree": {
+      "url": "http://example.com/repo"
+    }
   },
   "depends": {
     "image-type": "edge-container",

test/configs/edge-ostree-pull-device.json

@@ -5,8 +5,10 @@
       "installation_device": "/dev/sda"
     }
   },
-  "ostree": {
-    "url": "http://example.com/repo"
+  "options": {
+    "ostree": {
+      "url": "http://example.com/repo"
+    }
   },
   "depends": {
     "image-type": "edge-container",

test/configs/edge-ostree-pull-empty.json

@@ -1,7 +1,9 @@
 {
   "name": "edge-ostree-pull-empty",
-  "ostree": {
-    "url": "http://example.com/repo"
+  "options": {
+    "ostree": {
+      "url": "http://example.com/repo"
+    }
   },
   "depends": {
     "image-type": "edge-container",

test/configs/edge-ostree-pull-fips.json

@@ -5,8 +5,10 @@
       "fips": true
     }
   },
-  "ostree": {
-    "url": "http://example.com/repo"
+  "options": {
+    "ostree": {
+      "url": "http://example.com/repo"
+    }
   },
   "depends": {
     "image-type": "edge-container",

test/configs/edge-ostree-pull-user-fips.json

@@ -18,7 +18,9 @@
     "config": "empty.json",
     "image-type": "edge-container"
   },
-  "ostree": {
-    "url": "http://example.com/repo"
+  "options": {
+    "ostree": {
+      "url": "http://example.com/repo"
+    }
   }
 }

test/configs/edge-ostree-pull-user.json

@@ -17,7 +17,9 @@
     "config": "empty.json",
     "image-type": "edge-container"
   },
-  "ostree": {
-    "url": "http://example.com/repo"
+  "options": {
+    "ostree": {
+      "url": "http://example.com/repo"
+    }
   }
 }

test/configs/iot-customizations-full.json

@@ -1,7 +1,9 @@
 {
   "name": "iot-customizations-full",
-  "ostree": {
-    "ref": "test/fedora/iot"
+  "options": {
+    "ostree": {
+      "ref": "test/fedora/iot"
+    }
   },
   "blueprint": {
     "customizations": {