MGMT-16721: Fix for assisted service crash

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 makign appropriate checks to ensure that neither of these pointers are nil.
openshift · Feb 4, 2024 · f8a2ff2 · f8a2ff2
1 parent df15ca0
commit f8a2ff2
Show file tree

Hide file tree

Showing 2 changed files with 86 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,11 @@ func (c *tangConnectivityCheckCmd) getTangServersFromHostIgnition(host *models.H
 		if err != nil {
 			return nil, err
 		}
+		if luks == nil || 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
+			c.log.Warn("luks 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