fix: check if http.Response is nil before closing body

Enda Phelan · Enda Phelan · commit 24c5d0d794ea · 2021-09-01T14:39:36.000+01:00
diff --git a/pkg/cmd/kafka/consumergroup/delete/delete.go b/pkg/cmd/kafka/consumergroup/delete/delete.go
@@ -92,7 +92,9 @@ func runCmd(opts *Options) error {
 	ctx := context.Background()
 
 	_, httpRes, err := api.GroupsApi.GetConsumerGroupById(ctx, opts.id).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 
 	cgIDPair := localize.NewEntry("ID", opts.id)
 	kafkaNameTmplPair := localize.NewEntry("InstanceName", kafkaInstance.GetName())
@@ -122,14 +124,15 @@ func runCmd(opts *Options) error {
 	}
 
 	httpRes, err = api.GroupsApi.DeleteConsumerGroupById(ctx, opts.id).Execute()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 
 	if err != nil {
 		if httpRes == nil {
 			return err
 		}
 
-		defer httpRes.Body.Close()
-
 		operationTmplPair := localize.NewEntry("Operation", "delete")
 
 		switch httpRes.StatusCode {
diff --git a/pkg/cmd/kafka/consumergroup/describe/describe.go b/pkg/cmd/kafka/consumergroup/describe/describe.go
@@ -114,7 +114,10 @@ func runCmd(opts *Options) error {
 	ctx := context.Background()
 
 	consumerGroupData, httpRes, err := api.GroupsApi.GetConsumerGroupById(ctx, opts.id).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
+
 	if err != nil {
 		if httpRes == nil {
 			return err
diff --git a/pkg/cmd/kafka/consumergroup/list/list.go b/pkg/cmd/kafka/consumergroup/list/list.go
@@ -133,7 +133,9 @@ func runList(opts *Options) (err error) {
 	req = req.Page(opts.page)
 
 	consumerGroupData, httpRes, err := req.Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 	if err != nil {
 		if httpRes == nil {
 			return err
diff --git a/pkg/cmd/kafka/create/create.go b/pkg/cmd/kafka/create/create.go
@@ -180,7 +180,9 @@ func runCreate(opts *Options) error {
 	a = a.KafkaRequestPayload(*payload)
 	a = a.Async(true)
 	response, httpRes, err := a.Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 
 	if httpRes.StatusCode == http.StatusBadRequest {
 		return errors.New(opts.localizer.MustLocalize("kafka.create.error.conflictError", localize.NewEntry("Name", payload.Name)))
@@ -295,7 +297,10 @@ func promptKafkaPayload(opts *Options) (payload *kafkamgmtclient.KafkaRequestPay
 
 	// fetch all cloud available providers
 	cloudProviderResponse, httpRes, err := api.Kafka().GetCloudProviders(context.Background()).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
+
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/cmd/kafka/describe/describe.go b/pkg/cmd/kafka/describe/describe.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"net/http"
 
 	flagutil "github.com/redhat-developer/app-services-cli/pkg/cmdutil/flags"
 	"github.com/redhat-developer/app-services-cli/pkg/connection"
@@ -103,14 +104,21 @@ func runDescribe(opts *Options) error {
 	api := connection.API()
 
 	var kafkaInstance *kafkamgmtclient.KafkaRequest
+	var httpRes *http.Response
 	ctx := context.Background()
 	if opts.name != "" {
-		kafkaInstance, _, err = kafka.GetKafkaByName(ctx, api.Kafka(), opts.name)
+		kafkaInstance, httpRes, err = kafka.GetKafkaByName(ctx, api.Kafka(), opts.name)
+		if httpRes != nil {
+			defer httpRes.Body.Close()
+		}
 		if err != nil {
 			return err
 		}
 	} else {
-		kafkaInstance, _, err = kafka.GetKafkaByID(ctx, api.Kafka(), opts.id)
+		kafkaInstance, httpRes, err = kafka.GetKafkaByID(ctx, api.Kafka(), opts.id)
+		if httpRes != nil {
+			defer httpRes.Body.Close()
+		}
 		if err != nil {
 			return err
 		}
diff --git a/pkg/cmd/kafka/topic/create/create.go b/pkg/cmd/kafka/topic/create/create.go
@@ -172,7 +172,9 @@ func runCmd(opts *Options) error {
 	createTopicReq = createTopicReq.NewTopicInput(topicInput)
 
 	response, httpRes, err := createTopicReq.Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 
 	if err != nil {
 		if httpRes == nil {
diff --git a/pkg/cmd/kafka/topic/delete/delete.go b/pkg/cmd/kafka/topic/delete/delete.go
@@ -97,7 +97,9 @@ func runCmd(opts *Options) error {
 
 	// perform delete topic API request
 	_, httpRes, err := api.TopicsApi.GetTopic(context.Background(), opts.topicName).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 
 	topicNameTmplPair := localize.NewEntry("TopicName", opts.topicName)
 	kafkaNameTmplPair := localize.NewEntry("InstanceName", kafkaInstance.GetName())
@@ -126,6 +128,9 @@ func runCmd(opts *Options) error {
 
 	// perform delete topic API request
 	httpRes, err = api.TopicsApi.DeleteTopic(context.Background(), opts.topicName).Execute()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 	if err != nil {
 		if httpRes == nil {
 			return err
@@ -147,7 +152,6 @@ func runCmd(opts *Options) error {
 			return err
 		}
 	}
-	defer httpRes.Body.Close()
 
 	opts.Logger.Info(opts.localizer.MustLocalize("kafka.topic.delete.log.info.topicDeleted", topicNameTmplPair, kafkaNameTmplPair))
 
diff --git a/pkg/cmd/kafka/topic/describe/describe.go b/pkg/cmd/kafka/topic/describe/describe.go
@@ -105,7 +105,9 @@ func runCmd(opts *Options) error {
 
 	// fetch the topic
 	topicResponse, httpRes, err := api.TopicsApi.GetTopic(context.Background(), opts.name).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 
 	if err != nil {
 		if httpRes == nil {
diff --git a/pkg/cmd/kafka/topic/list/list.go b/pkg/cmd/kafka/topic/list/list.go
@@ -137,6 +137,9 @@ func runCmd(opts *Options) error {
 	a = a.Page(opts.page)
 
 	topicData, httpRes, err := a.Execute()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 	if err != nil {
 		if httpRes == nil {
 			return err
@@ -158,8 +161,6 @@ func runCmd(opts *Options) error {
 		}
 	}
 
-	defer httpRes.Body.Close()
-
 	if topicData.GetTotal() == 0 && opts.output == "" {
 		opts.Logger.Info(opts.localizer.MustLocalize("kafka.topic.list.log.info.noTopics", localize.NewEntry("InstanceName", kafkaInstance.GetName())))
 
diff --git a/pkg/cmd/kafka/topic/update/update.go b/pkg/cmd/kafka/topic/update/update.go
@@ -220,7 +220,9 @@ func runCmd(opts *Options) error {
 	var needsUpdate bool
 
 	topic, httpRes, err := api.TopicsApi.GetTopic(context.Background(), opts.name).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 
 	topicNameTmplPair := localize.NewEntry("TopicName", opts.name)
 	kafkaNameTmplPair := localize.NewEntry("InstanceName", kafkaInstance.GetName())
@@ -274,7 +276,9 @@ func runCmd(opts *Options) error {
 
 	// update the topic
 	response, httpRes, err := updateTopicReq.Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 
 	// handle error
 	if err != nil {
@@ -326,7 +330,9 @@ func runInteractivePrompt(opts *Options) (err error) {
 
 	// check if topic exists
 	topic, httpRes, err := api.TopicsApi.GetTopic(context.Background(), opts.name).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 
 	topicNameTmplPair := localize.NewEntry("TopicName", opts.name)
 	kafkaNameTmplPair := localize.NewEntry("InstanceName", kafkaInstance.GetName())
diff --git a/pkg/cmd/serviceaccount/delete/delete.go b/pkg/cmd/serviceaccount/delete/delete.go
@@ -77,7 +77,9 @@ func runDelete(opts *Options) (err error) {
 	}
 
 	_, httpRes, err := connection.API().ServiceAccount().GetServiceAccountById(context.Background(), opts.id).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 	if err != nil {
 		if httpRes == nil {
 			return err
@@ -115,7 +117,9 @@ func deleteServiceAccount(opts *Options) error {
 	}
 
 	_, httpRes, err := connection.API().ServiceAccount().DeleteServiceAccountById(context.Background(), opts.id).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 	if err != nil {
 		if httpRes == nil {
 			return err
diff --git a/pkg/cmd/serviceaccount/describe/describe.go b/pkg/cmd/serviceaccount/describe/describe.go
@@ -71,7 +71,9 @@ func runDescribe(opts *Options) error {
 	api := connection.API()
 
 	res, httpRes, err := api.ServiceAccount().GetServiceAccountById(context.Background(), opts.id).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 	if err != nil {
 		if httpRes == nil {
 			return err
diff --git a/pkg/cmd/serviceaccount/resetcredentials/reset_credentials.go b/pkg/cmd/serviceaccount/resetcredentials/reset_credentials.go
@@ -108,7 +108,10 @@ func runResetCredentials(opts *Options) (err error) {
 	api := connection.API()
 
 	serviceacct, httpRes, err := api.ServiceAccount().GetServiceAccountById(context.Background(), opts.id).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
+
 	if err != nil {
 		return err
 	}
@@ -182,7 +185,9 @@ func resetCredentials(name string, opts *Options) (*kafkamgmtclient.ServiceAccou
 	opts.Logger.Debug(opts.localizer.MustLocalize("serviceAccount.resetCredentials.log.debug.resettingCredentials", localize.NewEntry("Name", name)))
 
 	serviceacct, httpRes, err := api.ServiceAccount().ResetServiceAccountCreds(context.Background(), opts.id).Execute()
-	defer httpRes.Body.Close()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+	}
 	if err != nil {
 		if httpRes == nil {
 			return nil, err
diff --git a/pkg/httputil/httputil.go b/pkg/httputil/httputil.go
@@ -0,0 +1,45 @@
+// Package httputil contains functions that act as middleware for api interactions
+package httputil
+
+import (
+	"net/http"
+	"net/http/httputil"
+
+	"github.com/redhat-developer/app-services-cli/pkg/logging"
+)
+
+// LoggingRoundTripper implements http.RoundTripper. When set as Transport of http.Client, it executes HTTP requests with logging.
+type LoggingRoundTripper struct {
+	Proxied http.RoundTripper
+	Logger  logging.Logger
+}
+
+// RoundTrip logs the http request and response in debug mode
+// for all errors, where status code >= 400
+func (c LoggingRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) {
+	resp, err := c.Proxied.RoundTrip(r)
+	if err != nil {
+		return nil, err
+	}
+
+	// only dump the HTTP request and response for errors
+	if resp.StatusCode < http.StatusBadRequest {
+		return resp, nil
+	}
+
+	requestDump, err := httputil.DumpRequest(r, true)
+	if err != nil {
+		return nil, err
+	}
+
+	c.Logger.Debug(string(requestDump))
+
+	responseDump, err := httputil.DumpResponse(resp, true)
+	if err != nil {
+		return nil, err
+	}
+
+	c.Logger.Debug(string(responseDump))
+
+	return resp, nil
+}
diff --git a/pkg/kafka/topic/validators.go b/pkg/kafka/topic/validators.go
@@ -154,12 +154,13 @@ func (v *Validator) ValidateNameIsAvailable(val interface{}) error {
 		return err
 	}
 
-	_, httpRes, _ := api.TopicsApi.GetTopic(context.Background(), name).Execute()
-
-	if httpRes != nil && httpRes.StatusCode == http.StatusOK {
-		return errors.New(v.Localizer.MustLocalize("kafka.topic.create.error.conflictError", localize.NewEntry("TopicName", name), localize.NewEntry("InstanceName", kafkaInstance.GetName())))
+	_, httpRes, err := api.TopicsApi.GetTopic(context.Background(), name).Execute()
+	if httpRes != nil {
+		defer httpRes.Body.Close()
+		if httpRes.StatusCode == http.StatusOK {
+			return errors.New(v.Localizer.MustLocalize("kafka.topic.create.error.conflictError", localize.NewEntry("TopicName", name), localize.NewEntry("InstanceName", kafkaInstance.GetName())))
+		}
 	}
-	defer httpRes.Body.Close()
 
-	return nil
+	return err
 }
