Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix coverity warnings #408

Merged
merged 10 commits into from
Nov 24, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .changes/v2.14.0/408-improvements.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Remove Coverity warnings from code [GH-408]
37 changes: 30 additions & 7 deletions govcd/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,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.
Expand All @@ -97,7 +100,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
Expand Down Expand Up @@ -191,15 +197,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
Expand Down Expand Up @@ -744,3 +754,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
}
18 changes: 11 additions & 7 deletions govcd/api_vcd_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
}

Expand Down
10 changes: 9 additions & 1 deletion govcd/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,14 @@ 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)
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 {
Expand Down Expand Up @@ -314,6 +321,7 @@ func uploadFiles(client *Client, vappTemplate *types.VAppTemplate, ovfFileDesc *
return err
}
}
uploadError = nil
return nil
}

Expand Down
3 changes: 1 addition & 2 deletions govcd/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

Expand Down
20 changes: 10 additions & 10 deletions govcd/edgegateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 0 additions & 4 deletions govcd/filter_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to reviewers: dead code removed

// 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
}

Expand Down
44 changes: 35 additions & 9 deletions govcd/filter_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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})
Expand Down Expand Up @@ -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()
}

Expand Down
9 changes: 8 additions & 1 deletion govcd/media.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,14 @@ func executeUpload(client *Client, media *types.Media, mediaFilePath, mediaName
uploadError: &uploadError,
}

go uploadFile(client, mediaFilePath, details)
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 {
Expand Down
5 changes: 2 additions & 3 deletions govcd/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package govcd
import (
"fmt"
"net/http"
"net/url"
"strings"

"github.com/vmware/go-vcloud-director/v2/types/v56"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
6 changes: 5 additions & 1 deletion govcd/nsxt_importable_switch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Loading