From 265c50b1dc4acac6d13065510d2b3cee3a16f131 Mon Sep 17 00:00:00 2001
From: Ashvitha <ashvitha.sridharan@hashicorp.com>
Date: Tue, 30 May 2023 14:26:09 -0400
Subject: [PATCH] Add safety checks for the client telemetry gateway payload in
 case it's down (#17511)

---
 agent/hcp/client/client.go      | 33 +++++++++++----
 agent/hcp/client/client_test.go | 74 +++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+), 9 deletions(-)

diff --git a/agent/hcp/client/client.go b/agent/hcp/client/client.go
index 420310450..212647c51 100644
--- a/agent/hcp/client/client.go
+++ b/agent/hcp/client/client.go
@@ -115,15 +115,7 @@ func (c *hcpClient) FetchTelemetryConfig(ctx context.Context) (*TelemetryConfig,
 		return nil, err
 	}
 
-	payloadConfig := resp.Payload.TelemetryConfig
-	return &TelemetryConfig{
-		Endpoint: payloadConfig.Endpoint,
-		Labels:   payloadConfig.Labels,
-		MetricsConfig: &MetricsConfig{
-			Filters:  payloadConfig.Metrics.IncludeList,
-			Endpoint: payloadConfig.Metrics.Endpoint,
-		},
-	}, nil
+	return convertTelemetryConfig(resp)
 }
 
 func (c *hcpClient) FetchBootstrap(ctx context.Context) (*BootstrapConfig, error) {
@@ -281,6 +273,29 @@ func (c *hcpClient) DiscoverServers(ctx context.Context) ([]string, error) {
 	return servers, nil
 }
 
+// convertTelemetryConfig validates the AgentTelemetryConfig payload and converts it into a TelemetryConfig object.
+func convertTelemetryConfig(resp *hcptelemetry.AgentTelemetryConfigOK) (*TelemetryConfig, error) {
+	if resp.Payload == nil {
+		return nil, fmt.Errorf("missing payload")
+	}
+
+	if resp.Payload.TelemetryConfig == nil {
+		return nil, fmt.Errorf("missing telemetry config")
+	}
+
+	payloadConfig := resp.Payload.TelemetryConfig
+	var metricsConfig MetricsConfig
+	if payloadConfig.Metrics != nil {
+		metricsConfig.Endpoint = payloadConfig.Metrics.Endpoint
+		metricsConfig.Filters = payloadConfig.Metrics.IncludeList
+	}
+	return &TelemetryConfig{
+		Endpoint:      payloadConfig.Endpoint,
+		Labels:        payloadConfig.Labels,
+		MetricsConfig: &metricsConfig,
+	}, nil
+}
+
 // Enabled verifies if telemetry is enabled by ensuring a valid endpoint has been retrieved.
 // It returns full metrics endpoint and true if a valid endpoint was obtained.
 func (t *TelemetryConfig) Enabled() (string, bool) {
diff --git a/agent/hcp/client/client_test.go b/agent/hcp/client/client_test.go
index 43ecf0fd5..8c8a6addd 100644
--- a/agent/hcp/client/client_test.go
+++ b/agent/hcp/client/client_test.go
@@ -4,6 +4,8 @@ import (
 	"context"
 	"testing"
 
+	"github.com/hashicorp/hcp-sdk-go/clients/cloud-consul-telemetry-gateway/preview/2023-04-14/client/consul_telemetry_service"
+	"github.com/hashicorp/hcp-sdk-go/clients/cloud-consul-telemetry-gateway/preview/2023-04-14/models"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
@@ -73,3 +75,75 @@ func TestFetchTelemetryConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestConvertTelemetryConfig(t *testing.T) {
+	t.Parallel()
+	for name, test := range map[string]struct {
+		resp                 *consul_telemetry_service.AgentTelemetryConfigOK
+		expectedTelemetryCfg *TelemetryConfig
+		wantErr              string
+	}{
+		"success": {
+			resp: &consul_telemetry_service.AgentTelemetryConfigOK{
+				Payload: &models.HashicorpCloudConsulTelemetry20230414AgentTelemetryConfigResponse{
+					TelemetryConfig: &models.HashicorpCloudConsulTelemetry20230414TelemetryConfig{
+						Endpoint: "https://test.com",
+						Labels:   map[string]string{"test": "test"},
+					},
+				},
+			},
+			expectedTelemetryCfg: &TelemetryConfig{
+				Endpoint:      "https://test.com",
+				Labels:        map[string]string{"test": "test"},
+				MetricsConfig: &MetricsConfig{},
+			},
+		},
+		"successWithMetricsConfig": {
+			resp: &consul_telemetry_service.AgentTelemetryConfigOK{
+				Payload: &models.HashicorpCloudConsulTelemetry20230414AgentTelemetryConfigResponse{
+					TelemetryConfig: &models.HashicorpCloudConsulTelemetry20230414TelemetryConfig{
+						Endpoint: "https://test.com",
+						Labels:   map[string]string{"test": "test"},
+						Metrics: &models.HashicorpCloudConsulTelemetry20230414TelemetryMetricsConfig{
+							Endpoint:    "https://metrics-test.com",
+							IncludeList: []string{"consul.raft.apply"},
+						},
+					},
+				},
+			},
+			expectedTelemetryCfg: &TelemetryConfig{
+				Endpoint: "https://test.com",
+				Labels:   map[string]string{"test": "test"},
+				MetricsConfig: &MetricsConfig{
+					Endpoint: "https://metrics-test.com",
+					Filters:  []string{"consul.raft.apply"},
+				},
+			},
+		},
+		"errorsWithNilPayload": {
+			resp:    &consul_telemetry_service.AgentTelemetryConfigOK{},
+			wantErr: "missing payload",
+		},
+		"errorsWithNilTelemetryConfig": {
+			resp: &consul_telemetry_service.AgentTelemetryConfigOK{
+				Payload: &models.HashicorpCloudConsulTelemetry20230414AgentTelemetryConfigResponse{},
+			},
+			wantErr: "missing telemetry config",
+		},
+	} {
+		test := test
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			telemetryCfg, err := convertTelemetryConfig(test.resp)
+			if test.wantErr != "" {
+				require.Error(t, err)
+				require.Nil(t, telemetryCfg)
+				require.Contains(t, err.Error(), test.wantErr)
+				return
+			}
+
+			require.NoError(t, err)
+			require.Equal(t, test.expectedTelemetryCfg, telemetryCfg)
+		})
+	}
+}