Skip to content

Commit

Permalink
Http client refactor (#300)
Browse files Browse the repository at this point in the history
* Add context injection and timeout to sapcontrol webservice

* Add ctx parameter to Discover interface

* Removed http.DefaultClient from production code

* Collector client http request with context

* Cloud package http client with context

* Long line linting
  • Loading branch information
CDimonaco committed Nov 21, 2023
1 parent 56711ae commit 0a54f8f
Show file tree
Hide file tree
Showing 27 changed files with 275 additions and 178 deletions.
7 changes: 4 additions & 3 deletions internal/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ type Config struct {

// NewAgent returns a new instance of Agent with the given configuration
func NewAgent(config *Config) (*Agent, error) {
collectorClient := collector.NewCollectorClient(config.DiscoveriesConfig.CollectorConfig, http.DefaultClient)
agentClient := http.Client{Timeout: 30 * time.Second}
collectorClient := collector.NewCollectorClient(config.DiscoveriesConfig.CollectorConfig, &agentClient)

discoveries := []discovery.Discovery{
discovery.NewClusterDiscovery(collectorClient, *config.DiscoveriesConfig),
Expand Down Expand Up @@ -137,7 +138,7 @@ func (a *Agent) Stop(ctxCancel context.CancelFunc) {
func (a *Agent) startDiscoverTicker(ctx context.Context, d discovery.Discovery) {

tick := func() {
result, err := d.Discover()
result, err := d.Discover(ctx)
if err != nil {
result = fmt.Sprintf("Error while running discovery '%s': %s", d.GetID(), err)
log.Errorln(result)
Expand All @@ -150,7 +151,7 @@ func (a *Agent) startDiscoverTicker(ctx context.Context, d discovery.Discovery)

func (a *Agent) startHeartbeatTicker(ctx context.Context) {
tick := func() {
err := a.collectorClient.Heartbeat()
err := a.collectorClient.Heartbeat(ctx)
if err != nil {
log.Errorf("Error while sending the heartbeat to the server: %s", err)
}
Expand Down
20 changes: 13 additions & 7 deletions internal/core/cloud/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Based on
package cloud

import (
"context"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -52,7 +53,7 @@ type Placement struct {
Region string `json:"region"`
}

func NewAWSMetadata(client HTTPClient) (*AWSMetadata, error) {
func NewAWSMetadata(ctx context.Context, client HTTPClient) (*AWSMetadata, error) {
var err error
awsMetadata := &AWSMetadata{
AmiID: "",
Expand Down Expand Up @@ -86,7 +87,7 @@ func NewAWSMetadata(client HTTPClient) (*AWSMetadata, error) {
}

firstElementsList := []string{fmt.Sprintf("%s/", awsMetadataResource)}
metadata, err := buildAWSMetadata(client, awsMetadataURL, firstElementsList)
metadata, err := buildAWSMetadata(ctx, client, awsMetadataURL, firstElementsList)
if err != nil {
return nil, err
}
Expand All @@ -104,13 +105,18 @@ func NewAWSMetadata(client HTTPClient) (*AWSMetadata, error) {
return awsMetadata, err
}

func buildAWSMetadata(client HTTPClient, url string, elements []string) (map[string]interface{}, error) {
func buildAWSMetadata(
ctx context.Context,
client HTTPClient,
url string,
elements []string,
) (map[string]interface{}, error) {
metadata := make(map[string]interface{})

for _, element := range elements {
newURL := url + element

response, err := requestMetadata(client, newURL)
response, err := requestMetadata(ctx, client, newURL)
if err != nil {
return metadata, err
}
Expand All @@ -119,7 +125,7 @@ func buildAWSMetadata(client HTTPClient, url string, elements []string) (map[str
currentElement := strings.Trim(element, "/")
newElements := strings.Split(fmt.Sprintf("%v", response), "\n")

metadata[currentElement], err = buildAWSMetadata(client, newURL, newElements)
metadata[currentElement], err = buildAWSMetadata(ctx, client, newURL, newElements)
if err != nil {
return nil, err
}
Expand All @@ -131,8 +137,8 @@ func buildAWSMetadata(client HTTPClient, url string, elements []string) (map[str
return metadata, nil
}

func requestMetadata(client HTTPClient, url string) (interface{}, error) {
req, _ := http.NewRequest(http.MethodGet, url, nil)
func requestMetadata(ctx context.Context, client HTTPClient, url string) (interface{}, error) {
req, _ := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)

resp, err := client.Do(req)
if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion internal/core/cloud/aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cloud_test

import (
"bytes"
"context"
"io"
"net/http"
"os"
Expand All @@ -24,6 +25,7 @@ func TestAWSMetadataTestSuite(t *testing.T) {
}

func (suite *AWSMetadataTestSuite) TestNewAWSMetadata() {
ctx := context.TODO()
clientMock := new(mocks.HTTPClient)

fixtures := []string{
Expand Down Expand Up @@ -65,7 +67,7 @@ func (suite *AWSMetadataTestSuite) TestNewAWSMetadata() {
).Once()
}

m, err := cloud.NewAWSMetadata(clientMock)
m, err := cloud.NewAWSMetadata(ctx, clientMock)

suite.NoError(err)

Expand Down
10 changes: 8 additions & 2 deletions internal/core/cloud/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package cloud

import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -142,7 +143,7 @@ type Subnet struct {
Prefix string `json:"prefix,omitempty"`
}

func NewAzureMetadata(client HTTPClient) (*AzureMetadata, error) {
func NewAzureMetadata(ctx context.Context, client HTTPClient) (*AzureMetadata, error) {
var err error
m := &AzureMetadata{
Compute: Compute{
Expand Down Expand Up @@ -220,7 +221,12 @@ func NewAzureMetadata(client HTTPClient) (*AzureMetadata, error) {
},
}

req, _ := http.NewRequest(http.MethodGet, fmt.Sprintf("http://%s/metadata/instance", azureAPIAddress), nil)
req, _ := http.NewRequestWithContext(
ctx,
http.MethodGet,
fmt.Sprintf("http://%s/metadata/instance", azureAPIAddress),
nil,
)
req.Header.Add("Metadata", "True")

q := req.URL.Query()
Expand Down
4 changes: 3 additions & 1 deletion internal/core/cloud/azure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cloud_test

import (
"bytes"
"context"
"io"
"net/http"
"os"
Expand All @@ -23,6 +24,7 @@ func TestAzureMetadataTestSuite(t *testing.T) {
}

func (suite *AzureMetadataTestSuite) TestNewAzureMetadata() {
ctx := context.TODO()
clientMock := new(mocks.HTTPClient)

aFile, _ := os.Open(helpers.GetFixturePath("discovery/azure/azure_metadata.json"))
Expand All @@ -38,7 +40,7 @@ func (suite *AzureMetadataTestSuite) TestNewAzureMetadata() {
response, nil,
)

m, err := cloud.NewAzureMetadata(clientMock)
m, err := cloud.NewAzureMetadata(ctx, clientMock)

expectedMeta := &cloud.AzureMetadata{
Compute: cloud.Compute{
Expand Down
5 changes: 3 additions & 2 deletions internal/core/cloud/gcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package cloud

import (
"bytes"
"context"
"encoding/json"
"io"
"net/http"
Expand Down Expand Up @@ -47,7 +48,7 @@ type GCPProject struct {
ProjectID string `json:"projectId,omitempty"`
}

func NewGCPMetadata(client HTTPClient) (*GCPMetadata, error) {
func NewGCPMetadata(ctx context.Context, client HTTPClient) (*GCPMetadata, error) {
var err error
m := &GCPMetadata{
Instance: GCPInstance{
Expand All @@ -63,7 +64,7 @@ func NewGCPMetadata(client HTTPClient) (*GCPMetadata, error) {
},
}

req, _ := http.NewRequest(http.MethodGet, gcpMetadataURL, nil)
req, _ := http.NewRequestWithContext(ctx, http.MethodGet, gcpMetadataURL, nil)
req.Header.Add("Metadata-Flavor", gcpMetadataFlavorHeader)

q := req.URL.Query()
Expand Down
4 changes: 3 additions & 1 deletion internal/core/cloud/gcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cloud_test

import (
"bytes"
"context"
"io"
"net/http"
"os"
Expand All @@ -23,6 +24,7 @@ func TestGcpMetadataTestSuite(t *testing.T) {
}

func (suite *GcpMetadataTestSuite) TestNewGCPMetadata() {
ctx := context.TODO()
clientMock := new(mocks.HTTPClient)

aFile, _ := os.Open(helpers.GetFixturePath("discovery/gcp/gcp_metadata.json"))
Expand All @@ -38,7 +40,7 @@ func (suite *GcpMetadataTestSuite) TestNewGCPMetadata() {
response, nil,
)

m, err := cloud.NewGCPMetadata(clientMock)
m, err := cloud.NewGCPMetadata(ctx, clientMock)

expectedMeta := &cloud.GCPMetadata{
Instance: cloud.GCPInstance{
Expand Down
13 changes: 9 additions & 4 deletions internal/core/cloud/metadata.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cloud

import (
"context"
"regexp"
"strings"

Expand Down Expand Up @@ -181,7 +182,11 @@ func (i *Identifier) IdentifyCloudProvider() (string, error) {
return "", nil
}

func NewCloudInstance(commandExecutor utils.CommandExecutor, client HTTPClient) (*Instance, error) {
func NewCloudInstance(
ctx context.Context,
commandExecutor utils.CommandExecutor,
client HTTPClient,
) (*Instance, error) {
var err error
var cloudMetadata interface{}

Expand All @@ -200,22 +205,22 @@ func NewCloudInstance(commandExecutor utils.CommandExecutor, client HTTPClient)
switch provider {
case Azure:
{
cloudMetadata, err = NewAzureMetadata(client)
cloudMetadata, err = NewAzureMetadata(ctx, client)
if err != nil {
return nil, err
}
}
case AWS:
{
awsMetadata, err := NewAWSMetadata(client)
awsMetadata, err := NewAWSMetadata(ctx, client)
if err != nil {
return nil, err
}
cloudMetadata = NewAWSMetadataDto(awsMetadata)
}
case GCP:
{
gcpMetadata, err := NewGCPMetadata(client)
gcpMetadata, err := NewGCPMetadata(ctx, client)
if err != nil {
return nil, err
}
Expand Down
17 changes: 12 additions & 5 deletions internal/core/cloud/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cloud_test

import (
"bytes"
"context"
"io"
"net/http"
"testing"
Expand Down Expand Up @@ -238,6 +239,8 @@ func (suite *CloudMetadataTestSuite) TestIdentifyCloudProviderNoCloud() {
}

func (suite *CloudMetadataTestSuite) TestNewCloudInstanceAzure() {
ctx := context.TODO()

suite.mockExecutor.
On("Exec", "dmidecode", "-s", "chassis-asset-tag").
Return(dmidecodeAzure(), nil)
Expand All @@ -253,7 +256,7 @@ func (suite *CloudMetadataTestSuite) TestNewCloudInstanceAzure() {
response, nil,
)

c, err := cloud.NewCloudInstance(suite.mockExecutor, suite.mockHTTPClient)
c, err := cloud.NewCloudInstance(ctx, suite.mockExecutor, suite.mockHTTPClient)

suite.NoError(err)
suite.Equal("azure", c.Provider)
Expand All @@ -263,6 +266,7 @@ func (suite *CloudMetadataTestSuite) TestNewCloudInstanceAzure() {
}

func (suite *CloudMetadataTestSuite) TestNewCloudInstanceAWS() {
ctx := context.TODO()
suite.mockExecutor.
On("Exec", "dmidecode", "-s", "chassis-asset-tag").
Return(dmidecodeEmpty(), nil).
Expand All @@ -289,7 +293,7 @@ func (suite *CloudMetadataTestSuite) TestNewCloudInstanceAWS() {
On("Do", mock.AnythingOfType("*http.Request")).
Return(response2, nil)

c, err := cloud.NewCloudInstance(suite.mockExecutor, suite.mockHTTPClient)
c, err := cloud.NewCloudInstance(ctx, suite.mockExecutor, suite.mockHTTPClient)

suite.NoError(err)
suite.Equal("aws", c.Provider)
Expand All @@ -299,6 +303,7 @@ func (suite *CloudMetadataTestSuite) TestNewCloudInstanceAWS() {
}

func (suite *CloudMetadataTestSuite) TestNewInstanceNutanix() {
ctx := context.TODO()
suite.mockExecutor.
On("Exec", "dmidecode", "-s", "chassis-asset-tag").
Return(dmidecodeEmpty(), nil).
Expand All @@ -311,7 +316,7 @@ func (suite *CloudMetadataTestSuite) TestNewInstanceNutanix() {
On("Exec", "dmidecode").
Return(dmidecodeNutanix(), nil)

c, err := cloud.NewCloudInstance(suite.mockExecutor, suite.mockHTTPClient)
c, err := cloud.NewCloudInstance(ctx, suite.mockExecutor, suite.mockHTTPClient)

suite.NoError(err)
suite.Equal("nutanix", c.Provider)
Expand All @@ -320,6 +325,7 @@ func (suite *CloudMetadataTestSuite) TestNewInstanceNutanix() {
}

func (suite *CloudMetadataTestSuite) TestNewInstanceKVM() {
ctx := context.TODO()
suite.mockExecutor.
On("Exec", "dmidecode", "-s", "chassis-asset-tag").
Return(dmidecodeEmpty(), nil).
Expand All @@ -334,7 +340,7 @@ func (suite *CloudMetadataTestSuite) TestNewInstanceKVM() {
On("Exec", "systemd-detect-virt").
Return(systemdDetectVirtKVM(), nil)

c, err := cloud.NewCloudInstance(suite.mockExecutor, suite.mockHTTPClient)
c, err := cloud.NewCloudInstance(ctx, suite.mockExecutor, suite.mockHTTPClient)

suite.NoError(err)
suite.Equal("kvm", c.Provider)
Expand All @@ -343,6 +349,7 @@ func (suite *CloudMetadataTestSuite) TestNewInstanceKVM() {
}

func (suite *CloudMetadataTestSuite) TestNewCloudInstanceNoCloud() {
ctx := context.TODO()
suite.mockExecutor.
On("Exec", "dmidecode", "-s", "chassis-asset-tag").
Return(dmidecodeEmpty(), nil).
Expand All @@ -357,7 +364,7 @@ func (suite *CloudMetadataTestSuite) TestNewCloudInstanceNoCloud() {
On("Exec", "systemd-detect-virt").
Return(systemdDetectVirtEmpty(), nil)

c, err := cloud.NewCloudInstance(suite.mockExecutor, suite.mockHTTPClient)
c, err := cloud.NewCloudInstance(ctx, suite.mockExecutor, suite.mockHTTPClient)

suite.NoError(err)
suite.Equal("", c.Provider)
Expand Down
9 changes: 5 additions & 4 deletions internal/core/sapsystem/sapcontrol.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package sapsystem

import (
"context"
"fmt"

"github.com/pkg/errors"
Expand All @@ -13,18 +14,18 @@ type SAPControl struct {
Properties []*sapcontrol.InstanceProperty
}

func NewSAPControl(w sapcontrol.WebService) (*SAPControl, error) {
properties, err := w.GetInstanceProperties()
func NewSAPControl(ctx context.Context, w sapcontrol.WebService) (*SAPControl, error) {
properties, err := w.GetInstanceProperties(ctx)
if err != nil {
return nil, errors.Wrap(err, "SAPControl web service error")
}

processes, err := w.GetProcessList()
processes, err := w.GetProcessList(ctx)
if err != nil {
return nil, errors.Wrap(err, "SAPControl web service error")
}

instances, err := w.GetSystemInstanceList()
instances, err := w.GetSystemInstanceList(ctx)
if err != nil {
return nil, errors.Wrap(err, "SAPControl web service error")
}
Expand Down
Loading

0 comments on commit 0a54f8f

Please sign in to comment.