Merge branch 'main' into update-agent-versions-8592656277

Author: cmacknz

.github/workflows/bump-agent-versions.sh

@@ -17,10 +17,10 @@ else
     git diff -p
     git add ".agent-versions.json"
 
-		nl=$'\n' # otherwise the new line character is not recognized properly
-		commit_desc="This file is used for picking agent versions in integration tests.${nl}${nl}The file's content is based on responses from https://www.elastic.co/api/product_versions and https://snapshots.elastic.co${nl}${nl}The current update is generated based on the following requirements:${nl}${nl}\`\`\`json${nl}${version_requirements}${nl}\`\`\`"
+    nl=$'\n' # otherwise the new line character is not recognized properly
+    commit_desc="This file is used for picking agent versions in integration tests.${nl}${nl}The file's content is based on responses from https://www.elastic.co/api/product_versions and https://snapshots.elastic.co${nl}${nl}The current update is generated based on the following requirements:${nl}${nl}\`\`\`json${nl}${version_requirements}${nl}\`\`\`"
 
-		git commit -m "[$GITHUB_REF_NAME][Automation] Update .agent-versions.json" -m "$commit_desc"
+    git commit -m "[$GITHUB_REF_NAME][Automation] Update .agent-versions.json" -m "$commit_desc"
     git push --set-upstream origin "update-agent-versions-$GITHUB_RUN_ID"
     gh pr create \
        --base "$GITHUB_REF_NAME" \
@@ -31,4 +31,5 @@ else
        --label 'skip-changelog' \
        --label 'backport-skip' \
        --repo $GITHUB_REPOSITORY
+    echo "::set-output name=pr::true" # set step output for notifications
 fi

.github/workflows/bump-agent-versions.yml

@@ -40,6 +40,7 @@ jobs:
           install-only: true
 
       - name: Update versions file
+        id: update
         env:
           GH_TOKEN: ${{ env.GITHUB_TOKEN }}
         run: ./.github/workflows/bump-agent-versions.sh
@@ -53,7 +54,7 @@ jobs:
           message: ":traffic_cone: Elastic Agent versions file update failed: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
           channel: "#ingest-notifications"
 
-      - if: ${{ success() }}
+      - if: ${{ success() && steps.update.output.pr == true}}
         uses: elastic/apm-pipeline-library/.github/actions/slack-message@current
         with:
           url: ${{ secrets.VAULT_ADDR }}

changelog/fragments/1711474205-Reduce-false-positives-in-logging-an-API-switch-request-from-Fleet-server.yaml

@@ -0,0 +1,34 @@
+# 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: Reduce false positives in logging an API switch request from Fleet server
+
+# 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
+pr: https://github.com/elastic/elastic-agent/pull/4481
+
+# 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
+issue: https://github.com/elastic/elastic-agent/issues/4477

internal/pkg/fleetapi/client/client_test.go

@@ -183,6 +183,37 @@ func TestElasticApiVersion(t *testing.T) {
 		}
 	})
 
