Skip to content

Commit

Permalink
[Fleet] Don't throw concurrent installation error when encountering c…
Browse files Browse the repository at this point in the history
…onflicts (elastic#173190)

## Summary

Closes elastic#171986

## How to test

You'll need to get into a state where a tag with a conflicting ID exists
in a second Kibana space outside of the default space.

It seems like the tags API doesn't support providing an ID at least from
the REST API, so that makes creating duplicate tags in other spaces a
bit challenging.

e.g. 

```
POST kbn:api/saved_objects_tagging/tags/create
{
  "id": "my-tag",
  "name": "My tag",
  "description": "",
  "color": "#FFFFFF"
}
```

results in 

```
{
  "statusCode": 400,
  "error": "Bad Request",
  "message": "[request body.id]: definition for this key is missing"
}
```

So, in order to enable creating tags with ID's easily, I made some quick
code changes to the tags service

```diff
diff --git a/x-pack/plugins/saved_objects_tagging/server/routes/tags/create_tag.ts b/x-pack/plugins/saved_objects_tagging/server/routes/tags/create_tag.ts
index 0c48168eed2..fa0e7fbe7b9 100644
--- a/x-pack/plugins/saved_objects_tagging/server/routes/tags/create_tag.ts
+++ b/x-pack/plugins/saved_objects_tagging/server/routes/tags/create_tag.ts
@@ -6,6 +6,7 @@
  */
 
 import { schema } from '@kbn/config-schema';
+import { omit } from 'lodash';
 import type { TagsPluginRouter } from '../../types';
 import { TagValidationError } from '../../services/tags';
 
@@ -15,6 +16,7 @@ export const registerCreateTagRoute = (router: TagsPluginRouter) => {
       path: '/api/saved_objects_tagging/tags/create',
       validate: {
         body: schema.object({
+          id: schema.maybe(schema.string()),
           name: schema.string(),
           description: schema.string(),
           color: schema.string(),
@@ -32,7 +34,7 @@ export const registerCreateTagRoute = (router: TagsPluginRouter) => {
           });
         }
 
-        const tag = await tagsClient.create(req.body);
+        const tag = await tagsClient.create(omit(req.body, 'id'), { id: req.body.id });
         return res.ok({
           body: {
             tag,

```

With those changes in place (I don't think committing them is
necessary), I was able to create a tag in my second space e.g.

```
POST kbn:api/saved_objects_tagging/tags/create
{
  "id": "fleet-pkg-nginx-default",
  "name": "Nginx duplicate tag",
  "description": "",
  "color": "#DADADA"
}
```

Then, from my default space I installed the nginx package

```
POST kbn:/api/fleet/epm/packages/nginx
```

This throws the concurrent install error as expected on `main`

```
{
  "statusCode": 409,
  "error": "Conflict",
  "message": "Concurrent installation or upgrade of nginx-1.17.0 detected, aborting. Original error: Saved object [tag/fleet-pkg-nginx-default] conflict"
}
```

With this PR, we get this error instead

```
{
  "statusCode": 400,
  "error": "Bad Request",
  "message": "Saved Object conflict encountered while installing nginx-1.17.0. There may be a conflicting Saved Object saved to another Space. Original error: Saved object [tag/fleet-pkg-nginx-default] conflict"
}
```

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
2 people authored and delanni committed Jan 11, 2024
1 parent f959abb commit 71be787
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 6 deletions.
4 changes: 4 additions & 0 deletions x-pack/plugins/fleet/server/errors/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
AgentRequestInvalidError,
PackagePolicyRequestError,
FleetNotFoundError,
PackageSavedObjectConflictError,
} from '.';

