diff --git a/agent/hcp/bootstrap/bootstrap_test.go b/agent/hcp/bootstrap/bootstrap_test.go index 74b57e5f50ab..8aced71c4461 100644 --- a/agent/hcp/bootstrap/bootstrap_test.go +++ b/agent/hcp/bootstrap/bootstrap_test.go @@ -156,12 +156,13 @@ func TestLoadConfig_Persistence(t *testing.T) { initial, err := baseLoader(nil) require.NoError(t, err) + ctx := context.Background() // Override the client TLS config so that the test server can be trusted. initial.RuntimeConfig.Cloud.WithTLSConfig(clientTLS) - client, err := hcpclient.NewClient(initial.RuntimeConfig.Cloud) + client, err := hcpclient.NewClient(initial.RuntimeConfig.Cloud, ctx) require.NoError(t, err) - loader, err := LoadConfig(context.Background(), client, initial.RuntimeConfig.DataDir, baseLoader, ui) + loader, err := LoadConfig(ctx, client, initial.RuntimeConfig.DataDir, baseLoader, ui) require.NoError(t, err) // Load the agent config with the potentially wrapped loader. diff --git a/agent/hcp/client/client.go b/agent/hcp/client/client.go index 9e3990ad7f3b..83cfc2a560f0 100644 --- a/agent/hcp/client/client.go +++ b/agent/hcp/client/client.go @@ -6,15 +6,13 @@ package client import ( "context" "fmt" - "regexp" "strconv" - "strings" "time" httptransport "github.com/go-openapi/runtime/client" "github.com/go-openapi/strfmt" - "github.com/hashicorp/go-multierror" + "github.com/hashicorp/go-hclog" hcptelemetry "github.com/hashicorp/hcp-sdk-go/clients/cloud-consul-telemetry-gateway/preview/2023-04-14/client/consul_telemetry_service" hcpgnm "github.com/hashicorp/hcp-sdk-go/clients/cloud-global-network-manager-service/preview/2022-02-15/client/global_network_manager_service" gnmmod "github.com/hashicorp/hcp-sdk-go/clients/cloud-global-network-manager-service/preview/2022-02-15/models" @@ -38,21 +36,6 @@ type Client interface { DiscoverServers(ctx context.Context) ([]string, error) } -// MetricsConfig holds metrics specific configuration for the TelemetryConfig. -// The endpoint field overrides the TelemetryConfig endpoint. -type MetricsConfig struct { - Filters []string - Endpoint string -} - -// TelemetryConfig contains configuration for telemetry data forwarded by Consul servers -// to the HCP Telemetry gateway. -type TelemetryConfig struct { - Endpoint string - Labels map[string]string - MetricsConfig *MetricsConfig -} - type BootstrapConfig struct { Name string BootstrapExpect int @@ -70,9 +53,10 @@ type hcpClient struct { gnm hcpgnm.ClientService tgw hcptelemetry.ClientService resource resource.Resource + logger hclog.Logger } -func NewClient(cfg config.CloudConfig) (Client, error) { +func NewClient(cfg config.CloudConfig, ctx context.Context) (Client, error) { client := &hcpClient{ cfg: cfg, } @@ -90,6 +74,7 @@ func NewClient(cfg config.CloudConfig) (Client, error) { client.gnm = hcpgnm.New(client.hc, nil) client.tgw = hcptelemetry.New(client.hc, nil) + client.logger = hclog.FromContext(ctx).Named("hcp_client") return client, nil } @@ -115,10 +100,14 @@ func (c *hcpClient) FetchTelemetryConfig(ctx context.Context) (*TelemetryConfig, resp, err := c.tgw.AgentTelemetryConfig(params, nil) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to fetch from HCP: %w", err) } - return convertTelemetryConfig(resp) + if err := validateAgentTelemetryConfigPayload(resp); err != nil { + return nil, fmt.Errorf("invalid response payload: %w", err) + } + + return convertAgentTelemetryResponse(resp, c.logger, c.cfg) } func (c *hcpClient) FetchBootstrap(ctx context.Context) (*BootstrapConfig, error) { @@ -275,89 +264,3 @@ 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) { - endpoint := t.Endpoint - if override := t.MetricsConfig.Endpoint; override != "" { - endpoint = override - } - - if endpoint == "" { - return "", false - } - - // The endpoint from Telemetry Gateway is a domain without scheme, and without the metrics path, so they must be added. - return endpoint + metricsGatewayPath, true -} - -// DefaultLabels returns a set of string pairs that must be added as attributes to all exported telemetry data. -func (t *TelemetryConfig) DefaultLabels(cfg config.CloudConfig) map[string]string { - labels := make(map[string]string) - nodeID := string(cfg.NodeID) - if nodeID != "" { - labels["node_id"] = nodeID - } - if cfg.NodeName != "" { - labels["node_name"] = cfg.NodeName - } - - for k, v := range t.Labels { - labels[k] = v - } - - return labels -} - -// FilterRegex returns a valid regex used to filter metrics. -// It will fail if there are 0 valid regex filters given. -func (t *TelemetryConfig) FilterRegex() (*regexp.Regexp, error) { - var mErr error - filters := t.MetricsConfig.Filters - validFilters := make([]string, 0, len(filters)) - for _, filter := range filters { - _, err := regexp.Compile(filter) - if err != nil { - mErr = multierror.Append(mErr, fmt.Errorf("compilation of filter %q failed: %w", filter, err)) - continue - } - validFilters = append(validFilters, filter) - } - - if len(validFilters) == 0 { - return nil, multierror.Append(mErr, fmt.Errorf("no valid filters")) - } - - // Combine the valid regex strings with an OR. - finalRegex := strings.Join(validFilters, "|") - composedRegex, err := regexp.Compile(finalRegex) - if err != nil { - return nil, fmt.Errorf("failed to compile regex: %w", err) - } - - return composedRegex, nil -} diff --git a/agent/hcp/client/client_test.go b/agent/hcp/client/client_test.go index 8587b0cfec89..ac228af3c57e 100644 --- a/agent/hcp/client/client_test.go +++ b/agent/hcp/client/client_test.go @@ -2,257 +2,91 @@ package client import ( "context" + "fmt" "testing" - "github.com/hashicorp/consul/agent/hcp/config" - "github.com/hashicorp/consul/types" - "github.com/hashicorp/hcp-sdk-go/clients/cloud-consul-telemetry-gateway/preview/2023-04-14/client/consul_telemetry_service" + "github.com/go-openapi/runtime" + "github.com/hashicorp/go-hclog" + hcptelemetry "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" ) -func TestFetchTelemetryConfig(t *testing.T) { - t.Parallel() - for name, test := range map[string]struct { - metricsEndpoint string - expect func(*MockClient) - disabled bool - }{ - "success": { - expect: func(mockClient *MockClient) { - mockClient.EXPECT().FetchTelemetryConfig(mock.Anything).Return(&TelemetryConfig{ - Endpoint: "https://test.com", - MetricsConfig: &MetricsConfig{ - Endpoint: "", - }, - }, nil) - }, - metricsEndpoint: "https://test.com/v1/metrics", - }, - "overrideMetricsEndpoint": { - expect: func(mockClient *MockClient) { - mockClient.EXPECT().FetchTelemetryConfig(mock.Anything).Return(&TelemetryConfig{ - Endpoint: "https://test.com", - MetricsConfig: &MetricsConfig{ - Endpoint: "https://test.com", - }, - }, nil) - }, - metricsEndpoint: "https://test.com/v1/metrics", - }, - "disabledWithEmptyEndpoint": { - expect: func(mockClient *MockClient) { - mockClient.EXPECT().FetchTelemetryConfig(mock.Anything).Return(&TelemetryConfig{ - Endpoint: "", - MetricsConfig: &MetricsConfig{ - Endpoint: "", - }, - }, nil) - }, - disabled: true, - }, - } { - test := test - t.Run(name, func(t *testing.T) { - t.Parallel() - - mock := NewMockClient(t) - test.expect(mock) - - telemetryCfg, err := mock.FetchTelemetryConfig(context.Background()) - require.NoError(t, err) - - if test.disabled { - endpoint, ok := telemetryCfg.Enabled() - require.False(t, ok) - require.Empty(t, endpoint) - return - } - - endpoint, ok := telemetryCfg.Enabled() +type mockTGW struct { + mockResponse *hcptelemetry.AgentTelemetryConfigOK + mockError error +} - require.True(t, ok) - require.Equal(t, test.metricsEndpoint, endpoint) - }) - } +func (m *mockTGW) AgentTelemetryConfig(params *hcptelemetry.AgentTelemetryConfigParams, authInfo runtime.ClientAuthInfoWriter, opts ...hcptelemetry.ClientOption) (*hcptelemetry.AgentTelemetryConfigOK, error) { + return m.mockResponse, m.mockError +} +func (m *mockTGW) GetLabelValues(params *hcptelemetry.GetLabelValuesParams, authInfo runtime.ClientAuthInfoWriter, opts ...hcptelemetry.ClientOption) (*hcptelemetry.GetLabelValuesOK, error) { + return hcptelemetry.NewGetLabelValuesOK(), nil +} +func (m *mockTGW) QueryRangeBatch(params *hcptelemetry.QueryRangeBatchParams, authInfo runtime.ClientAuthInfoWriter, opts ...hcptelemetry.ClientOption) (*hcptelemetry.QueryRangeBatchOK, error) { + return hcptelemetry.NewQueryRangeBatchOK(), nil } +func (m *mockTGW) SetTransport(transport runtime.ClientTransport) {} -func TestConvertTelemetryConfig(t *testing.T) { +func TestFetchTelemetryConfig(t *testing.T) { t.Parallel() - for name, test := range map[string]struct { - resp *consul_telemetry_service.AgentTelemetryConfigOK - expectedTelemetryCfg *TelemetryConfig - wantErr string + for name, tc := range map[string]struct { + mockResponse *hcptelemetry.AgentTelemetryConfigOK + mockError error + 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{}, + "errorsWithFetchFailure": { + mockError: fmt.Errorf("failed to fetch from HCP"), + mockResponse: nil, + wantErr: "failed to fetch from HCP", + }, + "errorsWithInvalidPayload": { + mockResponse: &hcptelemetry.AgentTelemetryConfigOK{ + Payload: &models.HashicorpCloudConsulTelemetry20230414AgentTelemetryConfigResponse{}, }, + mockError: nil, + wantErr: "invalid response payload", }, - "successWithMetricsConfig": { - resp: &consul_telemetry_service.AgentTelemetryConfigOK{ + "success:": { + mockResponse: &hcptelemetry.AgentTelemetryConfigOK{ Payload: &models.HashicorpCloudConsulTelemetry20230414AgentTelemetryConfigResponse{ + RefreshConfig: &models.HashicorpCloudConsulTelemetry20230414RefreshConfig{ + RefreshInterval: "1s", + }, TelemetryConfig: &models.HashicorpCloudConsulTelemetry20230414TelemetryConfig{ Endpoint: "https://test.com", - Labels: map[string]string{"test": "test"}, + Labels: map[string]string{"test": "123"}, Metrics: &models.HashicorpCloudConsulTelemetry20230414TelemetryMetricsConfig{ - Endpoint: "https://metrics-test.com", - IncludeList: []string{"consul.raft.apply"}, + IncludeList: []string{"consul"}, }, }, }, }, - 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) - }) - } -} - -func Test_DefaultLabels(t *testing.T) { - for name, tc := range map[string]struct { - cfg config.CloudConfig - expectedLabels map[string]string - }{ - "Success": { - cfg: config.CloudConfig{ - NodeID: types.NodeID("nodeyid"), - NodeName: "nodey", - }, - expectedLabels: map[string]string{ - "node_id": "nodeyid", - "node_name": "nodey", - }, - }, - - "NoNodeID": { - cfg: config.CloudConfig{ - NodeID: types.NodeID(""), - NodeName: "nodey", - }, - expectedLabels: map[string]string{ - "node_name": "nodey", - }, - }, - "NoNodeName": { - cfg: config.CloudConfig{ - NodeID: types.NodeID("nodeyid"), - NodeName: "", - }, - expectedLabels: map[string]string{ - "node_id": "nodeyid", - }, - }, - "Empty": { - cfg: config.CloudConfig{ - NodeID: "", - NodeName: "", - }, - expectedLabels: map[string]string{}, - }, - } { - t.Run(name, func(t *testing.T) { - tCfg := &TelemetryConfig{} - labels := tCfg.DefaultLabels(tc.cfg) - require.Equal(t, labels, tc.expectedLabels) - }) - } -} - -func TestFilterRegex(t *testing.T) { - t.Parallel() - for name, tc := range map[string]struct { - filters []string - expectedRegexString string - matches []string - wantErr string - wantMatch bool - }{ - "badFilterRegex": { - filters: []string{"(*LF)"}, - wantErr: "no valid filters", - }, - "failsWithNoRegex": { - filters: []string{}, - wantErr: "no valid filters", - }, - "matchFound": { - filters: []string{"raft.*", "mem.*"}, - expectedRegexString: "raft.*|mem.*", - matches: []string{"consul.raft.peers", "consul.mem.heap_size"}, - wantMatch: true, - }, - "matchNotFound": { - filters: []string{"mem.*"}, - matches: []string{"consul.raft.peers", "consul.txn.apply"}, - expectedRegexString: "mem.*", - wantMatch: false, + mockError: nil, }, } { tc := tc t.Run(name, func(t *testing.T) { t.Parallel() - cfg := &TelemetryConfig{ - MetricsConfig: &MetricsConfig{ - Filters: tc.filters, + c := &hcpClient{ + tgw: &mockTGW{ + mockError: tc.mockError, + mockResponse: tc.mockResponse, }, + logger: hclog.NewNullLogger(), } - f, err := cfg.FilterRegex() + telemetryCfg, err := c.FetchTelemetryConfig(context.Background()) if tc.wantErr != "" { require.Error(t, err) require.Contains(t, err.Error(), tc.wantErr) + require.Nil(t, telemetryCfg) return } require.NoError(t, err) - require.Equal(t, tc.expectedRegexString, f.String()) - for _, metric := range tc.matches { - m := f.MatchString(metric) - require.Equal(t, tc.wantMatch, m) - } + require.NotNil(t, telemetryCfg) }) } } diff --git a/agent/hcp/discover/discover.go b/agent/hcp/discover/discover.go index 12024b7dd6a0..7fbead95e0fe 100644 --- a/agent/hcp/discover/discover.go +++ b/agent/hcp/discover/discover.go @@ -32,12 +32,13 @@ func (p *Provider) Addrs(args map[string]string, l *log.Logger) ([]string, error return nil, err } - client, err := hcpclient.NewClient(cfg.CloudConfig) + ctx := context.Background() + client, err := hcpclient.NewClient(cfg.CloudConfig, ctx) if err != nil { return nil, err } - ctx, cancel := context.WithTimeout(context.Background(), cfg.timeout) + ctx, cancel := context.WithTimeout(ctx, cfg.timeout) defer cancel() servers, err := client.DiscoverServers(ctx) if err != nil { diff --git a/command/agent/agent.go b/command/agent/agent.go index c241fa2b7bc8..07d297cfa9cb 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -169,7 +169,7 @@ func (c *cmd) run(args []string) int { return 1 } if res.RuntimeConfig.IsCloudEnabled() { - client, err := hcpclient.NewClient(res.RuntimeConfig.Cloud) + client, err := hcpclient.NewClient(res.RuntimeConfig.Cloud, ctx) if err != nil { ui.Error("error building HCP HTTP client: " + err.Error()) return 1 diff --git a/go.mod b/go.mod index ed8caab6ee01..145df55a418b 100644 --- a/go.mod +++ b/go.mod @@ -62,7 +62,7 @@ require ( github.com/hashicorp/golang-lru v0.5.4 github.com/hashicorp/hcl v1.0.0 github.com/hashicorp/hcp-scada-provider v0.2.3 - github.com/hashicorp/hcp-sdk-go v0.48.0 + github.com/hashicorp/hcp-sdk-go v0.54.0 github.com/hashicorp/hil v0.0.0-20200423225030-a18a1cd20038 github.com/hashicorp/memberlist v0.5.0 github.com/hashicorp/raft v1.5.0 diff --git a/go.sum b/go.sum index 0eae0ab4af9f..4d302f1259bd 100644 --- a/go.sum +++ b/go.sum @@ -554,8 +554,8 @@ github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/hashicorp/hcp-scada-provider v0.2.3 h1:AarYR+/Pcv+cMvPdAlb92uOBmZfEH6ny4+DT+4NY2VQ= github.com/hashicorp/hcp-scada-provider v0.2.3/go.mod h1:ZFTgGwkzNv99PLQjTsulzaCplCzOTBh0IUQsPKzrQFo= -github.com/hashicorp/hcp-sdk-go v0.48.0 h1:LWpFR7YVDz4uG4C/ixcy2tRbg7/BgjMcTh1bRkKaeBQ= -github.com/hashicorp/hcp-sdk-go v0.48.0/go.mod h1:hZqky4HEzsKwvLOt4QJlZUrjeQmb4UCZUhDP2HyQFfc= +github.com/hashicorp/hcp-sdk-go v0.54.0 h1:7cyymgoRFGzmmUHohuzi8Jf3u5zShZpfg+2EHTXJh3M= +github.com/hashicorp/hcp-sdk-go v0.54.0/go.mod h1:hZqky4HEzsKwvLOt4QJlZUrjeQmb4UCZUhDP2HyQFfc= github.com/hashicorp/hil v0.0.0-20200423225030-a18a1cd20038 h1:n9J0rwVWXDpNd5iZnwY7w4WZyq53/rROeI7OVvLW8Ok= github.com/hashicorp/hil v0.0.0-20200423225030-a18a1cd20038/go.mod h1:n2TSygSNwsLJ76m8qFXTSc7beTb+auJxYdqrnoqwZWE= github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64=