+	t.Run("verify that we don't log a generic 400 status as a downgrade request", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+
+		mux := http.NewServeMux()
+		mux.HandleFunc("/genericbadrequest", func(writer http.ResponseWriter, request *http.Request) {
+			assert.Equal(t, request.Header.Get(elasticApiVersionHeaderKey), defaultFleetApiVersion)
+			// request return a 400 with the defaultFleetApiVersion (just testing that we don't log that as a downgrade request)
+			writer.Header().Add(elasticApiVersionHeaderKey, defaultFleetApiVersion)
+			writer.WriteHeader(http.StatusBadRequest)
+		})
+
+		ts := httptest.NewServer(mux)
+		defer ts.Close()
+
+		testLogger, obsLogs := logger.NewTesting("testElasticApiVersion")
+
+		clt, err := NewWithConfig(testLogger, remote.Config{
+			Hosts: []string{ts.URL},
+		})
+		require.NoError(t, err)
+
+		resp, err := clt.Send(ctx, http.MethodGet, "/genericbadrequest", nil, nil, nil)
+		if assert.NoError(t, err) {
+			defer resp.Body.Close()
+		}
+		logs := obsLogs.FilterMessageSnippet("fleet requested a different api version").All()
+		t.Logf("retrieved logs: %v", logs)
+		assert.Empty(t, logs, "downgrade response should not be logged when the fleet api version is the same as the request")
+	})
+
 	t.Run("verify that we log a downgrade request", func(t *testing.T) {
 		ctx, cancel := context.WithCancel(context.Background())
 		defer cancel()

internal/pkg/remote/client.go

@@ -27,6 +27,7 @@ import (
 
 const (
 	retryOnBadConnTimeout = 5 * time.Minute
+	requestIDHeaderName   = "X-Request-ID"
 )
 
 type wrapperFunc func(rt http.RoundTripper) (http.RoundTripper, error)
@@ -194,7 +195,7 @@ func (c *Client) Send(
 
 		// If available, add the request id as an HTTP header
 		if reqID != "" {
-			req.Header.Add("X-Request-ID", reqID)
+			req.Header.Add(requestIDHeaderName, reqID)
 		}
 
 		// copy headers.
@@ -220,27 +221,30 @@ func (c *Client) Send(
 			c.log.With("error", err).Debugf(msg)
 			continue
 		}
-		c.checkApiVersionHeaders(reqID, resp)
+		c.checkApiVersionHeaders(req, resp)
 
 		return resp, nil
 	}
 
 	return nil, fmt.Errorf("all hosts failed: %w", multiErr)
 }
 
-func (c *Client) checkApiVersionHeaders(reqID string, resp *http.Response) {
+func (c *Client) checkApiVersionHeaders(req *http.Request, resp *http.Response) {
 	const elasticApiVersionHeaderKey = "Elastic-Api-Version"
 	const warningHeaderKey = "Warning"
 
 	warning := resp.Header.Get(warningHeaderKey)
+	requestID := req.Header.Get(requestIDHeaderName)
 	if warning != "" {
-		c.log.With("http.request.id", reqID).Warnf("warning in fleet response: %q", warning)
+		c.log.With("http.request.id", requestID).Warnf("warning in fleet response: %q", warning)
 	}
 
-	if downgradeVersion := resp.Header.Get(elasticApiVersionHeaderKey); resp.StatusCode == http.StatusBadRequest && downgradeVersion != "" {
+	requestAPIVersion := req.Header.Get(elasticApiVersionHeaderKey)
+	downgradeVersion := resp.Header.Get(elasticApiVersionHeaderKey)
+	if resp.StatusCode == http.StatusBadRequest && downgradeVersion != "" && downgradeVersion != requestAPIVersion {
 		// fleet server requested a downgrade to a different api version, we should bubble up an error until some kind
 		// of fallback mechanism can instantiate the requested version. This is not yet implemented so we log an error
-		c.log.With("http.request.id", reqID).Errorf("fleet requested a different api version %q but this is currently not implemented", downgradeVersion)
+		c.log.With("http.request.id", requestID).Errorf("fleet requested a different api version %q but this is currently not implemented", downgradeVersion)
 	}
 }
 

testing/upgradetest/upgrader.go

@@ -330,7 +330,8 @@ func PerformUpgrade(
 		// we can determine this state by the EOF error coming from the server.
 		// If the server is just unavailable/not running, we should not succeed.
 		// Starting with version 8.13.2, this is handled by the upgrade command itself.
-		isConnectionInterrupted := strings.Contains(err.Error(), "Unavailable") && strings.Contains(err.Error(), "EOF")
+		outputString := string(upgradeOutput)
+		isConnectionInterrupted := strings.Contains(outputString, "Unavailable") && strings.Contains(outputString, "EOF")
 		if !isConnectionInterrupted {
 			return fmt.Errorf("failed to start agent upgrade to version %q: %w\n%s", endVersionInfo.Binary.Version, err, upgradeOutput)
 		}