Repository: rancher/cluster-api-provider-rke2
Pull request #402 (2024-08-12 10:36:33)

Fix cisProfile option with ignition format
Currently there are some stray tabs which break the rendered output, replace these with spaces and add some unit test coverage.

kind/bug

**What this PR does / why we need it**:

Currently the cisProfile option is broken when selecting ignition format for Leap/SLEMicro deployments

Fixes: #401


**Checklist**:
<!-- Put an "X" character inside the brackets of each completed task. Some may be optional depending on the PR in which case these can be deleted -->

- [ ] squashed commits into logical changes
- [ ] includes documentation
- [X] adds unit tests
- [ ] adds or updates e2e tests


hardys: Review welcome but marking this as a draft until I've tested it e2e locally
hardys: > @hardys let's rebase before re-running e2e tests on this one, thanks!

Will do, just want to complete some more local testing then will rebase and remove the draft status

Also looks like we'll need https://github.com/rancher/cluster-api-provider-rke2/pull/301 so good to see that may be getting revived
hardys: After testing locally I found another issue:

```
	converting base config to Ignition: error converting to Ignition: warning at $.storage.files.1.mode, line 41 col 13: unreasonable mode would be reasonable if specified in octal; remember to add a leading zero
```

I think that's because the [mode constant here](https://github.com/rancher/cluster-api-provider-rke2/blob/main/pkg/consts/global_consts.go#L49) needs updating, will add to this PR and re-test
hardys: I found some more issues which should now be resolved:

- The CIS prepration script was copies to /opt which isn't writable for Leap/SLEMicro, moved to /etc for both ignition/cloud-init cases so it's alongside the configuration files
- The script was running too early in the ignition flow, prior to the RKE2 installation, so the sysctl file was not yet present
- The script logic to find the sysctl file did not include the path used by the RKE2 installer, now added
hardys: > The CIS prepration script was copies to /opt which isn't writable for Leap/SLEMicro, moved to /etc for both ignition/cloud-init cases so it's alongside the configuration files

The latest iteration takes a different approach and mounts the /opt subvolume as described in https://en.opensuse.org/Portal:MicroOS/Ignition#Mounts - I'm still not sure writing the file to /opt is ideal, but with this approach I can avoid changing any of the existing cloud-init related code.
hardys: @alexander-demicev @furkatgofurov7 this is passing CI and now ready for review - please take a look when you get a moment, thanks!

