MGMT-16721: Handle luks == nil and luks.Clevis == nil

In line 41 of internal/host/hostcommands/tang_connectivity_check_cmd.go Both `luks` and `luks.Clevis` are pointers which can be nil, there was no checking in place to ensure that we avoid a nil pointer dereference This PR fixes that by making appropriate checks to ensure that neither of these pointers are nil. There are some unanswered questions around why luks.Clevis would be nil and whether or not we should take other actions, this will be investigated in https://issues.redhat.com/browse/MGMT-16721
openshift · Feb 7, 2024 · 3e2431f · 3e2431f
1 parent df15ca0
commit 3e2431f
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 29 deletions.
diff --git a/internal/host/hostcommands/tang_connectivity_check_cmd.go b/internal/host/hostcommands/tang_connectivity_check_cmd.go
@@ -38,6 +38,15 @@ func (c *tangConnectivityCheckCmd) getTangServersFromHostIgnition(host *models.H
 		if err != nil {
 			return nil, err
 		}
+		if luks == nil {
+			return nil, nil
+		}
+		if luks.Clevis == nil {
+			// TODO: not sure how this could happen or whether its possibly valid, for now pretend encryption is disabled just so we avoid a nil pointer dereference
+			// Investigation of this will be carried out in https://issues.redhat.com/browse/MGMT-16721
+			c.log.Warn("luks clevis configuration is missing or incomplete in the host ignition, disk encryption will be assumed to be disabled.")
+			return nil, nil
+		}
 		if len(luks.Clevis.Tang) != 0 {
 			if tangServers, err = json.Marshal(luks.Clevis.Tang); err != nil {
 				return nil, err

diff --git a/internal/host/hostcommands/tang_connectivity_check_cmd_test.go b/internal/host/hostcommands/tang_connectivity_check_cmd_test.go
@@ -39,36 +39,88 @@ var _ = Describe("tangConnectivitycheckcmd", func() {
 
 	})
 
-	It("get_step: Tang imported cluster should use host ignition", func() {
+	const hostIgnition = `{
+		"ignition": {
+		  "config": {},
+		  "version": "3.2.0"
+		},
+		"storage": {
+			"luks": [
+				{
+				  "clevis": {
+					"tang": [
+					  {
+						"thumbprint": "nWW89qAs1hDPKiIcae-ey2cQmUk",
+						"url": "http://foo.bar"
+					  }
+					]
+				  },
+				  "device": "/dev/disk/by-partlabel/root",
+				  "name": "root",
+				  "options": [
+					"--cipher",
+					"aes-cbc-essiv:sha256"
+				  ],
+				  "wipeVolume": true
+				}
+			],
+		  "files": []
+		}
+	  }`
+
+	const hostIgnitionWithoutLuks = `{
+		"ignition": {
+		  "config": {},
+		  "version": "3.2.0"
+		},
+		"storage": {
+		  "files": []
+		}
+	  }`
+
+	const hostIgnitionWithoutClevis = `{
+		"ignition": {
+		  "config": {},
+		  "version": "3.2.0"
+		},
+		"storage": {
+			"luks": [
+			],
+		  "files": []
+		}
+	  }`
 
-		const hostIgnition = `{
-			"ignition": {
-			  "config": {},
-			  "version": "3.2.0"
-			},
-			"storage": {
-				"luks": [
-					{
-					  "clevis": {
-						"tang": [
-						  {
-							"thumbprint": "nWW89qAs1hDPKiIcae-ey2cQmUk",
-							"url": "http://foo.bar"
-						  }
-						]
-					  },
-					  "device": "/dev/disk/by-partlabel/root",
-					  "name": "root",
-					  "options": [
-						"--cipher",
-						"aes-cbc-essiv:sha256"
-					  ],
-					  "wipeVolume": true
-					}
-				],
-			  "files": []
-			}
-		  }`
+	It("Should not panic if Luks is undefined", func() {
+		imported := true
+		cluster.Imported = &imported
+		Expect(db.Create(&cluster).Error).ShouldNot(HaveOccurred())
+		apiVipConnectivity, err := json.Marshal(models.APIVipConnectivityResponse{
+			IsSuccess: true,
+			Ignition:  hostIgnitionWithoutLuks,
+		})
+		Expect(err).ToNot(HaveOccurred())
+		host.APIVipConnectivity = string(apiVipConnectivity)
+		Expect(db.Save(&host).Error).ShouldNot(HaveOccurred())
+		stepReply, stepErr = tangConnectivityCheckCmd.GetSteps(ctx, &host)
+		Expect(stepErr).ShouldNot(HaveOccurred())
+	})
+
+	It("Should not panic if Clevis is undefined", func() {
+		imported := true
+		cluster.Imported = &imported
+		Expect(db.Create(&cluster).Error).ShouldNot(HaveOccurred())
+		apiVipConnectivity, err := json.Marshal(models.APIVipConnectivityResponse{
+			IsSuccess: true,
+			Ignition:  hostIgnitionWithoutClevis,
+		})
+		Expect(err).ToNot(HaveOccurred())
+		host.APIVipConnectivity = string(apiVipConnectivity)
+		Expect(db.Save(&host).Error).ShouldNot(HaveOccurred())
+		stepReply, stepErr = tangConnectivityCheckCmd.GetSteps(ctx, &host)
+		Expect(stepErr).ShouldNot(HaveOccurred())
+	})
+
+	It("get_step: Tang imported cluster should use host ignition", func() {
 
 		By("Set cluster to imported and inject host ignition for test", func() {
 			imported := true