Improve Test_InsertOrEjectMedia, address gosec issues (#608)

.changes/v2.22.0/608-notes.md

@@ -0,0 +1,3 @@
+* Improved test `Test_InsertOrEjectMedia` [GH-608]
+* Addressed `gosec` 2.17.0 errors [GH-608]
+

.github/workflows/check-code.yml

@@ -13,7 +13,7 @@ jobs:
     - name: Set up Go 1.x
       uses: actions/setup-go@v3
       with:
-        go-version: '1.19'
+        go-version: '1.21'
 
     - name: Check out code into the Go module directory
       uses: actions/checkout@v3

.github/workflows/check-security.yml

@@ -9,7 +9,7 @@ jobs:
       - name: Set up Go 1.x
         uses: actions/setup-go@v3
         with:
-          go-version: '1.19'
+          go-version: '1.21'
       - name: Checkout Source
         uses: actions/checkout@v2
       - name: gosec

govcd/adminorg_test.go

@@ -117,17 +117,16 @@ func (vcd *TestVCD) TestAdminOrg_SetLease(check *C) {
 		},
 	}
 
-	for _, info := range leaseData {
-
+	for infoIndex, info := range leaseData {
 		fmt.Printf("update lease params %v\n", info)
 		// Change the lease parameters for both vapp and vApp template
-		adminOrg.AdminOrg.OrgSettings.OrgVAppLeaseSettings.StorageLeaseSeconds = &info.vappStorageLease
-		adminOrg.AdminOrg.OrgSettings.OrgVAppLeaseSettings.DeploymentLeaseSeconds = &info.deploymentLeaseSeconds
-		adminOrg.AdminOrg.OrgSettings.OrgVAppLeaseSettings.PowerOffOnRuntimeLeaseExpiration = &info.powerOffOnRuntimeLeaseExpiration
-		adminOrg.AdminOrg.OrgSettings.OrgVAppLeaseSettings.DeleteOnStorageLeaseExpiration = &info.vappDeleteOnStorageLeaseExpiration
+		adminOrg.AdminOrg.OrgSettings.OrgVAppLeaseSettings.StorageLeaseSeconds = &leaseData[infoIndex].vappStorageLease
+		adminOrg.AdminOrg.OrgSettings.OrgVAppLeaseSettings.DeploymentLeaseSeconds = &leaseData[infoIndex].deploymentLeaseSeconds
+		adminOrg.AdminOrg.OrgSettings.OrgVAppLeaseSettings.PowerOffOnRuntimeLeaseExpiration = &leaseData[infoIndex].powerOffOnRuntimeLeaseExpiration
+		adminOrg.AdminOrg.OrgSettings.OrgVAppLeaseSettings.DeleteOnStorageLeaseExpiration = &leaseData[infoIndex].vappDeleteOnStorageLeaseExpiration
 
-		adminOrg.AdminOrg.OrgSettings.OrgVAppTemplateSettings.StorageLeaseSeconds = &info.vappTemplateStorageLease
-		adminOrg.AdminOrg.OrgSettings.OrgVAppTemplateSettings.DeleteOnStorageLeaseExpiration = &info.vappTemplateDeleteOnStorageLeaseExpiration
+		adminOrg.AdminOrg.OrgSettings.OrgVAppTemplateSettings.StorageLeaseSeconds = &leaseData[infoIndex].vappTemplateStorageLease
+		adminOrg.AdminOrg.OrgSettings.OrgVAppTemplateSettings.DeleteOnStorageLeaseExpiration = &leaseData[infoIndex].vappTemplateDeleteOnStorageLeaseExpiration
 
 		task, err := adminOrg.Update()
 		check.Assert(err, IsNil)

govcd/api_vcd_test.go

@@ -77,19 +77,20 @@ const (
 	TestVdcFindDiskByHREF         = "TestVdcFindDiskByHREF"
 	TestFindDiskByHREF            = "TestFindDiskByHREF"
 	TestDisk                      = "TestDisk"
-	TestVMAttachOrDetachDisk      = "TestVMAttachOrDetachDisk"
-	TestVMAttachDisk              = "TestVMAttachDisk"
-	TestVMDetachDisk              = "TestVMDetachDisk"
-	TestCreateExternalNetwork     = "TestCreateExternalNetwork"
-	TestDeleteExternalNetwork     = "TestDeleteExternalNetwork"
-	TestLbServiceMonitor          = "TestLbServiceMonitor"
-	TestLbServerPool              = "TestLbServerPool"
-	TestLbAppProfile              = "TestLbAppProfile"
-	TestLbAppRule                 = "TestLbAppRule"
-	TestLbVirtualServer           = "TestLbVirtualServer"
-	TestLb                        = "TestLb"
-	TestNsxvSnatRule              = "TestNsxvSnatRule"
-	TestNsxvDnatRule              = "TestNsxvDnatRule"
+	// #nosec G101 -- Not a credential
+	TestVMAttachOrDetachDisk  = "TestVMAttachOrDetachDisk"
+	TestVMAttachDisk          = "TestVMAttachDisk"
+	TestVMDetachDisk          = "TestVMDetachDisk"
+	TestCreateExternalNetwork = "TestCreateExternalNetwork"
+	TestDeleteExternalNetwork = "TestDeleteExternalNetwork"
+	TestLbServiceMonitor      = "TestLbServiceMonitor"
+	TestLbServerPool          = "TestLbServerPool"
+	TestLbAppProfile          = "TestLbAppProfile"
+	TestLbAppRule             = "TestLbAppRule"
+	TestLbVirtualServer       = "TestLbVirtualServer"
+	TestLb                    = "TestLb"
+	TestNsxvSnatRule          = "TestNsxvSnatRule"
+	TestNsxvDnatRule          = "TestNsxvDnatRule"
 )
 
 const (

govcd/nsxt_alb_controllers.go

@@ -132,15 +132,7 @@ func (vcdClient *VCDClient) GetAlbControllerByUrl(url string) (*NsxtAlbControlle
 		}
 	}
 
-	if len(filteredControllers) == 0 {
-		return nil, fmt.Errorf("%s could not find ALB Controller by Url '%s'", ErrorEntityNotFound, url)
-	}
-
-	if len(filteredControllers) > 1 {
-		return nil, fmt.Errorf("found more than 1 ALB Controller by Url '%s'", url)
-	}
-
-	return filteredControllers[0], nil
+	return oneOrError("url", url, filteredControllers)
 }
 
 // CreateNsxtAlbController creates controller with supplied albControllerConfig configuration

govcd/vm_test.go

@@ -450,10 +450,32 @@ func (vcd *TestVCD) Test_InsertOrEjectMedia(check *C) {
 	err = ejectMediaTask.WaitTaskCompletion()
 	check.Assert(err, IsNil)
 
-	//verify
 	err = vm.Refresh()
 	check.Assert(err, IsNil)
-	check.Assert(isMediaInjected(vm.VM.VirtualHardwareSection.Item), Equals, false)
+
+	// Expecting outcome to be 'false', but VCD sometimes lags to report that media is ejected in VM
+	// state (even after ejection task is finished).
+	// In such cases, do additional VM refresh and try to see if the structure has no media anymore
+	mediaInjected := isMediaInjected(vm.VM.VirtualHardwareSection.Item)
+	retryCount := 5
+	if mediaInjected {
+		// Run a few more attempts every second to see if the VM finally shows no media being
+		// present in VirtualHardwareItem structure
+		for a := 0; a < retryCount; a++ {
+			fmt.Printf("attempt %d\n", a)
+			err = vm.Refresh()
+			if err != nil {
+				fmt.Printf("error refreshing VM: %s\n", err)
+			}
+			retryIsMediaInjected := isMediaInjected(vm.VM.VirtualHardwareSection.Item)
+			if !retryIsMediaInjected {
+				fmt.Printf("media error recovered at %d refresh attempt\n", a)
+				check.SucceedNow()
+			}
+			time.Sleep(time.Second)
+		}
+		check.Errorf("error was not recovered after %d attempts - ejection FAILED", retryCount)
+	}
 }
 
 // Test Insert or Eject Media for VM
@@ -1404,6 +1426,7 @@ func (vcd *TestVCD) Test_AddNewEmptyVMMultiNIC(check *C) {
 func (vcd *TestVCD) Test_UpdateVmSpecSection(check *C) {
 	fmt.Printf("Running: %s\n", check.TestName())
 
+	// #nosec G101 -- Not a credential
 	vmName := "Test_UpdateVmSpecSection"
 	if vcd.skipVappTests {
 		check.Skip("Skipping test because vApp wasn't properly created")

util/logging.go

@@ -41,6 +41,7 @@ const (
 	envLogSkipHttpReq = "GOVCD_LOG_SKIP_HTTP_REQ"
 
 	// Name of the environment variable that enables logging of HTTP responses
+	// #nosec G101 -- Not a credential
 	envLogSkipHttpResp = "GOVCD_LOG_SKIP_HTTP_RESP"
 
 	// Name of the environment variable with a custom list of of responses to skip from logging