From d2c0b8735d4e8e8ea6c5cd3e7726d92303d839ef Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 15 Feb 2024 14:07:03 +0100 Subject: [PATCH] Pass context with timeout to FQDN lookup (#4147) (#4263) * Pass context with timeout to FQDN lookup * Add temporary gomod replace * Add CHANGELOG fragment * Updating dependency * Update method calls * Extract function * Fallback to OS-provided hostname on FQDN lookup error * Refactoring: reuse * Adding unit tests * Add comment (cherry picked from commit 4c9d8658cd3c929167051b7e35354bd20009c2fd) Co-authored-by: Shaunak Kashyap Co-authored-by: Pierre HILBERT --- changelog/fragments/1706213166-fqdn-ctx.yaml | 32 ++++++++ .../agent/application/info/agent_metadata.go | 24 ++---- .../pkg/composable/providers/host/host.go | 16 +--- internal/pkg/util/host.go | 33 +++++++++ internal/pkg/util/host_test.go | 73 +++++++++++++++++++ 5 files changed, 147 insertions(+), 31 deletions(-) create mode 100644 changelog/fragments/1706213166-fqdn-ctx.yaml create mode 100644 internal/pkg/util/host.go create mode 100644 internal/pkg/util/host_test.go diff --git a/changelog/fragments/1706213166-fqdn-ctx.yaml b/changelog/fragments/1706213166-fqdn-ctx.yaml new file mode 100644 index 00000000000..d17b1733a38 --- /dev/null +++ b/changelog/fragments/1706213166-fqdn-ctx.yaml @@ -0,0 +1,32 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: bug-fix + +# Change summary; a 80ish characters long description of the change. +summary: Set timeout of 1 minute for FQDN lookups + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. +#description: + +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: elastic-agent + +# PR URL; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +pr: https://github.com/elastic/elastic-agent/pull/4147 + +# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +#issue: https://github.com/owner/repo/1234 diff --git a/internal/pkg/agent/application/info/agent_metadata.go b/internal/pkg/agent/application/info/agent_metadata.go index d4b26d5f3ca..35ec9897e96 100644 --- a/internal/pkg/agent/application/info/agent_metadata.go +++ b/internal/pkg/agent/application/info/agent_metadata.go @@ -8,14 +8,16 @@ import ( "context" "fmt" + "github.com/elastic/elastic-agent/pkg/features" + "runtime" "strings" "github.com/elastic/elastic-agent/internal/pkg/agent/application/paths" "github.com/elastic/elastic-agent/internal/pkg/agent/errors" "github.com/elastic/elastic-agent/internal/pkg/release" + "github.com/elastic/elastic-agent/internal/pkg/util" "github.com/elastic/elastic-agent/pkg/core/logger" - "github.com/elastic/elastic-agent/pkg/features" "github.com/elastic/go-sysinfo" "github.com/elastic/go-sysinfo/types" @@ -149,15 +151,7 @@ func (i *AgentInfo) ECSMetadata(l *logger.Logger) (*ECSMeta, error) { } info := sysInfo.Info() - hostname := info.Hostname - if features.FQDN() { - fqdn, err := sysInfo.FQDN() - if err != nil { - l.Debugf("unable to lookup FQDN: %s, using hostname = %s", err.Error(), hostname) - } else { - hostname = fqdn - } - } + hostname := util.GetHostName(features.FQDN(), info, sysInfo, l) return &ECSMeta{ Elastic: &ElasticECSMeta{ @@ -205,15 +199,7 @@ func (i *AgentInfo) ECSMetadataFlatMap(l *logger.Logger) (map[string]interface{} } info := sysInfo.Info() - hostname := info.Hostname - if features.FQDN() { - fqdn, err := sysInfo.FQDN() - if err != nil { - l.Debugf("unable to lookup FQDN: %s, using hostname = %s", err.Error(), hostname) - } else { - hostname = fqdn - } - } + hostname := util.GetHostName(features.FQDN(), info, sysInfo, l) // Agent meta[agentIDKey] = i.agentID diff --git a/internal/pkg/composable/providers/host/host.go b/internal/pkg/composable/providers/host/host.go index d63189bf71a..afb48b32c05 100644 --- a/internal/pkg/composable/providers/host/host.go +++ b/internal/pkg/composable/providers/host/host.go @@ -11,14 +11,14 @@ import ( "runtime" "time" - "github.com/elastic/elastic-agent/pkg/features" - "github.com/elastic/go-sysinfo" - "github.com/elastic/elastic-agent/internal/pkg/agent/errors" "github.com/elastic/elastic-agent/internal/pkg/composable" "github.com/elastic/elastic-agent/internal/pkg/config" corecomp "github.com/elastic/elastic-agent/internal/pkg/core/composable" + "github.com/elastic/elastic-agent/internal/pkg/util" "github.com/elastic/elastic-agent/pkg/core/logger" + "github.com/elastic/elastic-agent/pkg/features" + "github.com/elastic/go-sysinfo" ) const ( @@ -137,15 +137,7 @@ func getHostInfo(log *logger.Logger) func() (map[string]interface{}, error) { } info := sysInfo.Info() - name := info.Hostname - if features.FQDN() { - fqdn, err := sysInfo.FQDN() - if err != nil { - log.Debugf("unable to lookup FQDN: %s, using hostname = %s", err.Error(), name) - } else { - name = fqdn - } - } + name := util.GetHostName(features.FQDN(), info, sysInfo, log) return map[string]interface{}{ "id": info.UniqueID, diff --git a/internal/pkg/util/host.go b/internal/pkg/util/host.go new file mode 100644 index 00000000000..490afd5744e --- /dev/null +++ b/internal/pkg/util/host.go @@ -0,0 +1,33 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package util + +import ( + "context" + "time" + + "github.com/elastic/elastic-agent/pkg/core/logger" + "github.com/elastic/go-sysinfo/types" +) + +// GetHostName returns the host's FQDN if the FDQN feature flag is enabled; otherwise, it +// returns the OS-provided hostname. +func GetHostName(isFqdnFeatureEnabled bool, hostInfo types.HostInfo, host types.Host, log *logger.Logger) string { + if !isFqdnFeatureEnabled { + return hostInfo.Hostname + } + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) + defer cancel() + + fqdn, err := host.FQDNWithContext(ctx) + if err != nil { + // If we are unable to lookup the FQDN, we fallback to the OS-provided hostname + log.Debugf("unable to lookup FQDN: %s, using hostname = %s", err.Error(), hostInfo.Hostname) + return hostInfo.Hostname + } + + return fqdn +} diff --git a/internal/pkg/util/host_test.go b/internal/pkg/util/host_test.go new file mode 100644 index 00000000000..12f8cab44e9 --- /dev/null +++ b/internal/pkg/util/host_test.go @@ -0,0 +1,73 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package util + +import ( + "context" + "errors" + "testing" + + "github.com/elastic/elastic-agent-libs/logp" + + "github.com/stretchr/testify/require" + + "github.com/elastic/go-sysinfo/types" +) + +func TestGetHostName(t *testing.T) { + cases := map[string]struct { + fqdnFeatureEnabled bool + hostInfo types.HostInfo + host types.Host + log *logp.Logger + + expected string + }{ + "fqdn_feature_disabled": { + fqdnFeatureEnabled: false, + hostInfo: types.HostInfo{Hostname: "foobar"}, + expected: "foobar", + }, + "fqdn_lookup_fails": { + fqdnFeatureEnabled: true, + hostInfo: types.HostInfo{Hostname: "foobar"}, + host: &mockHost{ + fqdn: "", + fqdnErr: errors.New("fqdn lookup failed while testing"), + }, + log: logp.NewLogger("testing"), + expected: "foobar", + }, + "fqdn_lookup_succeeds": { + fqdnFeatureEnabled: true, + hostInfo: types.HostInfo{Hostname: "foobar"}, + host: &mockHost{ + fqdn: "qux", + fqdnErr: nil, + }, + expected: "qux", + }, + } + + for name, test := range cases { + t.Run(name, func(t *testing.T) { + hostname := GetHostName(test.fqdnFeatureEnabled, test.hostInfo, test.host, test.log) + require.Equal(t, test.expected, hostname) + }) + } +} + +type mockHost struct { + fqdn string + fqdnErr error +} + +func (m *mockHost) CPUTime() (types.CPUTimes, error) { return types.CPUTimes{}, nil } +func (m *mockHost) Info() types.HostInfo { return types.HostInfo{} } +func (m *mockHost) Memory() (*types.HostMemoryInfo, error) { return nil, nil } +func (m *mockHost) FQDNWithContext(ctx context.Context) (string, error) { + return m.fqdn, m.fqdnErr +} +func (m *mockHost) FQDN() (string, error) { return m.FQDNWithContext(context.Background()) }