Check if the Fleet API version is different between request and response to detect an api version switch request (#4481)

* Check if the Fleet API version is different between request and response to detect an API version switch request

Author: pchila

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)
 	}
 }