Fix volume ID in NodeUnstage and NodeUnpublish tests; add closeLuksMo…

…untSource test cases - Updated volume ID from "test" to "3232-pvc1234" in NodeUnstageVolumeRequest and NodeUnpublishVolumeRequest tests. - Added comprehensive test cases for closeLuksMountSource, covering success and error scenarios.
linode · Sep 11, 2024 · 83184dc · 83184dc
1 parent 475c837
commit 83184dc
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 102 deletions.
diff --git a/internal/driver/luks.go b/internal/driver/luks.go
@@ -220,7 +220,9 @@ func (e *Encryption) luksOpen(ctx LuksContext, volume string) error {
 	return nil
 }
 
-// check is a given mapping under /dev/mapper is a luks volume
+// isLuksMapping checks if the specified volume ID is a LUKS volume 
+// by executing the 'cryptsetup status' command. It returns true if 
+// the volume is LUKS, false otherwise, along with any error encountered.
 func (e *Encryption) isLuksMapping(volume string) (bool, error) {
 	cryptsetupCmd, err := e.getCryptsetupCmd()
 	if err != nil {

diff --git a/internal/driver/nodeserver_elevated_test.go b/internal/driver/nodeserver_elevated_test.go
@@ -73,7 +73,7 @@ func TestNodeUnstageUnpublishVolume(t *testing.T) {
 			prepareFunc: defaultPrepareFunc,
 			call: func(target string) error {
 				req := &csi.NodeUnstageVolumeRequest{
-					VolumeId:          "test",
+					VolumeId:          "3232-pvc1234",
 					StagingTargetPath: target,
 				}
 
@@ -86,7 +86,7 @@ func TestNodeUnstageUnpublishVolume(t *testing.T) {
 			prepareFunc: defaultPrepareFunc,
 			call: func(target string) error {
 				req := &csi.NodeUnpublishVolumeRequest{
-					VolumeId:   "test",
+					VolumeId:   "3232-pvc1234",
 					TargetPath: target,
 				}
 

diff --git a/internal/driver/nodeserver_helpers_test.go b/internal/driver/nodeserver_helpers_test.go
@@ -796,105 +796,110 @@ func TestNodeServer_mountVolume(t *testing.T) {
 	}
 }
 
-// func TestNodeServer_closeLuksMountSource(t *testing.T) {
-// 	tests := []struct {
-// 		name               string
-// 		expectMounterCalls func(m *mocks.MockMounter)
-// 		expectExecCalls    func(m *mocks.MockExecutor, c *mocks.MockCommand)
-// 		expectFsCalls      func(m *mocks.MockFileSystem)
-// 		volumeID           string
-// 		wantErr            bool
-// 	}{
-// 		{
-// 			name: "Success - close mount source",
-// 			expectExecCalls: func(m *mocks.MockExecutor, c *mocks.MockCommand) {
-// 			},
-// 			volumeID:    "3232-pvc1234",
-// 			wantErr: false,
-// 		},
-// 		{
-// 			name: "Success - close LUKS mount source",
-// 			expectExecCalls: func(m *mocks.MockExecutor, c *mocks.MockCommand) {
-// 				m.EXPECT().LookPath(gomock.Any()).Return("/bin/test", nil)
-// 				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
-// 				c.EXPECT().CombinedOutput().Return([]byte("/dev/mapper/test"), nil)
-
-// 				m.EXPECT().LookPath(gomock.Any()).Return("/bin/test", nil)
-// 				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
-// 				c.EXPECT().CombinedOutput().Return([]byte("type: luks"), nil)
-
-// 				m.EXPECT().LookPath(gomock.Any()).Return("/bin/test", nil)
-// 				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
-// 				c.EXPECT().CombinedOutput().Return([]byte("test"), nil)
-// 			},
-// 			volumeID:    "3232-pvc1234",
-// 			wantErr: false,
-// 		},
-// 		{
-// 			name: "Error - failed to close LUKS mount source",
-// 			expectExecCalls: func(m *mocks.MockExecutor, c *mocks.MockCommand) {
-// 				m.EXPECT().LookPath(gomock.Any()).Return("/bin/test", nil)
-// 				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
-// 				c.EXPECT().CombinedOutput().Return([]byte("/dev/mapper/test"), nil)
-
-// 				m.EXPECT().LookPath(gomock.Any()).Return("/bin/test", nil)
-// 				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
-// 				c.EXPECT().CombinedOutput().Return([]byte("type: luks"), nil)
-
-// 				m.EXPECT().LookPath(gomock.Any()).Return("/bin/test", nil)
-// 				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
-// 				c.EXPECT().CombinedOutput().Return(nil, fmt.Errorf("Unexpected error"))
-// 			},
-// 			volumeID:    "3232-pvc1234",
-// 			wantErr: true,
-// 		},
-// 		{
-// 			name: "Error - failed to determine if mount is a luks mapping",
-// 			expectExecCalls: func(m *mocks.MockExecutor, c *mocks.MockCommand) {
-// 				m.EXPECT().LookPath(gomock.Any()).Return("/bin/test", nil)
-// 				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
-// 				c.EXPECT().CombinedOutput().Return([]byte("/dev/mapper/test"), nil)
-
-// 				m.EXPECT().LookPath(gomock.Any()).Return("", osexec.ErrNotFound)
-// 			},
-// 			volumeID:    "3232-pvc1234",
-// 			wantErr: true,
-// 		},
-// 	}
-// 	for _, tt := range tests {
-// 		t.Run(tt.name, func(t *testing.T) {
-
-// 			ctrl := gomock.NewController(t)
-// 			defer ctrl.Finish()
-
-// 			mockMounter := mocks.NewMockMounter(ctrl)
-// 			mockFileSystem := mocks.NewMockFileSystem(ctrl)
-// 			mockExec := mocks.NewMockExecutor(ctrl)
-// 			mockCommand := mocks.NewMockCommand(ctrl)
-
-// 			if tt.expectExecCalls != nil {
-// 				tt.expectExecCalls(mockExec, mockCommand)
-// 			}
-// 			if tt.expectFsCalls != nil {
-// 				tt.expectFsCalls(mockFileSystem)
-// 			}
-// 			if tt.expectMounterCalls != nil {
-// 				tt.expectMounterCalls(mockMounter)
-// 			}
-
-// 			ns := &NodeServer{
-// 				mounter: &mount.SafeFormatAndMount{
-// 					Interface: mockMounter,
-// 					Exec:      mockExec,
-// 				},
-// 				encrypt: NewLuksEncryption(mockExec, mockFileSystem),
-// 			}
-// 			if err := ns.closeLuksMountSource(context.Background(), tt.volumeID); (err != nil) != tt.wantErr {
-// 				t.Errorf("NodeServer.closeLuksMountSources() error = %v, wantErr %v", err, tt.wantErr)
-// 			}
-// 		})
-// 	}
-// }
+func TestNodeServer_closeLuksMountSource(t *testing.T) {
+	tests := []struct {
+		name               string
+		expectMounterCalls func(m *mocks.MockMounter)
+		expectExecCalls    func(m *mocks.MockExecutor, c *mocks.MockCommand)
+		expectFsCalls      func(m *mocks.MockFileSystem)
+		volumeID           string
+		wantErr            bool
+	}{
+		{
+			name: "Success - Able to close LUKS volume",
+			expectExecCalls: func(m *mocks.MockExecutor, c *mocks.MockCommand) {
+				// check if the luks volume is already open
+				m.EXPECT().LookPath(gomock.Any()).Return("/bin/cryptsetup", nil)
+				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
+				c.EXPECT().CombinedOutput().Return([]byte("type: luks"), nil)
+
+				// close the luks volume
+				m.EXPECT().LookPath(gomock.Any()).Return("/bin/cryptsetup", nil)
+				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
+				c.EXPECT().CombinedOutput().Return([]byte("success"), nil)
+			},
+			volumeID:    "3232-pvc1234",
+			wantErr: false,
+		},
+		{
+			name: "Success - If volume is not a LUKS volume",
+			expectExecCalls: func(m *mocks.MockExecutor, c *mocks.MockCommand) {
+				m.EXPECT().LookPath(gomock.Any()).Return("/bin/cryptsetup", nil)
+				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
+				c.EXPECT().CombinedOutput().Return([]byte("type: none"), nil)
+			},
+			volumeID:    "3232-pvc1234",
+			wantErr: false,
+		},
+		{
+			name: "Error - Unable to close LUKS volume",
+			expectExecCalls: func(m *mocks.MockExecutor, c *mocks.MockCommand) {
+				// check if the luks volume is already open
+				m.EXPECT().LookPath(gomock.Any()).Return("/bin/cryptsetup", nil)
+				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
+				c.EXPECT().CombinedOutput().Return([]byte("type: luks"), nil)
+
+				// close the luks volume
+				m.EXPECT().LookPath(gomock.Any()).Return("/bin/cryptsetup", nil)
+				m.EXPECT().Command(gomock.Any(), gomock.Any()).Return(c)
+				c.EXPECT().CombinedOutput().Return(nil, fmt.Errorf("Unable to close LUKS volume"))
+			},
+			volumeID:    "3232-pvc1234",
+			wantErr: true,
+		},
+		{
+			name: "Error - Invalid volume ID",
+			volumeID:    "3232-pvc-test-1234",
+			wantErr: true,
+		},
+		{
+			name: "Error - Invalid volume ID",
+			volumeID:    "3232-test1234",
+			wantErr: true,
+		},
+		{
+			name: "Error - unable to find cryptsetup",
+			expectExecCalls: func(m *mocks.MockExecutor, c *mocks.MockCommand) {
+				m.EXPECT().LookPath(gomock.Any()).Return("", osexec.ErrNotFound)
+			},
+			volumeID:    "3232-pvc1234",
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			mockMounter := mocks.NewMockMounter(ctrl)
+			mockFileSystem := mocks.NewMockFileSystem(ctrl)
+			mockExec := mocks.NewMockExecutor(ctrl)
+			mockCommand := mocks.NewMockCommand(ctrl)
+
+			if tt.expectExecCalls != nil {
+				tt.expectExecCalls(mockExec, mockCommand)
+			}
+			if tt.expectFsCalls != nil {
+				tt.expectFsCalls(mockFileSystem)
+			}
+			if tt.expectMounterCalls != nil {
+				tt.expectMounterCalls(mockMounter)
+			}
+
+			ns := &NodeServer{
+				mounter: &mount.SafeFormatAndMount{
+					Interface: mockMounter,
+					Exec:      mockExec,
+				},
+				encrypt: NewLuksEncryption(mockExec, mockFileSystem),
+			}
+			if err := ns.closeLuksMountSource(context.Background(), tt.volumeID); (err != nil) != tt.wantErr {
+				t.Errorf("NodeServer.closeLuksMountSources() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
 
 func Test_validateNodeExpandVolumeRequest(t *testing.T) {
 	tests := []struct {