Patch:
diff --git a/bootstrap/internal/ignition/butane/butane.go b/bootstrap/internal/ignition/butane/butane.go
index cd81310..a4432b5 100644
--- a/bootstrap/internal/ignition/butane/butane.go
+++ b/bootstrap/internal/ignition/butane/butane.go
@@ -38,9 +38,8 @@ import (
 // The rke2-install.service unit is enabled and is executed only once during the boot process to run the /etc/rke2-install.sh script.
 // This script installs and deploys RKE2, and performs pre and post-installation commands.
 // The ntpd.service unit is enabled only if NTP servers are specified.
-// The second section defines storage files for the system. It creates a file at /etc/rke2-install.sh. If CISEnabled is set to true,
-// it runs an additional CIS script to enforce system security standards. If NTP servers are specified,
-// it creates an NTP configuration file at /etc/ntp.conf.
+// The second section defines storage files for the system. It creates a file at /etc/rke2-install.sh.
+// If NTP servers are specified, it creates an NTP configuration file at /etc/ntp.conf.
 const (
 	butaneTemplate = `
 variant: fcos
@@ -67,6 +66,13 @@ systemd:
       enabled: true
     {{- end }}
 storage:
+  filesystems:
+    - path: /opt
+      device: "/dev/disk/by-partlabel/p.lxroot"
+      format: btrfs
+      wipe_filesystem: false
+      mount_options:
+       - "subvol=/@/opt"
   files:
     - path: /etc/ssh/sshd_config
       mode: 0600
@@ -114,10 +120,6 @@ storage:
           {{ . | Indent 10 }}
           {{- end }}
 
-		  {{- if .CISEnabled }}
-  		  /opt/rke2-cis-script.sh
-		  {{ end }}
-
           {{ range .DeployRKE2Commands }}
           {{ . | Indent 10 }}
           {{- end }}
diff --git a/bootstrap/internal/ignition/ignition.go b/bootstrap/internal/ignition/ignition.go
index 64996dd..8197921 100644
--- a/bootstrap/internal/ignition/ignition.go
+++ b/bootstrap/internal/ignition/ignition.go
@@ -29,6 +29,7 @@ const (
 	controlPlaneCommand          = "curl -sfL https://get.rke2.io | INSTALL_RKE2_VERSION=%[1]s sh -s - server"
 	airGappedWorkerCommand       = "INSTALL_RKE2_ARTIFACT_PATH=/opt/rke2-artifacts INSTALL_RKE2_TYPE=\"agent\" sh /opt/install.sh"
 	workerCommand                = "curl -sfL https://get.rke2.io | INSTALL_RKE2_VERSION=%[1]s INSTALL_RKE2_TYPE=\"agent\" sh -s -"
+	cisPreparationCommand        = "/opt/rke2-cis-script.sh"
 )
 
 var (
@@ -171,6 +172,11 @@ func getRKE2Commands(baseUserData *cloudinit.BaseUserData, command, airgappedCom
 		rke2Commands = append(rke2Commands, fmt.Sprintf(command, baseUserData.RKE2Version))
 	}
 
+	// If CISEnabled is set to true we run an additional script for CIS mode pre-requisite config
+	if baseUserData.CISEnabled {
+		rke2Commands = append(rke2Commands, cisPreparationCommand)
+	}
+
 	rke2Commands = append(rke2Commands, systemdServices...)
 
 	return rke2Commands, nil
diff --git a/pkg/consts/global_consts.go b/pkg/consts/global_consts.go
index 35677f4..38b71df 100644
--- a/pkg/consts/global_consts.go
+++ b/pkg/consts/global_consts.go
@@ -46,5 +46,5 @@ const (
 	DefaultFileMode = "0644"
 
 	// FileModeRootExecutable is the mode of the files created by the controller when the owner is root.
-	FileModeRootExecutable = "700"
+	FileModeRootExecutable = "0700"
 )
diff --git a/pkg/rke2/config.go b/pkg/rke2/config.go
index b2cc404..47c04a4 100644
--- a/pkg/rke2/config.go
+++ b/pkg/rke2/config.go
@@ -55,6 +55,7 @@ fi
 
 YUM_BASED_PARAM_FILE_FOUND=false
 TAR_BASED_PARAM_FILE_FOUND=false
+INSTALLER_BASED_PARAM_FILE_FOUND=false
 
 # Using RKE2 generated kernel parameters
 if [ -f /usr/share/rke2/rke2-cis-sysctl.conf ]; then
@@ -67,7 +68,12 @@ if [ -f /usr/local/share/rke2/rke2-cis-sysctl.conf ]; then
 		cp -f /usr/local/share/rke2/rke2-cis-sysctl.conf /etc/sysctl.d/90-rke2-cis.conf
 fi
 
-if [ "$YUM_BASED_PARAM_FILE_FOUND" = false ] && [ "$TAR_BASED_PARAM_FILE_FOUND" = false ]; then
+if [ -f /opt/rke2/share/rke2/rke2-cis-sysctl.conf ]; then
+		INSTALLER_BASED_PARAM_FILE_FOUND=true
+		cp -f /opt/rke2/share/rke2/rke2-cis-sysctl.conf /etc/sysctl.d/90-rke2-cis.conf
+fi
+
+if [ "$YUM_BASED_PARAM_FILE_FOUND" = false ] && [ "$TAR_BASED_PARAM_FILE_FOUND" = false ] && [ "$INSTALLER_BASED_PARAM_FILE_FOUND" = false ]; then
 		echo "No kernel parameters file found"
 		exit 1
 fi


Test patch:
diff --git a/bootstrap/internal/ignition/ignition_test.go b/bootstrap/internal/ignition/ignition_test.go
index 1336ac0..82af74f 100644
--- a/bootstrap/internal/ignition/ignition_test.go
+++ b/bootstrap/internal/ignition/ignition_test.go
@@ -18,11 +18,18 @@ package ignition
 
 import (
 	"fmt"
+	"compress/gzip"
+	"bytes"
+	"io/ioutil"
+	"encoding/base64"
+	"strings"
 	"testing"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	ignition "github.com/coreos/ignition/v2/config/v3_3"
+
 	bootstrapv1 "github.com/rancher/cluster-api-provider-rke2/bootstrap/api/v1beta1"
 	"github.com/rancher/cluster-api-provider-rke2/bootstrap/internal/cloudinit"
 )
@@ -75,24 +82,64 @@ var _ = Describe("NewJoinWorker", func() {
 	})
 
 	It("should return ignition data for worker", func() {
-		ignition, err := NewJoinWorker(input)
+		ignitionJson, err := NewJoinWorker(input)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(ignitionJson).ToNot(BeNil())
+
+		ign, reports, err := ignition.Parse(ignitionJson)
 		Expect(err).ToNot(HaveOccurred())
-		Expect(ignition).ToNot(BeNil())
+		Expect(reports.IsFatal()).To(BeFalse())
+
+		Expect(ign.Storage.Files).To(HaveLen(4))
+		Expect(ign.Storage.Files[0].Path).To(Equal("/etc/ssh/sshd_config"))
+		Expect(ign.Storage.Files[1].Path).To(Equal("/test/file"))
+		Expect(ign.Storage.Files[2].Path).To(Equal("/test/config"))
+		Expect(ign.Storage.Files[3].Path).To(Equal("/etc/rke2-install.sh"))
 	})
 
 	It("should return error if input is nil", func() {
 		input = nil
-		ignition, err := NewJoinWorker(input)
+		ignitionJson, err := NewJoinWorker(input)
 		Expect(err).To(HaveOccurred())
-		Expect(ignition).To(BeNil())
+		Expect(ignitionJson).To(BeNil())
 	})
 
 	It("should return error if base userdata is nil", func() {
 		input.BaseUserData = nil
-		ignition, err := NewJoinWorker(input)
+		ignitionJson, err := NewJoinWorker(input)
 		Expect(err).To(HaveOccurred())
-		Expect(ignition).To(BeNil())
+		Expect(ignitionJson).To(BeNil())
 	})
+
+	It("should add preparation script with CISEnabled", func() {
+		input.CISEnabled = true
+		ignitionJson, err := NewJoinWorker(input)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(ignitionJson).ToNot(BeNil())
+
+		ign, reports, err := ignition.Parse(ignitionJson)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(reports.IsFatal()).To(BeFalse())
+
+		Expect(ign.Storage.Files).To(HaveLen(4))
+		Expect(ign.Storage.Files[0].Path).To(Equal("/etc/ssh/sshd_config"))
+		Expect(ign.Storage.Files[1].Path).To(Equal("/test/file"))
+		Expect(ign.Storage.Files[2].Path).To(Equal("/test/config"))
+		Expect(ign.Storage.Files[3].Path).To(Equal("/etc/rke2-install.sh"))
+
+		// Check rke2-install.sh contains the call to rke2-cis-script.sh
+		// The ignition file contents is gzipped and base64 encoded, so unpack it first
+		scriptContentsEnc := strings.Split(*ign.Storage.Files[3].Contents.Source, ",")[1]
+		scriptContentsGzip, err := base64.StdEncoding.DecodeString(scriptContentsEnc)
+		Expect(err).ToNot(HaveOccurred())
+		reader := bytes.NewReader(scriptContentsGzip)
+		gzreader, err := gzip.NewReader(reader);
+		Expect(err).ToNot(HaveOccurred())
+		scriptContents, err := ioutil.ReadAll(gzreader)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(scriptContents).To(ContainSubstring("/opt/rke2-cis-script.sh"))
+	})
+
 })
 
 var _ = Describe("NewJoinControlPlane", func() {
@@ -125,23 +172,23 @@ var _ = Describe("NewJoinControlPlane", func() {
 	})
 
 	It("should return ignition data for control plane", func() {
-		ignition, err := NewJoinControlPlane(input)
+		ignitionJson, err := NewJoinControlPlane(input)
 		Expect(err).ToNot(HaveOccurred())
-		Expect(ignition).ToNot(BeNil())
+		Expect(ignitionJson).ToNot(BeNil())
 	})
 
 	It("should return error if input is nil", func() {
 		input = nil
-		ignition, err := NewJoinControlPlane(input)
+		ignitionJson, err := NewJoinControlPlane(input)
 		Expect(err).To(HaveOccurred())
-		Expect(ignition).To(BeNil())
+		Expect(ignitionJson).To(BeNil())
 	})
 
 	It("should return error if control plane input is nil", func() {
 		input.ControlPlaneInput = nil
-		ignition, err := NewJoinControlPlane(input)
+		ignitionJson, err := NewJoinControlPlane(input)
 		Expect(err).To(HaveOccurred())
-		Expect(ignition).To(BeNil())
+		Expect(ignitionJson).To(BeNil())
 	})
 })
 
@@ -175,23 +222,23 @@ var _ = Describe("NewInitControlPlane", func() {
 	})
 
 	It("should return ignition data for control plane", func() {
-		ignition, err := NewInitControlPlane(input)
+		ignitionJson, err := NewInitControlPlane(input)
 		Expect(err).ToNot(HaveOccurred())
-		Expect(ignition).ToNot(BeNil())
+		Expect(ignitionJson).ToNot(BeNil())
 	})
 
 	It("should return error if input is nil", func() {
 		input = nil
-		ignition, err := NewInitControlPlane(input)
+		ignitionJson, err := NewInitControlPlane(input)
 		Expect(err).To(HaveOccurred())
-		Expect(ignition).To(BeNil())
+		Expect(ignitionJson).To(BeNil())
 	})
 
 	It("should return error if control plane input is nil", func() {
 		input.ControlPlaneInput = nil
-		ignition, err := NewInitControlPlane(input)
+		ignitionJson, err := NewInitControlPlane(input)
 		Expect(err).To(HaveOccurred())
-		Expect(ignition).To(BeNil())
+		Expect(ignitionJson).To(BeNil())
 	})
 })