[Fleet] Update policy fails on duplicate check

[dgieselaar]

I ran into the following error when trying to update a policy:

FleetError: An integration policy with the name apm-1 already exists. Please rename it or choose a different name.

After listing all the policies it turned out that I indeed had two policies with the same name. This is something that should not be possible according to @criamico, but apparently there's a loophole where two policies can be created or updated to have the same name. I'm not sure how to be honest. I was trying to update an existing policy.

[elasticmachine]

Pinging @elastic/fleet (Team:Fleet)

[criamico]

Adding some notes from the slack conversation here plus some investigation:

The name uniqueness for package policies was introduced with #115212 (related ticket is here).

This specific error seems to stem from this code packagePolicyService.update

kibana/x-pack/plugins/fleet/server/services/package_policy.ts

Lines 508 to 520 in 4baa14a

	if (!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.`
	);
	}
	}

There is a similar check in packagePolicyService.create:

kibana/x-pack/plugins/fleet/server/services/package_policy.ts

Lines 151 to 163 in 4baa14a

	if (!options?.skipUniqueNameVerification) {
	const existingPoliciesWithName = await this.list(soClient, {
	perPage: 1,
	kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.name: "${packagePolicy.name}"`,
	});
	// Check that the name does not exist already
	if (existingPoliciesWithName.items.length > 0) {
	throw new FleetError(
	`An integration policy with the name ${packagePolicy.name} already exists. Please rename it or choose a different name.`
	);
	}
	}

What I noticed is that packagePolicyService.create is ever called only in two places in the code and only in one of them with option skipUniqueNameVerification: true

kibana/x-pack/plugins/fleet/server/services/agent_policy.ts

Lines 1091 to 1099 in 4baa14a

	await packagePolicyService.create(soClient, esClient, newPackagePolicy, {
	id,
	bumpRevision: bumpAgentPolicyRevison,
	skipEnsureInstalled: true,
	skipUniqueNameVerification: true,
	overwrite: true,
	force: true, // To add package to managed policy we need the force flag
	packageInfo,
	});

which in turn is called in service/preconfiguration.

The flag skipUniqueNameVerification was introduced in that place with this PR to handle preconfigured policies.

@dgieselaar by chance do you have any preconfigured policy in your setup? I think that this is the only way how we can get into this state, unless I'm missing something.

[dgieselaar]

@criamico yes. Can you maybe elaborate on what the consequences are for the user? cc @sqren

[criamico]

Well I think that the issue here can be that the user that creates preconfigured policies might get into this state were there are two policies with the same name. I'm not sure what other problems can stem from this bug, other than being unable to update both the policies as you experienced.

@nchaulet could you help on this?

[nchaulet]

Well I think that the issue here can be that the user that creates preconfigured policies might get into this state were there are two policies with the same name. I'm not sure what other problems can stem from this bug, other than being unable to update both the policies as you experienced.

I do not think there is another issue than being able to update both policies, and I think update updating one of the policy name here is probably the best way to fix that

[sorenlouv]

@nchaulet @criamico What's concerning me, is that Dario ran into this when APM calls fleet to update a policy here:

kibana/x-pack/plugins/apm/server/routes/fleet/api_keys/add_api_keys_to_policies_if_missing.ts

Lines 100 to 105 in c2e5381

	const newPolicy = await fleet.packagePolicyService.update(
	savedObjectsClient,
	internalESClient,
	policy.id,
	policyWithApiKeys
	);

Can you explain why the error would happen in the above flow?

[nchaulet]

@nchaulet @criamico What's concerning me, is that Dario ran into this when APM calls fleet to update a policy here:
Can you explain why the error would happen in the above flow?

We perform name validation in the update method it's possible to skip the verification by adding the { skipUniqueNameVerification: true }, as you do not edit the name here (maybe we could always skip the verification if name does not change, started a draft PR here #149944).

Not sure how we got with 2 package policies with the same name in the first place but I think it's possible that happen when:

• a user create a policy from the UI/API
• Than he edit the kibana config to add a preconfigured package policy (we will skip package verification here, as preconfigured package policy are mandatory for some cloud use case fleet server & APM and it's better to have some duplicate that could be user resolved later than breaking Fleet Server or APM in cloud)

Does it make sense to you?

[sorenlouv]

Does it make sense to you?

I'm not sure why it happened in Dario's case. It's not my impression that he was editing a policy. Instead I think this just happened due to the background job I mentioned above. I am concerned whether the change in #149944 could cause an extra APM policy to be created every time Kibana restarts.

I don't have any suggestions for how to fix it though. Will keep an eye on this and ping you if someone run into this again.

[nchaulet]

Should be resolved by #149944