Merge branch 'main' into dependabot/go_modules/github.com/elastic/elastic-transport-go/v8-8.4.0

cmacknz · web-flow · commit 7ea72a6ae2ce · 2024-02-14T16:08:41.000-05:00
diff --git a/.mergify.yml b/.mergify.yml
@@ -293,3 +293,16 @@ pull_request_rules:
         labels:
           - "backport"
         title: "[{{ destination_branch }}](backport #{{ number }}) {{ title }}"
+  - name: backport patches to 8.13 branch
+    conditions:
+      - merged
+      - label=backport-v8.13.0
+    actions:
+      backport:
+        assignees:
+          - "{{ author }}"
+        branches:
+          - "8.13"
+        labels:
+          - "backport"
+        title: "[{{ destination_branch }}](backport #{{ number }}) {{ title }}"
diff --git a/changelog/fragments/1706213166-fqdn-ctx.yaml 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/changelog/fragments/1707857612-windows_stop_before_uninstall.yaml b/changelog/fragments/1707857612-windows_stop_before_uninstall.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: On Windows make sure the service is stopped before uninstalling.
+
+# 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; a word indicating the component this changeset affects.
+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/owner/repo/1234
+
+# 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
@@ -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/agent/install/uninstall.go b/internal/pkg/agent/install/uninstall.go
@@ -54,13 +54,25 @@ func Uninstall(cfgFile, topPath, uninstallToken string, log *logp.Logger, pt *pr
 	if status == service.StatusRunning {
 		err := svc.Stop()
 		if err != nil {
-			pt.Describe("Failed to stop service")
+			pt.Describe("Failed to issue stop service")
 			return aerrors.New(
 				err,
-				fmt.Sprintf("failed to stop service (%s)", paths.ServiceName),
+				fmt.Sprintf("failed to issue stop service (%s)", paths.ServiceName),
 				aerrors.M("service", paths.ServiceName))
 		}
 	}
+	// The kardianos service manager can't tell the difference
+	// between 'Stopped' and 'StopPending' on Windows, so make
+	// sure the service is stopped.
+	err = isStopped(30*time.Second, 250*time.Millisecond, paths.ServiceName)
+	if err != nil {
+		pt.Describe("Failed to complete stop of service")
+		return aerrors.New(
+			err,
+			fmt.Sprintf("failed to complete stop service (%s)", paths.ServiceName),
+			aerrors.M("service", paths.ServiceName))
+	}
+
 	pt.Describe("Successfully stopped service")
 
 	// kill any running watcher
diff --git a/internal/pkg/agent/install/wait_service_other.go b/internal/pkg/agent/install/wait_service_other.go
@@ -0,0 +1,17 @@
+// 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.
+
+//go:build !windows
+
+package install
+
+import (
+	"time"
+)
+
+// isStopped waits until the service has stopped.  On non Windows
+// systems this isn't necessary so just return.
+func isStopped(timeout time.Duration, interval time.Duration, service string) error {
+	return nil
+}
diff --git a/internal/pkg/agent/install/wait_service_windows.go b/internal/pkg/agent/install/wait_service_windows.go
@@ -0,0 +1,60 @@
+// 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.
+
+//go:build windows
+
+package install
+
+import (
+	"fmt"
+	"time"
+
+	"golang.org/x/sys/windows/svc"
+	"golang.org/x/sys/windows/svc/mgr"
+)
+
+// isStopped queries the Windows service manager to see if the state
+// of the service is stopped.  It will repeat the query every
+// 'interval' until the 'timeout' is reached.  It returns nil if the
+// system is stopped within the timeout period.  An error is returned
+// if the service doesn't stop before the timeout or if there are
+// errors communicating with the service manager.
+func isStopped(timeout time.Duration, interval time.Duration, service string) error {
+	var err error
+	var status svc.Status
+
+	m, err := mgr.Connect()
+	if err != nil {
+		return fmt.Errorf("failed to connect to service manager: %w", err)
+	}
+	defer func() {
+		_ = m.Disconnect()
+	}()
+
+	s, err := m.OpenService(service)
+	if err != nil {
+		return fmt.Errorf("failed to open service (%s): %w", service, err)
+	}
+	defer s.Close()
+
+	ticker := time.NewTicker(interval)
+	defer ticker.Stop()
+	timer := time.NewTimer(timeout)
+	defer timer.Stop()
+
+	for {
+		select {
+		case <-ticker.C:
+			status, err = s.Query()
+			if err != nil {
+				return fmt.Errorf("error querying service (%s): %w", service, err)
+			}
+			if status.State == svc.Stopped {
+				return nil
+			}
+		case <-timer.C:
+			return fmt.Errorf("timed out after %s waiting for service (%s) to stop, last state was: %d", timeout, service, status.State)
+		}
+	}
+}
diff --git 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
@@ -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
@@ -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()) }
diff --git a/testing/integration/install_test.go b/testing/integration/install_test.go
