[Fleet] Don't auto upgrade policies for AUTO_UPDATE packages

[kpollich]

Summary

Ref #114914 (review)

We had an error in our logic for automatic package policies update via the keep_policies_up_to_date flag that incorrectly upgraded policies for any package flagged as AUTO_UPDATE in

kibana/x-pack/plugins/fleet/common/constants/epm.ts

Lines 42 to 46 in 50e4ae7

	export const autoUpdatePackages = [
	FLEET_ENDPOINT_PACKAGE,
	FLEET_APM_PACKAGE,
	FLEET_SYNTHETICS_PACKAGE,
	];

It feels like this should have been caught by our unit tests, but I'm not sure whether we should try to mock this AUTO_UPDATE_PACKAGES list, or use a "hardcoded" package name in our tests. Open to suggestions here, as I would like to make sure this case is covered by automated tests.

[elasticmachine]

Pinging @elastic/fleet (Team:Fleet)

[nchaulet]

🚀 How complicated it will be to add a unit test for that? (it could be a great way to document what we want to be auto upgraded, ...)

[kpollich]

We do have a test now that I think has had its robustness improved by this PR:

kibana/x-pack/plugins/fleet/server/services/managed_package_policies.test.ts

Lines 35 to 80 in cba8333

	it('should not upgrade policies for non-managed package', async () => {
	const esClient = elasticsearchServiceMock.createClusterClient().asInternalUser;
	const soClient = savedObjectsClientMock.create();
	(packagePolicyService.get as jest.Mock).mockImplementationOnce(
	(savedObjectsClient: any, id: string) => {
	return {
	id,
	inputs: {},
	version: '',
	revision: 1,
	updated_at: '',
	updated_by: '',
	created_at: '',
	created_by: '',
	package: {
	name: 'non-managed-package',
	title: 'Non-Managed Package',
	version: '0.0.1',
	},
	};
	}
	);
	(packagePolicyService.getUpgradeDryRunDiff as jest.Mock).mockImplementationOnce(
	(savedObjectsClient: any, id: string) => {
	return {
	name: 'non-managed-package-policy',
	diff: [{ id: 'foo' }, { id: 'bar' }],
	hasErrors: false,
	};
	}
	);
	(getPackageInfo as jest.Mock).mockImplementationOnce(
	({ savedObjectsClient, pkgName, pkgVersion }) => ({
	name: pkgName,
	version: pkgVersion,
	keepPoliciesUpToDate: false,
	})
	);
	await upgradeManagedPackagePolicies(soClient, esClient, ['non-managed-package-id']);
	expect(packagePolicyService.upgrade).not.toBeCalled();
	});

I think this test gives us confidence that any package not flagged as keep_policies_up_to_date will not be auto-upgraded.

Maybe a good way to improve coverage here would be to move the shouldUpgradePolicies boolean into a function that can then be tested as a unit? Then we can assert something like "should not upgrade policies, even if package appears in AUTO_UPDATE_PACKAGES" - does that make sense?

[nchaulet]

Maybe a good way to improve coverage here would be to move the shouldUpgradePolicies boolean into a function that can then be tested as a unit? Then we can assert something like "should not upgrade policies, even if package appears in AUTO_UPDATE_PACKAGES" - does that make sense?

Ye it totally make sense

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 09e9313
• Storybooks Preview

Metrics [docs]

✅ unchanged

History

• 💛 Build #161472 was flaky 54a23e6
• 💔 Build #161462 failed f7dba3b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @kpollich

[kibanamachine]

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.