From 39994b02db1d4ecd23a2dcbc844991da1dcafd62 Mon Sep 17 00:00:00 2001
From: Michal Pristas <michal.pristas@gmail.com>
Date: Mon, 7 Sep 2020 09:16:16 +0200
Subject: [PATCH] [Ingest Manager] Remove Success from fleet contract (#20449)
 (#20947)

---
 .../cmd/fakewebapi/action_example.json        |  1 -
 .../dev-tools/cmd/fakewebapi/main.go          |  7 ++--
 .../pkg/agent/application/enroll_cmd_test.go  |  4 ---
 .../pkg/agent/application/fleet_acker_test.go |  2 +-
 .../pkg/agent/application/fleet_gateway.go    |  1 -
 .../agent/application/fleet_gateway_test.go   | 13 ++++----
 .../pkg/agent/application/lazy_acker_test.go  |  2 +-
 .../agent/application/managed_mode_test.go    |  1 -
 x-pack/elastic-agent/pkg/fleetapi/ack_cmd.go  |  6 ++--
 .../pkg/fleetapi/ack_cmd_test.go              | 10 ++----
 .../elastic-agent/pkg/fleetapi/checkin_cmd.go |  1 -
 .../pkg/fleetapi/checkin_cmd_test.go          | 32 +++----------------
 .../elastic-agent/pkg/fleetapi/enroll_cmd.go  |  6 ++--
 .../pkg/fleetapi/enroll_cmd_test.go           |  4 +--
 14 files changed, 22 insertions(+), 68 deletions(-)

diff --git a/x-pack/elastic-agent/dev-tools/cmd/fakewebapi/action_example.json b/x-pack/elastic-agent/dev-tools/cmd/fakewebapi/action_example.json
index d76fe58aed25..327b79ed3473 100644
--- a/x-pack/elastic-agent/dev-tools/cmd/fakewebapi/action_example.json
+++ b/x-pack/elastic-agent/dev-tools/cmd/fakewebapi/action_example.json
@@ -1,6 +1,5 @@
 {
   "action": "checkin",
-  "success": true,
   "actions": [
     {
       "type": "CONFIG_CHANGE",
diff --git a/x-pack/elastic-agent/dev-tools/cmd/fakewebapi/main.go b/x-pack/elastic-agent/dev-tools/cmd/fakewebapi/main.go
index 3a40641678a7..11d90d13773e 100644
--- a/x-pack/elastic-agent/dev-tools/cmd/fakewebapi/main.go
+++ b/x-pack/elastic-agent/dev-tools/cmd/fakewebapi/main.go
@@ -27,12 +27,11 @@ var (
 	mutex  sync.Mutex
 
 	pathCheckin     = regexp.MustCompile(`^/api/fleet/agents/(.+)/checkin`)
-	checkinResponse = response{Actions: make([]action, 0), Success: true}
+	checkinResponse = response{Actions: make([]action, 0)}
 )
 
 type response struct {
 	Actions []action `json:"actions"`
-	Success bool     `json:"success"`
 }
 
 type action interface{}
@@ -78,8 +77,7 @@ func handlerEnroll(w http.ResponseWriter, r *http.Request) {
 	}
 
 	response := &fleetapi.EnrollResponse{
-		Action:  "created",
-		Success: true,
+		Action: "created",
 		Item: fleetapi.EnrollItemResponse{
 			ID:                   "a4937110-e53e-11e9-934f-47a8e38a522c",
 			Active:               true,
@@ -147,7 +145,6 @@ func handlerAction(w http.ResponseWriter, r *http.Request) {
 
 	checkinResponse = resp
 	w.WriteHeader(http.StatusCreated)
-	w.Write([]byte(`{ "success": true }`))
 	log.Println("Action request: ", string(c))
 }
 
diff --git a/x-pack/elastic-agent/pkg/agent/application/enroll_cmd_test.go b/x-pack/elastic-agent/pkg/agent/application/enroll_cmd_test.go
index 97b79e8239e3..beab1b253d6f 100644
--- a/x-pack/elastic-agent/pkg/agent/application/enroll_cmd_test.go
+++ b/x-pack/elastic-agent/pkg/agent/application/enroll_cmd_test.go
@@ -54,7 +54,6 @@ func TestEnroll(t *testing.T) {
 				w.Write([]byte(`
 {
     "action": "created",
-    "success": true,
     "item": {
        "id": "a9328860-ec54-11e9-93c4-d72ab8a69391",
         "active": true,
@@ -108,7 +107,6 @@ func TestEnroll(t *testing.T) {
 				w.Write([]byte(`
 {
     "action": "created",
-    "success": true,
     "item": {
        "id": "a9328860-ec54-11e9-93c4-d72ab8a69391",
         "active": true,
@@ -170,7 +168,6 @@ func TestEnroll(t *testing.T) {
 				w.Write([]byte(`
 {
     "action": "created",
-    "success": true,
     "item": {
         "id": "a9328860-ec54-11e9-93c4-d72ab8a69391",
         "active": true,
@@ -231,7 +228,6 @@ func TestEnroll(t *testing.T) {
 				w.Write([]byte(`
 {
     "action": "created",
-    "success": true,
     "item": {
         "id": "a9328860-ec54-11e9-93c4-d72ab8a69391",
         "active": true,
diff --git a/x-pack/elastic-agent/pkg/agent/application/fleet_acker_test.go b/x-pack/elastic-agent/pkg/agent/application/fleet_acker_test.go
index 624824e14ec1..41e42df7376f 100644
--- a/x-pack/elastic-agent/pkg/agent/application/fleet_acker_test.go
+++ b/x-pack/elastic-agent/pkg/agent/application/fleet_acker_test.go
@@ -48,7 +48,7 @@ func TestAcker(t *testing.T) {
 		assert.EqualValues(t, 1, len(cr.Events))
 		assert.EqualValues(t, testID, cr.Events[0].ActionID)
 
-		resp := wrapStrToResp(http.StatusOK, `{ "actions": [], "success": true }`)
+		resp := wrapStrToResp(http.StatusOK, `{ "actions": [] }`)
 		return resp, nil
 	})
 
diff --git a/x-pack/elastic-agent/pkg/agent/application/fleet_gateway.go b/x-pack/elastic-agent/pkg/agent/application/fleet_gateway.go
index cd94380e6739..4bb9d2e6280a 100644
--- a/x-pack/elastic-agent/pkg/agent/application/fleet_gateway.go
+++ b/x-pack/elastic-agent/pkg/agent/application/fleet_gateway.go
@@ -213,7 +213,6 @@ func (f *fleetGateway) execute(ctx context.Context) (*fleetapi.CheckinResponse,
 			f.log.Warnf("retrieved unauthorized for '%d' times. Unrolling.", f.unauthCounter)
 			return &fleetapi.CheckinResponse{
 				Actions: []fleetapi.Action{&fleetapi.ActionUnenroll{ActionID: "", ActionType: "UNENROLL", IsDetected: true}},
-				Success: true,
 			}, nil
 		}
 
diff --git a/x-pack/elastic-agent/pkg/agent/application/fleet_gateway_test.go b/x-pack/elastic-agent/pkg/agent/application/fleet_gateway_test.go
index 61fd509d9956..bd9037416dcf 100644
--- a/x-pack/elastic-agent/pkg/agent/application/fleet_gateway_test.go
+++ b/x-pack/elastic-agent/pkg/agent/application/fleet_gateway_test.go
@@ -179,7 +179,7 @@ func TestFleetGateway(t *testing.T) {
 	) {
 		received := ackSeq(
 			client.Answer(func(headers http.Header, body io.Reader) (*http.Response, error) {
-				resp := wrapStrToResp(http.StatusOK, `{ "actions": [], "success": true }`)
+				resp := wrapStrToResp(http.StatusOK, `{ "actions": [] }`)
 				return resp, nil
 			}),
 			dispatcher.Answer(func(actions ...action) error {
@@ -220,8 +220,7 @@ func TestFleetGateway(t *testing.T) {
 			"type": "ANOTHER_ACTION",
 			"id": "id2"
 		}
-	],
-	"success": true
+	]
 }
 `)
 				return resp, nil
@@ -265,7 +264,7 @@ func TestFleetGateway(t *testing.T) {
 		for {
 			received := ackSeq(
 				client.Answer(func(headers http.Header, body io.Reader) (*http.Response, error) {
-					resp := wrapStrToResp(http.StatusOK, `{ "actions": [], "success": true }`)
+					resp := wrapStrToResp(http.StatusOK, `{ "actions": [] }`)
 					return resp, nil
 				}),
 				dispatcher.Answer(func(actions ...action) error {
@@ -305,7 +304,7 @@ func TestFleetGateway(t *testing.T) {
 
 				require.Equal(t, 1, len(cr.Events))
 
-				resp := wrapStrToResp(http.StatusOK, `{ "actions": [], "success": true }`)
+				resp := wrapStrToResp(http.StatusOK, `{ "actions": [] }`)
 				return resp, nil
 			}),
 			dispatcher.Answer(func(actions ...action) error {
@@ -358,7 +357,7 @@ func TestFleetGateway(t *testing.T) {
 
 		// Make sure that all API calls to the checkin API are successfull, the following will happen:
 		ch2 := client.Answer(func(headers http.Header, body io.Reader) (*http.Response, error) {
-			resp := wrapStrToResp(http.StatusOK, `{ "actions": [], "success": true }`)
+			resp := wrapStrToResp(http.StatusOK, `{ "actions": [] }`)
 			return resp, nil
 		})
 
@@ -424,7 +423,7 @@ func TestRetriesOnFailures(t *testing.T) {
 
 					require.Equal(t, 1, len(cr.Events))
 
-					resp := wrapStrToResp(http.StatusOK, `{ "actions": [], "success": true }`)
+					resp := wrapStrToResp(http.StatusOK, `{ "actions": [] }`)
 					return resp, nil
 				}),
 
diff --git a/x-pack/elastic-agent/pkg/agent/application/lazy_acker_test.go b/x-pack/elastic-agent/pkg/agent/application/lazy_acker_test.go
index aa48e9bd9499..24c708c0d91a 100644
--- a/x-pack/elastic-agent/pkg/agent/application/lazy_acker_test.go
+++ b/x-pack/elastic-agent/pkg/agent/application/lazy_acker_test.go
@@ -64,7 +64,7 @@ func TestLazyAcker(t *testing.T) {
 			assert.EqualValues(t, 1, len(cr.Events))
 		}
 
-		resp := wrapStrToResp(http.StatusOK, `{ "actions": [], "success": true }`)
+		resp := wrapStrToResp(http.StatusOK, `{ "actions": [] }`)
 		return resp, nil
 	})
 
diff --git a/x-pack/elastic-agent/pkg/agent/application/managed_mode_test.go b/x-pack/elastic-agent/pkg/agent/application/managed_mode_test.go
index 2958a51f688b..c14e65c29e7a 100644
--- a/x-pack/elastic-agent/pkg/agent/application/managed_mode_test.go
+++ b/x-pack/elastic-agent/pkg/agent/application/managed_mode_test.go
@@ -92,7 +92,6 @@ func (m *mockStreamStore) Shutdown() {}
 const fleetResponse = `
 {
 	"action": "checkin",
-	"success": true,
 	"actions": [{
 		"agent_id": "17e93530-7f42-11ea-9330-71e968b29fa4",
 		"type": "CONFIG_CHANGE",
diff --git a/x-pack/elastic-agent/pkg/fleetapi/ack_cmd.go b/x-pack/elastic-agent/pkg/fleetapi/ack_cmd.go
index a1b1a2451140..ac568c884d12 100644
--- a/x-pack/elastic-agent/pkg/fleetapi/ack_cmd.go
+++ b/x-pack/elastic-agent/pkg/fleetapi/ack_cmd.go
@@ -45,12 +45,10 @@ func (e *AckRequest) Validate() error {
 // AckResponse is the response send back from the server.
 // 200
 // {
-// 	 "action": "acks",
-// 	 "success": true
+// 	 "action": "acks"
 // }
 type AckResponse struct {
-	Action  string `json:"action"`
-	Success bool   `json:"success"`
+	Action string `json:"action"`
 }
 
 // Validate validates the response send from the server.
diff --git a/x-pack/elastic-agent/pkg/fleetapi/ack_cmd_test.go b/x-pack/elastic-agent/pkg/fleetapi/ack_cmd_test.go
index 945cce72a407..a9e3aebc25b5 100644
--- a/x-pack/elastic-agent/pkg/fleetapi/ack_cmd_test.go
+++ b/x-pack/elastic-agent/pkg/fleetapi/ack_cmd_test.go
@@ -20,12 +20,7 @@ func TestAck(t *testing.T) {
 
 	t.Run("Test ack roundtrip", withServerWithAuthClient(
 		func(t *testing.T) *http.ServeMux {
-			raw := `
-{
-    "action": "ack",
-    "success": true
-}
-`
+			raw := `{"action": "ack"}`
 			mux := http.NewServeMux()
 			path := fmt.Sprintf("/api/ingest_manager/fleet/agents/%s/acks", agentInfo.AgentID())
 			mux.HandleFunc(path, authHandler(func(w http.ResponseWriter, r *http.Request) {
@@ -63,7 +58,7 @@ func TestAck(t *testing.T) {
 
 			request := AckRequest{
 				Events: []AckEvent{
-					AckEvent{
+					{
 						EventType: "ACTION_RESULT",
 						SubType:   "ACKNOWLEDGED",
 						ActionID:  action.ID(),
@@ -73,7 +68,6 @@ func TestAck(t *testing.T) {
 
 			r, err := cmd.Execute(context.Background(), &request)
 			require.NoError(t, err)
-			require.True(t, r.Success)
 			require.Equal(t, "ack", r.Action)
 		},
 	))
diff --git a/x-pack/elastic-agent/pkg/fleetapi/checkin_cmd.go b/x-pack/elastic-agent/pkg/fleetapi/checkin_cmd.go
index 22a09f5e8927..58c80b0adf1a 100644
--- a/x-pack/elastic-agent/pkg/fleetapi/checkin_cmd.go
+++ b/x-pack/elastic-agent/pkg/fleetapi/checkin_cmd.go
@@ -49,7 +49,6 @@ func (e *CheckinRequest) Validate() error {
 // need to be executed or proxy to running processes.
 type CheckinResponse struct {
 	Actions Actions `json:"actions"`
-	Success bool    `json:"success"`
 }
 
 // Validate validates the response send from the server.
diff --git a/x-pack/elastic-agent/pkg/fleetapi/checkin_cmd_test.go b/x-pack/elastic-agent/pkg/fleetapi/checkin_cmd_test.go
index 43501c7fac75..953b86a260e1 100644
--- a/x-pack/elastic-agent/pkg/fleetapi/checkin_cmd_test.go
+++ b/x-pack/elastic-agent/pkg/fleetapi/checkin_cmd_test.go
@@ -79,8 +79,7 @@ func TestCheckin(t *testing.T) {
 					}]
 				}
 			}
-		}],
-		"success": true
+		}]
 	}
 	`
 			mux := http.NewServeMux()
@@ -98,7 +97,6 @@ func TestCheckin(t *testing.T) {
 
 			r, err := cmd.Execute(ctx, &request)
 			require.NoError(t, err)
-			require.True(t, r.Success)
 
 			require.Equal(t, 1, len(r.Actions))
 
@@ -142,8 +140,7 @@ func TestCheckin(t *testing.T) {
 	            "type": "WHAT_TO_DO_WITH_IT",
 	            "id": "id2"
 	        }
-	    ],
-	    "success": true
+	    ]
 	}
 	`
 			mux := http.NewServeMux()
@@ -161,7 +158,6 @@ func TestCheckin(t *testing.T) {
 
 			r, err := cmd.Execute(ctx, &request)
 			require.NoError(t, err)
-			require.True(t, r.Success)
 
 			require.Equal(t, 2, len(r.Actions))
 
@@ -178,12 +174,7 @@ func TestCheckin(t *testing.T) {
 
 	t.Run("When we receive no action", withServerWithAuthClient(
 		func(t *testing.T) *http.ServeMux {
-			raw := `
-	{
-	  "actions": [],
-		"success": true
-	}
-	`
+			raw := `{ "actions": [] }`
 			mux := http.NewServeMux()
 			path := fmt.Sprintf("/api/ingest_manager/fleet/agents/%s/checkin", agentInfo.AgentID())
 			mux.HandleFunc(path, authHandler(func(w http.ResponseWriter, r *http.Request) {
@@ -199,7 +190,6 @@ func TestCheckin(t *testing.T) {
 
 			r, err := cmd.Execute(ctx, &request)
 			require.NoError(t, err)
-			require.True(t, r.Success)
 
 			require.Equal(t, 0, len(r.Actions))
 		},
@@ -207,12 +197,7 @@ func TestCheckin(t *testing.T) {
 
 	t.Run("Meta are sent", withServerWithAuthClient(
 		func(t *testing.T) *http.ServeMux {
-			raw := `
-{
-  "actions": [],
-	"success": true
-}
-`
+			raw := `{"actions": []}`
 			mux := http.NewServeMux()
 			path := fmt.Sprintf("/api/ingest_manager/fleet/agents/%s/checkin", agentInfo.AgentID())
 			mux.HandleFunc(path, authHandler(func(w http.ResponseWriter, r *http.Request) {
@@ -239,7 +224,6 @@ func TestCheckin(t *testing.T) {
 
 			r, err := cmd.Execute(ctx, &request)
 			require.NoError(t, err)
-			require.True(t, r.Success)
 
 			require.Equal(t, 0, len(r.Actions))
 		},
@@ -247,12 +231,7 @@ func TestCheckin(t *testing.T) {
 
 	t.Run("No meta are sent when not provided", withServerWithAuthClient(
 		func(t *testing.T) *http.ServeMux {
-			raw := `
-	{
-	  "actions": [],
-		"success": true
-	}
-	`
+			raw := `{"actions": []}`
 			mux := http.NewServeMux()
 			path := fmt.Sprintf("/api/ingest_manager/fleet/agents/%s/checkin", agentInfo.AgentID())
 			mux.HandleFunc(path, authHandler(func(w http.ResponseWriter, r *http.Request) {
@@ -279,7 +258,6 @@ func TestCheckin(t *testing.T) {
 
 			r, err := cmd.Execute(ctx, &request)
 			require.NoError(t, err)
-			require.True(t, r.Success)
 
 			require.Equal(t, 0, len(r.Actions))
 		},
diff --git a/x-pack/elastic-agent/pkg/fleetapi/enroll_cmd.go b/x-pack/elastic-agent/pkg/fleetapi/enroll_cmd.go
index 55955f3edd56..3e6336fbc81a 100644
--- a/x-pack/elastic-agent/pkg/fleetapi/enroll_cmd.go
+++ b/x-pack/elastic-agent/pkg/fleetapi/enroll_cmd.go
@@ -112,7 +112,6 @@ func (e *EnrollRequest) Validate() error {
 // Example:
 // {
 //   "action": "created",
-//   "success": true,
 //   "item": {
 //     "id": "a4937110-e53e-11e9-934f-47a8e38a522c",
 //     "active": true,
@@ -126,9 +125,8 @@ func (e *EnrollRequest) Validate() error {
 //   }
 // }
 type EnrollResponse struct {
-	Action  string             `json:"action"`
-	Success bool               `json:"success"`
-	Item    EnrollItemResponse `json:"item"`
+	Action string             `json:"action"`
+	Item   EnrollItemResponse `json:"item"`
 }
 
 // EnrollItemResponse item response.
diff --git a/x-pack/elastic-agent/pkg/fleetapi/enroll_cmd_test.go b/x-pack/elastic-agent/pkg/fleetapi/enroll_cmd_test.go
index df341b0110eb..6533ad6643ee 100644
--- a/x-pack/elastic-agent/pkg/fleetapi/enroll_cmd_test.go
+++ b/x-pack/elastic-agent/pkg/fleetapi/enroll_cmd_test.go
@@ -43,8 +43,7 @@ func TestEnroll(t *testing.T) {
 				require.Equal(t, "linux", req.Metadata.Local.OS.Name)
 
 				response := &EnrollResponse{
-					Action:  "created",
-					Success: true,
+					Action: "created",
 					Item: EnrollItemResponse{
 						ID:                   "a4937110-e53e-11e9-934f-47a8e38a522c",
 						Active:               true,
@@ -87,7 +86,6 @@ func TestEnroll(t *testing.T) {
 
 			require.Equal(t, "my-access-api-key", resp.Item.AccessAPIKey)
 			require.Equal(t, "created", resp.Action)
-			require.True(t, resp.Success)
 		},
 	))