github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-06-28 11:05:39 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Fleet] Create and update package policy API return 409 conflict when names are not unique (#153533)

Browse source 
Closes https://github.com/elastic/kibana/issues/153026

## Summary

The agent policy API returns a 409 in the case the policy already
exists. The package policy API will now follow the same behavior.

### Checklist

Delete any items that are not applicable to this PR.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
This commit is contained in:
Cristina Amico 2023-03-23 14:30:49 +01:00 • committed by GitHub
parent 646221d1ac
commit d75df24f96
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 52 additions and 30 deletions
Download patch file Download diff file Expand all files Collapse all files

6
x-pack/plugins/fleet/common/openapi/bundled.json
View file

@ -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"
}
}
}

4
x-pack/plugins/fleet/common/openapi/bundled.yaml
View file

@ -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

2
x-pack/plugins/fleet/common/openapi/paths/package_policies.yaml
View file

@ -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:

2
x-pack/plugins/fleet/common/openapi/paths/package_policies@upgrade.yaml
View file

@ -36,3 +36,5 @@ post:
- success
'400':
$ref: ../components/responses/error.yaml
'409':
$ref: ../components/responses/error.yaml

4
x-pack/plugins/fleet/server/errors/handlers.ts
View file

@ -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
};

1
x-pack/plugins/fleet/server/errors/index.ts
View file

@ -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 {

47
x-pack/plugins/fleet/server/services/package_policy.ts
View file

@ -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<PackagePolicy['elasticsearch']>['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.`
);
}
}

8
x-pack/test/fleet_api_integration/apis/package_policy/create.ts
View file

@ -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 () {

8
x-pack/test/fleet_api_integration/apis/package_policy/update.ts
View file

@ -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 () {