diff --git a/x-pack/plugins/fleet/common/openapi/bundled.json b/x-pack/plugins/fleet/common/openapi/bundled.json index 9b9e9a64f2ac..90e0459cb2e5 100644 --- a/x-pack/plugins/fleet/common/openapi/bundled.json +++ b/x-pack/plugins/fleet/common/openapi/bundled.json @@ -3858,6 +3858,9 @@ }, "400": { "$ref": "#/components/responses/error" + }, + "409": { + "$ref": "#/components/responses/error" } }, "requestBody": { @@ -4066,6 +4069,9 @@ }, "400": { "$ref": "#/components/responses/error" + }, + "409": { + "$ref": "#/components/responses/error" } } } diff --git a/x-pack/plugins/fleet/common/openapi/bundled.yaml b/x-pack/plugins/fleet/common/openapi/bundled.yaml index 8f5436703f6f..e50525db886d 100644 --- a/x-pack/plugins/fleet/common/openapi/bundled.yaml +++ b/x-pack/plugins/fleet/common/openapi/bundled.yaml @@ -2406,6 +2406,8 @@ paths: - item '400': $ref: '#/components/responses/error' + '409': + $ref: '#/components/responses/error' requestBody: description: >- You should use inputs as an object and not use the deprecated inputs @@ -2537,6 +2539,8 @@ paths: - success '400': $ref: '#/components/responses/error' + '409': + $ref: '#/components/responses/error' /package_policies/upgrade/dryrun: post: summary: Dry run package policy upgrade diff --git a/x-pack/plugins/fleet/common/openapi/paths/package_policies.yaml b/x-pack/plugins/fleet/common/openapi/paths/package_policies.yaml index 0fe987e1727d..89a54608d231 100644 --- a/x-pack/plugins/fleet/common/openapi/paths/package_policies.yaml +++ b/x-pack/plugins/fleet/common/openapi/paths/package_policies.yaml @@ -47,6 +47,8 @@ post: - item '400': $ref: ../components/responses/error.yaml + '409': + $ref: ../components/responses/error.yaml requestBody: description: You should use inputs as an object and not use the deprecated inputs array. content: diff --git a/x-pack/plugins/fleet/common/openapi/paths/package_policies@upgrade.yaml b/x-pack/plugins/fleet/common/openapi/paths/package_policies@upgrade.yaml index 2b6e69d49c44..1837675a15f2 100644 --- a/x-pack/plugins/fleet/common/openapi/paths/package_policies@upgrade.yaml +++ b/x-pack/plugins/fleet/common/openapi/paths/package_policies@upgrade.yaml @@ -36,3 +36,5 @@ post: - success '400': $ref: ../components/responses/error.yaml + '409': + $ref: ../components/responses/error.yaml diff --git a/x-pack/plugins/fleet/server/errors/handlers.ts b/x-pack/plugins/fleet/server/errors/handlers.ts index 89811ca2bc7a..6a86f9b9f2c6 100644 --- a/x-pack/plugins/fleet/server/errors/handlers.ts +++ b/x-pack/plugins/fleet/server/errors/handlers.ts @@ -31,6 +31,7 @@ import { PackageFailedVerificationError, PackagePolicyNotFoundError, FleetUnauthorizedError, + PackagePolicyNameExistsError, } from '.'; type IngestErrorHandler = ( @@ -78,6 +79,9 @@ const getHTTPResponseCode = (error: FleetError): number => { if (error instanceof FleetUnauthorizedError) { return 403; // Unauthorized } + if (error instanceof PackagePolicyNameExistsError) { + return 409; // Conflict + } return 400; // Bad Request }; diff --git a/x-pack/plugins/fleet/server/errors/index.ts b/x-pack/plugins/fleet/server/errors/index.ts index 4856ff5294e4..84cf87d799f4 100644 --- a/x-pack/plugins/fleet/server/errors/index.ts +++ b/x-pack/plugins/fleet/server/errors/index.ts @@ -50,6 +50,7 @@ export class ConcurrentInstallOperationError extends FleetError {} export class AgentReassignmentError extends FleetError {} export class PackagePolicyIneligibleForUpgradeError extends FleetError {} export class PackagePolicyValidationError extends FleetError {} +export class PackagePolicyNameExistsError extends FleetError {} export class PackagePolicyNotFoundError extends FleetError {} export class BundledPackageNotFoundError extends FleetError {} export class HostedAgentPolicyRestrictionRelatedError extends FleetError { diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts index 8561b14dd9de..fe044f012dd1 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.ts @@ -71,6 +71,7 @@ import { PackagePolicyNotFoundError, HostedAgentPolicyRestrictionRelatedError, FleetUnauthorizedError, + PackagePolicyNameExistsError, } from '../errors'; import { NewPackagePolicySchema, PackagePolicySchema, UpdatePackagePolicySchema } from '../types'; import type { @@ -166,17 +167,7 @@ class PackagePolicyClientImpl implements PackagePolicyClient { // trailing whitespace causes issues creating API keys enrichedPackagePolicy.name = enrichedPackagePolicy.name.trim(); if (!options?.skipUniqueNameVerification) { - const existingPoliciesWithName = await this.list(soClient, { - perPage: 1, - kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.name: "${enrichedPackagePolicy.name}"`, - }); - - // Check that the name does not exist already - if (existingPoliciesWithName.items.length > 0) { - throw new FleetError( - `An integration policy with the name ${enrichedPackagePolicy.name} already exists. Please rename it or choose a different name.` - ); - } + await requireUniqueName(soClient, enrichedPackagePolicy); } let elasticsearchPrivileges: NonNullable['privileges']; @@ -548,17 +539,7 @@ class PackagePolicyClientImpl implements PackagePolicyClient { packagePolicy.name !== oldPackagePolicy.name && !options?.skipUniqueNameVerification ) { - // Check that the name does not exist already but exclude the current package policy - const existingPoliciesWithName = await this.list(soClient, { - perPage: SO_SEARCH_LIMIT, - kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.name:"${packagePolicy.name}"`, - }); - const filtered = (existingPoliciesWithName?.items || []).filter((p) => p.id !== id); - if (filtered.length > 0) { - throw new FleetError( - `An integration policy with the name ${packagePolicy.name} already exists. Please rename it or choose a different name.` - ); - } + await requireUniqueName(soClient, enrichedPackagePolicy, id); } let inputs = restOfPackagePolicy.inputs.map((input) => @@ -2251,3 +2232,25 @@ function deepMergeVars(original: any, override: any, keepOriginalValue = false): return result; } + +async function requireUniqueName( + soClient: SavedObjectsClientContract, + packagePolicy: UpdatePackagePolicy | NewPackagePolicy, + id?: string +) { + const existingPoliciesWithName = await packagePolicyService.list(soClient, { + perPage: SO_SEARCH_LIMIT, + kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.name:"${packagePolicy.name}"`, + }); + + const policiesToCheck = id + ? // Check that the name does not exist already but exclude the current package policy + (existingPoliciesWithName?.items || []).filter((p) => p.id !== id) + : existingPoliciesWithName?.items; + + if (policiesToCheck.length > 0) { + throw new PackagePolicyNameExistsError( + `An integration policy with the name ${packagePolicy.name} already exists. Please rename it or choose a different name.` + ); + } +} diff --git a/x-pack/test/fleet_api_integration/apis/package_policy/create.ts b/x-pack/test/fleet_api_integration/apis/package_policy/create.ts index 7def6a1801bf..82777e41eb5e 100644 --- a/x-pack/test/fleet_api_integration/apis/package_policy/create.ts +++ b/x-pack/test/fleet_api_integration/apis/package_policy/create.ts @@ -245,7 +245,7 @@ export default function (providerContext: FtrProviderContext) { .expect(400); }); - it('should return a 400 if there is another package policy with the same name', async function () { + it('should return a 409 if there is another package policy with the same name', async function () { await supertest .post(`/api/fleet/package_policies`) .set('kbn-xsrf', 'xxxx') @@ -279,10 +279,10 @@ export default function (providerContext: FtrProviderContext) { version: '0.1.0', }, }) - .expect(400); + .expect(409); }); - it('should return a 400 if there is a package policy with the same name on a different policy', async function () { + it('should return a 409 if there is a package policy with the same name on a different policy', async function () { const { body: agentPolicyResponse } = await supertest .post(`/api/fleet/agent_policies`) .set('kbn-xsrf', 'xxxx') @@ -325,7 +325,7 @@ export default function (providerContext: FtrProviderContext) { version: '0.1.0', }, }) - .expect(400); + .expect(409); }); it('should return a 400 with required variables not provided', async function () { diff --git a/x-pack/test/fleet_api_integration/apis/package_policy/update.ts b/x-pack/test/fleet_api_integration/apis/package_policy/update.ts index 2206527667a4..2cc2302a42b6 100644 --- a/x-pack/test/fleet_api_integration/apis/package_policy/update.ts +++ b/x-pack/test/fleet_api_integration/apis/package_policy/update.ts @@ -368,7 +368,7 @@ export default function (providerContext: FtrProviderContext) { }); }); - it('should return a 400 if there is another package policy with the same name', async function () { + it('should return a 409 if there is another package policy with the same name', async function () { await supertest .put(`/api/fleet/package_policies/${packagePolicyId2}`) .set('kbn-xsrf', 'xxxx') @@ -385,10 +385,10 @@ export default function (providerContext: FtrProviderContext) { version: '0.1.0', }, }) - .expect(400); + .expect(409); }); - it('should return a 400 if there is another package policy with the same name on a different policy', async function () { + it('should return a 409 if there is another package policy with the same name on a different policy', async function () { const { body: agentPolicyResponse } = await supertest .post(`/api/fleet/agent_policies`) .set('kbn-xsrf', 'xxxx') @@ -414,7 +414,7 @@ export default function (providerContext: FtrProviderContext) { version: '0.1.0', }, }) - .expect(400); + .expect(409); }); it('should work with frozen input vars', async function () {