Fix auto-removal of integrations when an unrelated setting is saved (#13743)

This commit is contained in:
Martin Angers 2023-09-06 14:31:40 -04:00 committed by GitHub
parent 72f5c5b30c
commit a0c950acf6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 120 additions and 31 deletions

View file

@ -0,0 +1 @@
* Fixed a bug where Jira and/or Zendesk integrations were being removed when an unrelated setting was changed.

View file

@ -615,9 +615,6 @@ type Service interface {
GetMDMAppleFileVaultSummary(ctx context.Context, teamID *uint) (*MDMAppleFileVaultSummary, error)
// GetMDMAppleEnrollmentProfileByToken returns the Apple enrollment from its secret token.
// TODO(mna): this may have to be removed if we don't end up supporting
// manual enrollment via a token (currently we only support it via Fleet
// Desktop, in the My Device page). See #8701.
GetMDMAppleEnrollmentProfileByToken(ctx context.Context, enrollmentToken string, enrollmentRef string) (profile []byte, err error)
// GetDeviceMDMAppleEnrollmentProfile loads the raw (PList-format) enrollment

View file

@ -392,32 +392,42 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle
}
}
delJira, err := fleet.ValidateJiraIntegrations(ctx, storedJiraByProjectKey, newAppConfig.Integrations.Jira)
if err != nil {
if errors.As(err, &fleet.IntegrationTestError{}) {
return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{
Message: err.Error(),
})
// NOTE: the frontend will always send all integrations back when making
// changes, so as soon as Jira or Zendesk has something set, it's fair to
// assume that integrations are being modified and we have the full set of
// those integrations. When deleting, it does send empty arrays (not nulls),
// so this is fine - e.g. when deleting the last integration it sends:
//
// {"integrations":{"zendesk":[],"jira":[]}}
//
if newAppConfig.Integrations.Jira != nil || newAppConfig.Integrations.Zendesk != nil {
delJira, err := fleet.ValidateJiraIntegrations(ctx, storedJiraByProjectKey, newAppConfig.Integrations.Jira)
if err != nil {
if errors.As(err, &fleet.IntegrationTestError{}) {
return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{
Message: err.Error(),
})
}
return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("Jira integration", err.Error()))
}
return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("Jira integration", err.Error()))
}
appConfig.Integrations.Jira = newAppConfig.Integrations.Jira
appConfig.Integrations.Jira = newAppConfig.Integrations.Jira
delZendesk, err := fleet.ValidateZendeskIntegrations(ctx, storedZendeskByGroupID, newAppConfig.Integrations.Zendesk)
if err != nil {
if errors.As(err, &fleet.IntegrationTestError{}) {
return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{
Message: err.Error(),
})
delZendesk, err := fleet.ValidateZendeskIntegrations(ctx, storedZendeskByGroupID, newAppConfig.Integrations.Zendesk)
if err != nil {
if errors.As(err, &fleet.IntegrationTestError{}) {
return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{
Message: err.Error(),
})
}
return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("Zendesk integration", err.Error()))
}
return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("Zendesk integration", err.Error()))
}
appConfig.Integrations.Zendesk = newAppConfig.Integrations.Zendesk
appConfig.Integrations.Zendesk = newAppConfig.Integrations.Zendesk
// if any integration was deleted, remove it from any team that uses it
if len(delJira)+len(delZendesk) > 0 {
if err := svc.ds.DeleteIntegrationsFromTeams(ctx, fleet.Integrations{Jira: delJira, Zendesk: delZendesk}); err != nil {
return nil, ctxerr.Wrap(ctx, err, "delete integrations from teams")
// if any integration was deleted, remove it from any team that uses it
if len(delJira)+len(delZendesk) > 0 {
if err := svc.ds.DeleteIntegrationsFromTeams(ctx, fleet.Integrations{Jira: delJira, Zendesk: delZendesk}); err != nil {
return nil, ctxerr.Wrap(ctx, err, "delete integrations from teams")
}
}
}

View file

@ -3611,6 +3611,16 @@ func (s *integrationTestSuite) TestExternalIntegrationsConfig() {
require.Equal(t, "qux2", config.Integrations.Jira[1].ProjectKey)
require.False(t, config.Integrations.Jira[1].EnableSoftwareVulnerabilities)
// make an unrelated appconfig change, should not remove the integrations
var appCfgResp appConfigResponse
s.DoJSON("PATCH", "/api/v1/fleet/config", json.RawMessage(`{
"org_info": {
"org_name": "test-integrations"
}
}`), http.StatusOK, &appCfgResp)
require.Equal(t, "test-integrations", appCfgResp.OrgInfo.OrgName)
require.Len(t, appCfgResp.Integrations.Jira, 2)
// delete first Jira integration
s.DoRaw("PATCH", "/api/v1/fleet/config", []byte(fmt.Sprintf(`{
"integrations": {
@ -3932,13 +3942,22 @@ func (s *integrationTestSuite) TestExternalIntegrationsConfig() {
}
}`, srvURL)), http.StatusOK)
// remove all integrations
s.DoRaw("PATCH", "/api/v1/fleet/config", []byte(`{
// if no jira nor zendesk integrations are provided, does not remove integrations
appCfgResp = appConfigResponse{}
s.DoJSON("PATCH", "/api/v1/fleet/config", json.RawMessage(`{
"integrations": {}
}`), http.StatusOK, &appCfgResp)
require.Len(t, appCfgResp.Integrations.Jira, 1)
// if explicitly-empty arrays are provided, remove all integrations
appCfgResp = appConfigResponse{}
s.DoJSON("PATCH", "/api/v1/fleet/config", json.RawMessage(`{
"integrations": {
"jira": [],
"zendesk": []
}
}`), http.StatusOK)
}`), http.StatusOK, &appCfgResp)
require.Len(t, appCfgResp.Integrations.Jira, 0)
// set environmental varible to use Zendesk test client
t.Setenv("TEST_ZENDESK_CLIENT", "true")
@ -4004,6 +4023,16 @@ func (s *integrationTestSuite) TestExternalIntegrationsConfig() {
require.Equal(t, int64(123), config.Integrations.Zendesk[1].GroupID)
require.False(t, config.Integrations.Zendesk[1].EnableSoftwareVulnerabilities)
// make an unrelated appconfig change, should not remove the integrations
appCfgResp = appConfigResponse{}
s.DoJSON("PATCH", "/api/v1/fleet/config", json.RawMessage(`{
"org_info": {
"org_name": "test-integrations-zendesk"
}
}`), http.StatusOK, &appCfgResp)
require.Equal(t, "test-integrations-zendesk", appCfgResp.OrgInfo.OrgName)
require.Len(t, appCfgResp.Integrations.Zendesk, 2)
// delete first Zendesk integration
s.DoRaw("PATCH", "/api/v1/fleet/config", []byte(fmt.Sprintf(`{
"integrations": {
@ -4398,6 +4427,14 @@ func (s *integrationTestSuite) TestExternalIntegrationsConfig() {
}
}`, srvURL)), http.StatusUnprocessableEntity)
// if no jira nor zendesk integrations are provided, does not remove integrations
appCfgResp = appConfigResponse{}
s.DoJSON("PATCH", "/api/v1/fleet/config", json.RawMessage(`{
"integrations": {}
}`), http.StatusOK, &appCfgResp)
require.Len(t, appCfgResp.Integrations.Jira, 1)
require.Len(t, appCfgResp.Integrations.Zendesk, 1)
// remove all integrations on exit, so that other tests can enable the
// webhook as needed
s.DoRaw("PATCH", "/api/v1/fleet/config", []byte(`{

View file

@ -1186,6 +1186,7 @@ func (s *integrationEnterpriseTestSuite) TestExternalIntegrationsTeamConfig() {
require.Len(t, getResp.Team.Config.Integrations.Zendesk, 0)
// disable the webhook and enable the automation
tmResp = teamResponse{}
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d", team.ID), fleet.TeamPayload{
Integrations: &fleet.TeamIntegrations{
Jira: []*fleet.TeamJiraIntegration{
@ -1206,6 +1207,24 @@ func (s *integrationEnterpriseTestSuite) TestExternalIntegrationsTeamConfig() {
require.Len(t, tmResp.Team.Config.Integrations.Jira, 1)
require.Equal(t, "qux", tmResp.Team.Config.Integrations.Jira[0].ProjectKey)
// update the team with an unrelated field, should not change integrations
tmResp = teamResponse{}
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d", team.ID), fleet.TeamPayload{
Description: ptr.String("team-desc"),
}, http.StatusOK, &tmResp)
require.Len(t, tmResp.Team.Config.Integrations.Jira, 1)
require.Equal(t, "team-desc", tmResp.Team.Description)
// make an unrelated appconfig change, should not remove the global integrations nor the teams'
var appCfgResp appConfigResponse
s.DoJSON("PATCH", "/api/v1/fleet/config", json.RawMessage(`{
"org_info": {
"org_name": "test-integrations"
}
}`), http.StatusOK, &appCfgResp)
require.Equal(t, "test-integrations", appCfgResp.OrgInfo.OrgName)
require.Len(t, appCfgResp.Integrations.Jira, 2)
// enable the webhook without changing the integration should fail (an integration is already enabled)
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d", team.ID), fleet.TeamPayload{WebhookSettings: &fleet.TeamWebhookSettings{
FailingPoliciesWebhook: fleet.FailingPoliciesWebhookSettings{
@ -1401,6 +1420,25 @@ func (s *integrationEnterpriseTestSuite) TestExternalIntegrationsTeamConfig() {
require.Equal(t, int64(122), tmResp.Team.Config.Integrations.Zendesk[0].GroupID)
require.Equal(t, int64(123), tmResp.Team.Config.Integrations.Zendesk[1].GroupID)
// update the team with an unrelated field, should not change integrations
tmResp = teamResponse{}
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d", team.ID), fleet.TeamPayload{
Description: ptr.String("team-desc-2"),
}, http.StatusOK, &tmResp)
require.Len(t, tmResp.Team.Config.Integrations.Zendesk, 2)
require.Equal(t, "team-desc-2", tmResp.Team.Description)
// make an unrelated appconfig change, should not remove the global integrations nor the teams'
appCfgResp = appConfigResponse{}
s.DoJSON("PATCH", "/api/v1/fleet/config", json.RawMessage(`{
"org_info": {
"org_name": "test-integrations-2"
}
}`), http.StatusOK, &appCfgResp)
require.Equal(t, "test-integrations-2", appCfgResp.OrgInfo.OrgName)
require.Len(t, appCfgResp.Integrations.Zendesk, 2)
require.Len(t, appCfgResp.Integrations.Jira, 2)
// enabling the second without disabling the first fails
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d", team.ID), fleet.TeamPayload{
Integrations: &fleet.TeamIntegrations{
@ -1629,9 +1667,15 @@ func (s *integrationEnterpriseTestSuite) TestExternalIntegrationsTeamConfig() {
require.False(t, tmResp.Team.Config.WebhookSettings.FailingPoliciesWebhook.Enable)
require.Empty(t, tmResp.Team.Config.WebhookSettings.FailingPoliciesWebhook.DestinationURL)
s.DoRaw("PATCH", "/api/v1/fleet/config", []byte(`{
"integrations": {}
}`), http.StatusOK)
appCfgResp = appConfigResponse{}
s.DoJSON("PATCH", "/api/v1/fleet/config", json.RawMessage(`{
"integrations": {
"jira": [],
"zendesk": []
}
}`), http.StatusOK, &appCfgResp)
require.Len(t, appCfgResp.Integrations.Jira, 0)
require.Len(t, appCfgResp.Integrations.Zendesk, 0)
}
func (s *integrationEnterpriseTestSuite) TestMacOSUpdatesConfig() {