diff --git a/.changes/v2.14.0/408-improvements.md b/.changes/v2.14.0/408-improvements.md new file mode 100644 index 000000000..fa6ad6174 --- /dev/null +++ b/.changes/v2.14.0/408-improvements.md @@ -0,0 +1 @@ +* Remove Coverity warnings from code [GH-408] diff --git a/govcd/api.go b/govcd/api.go index 5491243dd..1b848e923 100644 --- a/govcd/api.go +++ b/govcd/api.go @@ -88,7 +88,10 @@ func enableDebugShowRequest() { //lint:ignore U1000 this function is used on request for debugging purposes func disableDebugShowRequest() { debugShowRequestEnabled = false - _ = os.Setenv("GOVCD_SHOW_REQ", "") + err := os.Setenv("GOVCD_SHOW_REQ", "") + if err != nil { + util.Logger.Printf("[DEBUG - disableDebugShowRequest] error setting environment variable: %s", err) + } } // Enables the debugging hook to show responses as they are processed. @@ -101,7 +104,10 @@ func enableDebugShowResponse() { //lint:ignore U1000 this function is used on request for debugging purposes func disableDebugShowResponse() { debugShowResponseEnabled = false - _ = os.Setenv("GOVCD_SHOW_RESP", "") + err := os.Setenv("GOVCD_SHOW_RESP", "") + if err != nil { + util.Logger.Printf("[DEBUG - disableDebugShowResponse] error setting environment variable: %s", err) + } } // On-the-fly debug hook. If either debugShowRequestEnabled or the environment @@ -195,15 +201,19 @@ func (client *Client) newRequest(params map[string]string, notEncodedParams map[ // If the body contains data - try to read all contents for logging and re-create another // io.Reader with all contents to use it down the line var readBody []byte + var err error if body != nil { - readBody, _ = ioutil.ReadAll(body) + readBody, err = ioutil.ReadAll(body) + if err != nil { + util.Logger.Printf("[DEBUG - newRequest] error reading body: %s", err) + } body = bytes.NewReader(readBody) } - // Build the request, no point in checking for errors here as we're just - // passing a string version of an url.URL struct and http.NewRequest returns - // error only if can't process an url.ParseRequestURI(). - req, _ := http.NewRequest(method, reqUrl.String(), body) + req, err := http.NewRequest(method, reqUrl.String(), body) + if err != nil { + util.Logger.Printf("[DEBUG - newRequest] error getting new request: %s", err) + } if client.VCDAuthHeader != "" && client.VCDToken != "" { // Add the authorization header @@ -748,3 +758,16 @@ func (client *Client) RemoveCustomHeader() { client.customHeader = nil } } + +// --------------------------------------------------------------------- +// The following functions are needed to avoid strict Coverity warnings +// --------------------------------------------------------------------- + +// urlParseRequestURI returns a URL, discarding the error +func urlParseRequestURI(href string) *url.URL { + apiEndpoint, err := url.ParseRequestURI(href) + if err != nil { + util.Logger.Printf("[DEBUG - urlParseRequestURI] error parsing request URI: %s", err) + } + return apiEndpoint +} diff --git a/govcd/api_vcd_versions.go b/govcd/api_vcd_versions.go index ac29dff05..e49242e07 100644 --- a/govcd/api_vcd_versions.go +++ b/govcd/api_vcd_versions.go @@ -151,7 +151,10 @@ func (client *Client) vcdFetchSupportedVersions() error { func (client *Client) MaxSupportedVersion() (string, error) { versions := make([]*semver.Version, len(client.supportedVersions.VersionInfos)) for index, versionInfo := range client.supportedVersions.VersionInfos { - version, _ := semver.NewVersion(versionInfo.Version) + version, err := semver.NewVersion(versionInfo.Version) + if err != nil { + return "", fmt.Errorf("error parsing version %s: %s", versionInfo.Version, err) + } versions[index] = version } // Sort supported versions in order lowest-highest @@ -169,24 +172,24 @@ func (client *Client) MaxSupportedVersion() (string, error) { // vcdCheckSupportedVersion checks if there is at least one specified version exactly matching listed ones. // Format example "27.0" -func (client *Client) vcdCheckSupportedVersion(version string) (bool, error) { +func (client *Client) vcdCheckSupportedVersion(version string) error { return client.checkSupportedVersionConstraint(fmt.Sprintf("= %s", version)) } // Checks if there is at least one specified version matching the list returned by vCD. // Constraint format can be in format ">= 27.0, < 32",">= 30" ,"= 27.0". -func (client *Client) checkSupportedVersionConstraint(versionConstraint string) (bool, error) { +func (client *Client) checkSupportedVersionConstraint(versionConstraint string) error { for _, versionInfo := range client.supportedVersions.VersionInfos { versionMatch, err := client.apiVersionMatchesConstraint(versionInfo.Version, versionConstraint) if err != nil { - return false, fmt.Errorf("cannot match version: %s", err) + return fmt.Errorf("cannot match version: %s", err) } if versionMatch { - return true, nil + return nil } } - return false, fmt.Errorf("version %s is not supported", versionConstraint) + return fmt.Errorf("version %s is not supported", versionConstraint) } func (client *Client) apiVersionMatchesConstraint(version, versionConstraint string) (bool, error) { @@ -217,7 +220,8 @@ func (client *Client) validateAPIVersion() error { } // Check if version is supported - if ok, err := client.vcdCheckSupportedVersion(client.APIVersion); !ok || err != nil { + err = client.vcdCheckSupportedVersion(client.APIVersion) + if err != nil { return fmt.Errorf("API version %s is not supported: %s", client.APIVersion, err) } diff --git a/govcd/catalog.go b/govcd/catalog.go index ec1f9310a..5b053daa3 100644 --- a/govcd/catalog.go +++ b/govcd/catalog.go @@ -218,8 +218,17 @@ func (cat *Catalog) UploadOvf(ovaFileName, itemName, description string, uploadP uploadError := *new(error) - //sending upload process to background, this allows no to lock and return task to client - go uploadFiles(cat.client, vappTemplate, &ovfFileDesc, tmpDir, filesAbsPaths, uploadPieceSize, progressCallBack, &uploadError, isOvf) + // sending upload process to background, this allows not to lock and return task to client + // The error should be captured in uploadError, but just in case, we add a logging for the + // main error + go func() { + err = uploadFiles(cat.client, vappTemplate, &ovfFileDesc, tmpDir, filesAbsPaths, uploadPieceSize, progressCallBack, &uploadError, isOvf) + if err != nil { + util.Logger.Println(strings.Repeat("*", 80)) + util.Logger.Printf("*** [DEBUG - UploadOvf] error calling uploadFiles: %s\n", err) + util.Logger.Println(strings.Repeat("*", 80)) + } + }() var task Task for _, item := range vappTemplate.Tasks.Task { @@ -314,6 +323,7 @@ func uploadFiles(client *Client, vappTemplate *types.VAppTemplate, ovfFileDesc * return err } } + uploadError = nil return nil } diff --git a/govcd/disk.go b/govcd/disk.go index ba08f6b38..24c1242f7 100644 --- a/govcd/disk.go +++ b/govcd/disk.go @@ -214,11 +214,10 @@ func (disk *Disk) Delete() (Task, error) { // Refresh the disk information by disk href func (disk *Disk) Refresh() error { - util.Logger.Printf("[TRACE] Disk refresh, HREF: %s\n", disk.Disk.HREF) - if disk.Disk == nil || disk.Disk.HREF == "" { return fmt.Errorf("cannot refresh, Object is empty") } + util.Logger.Printf("[TRACE] Disk refresh, HREF: %s\n", disk.Disk.HREF) unmarshalledDisk := &types.Disk{} diff --git a/govcd/edgegateway.go b/govcd/edgegateway.go index ed9a41ebb..1ce36e4ed 100644 --- a/govcd/edgegateway.go +++ b/govcd/edgegateway.go @@ -120,7 +120,7 @@ func (egw *EdgeGateway) AddDhcpPool(network *types.OrgVDCNetwork, dhcppool []int for { buffer := bytes.NewBufferString(xml.Header + string(output)) - apiEndpoint, _ := url.ParseRequestURI(egw.EdgeGateway.HREF) + apiEndpoint := urlParseRequestURI(egw.EdgeGateway.HREF) apiEndpoint.Path += "/action/configureServices" req := egw.client.NewRequest(map[string]string{}, http.MethodPost, *apiEndpoint, buffer) @@ -197,7 +197,7 @@ func (egw *EdgeGateway) RemoveNATPortMapping(natType, externalIP, externalPort, NatService: newNatService, } - apiEndpoint, _ := url.ParseRequestURI(egw.EdgeGateway.HREF) + apiEndpoint := urlParseRequestURI(egw.EdgeGateway.HREF) apiEndpoint.Path += "/action/configureServices" // Return the task @@ -263,7 +263,7 @@ func (egw *EdgeGateway) RemoveNATRuleAsync(id string) (Task, error) { NatService: natServiceToUpdate, } - egwConfigureHref, _ := url.ParseRequestURI(egw.EdgeGateway.HREF) + egwConfigureHref := urlParseRequestURI(egw.EdgeGateway.HREF) egwConfigureHref.Path += "/action/configureServices" // Return the task @@ -428,7 +428,7 @@ func (egw *EdgeGateway) UpdateNatRuleAsync(natRule *types.NatRule) (Task, error) NatService: natServiceToUpdate, } - egwConfigureHref, _ := url.ParseRequestURI(egw.EdgeGateway.HREF) + egwConfigureHref := urlParseRequestURI(egw.EdgeGateway.HREF) egwConfigureHref.Path += "/action/configureServices" // Return the task @@ -506,7 +506,7 @@ func (egw *EdgeGateway) AddNATRuleAsync(ruleDetails NatRule) (Task, error) { NatService: newNatService, } - egwConfigureHref, _ := url.ParseRequestURI(egw.EdgeGateway.HREF) + egwConfigureHref := urlParseRequestURI(egw.EdgeGateway.HREF) egwConfigureHref.Path += "/action/configureServices" // Return the task @@ -654,7 +654,7 @@ func (egw *EdgeGateway) AddNATPortMappingWithUplink(network *types.OrgVDCNetwork NatService: newNatService, } - apiEndpoint, _ := url.ParseRequestURI(egw.EdgeGateway.HREF) + apiEndpoint := urlParseRequestURI(egw.EdgeGateway.HREF) apiEndpoint.Path += "/action/configureServices" // Return the task @@ -687,7 +687,7 @@ func (egw *EdgeGateway) CreateFirewallRules(defaultAction string, rules []*types for { buffer := bytes.NewBufferString(xml.Header + string(output)) - apiEndpoint, _ := url.ParseRequestURI(egw.EdgeGateway.HREF) + apiEndpoint := urlParseRequestURI(egw.EdgeGateway.HREF) apiEndpoint.Path += "/action/configureServices" req := egw.client.NewRequest(map[string]string{}, http.MethodPost, *apiEndpoint, buffer) @@ -837,7 +837,7 @@ func (egw *EdgeGateway) Remove1to1Mapping(internal, external string) (Task, erro // Fix newEdgeConfig.NatService.IsEnabled = true - apiEndpoint, _ := url.ParseRequestURI(egw.EdgeGateway.HREF) + apiEndpoint := urlParseRequestURI(egw.EdgeGateway.HREF) apiEndpoint.Path += "/action/configureServices" // Return the task @@ -932,7 +932,7 @@ func (egw *EdgeGateway) Create1to1Mapping(internal, external, description string newEdgeConfig.FirewallService.FirewallRule = append(newEdgeConfig.FirewallService.FirewallRule, fwout) - apiEndpoint, _ := url.ParseRequestURI(egw.EdgeGateway.HREF) + apiEndpoint := urlParseRequestURI(egw.EdgeGateway.HREF) apiEndpoint.Path += "/action/configureServices" // Return the task @@ -950,7 +950,7 @@ func (egw *EdgeGateway) AddIpsecVPN(ipsecVPNConfig *types.EdgeGatewayServiceConf ipsecVPNConfig.Xmlns = types.XMLNamespaceVCloud - apiEndpoint, _ := url.ParseRequestURI(egw.EdgeGateway.HREF) + apiEndpoint := urlParseRequestURI(egw.EdgeGateway.HREF) apiEndpoint.Path += "/action/configureServices" // Return the task diff --git a/govcd/filter_engine.go b/govcd/filter_engine.go index 8cfd1bf43..d091acfee 100644 --- a/govcd/filter_engine.go +++ b/govcd/filter_engine.go @@ -291,10 +291,6 @@ func searchByFilter(queryByMetadata queryByMetadataFunc, queryWithMetadataFields return nil, explanation, fmt.Errorf("search for oldest item failed. Empty dates found for items %v", emptyDatesFound) } } - if searchEarliest || searchLatest { - // We should never reach this point, as a failure for newest or oldest item was caught above, but just in case - return nil, explanation, fmt.Errorf("search for oldest or earliest item failed. No reason found") - } return candidatesByConditions, explanation, nil } diff --git a/govcd/filter_helpers.go b/govcd/filter_helpers.go index 470fb5606..93c764055 100644 --- a/govcd/filter_helpers.go +++ b/govcd/filter_helpers.go @@ -12,6 +12,7 @@ import ( "time" "github.com/vmware/go-vcloud-director/v2/types/v56" + "github.com/vmware/go-vcloud-director/v2/util" ) // This file contains functions that help create tests for filtering. @@ -152,18 +153,33 @@ func makeDateFilter(items []DateItem) ([]FilterMatch, error) { earliestFound = true } exactFilter := NewFilterDef() - _ = exactFilter.AddFilter(types.FilterDate, "=="+item.Date) + err = exactFilter.AddFilter(types.FilterDate, "=="+item.Date) + if err != nil { + return nil, fmt.Errorf("error adding filter '%s' '%s': %s", types.FilterDate, "=="+item.Date, err) + } filters = append(filters, FilterMatch{exactFilter, item.Name, item.Entity, item.EntityType}) } if earliestFound && latestFound && earliestDate != latestDate { earlyFilter := NewFilterDef() - _ = earlyFilter.AddFilter(types.FilterDate, "<"+latestDate) - _ = earlyFilter.AddFilter(types.FilterEarliest, "true") + err := earlyFilter.AddFilter(types.FilterDate, "<"+latestDate) + if err != nil { + return nil, err + } + err = earlyFilter.AddFilter(types.FilterEarliest, "true") + if err != nil { + return nil, err + } lateFilter := NewFilterDef() - _ = lateFilter.AddFilter(types.FilterDate, ">"+earliestDate) - _ = lateFilter.AddFilter(types.FilterLatest, "true") + err = lateFilter.AddFilter(types.FilterDate, ">"+earliestDate) + if err != nil { + return nil, err + } + err = lateFilter.AddFilter(types.FilterLatest, "true") + if err != nil { + return nil, err + } filters = append(filters, FilterMatch{earlyFilter, earliestName, earliestEntity, entityType}) filters = append(filters, FilterMatch{lateFilter, latestName, latestEntity, entityType}) @@ -454,15 +470,25 @@ func ipToRegex(ip string) string { // strToRegex creates a regular expression that matches perfectly with the input query func strToRegex(s string) string { var result strings.Builder - result.WriteString("^") + var err error + _, err = result.WriteString("^") + if err != nil { + util.Logger.Printf("[DEBUG - strToRegex] error writing to string: %s", err) + } for _, ch := range s { if ch == '.' { - result.WriteString(fmt.Sprintf("\\%c", ch)) + _, err = result.WriteString(fmt.Sprintf("\\%c", ch)) } else { - result.WriteString(fmt.Sprintf("[%c]", ch)) + _, err = result.WriteString(fmt.Sprintf("[%c]", ch)) + } + if err != nil { + util.Logger.Printf("[DEBUG - strToRegex] error writing to string: %s", err) } } - result.WriteString("$") + _, err = result.WriteString("$") + if err != nil { + util.Logger.Printf("[DEBUG - strToRegex] error writing to string: %s", err) + } return result.String() } diff --git a/govcd/media.go b/govcd/media.go index 9d3e5bbfa..019f4b477 100644 --- a/govcd/media.go +++ b/govcd/media.go @@ -132,7 +132,17 @@ func executeUpload(client *Client, media *types.Media, mediaFilePath, mediaName uploadError: &uploadError, } - go uploadFile(client, mediaFilePath, details) + // sending upload process to background, this allows not to lock and return task to client + // The error should be captured in details.uploadError, but just in case, we add a logging for the + // main error + go func() { + _, err = uploadFile(client, mediaFilePath, details) + if err != nil { + util.Logger.Println(strings.Repeat("*", 80)) + util.Logger.Printf("*** [DEBUG - executeUpload] error calling uploadFile: %s\n", err) + util.Logger.Println(strings.Repeat("*", 80)) + } + }() var task Task for _, item := range media.Tasks.Task { diff --git a/govcd/metadata.go b/govcd/metadata.go index 7c7013c04..7d616ad12 100644 --- a/govcd/metadata.go +++ b/govcd/metadata.go @@ -7,7 +7,6 @@ package govcd import ( "fmt" "net/http" - "net/url" "strings" "github.com/vmware/go-vcloud-director/v2/types/v56" @@ -116,7 +115,7 @@ func (vapp *VApp) DeleteMetadata(key string) (Task, error) { // Deletes metadata (type MetadataStringValue) from the vApp // TODO: Support all MetadataTypedValue types with this function func deleteMetadata(client *Client, key string, requestUri string) (Task, error) { - apiEndpoint, _ := url.ParseRequestURI(requestUri) + apiEndpoint := urlParseRequestURI(requestUri) apiEndpoint.Path += "/metadata/" + key // Return the task @@ -142,7 +141,7 @@ func addMetadata(client *Client, key string, value string, requestUri string) (T }, } - apiEndpoint, _ := url.ParseRequestURI(requestUri) + apiEndpoint := urlParseRequestURI(requestUri) apiEndpoint.Path += "/metadata/" + key // Return the task diff --git a/govcd/nsxt_importable_switch.go b/govcd/nsxt_importable_switch.go index d2fc1bd85..893f27a40 100644 --- a/govcd/nsxt_importable_switch.go +++ b/govcd/nsxt_importable_switch.go @@ -10,6 +10,7 @@ import ( "net/url" "github.com/vmware/go-vcloud-director/v2/types/v56" + "github.com/vmware/go-vcloud-director/v2/util" ) // NsxtImportableSwitch is a read only object to retrieve NSX-T segments (importable switches) to be used for Org VDC @@ -121,7 +122,10 @@ func getFilteredNsxtImportableSwitches(filter map[string]string, client *Client) apiEndpoint := client.VCDHREF endpoint := apiEndpoint.Scheme + "://" + apiEndpoint.Host + "/network/orgvdcnetworks/importableswitches/" // error below is ignored because it is a static endpoint - urlRef, _ := url.Parse(endpoint) + urlRef, err := url.Parse(endpoint) + if err != nil { + util.Logger.Printf("[DEBUG - getFilteredNsxtImportableSwitches] error parsing URL: %s", err) + } headAccept := http.Header{} headAccept.Set("Accept", types.JSONMime) diff --git a/govcd/openapi.go b/govcd/openapi.go index c3e5989d7..d83dfa374 100644 --- a/govcd/openapi.go +++ b/govcd/openapi.go @@ -284,7 +284,7 @@ func (client *Client) OpenApiPostItem(apiVersion string, urlRef *url.URL, params // Task Owner ID is the ID of created object. ID must be used (although HREF exists in task) because HREF points to // old XML API and here we need to pull data from OpenAPI. - newObjectUrl, _ := url.ParseRequestURI(urlRefCopy.String() + task.Task.Owner.ID) + newObjectUrl := urlParseRequestURI(urlRefCopy.String() + task.Task.Owner.ID) err = client.OpenApiGetItem(apiVersion, newObjectUrl, nil, outType, additionalHeader) if err != nil { return fmt.Errorf("error retrieving item after creation: %s", err) @@ -635,14 +635,17 @@ func (client *Client) newOpenApiRequest(apiVersion string, params url.Values, me // io.Reader with all contents to use it down the line var readBody []byte if body != nil { - readBody, _ = ioutil.ReadAll(body) + readBody, err := ioutil.ReadAll(body) + if err != nil { + util.Logger.Printf("[DEBUG - newOpenApiRequest] error reading body: %s", err) + } body = bytes.NewReader(readBody) } - // Build the request, no point in checking for errors here as we're just - // passing a string version of an url.URL struct and http.NewRequest returns - // error only if can't process an url.ParseRequestURI(). - req, _ := http.NewRequest(method, reqUrlCopy.String(), body) + req, err := http.NewRequest(method, reqUrlCopy.String(), body) + if err != nil { + util.Logger.Printf("[DEBUG - newOpenApiRequest] error getting new request: %s", err) + } if client.VCDAuthHeader != "" && client.VCDToken != "" { // Add the authorization header @@ -776,6 +779,9 @@ func defaultPageSize(queryParams url.Values, defaultPageSize string) url.Values // copyUrlRef creates a copy of URL reference by re-parsing it func copyUrlRef(in *url.URL) *url.URL { // error is ignored because we expect to have correct URL supplied and this greatly simplifies code inside. - newUrlRef, _ := url.Parse(in.String()) + newUrlRef, err := url.Parse(in.String()) + if err != nil { + util.Logger.Printf("[DEBUG - copyUrlRef] error parsing URL: %s", err) + } return newUrlRef } diff --git a/govcd/openapi_endpoints.go b/govcd/openapi_endpoints.go index 0fe74d40b..402ae1783 100644 --- a/govcd/openapi_endpoints.go +++ b/govcd/openapi_endpoints.go @@ -132,7 +132,10 @@ func (client *Client) getOpenApiHighestElevatedVersion(endpoint string) (string, versionsRaw := elevatedVersionSlice versions := make([]*version.Version, len(versionsRaw)) for i, raw := range versionsRaw { - v, _ := version.NewVersion(raw) + v, err := version.NewVersion(raw) + if err != nil { + return "", fmt.Errorf("error evaluating version %s: %s", raw, err) + } versions[i] = v } sort.Sort(sort.Reverse(version.Collection(versions))) diff --git a/govcd/orgvdcnetwork.go b/govcd/orgvdcnetwork.go index a2e366303..040d22236 100644 --- a/govcd/orgvdcnetwork.go +++ b/govcd/orgvdcnetwork.go @@ -60,7 +60,7 @@ func (orgVdcNet *OrgVDCNetwork) Delete() (Task, error) { return Task{}, fmt.Errorf("error refreshing network: %s", err) } pathArr := strings.Split(orgVdcNet.OrgVDCNetwork.HREF, "/") - apiEndpoint, _ := url.ParseRequestURI(orgVdcNet.OrgVDCNetwork.HREF) + apiEndpoint := urlParseRequestURI(orgVdcNet.OrgVDCNetwork.HREF) apiEndpoint.Path = "/api/admin/network/" + pathArr[len(pathArr)-1] var resp *http.Response diff --git a/govcd/system.go b/govcd/system.go index cb551cbe5..f4426abfa 100644 --- a/govcd/system.go +++ b/govcd/system.go @@ -687,19 +687,23 @@ func CreateExternalNetwork(vcdClient *VCDClient, externalNetworkData *types.Exte externalNetHREF := vcdClient.Client.VCDHREF externalNetHREF.Path += "/admin/extension/externalnets" + if externalNetwork.Configuration == nil { + externalNetwork.Configuration = &types.NetworkConfiguration{} + } externalNetwork.Configuration.FenceMode = "isolated" // Return the task task, err := vcdClient.Client.ExecuteTaskRequest(externalNetHREF.String(), http.MethodPost, types.MimeExternalNetwork, "error instantiating a new ExternalNetwork: %s", externalNetwork) - // Real task in task array - if err == nil { - if task.Task != nil && task.Task.Tasks != nil && len(task.Task.Tasks.Task) == 0 { - return Task{}, fmt.Errorf("create external network task wasn't found") - } - task.Task = task.Task.Tasks.Task[0] + if err != nil { + return Task{}, err + } + if task.Task == nil || task.Task.Tasks == nil || len(task.Task.Tasks.Task) == 0 { + return Task{}, fmt.Errorf("create external network task wasn't found") } + // Real task in task array + task.Task = task.Task.Tasks.Task[0] return task, err } diff --git a/govcd/task.go b/govcd/task.go index 618db133b..24c5f588e 100644 --- a/govcd/task.go +++ b/govcd/task.go @@ -54,7 +54,7 @@ func (task *Task) Refresh() error { return fmt.Errorf("cannot refresh, Object is empty") } - refreshUrl, _ := url.ParseRequestURI(task.Task.HREF) + refreshUrl := urlParseRequestURI(task.Task.HREF) req := task.client.NewRequest(map[string]string{}, http.MethodGet, *refreshUrl, nil) diff --git a/govcd/upload.go b/govcd/upload.go index 82f987e36..eaa170a8f 100644 --- a/govcd/upload.go +++ b/govcd/upload.go @@ -185,7 +185,10 @@ func uploadPartFile(client *Client, part []byte, partDataSize int64, uDetails up if err != nil { return fmt.Errorf("file upload failed. Err: %s", err) } - response.Body.Close() + err = response.Body.Close() + if err != nil { + return fmt.Errorf("file closing failed. Err: %s", err) + } uDetails.callBack(uDetails.uploadedBytesForCallback+partDataSize, uDetails.allFilesSize) @@ -197,8 +200,11 @@ func makeEmptyRequest(client *Client) { apiEndpoint := client.VCDHREF apiEndpoint.Path += "/query?type=task&format=records&page=1&pageSize=5&" - _, _ = client.ExecuteRequest(apiEndpoint.String(), http.MethodGet, + _, err := client.ExecuteRequest(apiEndpoint.String(), http.MethodGet, "", "error making empty request: %s", nil, nil) + if err != nil { + util.Logger.Printf("[DEBUG - makeEmptyRequest] error executing request: %s", err) + } } func getUploadLink(files *types.FilesList) (*url.URL, error) { diff --git a/govcd/vapp.go b/govcd/vapp.go index 899ea055f..4080e8ea9 100644 --- a/govcd/vapp.go +++ b/govcd/vapp.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "net/http" - "net/url" "strconv" "time" @@ -241,7 +240,7 @@ func addNewVMW(vapp *VApp, name string, vappTemplate VAppTemplate, // Inject network config vAppComposition.SourcedItem.InstantiationParams.NetworkConnectionSection = network - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/action/recomposeVApp" // Return the task @@ -283,7 +282,7 @@ func (vapp *VApp) RemoveVM(vm VM) error { }, } - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/action/recomposeVApp" deleteTask, err := vapp.client.ExecuteTaskRequest(apiEndpoint.String(), http.MethodPost, @@ -307,7 +306,7 @@ func (vapp *VApp) PowerOn() (Task, error) { return Task{}, fmt.Errorf("error powering on vApp: %s", err) } - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/power/action/powerOn" // Return the task @@ -317,7 +316,7 @@ func (vapp *VApp) PowerOn() (Task, error) { func (vapp *VApp) PowerOff() (Task, error) { - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/power/action/powerOff" // Return the task @@ -328,7 +327,7 @@ func (vapp *VApp) PowerOff() (Task, error) { func (vapp *VApp) Reboot() (Task, error) { - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/power/action/reboot" // Return the task @@ -338,7 +337,7 @@ func (vapp *VApp) Reboot() (Task, error) { func (vapp *VApp) Reset() (Task, error) { - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/power/action/reset" // Return the task @@ -348,7 +347,7 @@ func (vapp *VApp) Reset() (Task, error) { func (vapp *VApp) Suspend() (Task, error) { - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/power/action/suspend" // Return the task @@ -358,7 +357,7 @@ func (vapp *VApp) Suspend() (Task, error) { func (vapp *VApp) Shutdown() (Task, error) { - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/power/action/shutdown" // Return the task @@ -373,7 +372,7 @@ func (vapp *VApp) Undeploy() (Task, error) { UndeployPowerAction: "powerOff", } - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/action/undeploy" // Return the task @@ -388,7 +387,7 @@ func (vapp *VApp) Deploy() (Task, error) { PowerOn: false, } - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/action/deploy" // Return the task @@ -435,7 +434,7 @@ func (vapp *VApp) Customize(computername, script string, changeSid bool) (Task, ChangeSid: takeBoolPointer(changeSid), } - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.Children.VM[0].HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.Children.VM[0].HREF) apiEndpoint.Path += "/guestCustomizationSection/" // Return the task @@ -548,7 +547,7 @@ func (vapp *VApp) ChangeCPUCountWithCore(virtualCpuCount int, coresPerSocket *in }, } - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.Children.VM[0].HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.Children.VM[0].HREF) apiEndpoint.Path += "/virtualHardwareSection/cpu" // Return the task @@ -639,7 +638,7 @@ func (vapp *VApp) SetOvf(parameters map[string]string) (Task, error) { ProductSection: vapp.VApp.Children.VM[0].ProductSection, } - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.Children.VM[0].HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.Children.VM[0].HREF) apiEndpoint.Path += "/productSections" // Return the task @@ -697,7 +696,7 @@ func (vapp *VApp) ChangeNetworkConfig(networks []map[string]interface{}, ip stri } - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.Children.VM[0].HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.Children.VM[0].HREF) apiEndpoint.Path += "/networkConnectionSection/" // Return the task @@ -739,7 +738,7 @@ func (vapp *VApp) ChangeMemorySize(size int) (Task, error) { }, } - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.Children.VM[0].HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.Children.VM[0].HREF) apiEndpoint.Path += "/virtualHardwareSection/memory" // Return the task @@ -1007,6 +1006,9 @@ func (vapp *VApp) UpdateNetworkAsync(networkSettingsToUpdate *VappNetworkSetting if networkToUpdate == (types.VAppNetworkConfiguration{}) { return Task{}, fmt.Errorf("not found network to update with Id %s", networkSettingsToUpdate.ID) } + if networkToUpdate.Configuration == nil { + networkToUpdate.Configuration = &types.NetworkConfiguration{} + } networkToUpdate.Configuration.RetainNetInfoAcrossDeployments = networkSettingsToUpdate.RetainIpMacEnabled // new network to connect if networkToUpdate.Configuration.ParentNetwork == nil && orgNetwork != nil { @@ -1119,6 +1121,9 @@ func (vapp *VApp) UpdateOrgNetworkAsync(networkSettingsToUpdate *VappNetworkSett fenceMode = types.FenceModeNAT } + if networkToUpdate.Configuration == nil { + networkToUpdate.Configuration = &types.NetworkConfiguration{} + } networkToUpdate.Configuration.RetainNetInfoAcrossDeployments = networkSettingsToUpdate.RetainIpMacEnabled networkToUpdate.Configuration.FenceMode = fenceMode @@ -1243,7 +1248,7 @@ func updateNetworkConfigurations(vapp *VApp, networkConfigurations []types.VAppN NetworkConfig: networkConfigurations, } - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/networkConfigSection/" // Return the task diff --git a/govcd/vm.go b/govcd/vm.go index 3cc6cfa17..062abdec3 100644 --- a/govcd/vm.go +++ b/govcd/vm.go @@ -8,7 +8,6 @@ import ( "fmt" "net" "net/http" - "net/url" "strconv" "strings" "time" @@ -154,7 +153,7 @@ func (client *Client) FindVMByHREF(vmHREF string) (VM, error) { func (vm *VM) PowerOn() (Task, error) { - apiEndpoint, _ := url.ParseRequestURI(vm.VM.HREF) + apiEndpoint := urlParseRequestURI(vm.VM.HREF) apiEndpoint.Path += "/power/action/powerOn" // Return the task @@ -179,7 +178,7 @@ func (vm *VM) PowerOnAndForceCustomization() error { return fmt.Errorf("VM %s must be undeployed before forcing customization", vm.VM.Name) } - apiEndpoint, _ := url.ParseRequestURI(vm.VM.HREF) + apiEndpoint := urlParseRequestURI(vm.VM.HREF) apiEndpoint.Path += "/action/deploy" powerOnAndCustomize := &types.DeployVAppParams{ @@ -205,7 +204,7 @@ func (vm *VM) PowerOnAndForceCustomization() error { func (vm *VM) PowerOff() (Task, error) { - apiEndpoint, _ := url.ParseRequestURI(vm.VM.HREF) + apiEndpoint := urlParseRequestURI(vm.VM.HREF) apiEndpoint.Path += "/power/action/powerOff" // Return the task @@ -255,7 +254,7 @@ func (vm *VM) ChangeCPUCountWithCore(virtualCpuCount int, coresPerSocket *int) ( }, } - apiEndpoint, _ := url.ParseRequestURI(vm.VM.HREF) + apiEndpoint := urlParseRequestURI(vm.VM.HREF) apiEndpoint.Path += "/virtualHardwareSection/cpu" // Return the task @@ -297,9 +296,10 @@ func (vm *VM) updateNicParameters(networks []map[string]interface{}, networkSect ipAllocationMode = types.IPAllocationModeDHCP // TODO v3.0 remove until here when deprecated `ip` and `network_name` attributes are removed - case ipIsSet && net.ParseIP(ipFieldString) != nil && (network["ip_allocation_mode"].(string) == types.IPAllocationModeManual): - ipAllocationMode = types.IPAllocationModeManual - ipAddress = ipFieldString + // Removed for Coverity warning: dead code - We can reinstate after removing above code + //case ipIsSet && net.ParseIP(ipFieldString) != nil && (network["ip_allocation_mode"].(string) == types.IPAllocationModeManual): + // ipAllocationMode = types.IPAllocationModeManual + // ipAddress = ipFieldString default: // New networks functionality. IP was not set and we're defaulting to provided ip_allocation_mode (only manual requires the IP) ipAllocationMode = network["ip_allocation_mode"].(string) } @@ -350,7 +350,7 @@ func (vm *VM) ChangeNetworkConfig(networks []map[string]interface{}) (Task, erro networkSection.Ovf = types.XMLNamespaceOVF networkSection.Info = "Specifies the available VM network connections" - apiEndpoint, _ := url.ParseRequestURI(vm.VM.HREF) + apiEndpoint := urlParseRequestURI(vm.VM.HREF) apiEndpoint.Path += "/networkConnectionSection/" // Return the task @@ -386,7 +386,7 @@ func (vm *VM) ChangeMemorySize(size int) (Task, error) { }, } - apiEndpoint, _ := url.ParseRequestURI(vm.VM.HREF) + apiEndpoint := urlParseRequestURI(vm.VM.HREF) apiEndpoint.Path += "/virtualHardwareSection/memory" // Return the task @@ -466,7 +466,7 @@ func (vm *VM) Customize(computerName, script string, changeSid bool) (Task, erro ChangeSid: takeBoolPointer(changeSid), } - apiEndpoint, _ := url.ParseRequestURI(vm.VM.HREF) + apiEndpoint := urlParseRequestURI(vm.VM.HREF) apiEndpoint.Path += "/guestCustomizationSection/" // Return the task @@ -482,7 +482,7 @@ func (vm *VM) Undeploy() (Task, error) { UndeployPowerAction: "powerOff", } - apiEndpoint, _ := url.ParseRequestURI(vm.VM.HREF) + apiEndpoint := urlParseRequestURI(vm.VM.HREF) apiEndpoint.Path += "/action/undeploy" // Return the task @@ -530,11 +530,10 @@ func (vm *VM) attachOrDetachDisk(diskParams *types.DiskAttachOrDetachParams, rel // https://vdc-download.vmware.com/vmwb-repository/dcr-public/1b6cf07d-adb3-4dba-8c47-9c1c92b04857/ // 241956dd-e128-4fcc-8131-bf66e1edd895/vcloud_sp_api_guide_30_0.pdf func (vm *VM) AttachDisk(diskParams *types.DiskAttachOrDetachParams) (Task, error) { - util.Logger.Printf("[TRACE] Attach disk, HREF: %s\n", diskParams.Disk.HREF) - - if diskParams.Disk == nil { + if diskParams == nil || diskParams.Disk == nil || diskParams.Disk.HREF == "" { return Task{}, fmt.Errorf("could not find disk info for attach") } + util.Logger.Printf("[TRACE] Attach disk, HREF: %s\n", diskParams.Disk.HREF) return vm.attachOrDetachDisk(diskParams, types.RelDiskAttach) } @@ -547,11 +546,11 @@ func (vm *VM) AttachDisk(diskParams *types.DiskAttachOrDetachParams) (Task, erro // https://vdc-download.vmware.com/vmwb-repository/dcr-public/1b6cf07d-adb3-4dba-8c47-9c1c92b04857/ // 241956dd-e128-4fcc-8131-bf66e1edd895/vcloud_sp_api_guide_30_0.pdf func (vm *VM) DetachDisk(diskParams *types.DiskAttachOrDetachParams) (Task, error) { - util.Logger.Printf("[TRACE] Detach disk, HREF: %s\n", diskParams.Disk.HREF) - if diskParams.Disk == nil { + if diskParams == nil || diskParams.Disk == nil || diskParams.Disk.HREF == "" { return Task{}, fmt.Errorf("could not find disk info for detach") } + util.Logger.Printf("[TRACE] Detach disk, HREF: %s\n", diskParams.Disk.HREF) return vm.attachOrDetachDisk(diskParams, types.RelDiskDetach) } @@ -714,7 +713,7 @@ func (vm *VM) insertOrEjectMedia(mediaParams *types.MediaInsertOrEjectParams, li // https://code.vmware.com/apis/287/vcloud#/doc/doc/operations/GET-VmPendingQuestion.html func (vm *VM) GetQuestion() (types.VmPendingQuestion, error) { - apiEndpoint, _ := url.ParseRequestURI(vm.VM.HREF) + apiEndpoint := urlParseRequestURI(vm.VM.HREF) apiEndpoint.Path += "/question" req := vm.client.NewRequest(map[string]string{}, http.MethodGet, *apiEndpoint, nil) @@ -761,7 +760,7 @@ func (vm *VM) AnswerQuestion(questionId string, choiceId int) error { ChoiceId: choiceId, } - apiEndpoint, _ := url.ParseRequestURI(vm.VM.HREF) + apiEndpoint := urlParseRequestURI(vm.VM.HREF) apiEndpoint.Path += "/question/action/answer" return vm.client.ExecuteRequestWithoutResponse(apiEndpoint.String(), http.MethodPost, @@ -780,7 +779,7 @@ func (vm *VM) ToggleHardwareVirtualization(isEnabled bool) (Task, error) { return Task{}, fmt.Errorf("hardware virtualization can be changed from powered off state, status: %s", vmStatus) } - apiEndpoint, _ := url.ParseRequestURI(vm.VM.HREF) + apiEndpoint := urlParseRequestURI(vm.VM.HREF) if isEnabled { apiEndpoint.Path += "/action/enableNestedHypervisor" } else { @@ -1650,7 +1649,7 @@ func addEmptyVmAsyncV10(vapp *VApp, reComposeVAppParams *types.RecomposeVAppPara if err != nil { return Task{}, err } - apiEndpoint, _ := url.ParseRequestURI(vapp.VApp.HREF) + apiEndpoint := urlParseRequestURI(vapp.VApp.HREF) apiEndpoint.Path += "/action/recomposeVApp" reComposeVAppParams.XmlnsVcloud = types.XMLNamespaceVCloud diff --git a/util/tar.go b/util/tar.go index 0234d7b8f..88460b9b4 100644 --- a/util/tar.go +++ b/util/tar.go @@ -100,13 +100,19 @@ func Unpack(tarFile string) ([]string, string, error) { filePaths = append(filePaths, newFile.Name()) if err := isExtractedFileValid(newFile, expectedFileSize); err != nil { - newFile.Close() + errClose := newFile.Close() + if errClose != nil { + Logger.Printf("[DEBUG - Unpack] error closing newFile: %s", errClose) + } return filePaths, dst, err } - // manually close here after each newFile operation; defering would cause each newFile close + // manually close here after each newFile operation; deferring would cause each newFile close // to wait until all operations have completed. - newFile.Close() + errClose := newFile.Close() + if errClose != nil { + Logger.Printf("[DEBUG - Unpack] error closing newFile: %s", errClose) + } } } }