Fix/remove mandatory name editing (#2120)

* remove mandatory name for editing agent group * remove mandatory name for editing agent * remove mandatory name for editing dataset * fix policies service unit test
orb-community · Dec 26, 2022 · fa9adaf · fa9adaf
1 parent 0854eac
commit fa9adaf
Show file tree

Hide file tree

Showing 13 changed files with 306 additions and 113 deletions.
diff --git a/fleet/agent_group_service.go b/fleet/agent_group_service.go
@@ -116,6 +116,15 @@ func (svc fleetService) EditAgentGroup(ctx context.Context, token string, group
 		return AgentGroup{}, err
 	}
 
+	currentAgentGroup, err := svc.agentGroupRepository.RetrieveByID(ctx, group.ID, ownerID)
+	if err != nil {
+		return AgentGroup{}, err
+	}
+
+	if newName := group.Name.String(); newName == "" {
+		group.Name = currentAgentGroup.Name
+	}
+
 	ag, err := svc.agentGroupRepository.Update(ctx, ownerID, group)
 	if err != nil {
 		return AgentGroup{}, err

diff --git a/fleet/agent_group_service_test.go b/fleet/agent_group_service_test.go
@@ -326,39 +326,58 @@ func TestUpdateAgentGroup(t *testing.T) {
 	}
 	_, err = fleetService.CreateAgent(context.Background(), token, agent)
 
-	matching := types.Metadata{"total": 0, "online": 0}
-	wrongAgentGroup := fleet.AgentGroup{ID: wrongID}
-	readyOnlyAgentGroup := fleet.AgentGroup{ID: wrongID, MatchingAgents: matching}
 	cases := map[string]struct {
-		group fleet.AgentGroup
-		token string
-		err   error
+		group         fleet.AgentGroup
+		expectedGroup fleet.AgentGroup
+		token         string
+		err           error
 	}{
 		"update existing agent": {
-			group: ag,
-			token: token,
-			err:   nil,
+			group:         ag,
+			expectedGroup: ag,
+			token:         token,
+			err:           nil,
 		},
 		"update group with wrong credentials": {
 			group: ag,
 			token: invalidToken,
 			err:   fleet.ErrUnauthorizedAccess,
 		},
 		"update a non-existing group": {
-			group: wrongAgentGroup,
+			group: fleet.AgentGroup{ID: wrongID},
 			token: token,
 			err:   fleet.ErrNotFound,
 		},
 		"update group read only fields": {
-			group: readyOnlyAgentGroup,
+			group: fleet.AgentGroup{
+				ID:             wrongID,
+				MatchingAgents: types.Metadata{"total": 0, "online": 0},
+			},
 			token: token,
 			err:   errors.ErrUpdateEntity,
 		},
+		"update existing agent without name": {
+			group: fleet.AgentGroup{
+				ID:        ag.ID,
+				MFOwnerID: ag.MFOwnerID,
+			},
+			expectedGroup: fleet.AgentGroup{
+				Name:        ag.Name,
+				Tags:        ag.Tags,
+				Description: ag.Description,
+			},
+			token: token,
+			err:   nil,
+		},
 	}
 
 	for desc, tc := range cases {
 		t.Run(desc, func(t *testing.T) {
-			_, err := fleetService.EditAgentGroup(context.Background(), tc.token, tc.group)
+			agentGroupTest, err := fleetService.EditAgentGroup(context.Background(), tc.token, tc.group)
+			if err == nil {
+				assert.Equal(t, tc.expectedGroup.Description, agentGroupTest.Description, fmt.Sprintf("%s: expected %s got %s", desc, tc.expectedGroup.Description, agentGroupTest.Description))
+				assert.Equal(t, tc.expectedGroup.Name, agentGroupTest.Name, fmt.Sprintf("%s: expected %s got %s", desc, tc.expectedGroup.Name, agentGroupTest.Name))
+			}
 			assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %d got %d", desc, tc.err, err))
 		})
 	}

diff --git a/fleet/agent_service.go b/fleet/agent_service.go
@@ -186,6 +186,15 @@ func (svc fleetService) EditAgent(ctx context.Context, token string, agent Agent
 	}
 	agent.MFOwnerID = ownerID
 
+	currentAgent, err := svc.agentRepo.RetrieveByID(ctx, ownerID, agent.MFThingID)
+	if err != nil {
+		return Agent{}, err
+	}
+
+	if newName := agent.Name.String(); newName == "" {
+		agent.Name = currentAgent.Name
+	}
+
 	err = svc.agentRepo.UpdateAgentByID(ctx, ownerID, agent)
 	if err != nil {
 		return Agent{}, err

diff --git a/fleet/agent_service_test.go b/fleet/agent_service_test.go
@@ -257,32 +257,48 @@ func TestUpdateAgent(t *testing.T) {
 		Tags: map[string]string{"test": "true"},
 	})
 
-	wrongAgentGroup := fleet.Agent{MFThingID: wrongID}
 	cases := map[string]struct {
-		group fleet.Agent
-		token string
-		err   error
+		agent         fleet.Agent
+		expectedAgent fleet.Agent
+		token         string
+		err           error
 	}{
 		"update existing agent": {
-			group: ag,
-			token: token,
-			err:   nil,
+			agent:         ag,
+			expectedAgent: ag,
+			token:         token,
+			err:           nil,
 		},
 		"update group with wrong credentials": {
-			group: ag,
+			agent: ag,
 			token: invalidToken,
 			err:   fleet.ErrUnauthorizedAccess,
 		},
 		"update a non-existing group": {
-			group: wrongAgentGroup,
+			agent: fleet.Agent{MFThingID: wrongID},
 			token: token,
 			err:   fleet.ErrNotFound,
 		},
+		"update existing agent without name": {
+			agent: fleet.Agent{
+				MFThingID: ag.MFThingID,
+				MFOwnerID: ag.MFOwnerID,
+			},
+			expectedAgent: fleet.Agent{
+				Name:    ag.Name,
+				OrbTags: ag.OrbTags,
+			},
+			token: token,
+			err:   nil,
+		},
 	}
 
 	for desc, tc := range cases {
 		t.Run(desc, func(t *testing.T) {
-			_, err := fleetService.EditAgent(context.Background(), tc.token, tc.group)
+			agentTest, err := fleetService.EditAgent(context.Background(), tc.token, tc.agent)
+			if err == nil {
+				assert.Equal(t, tc.expectedAgent.Name, agentTest.Name, fmt.Sprintf("%s: expected %s got %s", desc, tc.expectedAgent.Name, agentTest.Name))
+			}
 			assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %d got %d", desc, tc.err, err))
 		})
 	}

diff --git a/fleet/api/http/endpoint.go b/fleet/api/http/endpoint.go
@@ -117,9 +117,12 @@ func editAgentGroupEndpoint(svc fleet.Service) endpoint.Endpoint {
 			return agentGroupRes{}, err
 		}
 
-		validName, err := types.NewIdentifier(req.Name)
-		if err != nil {
-			return agentGroupRes{}, err
+		var validName types.Identifier
+		if req.Name != "" {
+			validName, err = types.NewIdentifier(req.Name)
+			if err != nil {
+				return agentGroupRes{}, errors.Wrap(errors.ErrMalformedEntity, err)
+			}
 		}
 		ag := fleet.AgentGroup{
 			ID:          req.id,
@@ -352,9 +355,12 @@ func editAgentEndpoint(svc fleet.Service) endpoint.Endpoint {
 			return nil, err
 		}
 
-		validName, err := types.NewIdentifier(req.Name)
-		if err != nil {
-			return nil, err
+		var validName types.Identifier
+		if req.Name != "" {
+			validName, err = types.NewIdentifier(req.Name)
+			if err != nil {
+				return nil, errors.Wrap(errors.ErrMalformedEntity, err)
+			}
 		}
 		agent := fleet.Agent{
 			Name:      validName,