[Fleet][spacetime] Improve fleet logging and error handling (#172657)

Solves #170953 ## Summary Improving logging in various parts of Fleet: - Package lifecycle - Package policy service - Agent policy service - Outputs - Fleet Server Hosts - Proxies Also improving errors handling by using more specific typed errors instead of generic error. ### Checklist - [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 --------- Co-authored-by: Kibana Machine <[email protected]>
elastic · Dec 12, 2023 · f90a489 · f90a489
1 parent fa3b6f4
commit f90a489
Show file tree

Hide file tree

Showing 44 changed files with 481 additions and 160 deletions.
diff --git a/x-pack/plugins/fleet/scripts/verify_test_packages/verify_test_packages.test.ts b/x-pack/plugins/fleet/scripts/verify_test_packages/verify_test_packages.test.ts
@@ -5,9 +5,30 @@
  * 2.0.
  */
 
+import { securityMock } from '@kbn/security-plugin/server/mocks';
+import { loggerMock } from '@kbn/logging-mocks';
+
+import type { Logger } from '@kbn/core/server';
+
+import { appContextService } from '../../server/services/app_context';
+
 import { verifyAllTestPackages } from './verify_test_packages';
 
+jest.mock('../../server/services/app_context');
+
+const mockedAppContextService = appContextService as jest.Mocked<typeof appContextService>;
+mockedAppContextService.getSecuritySetup.mockImplementation(() => ({
+  ...securityMock.createSetup(),
+}));
+
+let mockedLogger: jest.Mocked<Logger>;
+
 describe('Test packages', () => {
+  beforeEach(() => {
+    mockedLogger = loggerMock.create();
+    mockedAppContextService.getLogger.mockReturnValue(mockedLogger);
+  });
+
   test('All test packages should be valid (node scripts/verify_test_packages) ', async () => {
     const { errors } = await verifyAllTestPackages();
     expect(errors).toEqual([]);

diff --git a/x-pack/plugins/fleet/server/errors/handlers.ts b/x-pack/plugins/fleet/server/errors/handlers.ts
@@ -20,18 +20,14 @@ import { UninstallTokenError } from '../../common/errors';
 import { appContextService } from '../services';
 
 import {
-  AgentNotFoundError,
-  AgentActionNotFoundError,
   AgentPolicyNameExistsError,
   ConcurrentInstallOperationError,
   FleetError,
-  PackageNotFoundError,
   PackageUnsupportedMediaTypeError,
   RegistryConnectionError,
   RegistryError,
   RegistryResponseError,
   PackageFailedVerificationError,
-  PackagePolicyNotFoundError,
   FleetUnauthorizedError,
   PackagePolicyNameExistsError,
   PackageOutdatedError,
@@ -41,6 +37,11 @@ import {
   PackageESError,
   KibanaSOReferenceError,
   PackageAlreadyInstalledError,
+  AgentPolicyInvalidError,
+  EnrollmentKeyNameExistsError,
+  AgentRequestInvalidError,
+  PackagePolicyRequestError,
+  FleetNotFoundError,
 } from '.';
 
 type IngestErrorHandler = (
@@ -71,24 +72,31 @@ const getHTTPResponseCode = (error: FleetError): number => {
   if (error instanceof KibanaSOReferenceError) {
     return 400;
   }
+  if (error instanceof AgentPolicyInvalidError) {
+    return 400;
+  }
+  if (error instanceof AgentRequestInvalidError) {
+    return 400;
+  }
+  if (error instanceof PackagePolicyRequestError) {
+    return 400;
+  }
   // Unauthorized
   if (error instanceof FleetUnauthorizedError) {
     return 403;
   }
   // Not Found
-  if (error instanceof PackageNotFoundError || error instanceof PackagePolicyNotFoundError) {
-    return 404;
-  }
-  if (error instanceof AgentNotFoundError) {
-    return 404;
-  }
-  if (error instanceof AgentActionNotFoundError) {
+  if (error instanceof FleetNotFoundError) {
     return 404;
   }
+
   // Conflict
   if (error instanceof AgentPolicyNameExistsError) {
     return 409;
   }
+  if (error instanceof EnrollmentKeyNameExistsError) {
+    return 409;
+  }
   if (error instanceof ConcurrentInstallOperationError) {
     return 409;
   }

diff --git a/x-pack/plugins/fleet/server/errors/index.ts b/x-pack/plugins/fleet/server/errors/index.ts
@@ -28,7 +28,7 @@ export class RegistryResponseError extends RegistryError {
 }
 
 // Package errors
-export class PackageNotFoundError extends FleetError {}
+
 export class PackageOutdatedError extends FleetError {}
 export class PackageFailedVerificationError extends FleetError {
   constructor(pkgName: string, pkgVersion: string) {
@@ -43,20 +43,24 @@ export class PackageInvalidArchiveError extends FleetError {}
 export class PackageRemovalError extends FleetError {}
 export class PackageESError extends FleetError {}
 export class ConcurrentInstallOperationError extends FleetError {}
-export class BundledPackageLocationNotFoundError extends FleetError {}
+
 export class KibanaSOReferenceError extends FleetError {}
 export class PackageAlreadyInstalledError extends FleetError {}
 
 export class AgentPolicyError extends FleetError {}
-export class AgentPolicyNotFoundError extends FleetError {}
-export class AgentNotFoundError extends FleetError {}
-export class AgentActionNotFoundError extends FleetError {}
+export class AgentRequestInvalidError extends FleetError {}
+export class AgentPolicyInvalidError extends FleetError {}
+
 export class AgentPolicyNameExistsError extends AgentPolicyError {}
 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 BundledPackageLocationNotFoundError extends FleetError {}
+
+export class PackagePolicyRequestError extends FleetError {}
+
+export class EnrollmentKeyNameExistsError extends FleetError {}
 export class HostedAgentPolicyRestrictionRelatedError extends FleetError {
   constructor(message = 'Cannot perform that action') {
     super(
@@ -75,12 +79,27 @@ export class FleetEncryptedSavedObjectEncryptionKeyRequired extends FleetError {
 export class FleetSetupError extends FleetError {}
 export class GenerateServiceTokenError extends FleetError {}
 export class FleetUnauthorizedError extends FleetError {}
+export class FleetNotFoundError extends FleetError {}
 
 export class OutputUnauthorizedError extends FleetError {}
 export class OutputInvalidError extends FleetError {}
 export class OutputLicenceError extends FleetError {}
 export class DownloadSourceError extends FleetError {}
 
+// Not found errors
+export class AgentNotFoundError extends FleetNotFoundError {}
+export class AgentPolicyNotFoundError extends FleetNotFoundError {}
+export class AgentActionNotFoundError extends FleetNotFoundError {}
+export class DownloadSourceNotFound extends FleetNotFoundError {}
+export class EnrollmentKeyNotFoundError extends FleetNotFoundError {}
+export class FleetServerHostNotFoundError extends FleetNotFoundError {}
+export class SigningServiceNotFoundError extends FleetNotFoundError {}
+export class InputNotFoundError extends FleetNotFoundError {}
+export class OutputNotFoundError extends FleetNotFoundError {}
+export class PackageNotFoundError extends FleetNotFoundError {}
+export class PackagePolicyNotFoundError extends FleetNotFoundError {}
+export class StreamNotFoundError extends FleetNotFoundError {}
+
 export class FleetServerHostUnauthorizedError extends FleetUnauthorizedError {}
 export class FleetProxyUnauthorizedError extends FleetUnauthorizedError {}
 

diff --git a/x-pack/plugins/fleet/server/integration_tests/helpers/index.ts b/x-pack/plugins/fleet/server/integration_tests/helpers/index.ts
@@ -8,6 +8,8 @@
 import { adminTestUser } from '@kbn/test';
 import { getSupertest, type createRoot, type HttpMethod } from '@kbn/core-test-helpers-kbn-server';
 
+import { FleetError } from '../../errors';
+
 type Root = ReturnType<typeof createRoot>;
 
 export * from './docker_registry_helper';
@@ -18,7 +20,7 @@ export const waitForFleetSetup = async (root: Root) => {
     const resp = await statusApi.send();
     const fleetStatus = resp.body?.status?.plugins?.fleet;
     if (fleetStatus?.meta?.error) {
-      throw new Error(`Setup failed: ${JSON.stringify(fleetStatus)}`);
+      throw new FleetError(`Setup failed: ${JSON.stringify(fleetStatus)}`);
     }
 
     return !fleetStatus || fleetStatus?.summary === 'Fleet is setting up';

diff --git a/x-pack/plugins/fleet/server/routes/agent/source_uri_utils.ts b/x-pack/plugins/fleet/server/routes/agent/source_uri_utils.ts
@@ -9,6 +9,7 @@ import type { SavedObjectsClientContract } from '@kbn/core/server';
 
 import { downloadSourceService } from '../../services';
 import type { AgentPolicy } from '../../types';
+import { FleetError, DownloadSourceNotFound } from '../../errors';
 
 export const getSourceUriForAgentPolicy = async (
   soClient: SavedObjectsClientContract,
@@ -17,12 +18,12 @@ export const getSourceUriForAgentPolicy = async (
   const defaultDownloadSourceId = await downloadSourceService.getDefaultDownloadSourceId(soClient);
 
   if (!defaultDownloadSourceId) {
-    throw new Error('Default download source host is not setup');
+    throw new FleetError('Default download source host is not setup');
   }
   const downloadSourceId: string = agentPolicy.download_source_id || defaultDownloadSourceId;
   const downloadSource = await downloadSourceService.get(soClient, downloadSourceId);
   if (!downloadSource) {
-    throw new Error(`Download source host not found ${downloadSourceId}`);
+    throw new DownloadSourceNotFound(`Download source host not found ${downloadSourceId}`);
   }
   return { host: downloadSource.host, proxy_id: downloadSource.proxy_id };
 };
diff --git a/x-pack/plugins/fleet/server/routes/agent/upgrade_handler.test.ts b/x-pack/plugins/fleet/server/routes/agent/upgrade_handler.test.ts
@@ -15,7 +15,7 @@ describe('upgrade handler', () => {
 
     it('should throw if upgrade version is higher than kibana version', () => {
       expect(() => checkKibanaVersion('8.5.0', '8.4.0')).toThrowError(
-        'cannot upgrade agent to 8.5.0 because it is higher than the installed kibana version 8.4.0'
+        'Cannot upgrade agent to 8.5.0 because it is higher than the installed kibana version 8.4.0'
       );
     });
 

diff --git a/x-pack/plugins/fleet/server/routes/agent/upgrade_handler.ts b/x-pack/plugins/fleet/server/routes/agent/upgrade_handler.ts
@@ -19,7 +19,7 @@ import type { PostAgentUpgradeResponse } from '../../../common/types';
 import type { PostAgentUpgradeRequestSchema, PostBulkAgentUpgradeRequestSchema } from '../../types';
 import * as AgentService from '../../services/agents';
 import { appContextService } from '../../services';
-import { defaultFleetErrorHandler } from '../../errors';
+import { defaultFleetErrorHandler, AgentRequestInvalidError } from '../../errors';
 import {
   getRecentUpgradeInfoForAgent,
   isAgentUpgradeable,
@@ -187,14 +187,15 @@ export const postBulkAgentsUpgradeHandler: RequestHandler<
 export const checkKibanaVersion = (version: string, kibanaVersion: string, force = false) => {
   // get version number only in case "-SNAPSHOT" is in it
   const kibanaVersionNumber = semverCoerce(kibanaVersion)?.version;
-  if (!kibanaVersionNumber) throw new Error(`kibanaVersion ${kibanaVersionNumber} is not valid`);
+  if (!kibanaVersionNumber)
+    throw new AgentRequestInvalidError(`KibanaVersion ${kibanaVersionNumber} is not valid`);
   const versionToUpgradeNumber = semverCoerce(version)?.version;
   if (!versionToUpgradeNumber)
-    throw new Error(`version to upgrade ${versionToUpgradeNumber} is not valid`);
+    throw new AgentRequestInvalidError(`Version to upgrade ${versionToUpgradeNumber} is not valid`);
 
   if (!force && semverGt(versionToUpgradeNumber, kibanaVersionNumber)) {
-    throw new Error(
-      `cannot upgrade agent to ${versionToUpgradeNumber} because it is higher than the installed kibana version ${kibanaVersionNumber}`
+    throw new AgentRequestInvalidError(
+      `Cannot upgrade agent to ${versionToUpgradeNumber} because it is higher than the installed kibana version ${kibanaVersionNumber}`
     );
   }
 
@@ -205,8 +206,8 @@ export const checkKibanaVersion = (version: string, kibanaVersion: string, force
 
   // When force is enabled, only the major and minor versions are checked
   if (force && !(kibanaMajorGt || kibanaMajorEqMinorGte)) {
-    throw new Error(
-      `cannot force upgrade agent to ${versionToUpgradeNumber} because it does not satisfy the major and minor of the installed kibana version ${kibanaVersionNumber}`
+    throw new AgentRequestInvalidError(
+      `Cannot force upgrade agent to ${versionToUpgradeNumber} because it does not satisfy the major and minor of the installed kibana version ${kibanaVersionNumber}`
     );
   }
 };
@@ -228,8 +229,8 @@ const checkFleetServerVersion = (
   }
 
   if (!force && semverGt(versionToUpgradeNumber, maxFleetServerVersion)) {
-    throw new Error(
-      `cannot upgrade agent to ${versionToUpgradeNumber} because it is higher than the latest fleet server version ${maxFleetServerVersion}`
+    throw new AgentRequestInvalidError(
+      `Cannot upgrade agent to ${versionToUpgradeNumber} because it is higher than the latest fleet server version ${maxFleetServerVersion}`
     );
   }
 
@@ -241,8 +242,8 @@ const checkFleetServerVersion = (
 
   // When force is enabled, only the major and minor versions are checked
   if (force && !(fleetServerMajorGt || fleetServerMajorEqMinorGte)) {
-    throw new Error(
-      `cannot force upgrade agent to ${versionToUpgradeNumber} because it does not satisfy the major and minor of the latest fleet server version ${maxFleetServerVersion}`
+    throw new AgentRequestInvalidError(
+      `Cannot force upgrade agent to ${versionToUpgradeNumber} because it does not satisfy the major and minor of the latest fleet server version ${maxFleetServerVersion}`
     );
   }
 };
diff --git a/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts b/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts
@@ -6,7 +6,6 @@
  */
 
 import type { TypeOf } from '@kbn/config-schema';
-import Boom from '@hapi/boom';
 
 import { SavedObjectsErrorHelpers } from '@kbn/core/server';
 import type { RequestHandler } from '@kbn/core/server';
@@ -43,7 +42,11 @@ import type {
   UpgradePackagePolicyResponse,
 } from '../../../common/types';
 import { installationStatuses, inputsFormat } from '../../../common/constants';
-import { defaultFleetErrorHandler, PackagePolicyNotFoundError } from '../../errors';
+import {
+  defaultFleetErrorHandler,
+  PackagePolicyNotFoundError,
+  PackagePolicyRequestError,
+} from '../../errors';
 import { getInstallations, getPackageInfo } from '../../services/epm/packages';
 import { PACKAGES_SAVED_OBJECT_TYPE, SO_SEARCH_LIMIT } from '../../constants';
 import {
@@ -244,7 +247,7 @@ export const createPackagePolicyHandler: FleetRequestHandler<
     let newPackagePolicy: NewPackagePolicy;
     if (isSimplifiedCreatePackagePolicyRequest(newPolicy)) {
       if (!pkg) {
-        throw new Error('Package is required');
+        throw new PackagePolicyRequestError('Package is required');
       }
       const pkgInfo = await getPackageInfo({
         savedObjectsClient: soClient,
@@ -311,7 +314,7 @@ export const updatePackagePolicyHandler: FleetRequestHandler<
   const packagePolicy = await packagePolicyService.get(soClient, request.params.packagePolicyId);
 
   if (!packagePolicy) {
-    throw Boom.notFound('Package policy not found');
+    throw new PackagePolicyNotFoundError('Package policy not found');
   }
 
   if (limitedToPackages && limitedToPackages.length) {
@@ -337,7 +340,7 @@ export const updatePackagePolicyHandler: FleetRequestHandler<
       isSimplifiedCreatePackagePolicyRequest(body as unknown as SimplifiedPackagePolicy)
     ) {
       if (!pkg) {
-        throw new Error('package is required');
+        throw new PackagePolicyRequestError('Package is required');
       }
       const pkgInfo = await getPackageInfo({
         savedObjectsClient: soClient,

diff --git a/x-pack/plugins/fleet/server/services/agent_policies/outputs_helpers.ts b/x-pack/plugins/fleet/server/services/agent_policies/outputs_helpers.ts
@@ -12,7 +12,7 @@ import { LICENCE_FOR_PER_POLICY_OUTPUT, outputType } from '../../../common/const
 import { policyHasFleetServer, policyHasSyntheticsIntegration } from '../../../common/services';
 import { appContextService } from '..';
 import { outputService } from '../output';
-import { OutputInvalidError, OutputLicenceError } from '../../errors';
+import { OutputInvalidError, OutputLicenceError, OutputNotFoundError } from '../../errors';
 
 /**
  * Get the data output for a given agent policy
@@ -28,7 +28,7 @@ export async function getDataOutputForAgentPolicy(
     agentPolicy.data_output_id || (await outputService.getDefaultDataOutputId(soClient));
 
   if (!dataOutputId) {
-    throw new Error('No default data output found.');
+    throw new OutputNotFoundError('No default data output found.');
   }
 
   return outputService.get(soClient, dataOutputId);

diff --git a/x-pack/plugins/fleet/server/services/agent_policies/package_policies_to_agent_permissions.ts b/x-pack/plugins/fleet/server/services/agent_policies/package_policies_to_agent_permissions.ts
@@ -24,6 +24,7 @@ import type {
   RegistryDataStreamPrivileges,
 } from '../../../common/types';
 import { PACKAGE_POLICY_DEFAULT_INDEX_PRIVILEGES } from '../../constants';
+import { PackagePolicyRequestError } from '../../errors';
 
 import type { PackagePolicy } from '../../types';
 import { pkgToPkgKey } from '../epm/registry';
@@ -46,7 +47,7 @@ export function storedPackagePoliciesToAgentPermissions(
 ): FullAgentPolicyOutputPermissions | undefined {
   // I'm not sure what permissions to return for this case, so let's return the defaults
   if (!packagePolicies) {
-    throw new Error(
+    throw new PackagePolicyRequestError(
       'storedPackagePoliciesToAgentPermissions should be called with a PackagePolicy'
     );
   }
@@ -57,7 +58,9 @@ export function storedPackagePoliciesToAgentPermissions(
 
   const permissionEntries = packagePolicies.map((packagePolicy) => {
     if (!packagePolicy.package) {
-      throw new Error(`No package for package policy ${packagePolicy.name ?? packagePolicy.id}`);
+      throw new PackagePolicyRequestError(
+        `No package for package policy ${packagePolicy.name ?? packagePolicy.id}`
+      );
     }
 
     const pkg = packageInfoCache.get(pkgToPkgKey(packagePolicy.package))!;