Skip to content

Commit

Permalink
[Ingest Manager] Remove Success from fleet contract (elastic#20449)
Browse files Browse the repository at this point in the history
  • Loading branch information
michalpristas authored Sep 2, 2020
1 parent 1757cec commit a2c7258
Show file tree
Hide file tree
Showing 14 changed files with 22 additions and 68 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{
"action": "checkin",
"success": true,
"actions": [
{
"type": "CONFIG_CHANGE",
Expand Down
7 changes: 2 additions & 5 deletions x-pack/elastic-agent/dev-tools/cmd/fakewebapi/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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))
}

Expand Down
4 changes: 0 additions & 4 deletions x-pack/elastic-agent/pkg/agent/application/enroll_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
13 changes: 6 additions & 7 deletions x-pack/elastic-agent/pkg/agent/application/fleet_gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -220,8 +220,7 @@ func TestFleetGateway(t *testing.T) {
"type": "ANOTHER_ACTION",
"id": "id2"
}
],
"success": true
]
}
`)
return resp, nil
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
})

Expand Down Expand Up @@ -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
}),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 2 additions & 4 deletions x-pack/elastic-agent/pkg/fleetapi/ack_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 2 additions & 8 deletions x-pack/elastic-agent/pkg/fleetapi/ack_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -63,7 +58,7 @@ func TestAck(t *testing.T) {

request := AckRequest{
Events: []AckEvent{
AckEvent{
{
EventType: "ACTION_RESULT",
SubType: "ACKNOWLEDGED",
ActionID: action.ID(),
Expand All @@ -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)
},
))
Expand Down
1 change: 0 additions & 1 deletion x-pack/elastic-agent/pkg/fleetapi/checkin_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
32 changes: 5 additions & 27 deletions x-pack/elastic-agent/pkg/fleetapi/checkin_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ func TestCheckin(t *testing.T) {
}]
}
}
}],
"success": true
}]
}
`
mux := http.NewServeMux()
Expand All @@ -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))

Expand Down Expand Up @@ -142,8 +140,7 @@ func TestCheckin(t *testing.T) {
"type": "WHAT_TO_DO_WITH_IT",
"id": "id2"
}
],
"success": true
]
}
`
mux := http.NewServeMux()
Expand All @@ -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))

Expand All @@ -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) {
Expand All @@ -199,20 +190,14 @@ 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))
},
))

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) {
Expand All @@ -239,20 +224,14 @@ 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))
},
))

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) {
Expand All @@ -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))
},
Expand Down
6 changes: 2 additions & 4 deletions x-pack/elastic-agent/pkg/fleetapi/enroll_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ func (e *EnrollRequest) Validate() error {
// Example:
// {
// "action": "created",
// "success": true,
// "item": {
// "id": "a4937110-e53e-11e9-934f-47a8e38a522c",
// "active": true,
Expand All @@ -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.
Expand Down
4 changes: 1 addition & 3 deletions x-pack/elastic-agent/pkg/fleetapi/enroll_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
},
))

Expand Down

0 comments on commit a2c7258

Please sign in to comment.