type IngestErrorHandler = (
Expand Down Expand Up @@ -100,6 +101,9 @@ const getHTTPResponseCode = (error: FleetError): number => {
if (error instanceof ConcurrentInstallOperationError) {
return 409;
}
if (error instanceof PackageSavedObjectConflictError) {
return 409;
}
if (error instanceof PackagePolicyNameExistsError) {
return 409;
}
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/server/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export class PackageInvalidArchiveError extends FleetError {}
export class PackageRemovalError extends FleetError {}
export class PackageESError extends FleetError {}
export class ConcurrentInstallOperationError extends FleetError {}
export class PackageSavedObjectConflictError extends FleetError {}

export class KibanaSOReferenceError extends FleetError {}
export class PackageAlreadyInstalledError extends FleetError {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { savedObjectsClientMock, elasticsearchServiceMock } from '@kbn/core/serv
import { loggerMock } from '@kbn/logging-mocks';
import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common/constants';

import { ConcurrentInstallOperationError } from '../../../errors';
import { ConcurrentInstallOperationError, PackageSavedObjectConflictError } from '../../../errors';

import type { Installation } from '../../../../common';

Expand Down Expand Up @@ -254,7 +254,6 @@ describe('_installPackage', () => {
});

describe('when package is stuck in `installing`', () => {
afterEach(() => {});
const mockInstalledPackageSo: SavedObject<Installation> = {
id: 'mocked-package',
attributes: {
Expand Down Expand Up @@ -387,4 +386,56 @@ describe('_installPackage', () => {
});
});
});

it('surfaces saved object conflicts error', () => {
appContextService.start(
createAppContextStartContractMock({
internal: {
disableILMPolicies: false,
disableProxies: false,
fleetServerStandalone: false,
onlyAllowAgentUpgradeToKnownVersions: false,
retrySetupOnBoot: false,
registry: {
kibanaVersionCheckEnabled: true,
capabilities: [],
excludePackages: [],
},
},
})
);

mockedInstallKibanaAssetsAndReferences.mockRejectedValueOnce(
new PackageSavedObjectConflictError('test')
);

expect(
_installPackage({
savedObjectsClient: soClient,
// @ts-ignore
savedObjectsImporter: jest.fn(),
esClient,
logger: loggerMock.create(),
packageInstallContext: {
packageInfo: {
title: 'title',
name: 'xyz',
version: '4.5.6',
description: 'test',
type: 'integration',
categories: ['cloud', 'custom'],
format_version: 'string',
release: 'experimental',
conditions: { kibana: { version: 'x.y.z' } },
owner: { github: 'elastic/fleet' },
} as any,
assetsMap: new Map(),
paths: [],
},
installType: 'install',
installSource: 'registry',
spaceId: DEFAULT_SPACE_ID,
})
).rejects.toThrowError(PackageSavedObjectConflictError);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import { installTransforms } from '../elasticsearch/transform/install';
import { installMlModel } from '../elasticsearch/ml_model';
import { installIlmForDataStream } from '../elasticsearch/datastream_ilm/install';
import { saveArchiveEntriesFromAssetsMap } from '../archive/storage';
import { ConcurrentInstallOperationError } from '../../../errors';
import { ConcurrentInstallOperationError, PackageSavedObjectConflictError } from '../../../errors';
import { appContextService, packagePolicyService } from '../..';

import { auditLoggingService } from '../../audit_logging';
Expand Down Expand Up @@ -387,10 +387,12 @@ export async function _installPackage({
return [...installedKibanaAssetsRefs, ...esReferences];
} catch (err) {
if (SavedObjectsErrorHelpers.isConflictError(err)) {
throw new ConcurrentInstallOperationError(
`Concurrent installation or upgrade of ${pkgName || 'unknown'}-${
throw new PackageSavedObjectConflictError(
`Saved Object conflict encountered while installing ${pkgName || 'unknown'}-${
pkgVersion || 'unknown'
} detected, aborting. Original error: ${err.message}`
}. There may be a conflicting Saved Object saved to another Space. Original error: ${
err.message
}`
);
} else {
throw err;
Expand Down

0 comments on commit 71be787

Please sign in to comment.