[Ingest Manager] API sends 404 when package config id is missing (#73212) (#73659)

* Add test to confirm missing config responds w/ 404

Currently failing with a 500 as in #66388

* Use after() to remove items added by test.

The test initally failed with a 500 when the `after` was added. Debugging narrowed it down to a missing default config.

getDefaultAgentConfigId errors if there isn't a default config.  The config is added by `setupIngestManager` which _was_ always called during plugin#start but is no longer.

We could add the setup call to the test/suite, but instead I changed AgentConfigService.delete to use ensureDefaultAgentConfig instead of getDefaultAgentConfigId.

ensureDefaultAgentConfig adds one if it's missing. The check in delete is to make sure we don't delete the default config. We can still do that and now we add a config if it wasn't already there (which seems like A Good Thing)

* Fix package config path in OpenApi spec

* Return 404 if package config id is invalid/missing

* Change test for error displayed text

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Author: John Schulz

x-pack/plugins/ingest_manager/common/openapi/spec_oas3.json

@@ -922,7 +922,7 @@
       },
       "parameters": []
     },
-    "/packageConfigs": {
+    "/package_configs": {
       "get": {
         "summary": "PackageConfigs - List",
         "tags": [],
@@ -1237,7 +1237,7 @@
         ]
       }
     },
-    "/packageConfigs/{packageConfigId}": {
+    "/package_configs/{packageConfigId}": {
       "get": {
         "summary": "PackageConfigs - Info",
         "tags": [],

x-pack/plugins/ingest_manager/server/routes/package_config/handlers.ts

@@ -5,7 +5,7 @@
  */
 import { TypeOf } from '@kbn/config-schema';
 import Boom from 'boom';
-import { RequestHandler } from 'src/core/server';
+import { RequestHandler, SavedObjectsErrorHelpers } from '../../../../../../src/core/server';
 import { appContextService, packageConfigService } from '../../services';
 import { getPackageInfo } from '../../services/epm/packages';
 import {
@@ -49,26 +49,31 @@ export const getOnePackageConfigHandler: RequestHandler<TypeOf<
   typeof GetOnePackageConfigRequestSchema.params
 >> = async (context, request, response) => {
   const soClient = context.core.savedObjects.client;
+  const { packageConfigId } = request.params;
+  const notFoundResponse = () =>
+    response.notFound({ body: { message: `Package config ${packageConfigId} not found` } });
+
   try {
-    const packageConfig = await packageConfigService.get(soClient, request.params.packageConfigId);
+    const packageConfig = await packageConfigService.get(soClient, packageConfigId);
     if (packageConfig) {
       return response.ok({
         body: {
           item: packageConfig,
           success: true,
         },
       });
+    } else {
+      return notFoundResponse();
+    }
+  } catch (e) {
+    if (SavedObjectsErrorHelpers.isNotFoundError(e)) {
+      return notFoundResponse();
     } else {
       return response.customError({
-        statusCode: 404,
-        body: { message: 'Package config not found' },
+        statusCode: 500,
+        body: { message: e.message },
       });
     }
-  } catch (e) {
-    return response.customError({
-      statusCode: 500,
-      body: { message: e.message },
-    });
   }
 };
 

x-pack/plugins/ingest_manager/server/services/agent_config.ts

@@ -334,7 +334,7 @@ class AgentConfigService {
       throw new Error('Agent configuration not found');
     }
 
-    const defaultConfigId = await this.getDefaultAgentConfigId(soClient);
+    const { id: defaultConfigId } = await this.ensureDefaultAgentConfig(soClient);
     if (id === defaultConfigId) {
       throw new Error('The default agent configuration cannot be deleted');
     }

x-pack/test/ingest_manager_api_integration/apis/index.js

@@ -22,6 +22,7 @@ export default function ({ loadTestFile }) {
     // Package configs
     loadTestFile(require.resolve('./package_config/create'));
     loadTestFile(require.resolve('./package_config/update'));
+    loadTestFile(require.resolve('./package_config/get'));
     // Agent config
     loadTestFile(require.resolve('./agent_config/index'));
   });

x-pack/test/ingest_manager_api_integration/apis/package_config/get.ts

@@ -0,0 +1,89 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import expect from '@kbn/expect';
+import { FtrProviderContext } from '../../../api_integration/ftr_provider_context';
+import { skipIfNoDockerRegistry } from '../../helpers';
+
+export default function (providerContext: FtrProviderContext) {
+  const { getService } = providerContext;
+  const supertest = getService('supertest');
+  const dockerServers = getService('dockerServers');
+
+  const server = dockerServers.get('registry');
+  // use function () {} and not () => {} here
+  // because `this` has to point to the Mocha context
+  // see https://mochajs.org/#arrow-functions
+
+  describe('Package Config - get by id', async function () {
+    skipIfNoDockerRegistry(providerContext);
+    let agentConfigId: string;
+    let packageConfigId: string;
+
+    before(async function () {
+      if (!server.enabled) {
+        return;
+      }
+      const { body: agentConfigResponse } = await supertest
+        .post(`/api/ingest_manager/agent_configs`)
+        .set('kbn-xsrf', 'xxxx')
+        .send({
+          name: 'Test config',
+          namespace: 'default',
+        });
+      agentConfigId = agentConfigResponse.item.id;
+
+      const { body: packageConfigResponse } = await supertest
+        .post(`/api/ingest_manager/package_configs`)
+        .set('kbn-xsrf', 'xxxx')
+        .send({
+          name: 'filetest-1',
+          description: '',
+          namespace: 'default',
+          config_id: agentConfigId,
+          enabled: true,
+          output_id: '',
+          inputs: [],
+          package: {
+            name: 'filetest',
+            title: 'For File Tests',
+            version: '0.1.0',
+          },
+        });
+      packageConfigId = packageConfigResponse.item.id;
+    });
+
+    after(async function () {
+      if (!server.enabled) {
+        return;
+      }
+
+      await supertest
+        .post(`/api/ingest_manager/agent_configs/delete`)
+        .set('kbn-xsrf', 'xxxx')
+        .send({ agentConfigId })
+        .expect(200);
+
+      await supertest
+        .post(`/api/ingest_manager/package_configs/delete`)
+        .set('kbn-xsrf', 'xxxx')
+        .send({ packageConfigIds: [packageConfigId] })
+        .expect(200);
+    });
+
+    it('should succeed with a valid id', async function () {
+      const { body: apiResponse } = await supertest
+        .get(`/api/ingest_manager/package_configs/${packageConfigId}`)
+        .expect(200);
+
+      expect(apiResponse.success).to.be(true);
+    });
+
+    it('should return a 404 with an invalid id', async function () {
+      await supertest.get(`/api/ingest_manager/package_configs/IS_NOT_PRESENT`).expect(404);
+    });
+  });
+}

x-pack/test/security_solution_endpoint/apps/endpoint/policy_details.ts

@@ -27,7 +27,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
         await pageObjects.policy.navigateToPolicyDetails('invalid-id');
         await testSubjects.existOrFail('policyDetailsIdNotFoundMessage');
         expect(await testSubjects.getVisibleText('policyDetailsIdNotFoundMessage')).to.equal(
-          'Saved object [ingest-package-configs/invalid-id] not found'
+          'Package config invalid-id not found'
         );
       });
     });