RHINENG-21568: Use our local systems table when generating playbooks

db/seeders/20180101000000-systems.js

@@ -14,7 +14,8 @@ const SPECIAL_SYSTEMS = {
     '35f36364-6007-4ecc-9666-c4f8d354be9f': { hostname: '35e9b452-e405-499c-9c6e-120010b7b465.example.com' },
     '1040856f-b772-44c7-83a9-eea4813c4be8': { 
         hostname: '1040856f-b772-44c7-83a9-eea4813c4be8.example.com',
-        display_name: 'test-system-1'
+        display_name: 'test-system-1',
+        ansible_host: '1040856f-b772-44c7-83a9-eea4813c4be8.ansible.example.com'
     }
 };
 

src/connectors/Connector.js

@@ -47,6 +47,11 @@ module.exports = class Connector {
                 throw e;
             }
 
+            // UNKNOWN_SYSTEM errors with notFoundIds bubble up directly
+            if (e.notFoundIds) {
+                throw e;
+            }
+
             // Log request and response details for HTTP 400 errors
             if (e instanceof StatusCodeError && e.statusCode === 400) {
                 log.error({

src/connectors/http.js

@@ -34,7 +34,15 @@ function doHttp (options, cached, metrics) {
             case 207:
             case 304: return res;
             case 403: throw new errors.Forbidden("Access denied.");
-            case 404: return null;
+            case 404:
+                // If response contains not_found_ids (Inventory API), throw UNKNOWN_SYSTEM
+                if (res.body?.not_found_ids) {
+                    const notFoundIds = res.body.not_found_ids;
+                    const err = new errors.BadRequest('UNKNOWN_SYSTEM', `Unknown system identifier "${notFoundIds.join(', ')}"`);
+                    err.notFoundIds = notFoundIds;
+                    throw err;
+                }
+                return null;
             default: throw new StatusCodeError(res.statusCode, opts, res.body);
         }
     });

src/connectors/inventory/impl.js

@@ -81,6 +81,11 @@ module.exports = new class extends Connector {
                 e.error.details.message = 'Access to inventory service denied. You don\'t have the required \'inventory:hosts:read\' permission. Please check your RBAC permissions.';
                 throw e;
             }
+
+            // UNKNOWN_SYSTEM errors (from 404) bubble up directly
+            if (e.notFoundIds) {
+                throw e;
+            }
 
             if (retries > 0) {
                 log.warn({ error: e, ids, retries }, 'Inventory fetch failed. Retrying');
@@ -90,8 +95,7 @@ module.exports = new class extends Connector {
             throw e;
         }
 
-        // Inventory returns 404 with message "One or more hosts not found." when one or more requested hosts don't exist
-        // doHttp returns null when the response status is 404
+        // Handle 404 without not_found_ids (unexpected case, but be defensive)
         if (response === null) {
             throw new errors.BadRequest('UNKNOWN_SYSTEM', 'One or more requested systems do not exist in Inventory');
         }
@@ -164,6 +168,11 @@ module.exports = new class extends Connector {
                 e.error.details.message = 'Access to inventory service denied. You don\'t have the required \'inventory:hosts:read\' permission. Please check your RBAC permissions.';
                 throw e;
             }
+
+            // UNKNOWN_SYSTEM errors (from 404) bubble up directly
+            if (e.notFoundIds) {
+                throw e;
+            }
 
             if (retries > 0) {
                 log.warn({ error: e, ids, retries }, 'Inventory fetch failed. Retrying');
@@ -173,8 +182,7 @@ module.exports = new class extends Connector {
             throw e;
         }
 
-        // Inventory returns 404 with message "One or more hosts not found." when one or more requested hosts don't exist
-        // doHttp returns null when the response status is 404
+        // Handle 404 without not_found_ids (unexpected case, but be defensive)
         if (response === null) {
             throw new errors.BadRequest('UNKNOWN_SYSTEM', 'One or more requested systems do not exist in Inventory');
         }
@@ -188,6 +196,38 @@ module.exports = new class extends Connector {
         return validate(transformed);
     }
 
+    /**
+     * Handles 404 from Inventory API gracefully by removing not_found_ids and retrying with remaining IDs.
+     * Returns partial results of found systems.
+     */
+    async getSystemDetailsBatchPartial (ids = [], refresh = false) {
+        if (ids.length === 0) {
+            return {};
+        }
+
+        try {
+            return await this.getSystemDetailsBatch(ids, refresh);
+        } catch (e) {
+            // Check if it's an UNKNOWN_SYSTEM error with notFoundIds
+            if (!e.notFoundIds) {
+                throw e;
+            }
+
+            // Some systems not found - retry with remaining IDs
+            const notFoundIds = e.notFoundIds;
+            const remainingIds = _.difference(ids, notFoundIds);
+
+            log.warn({ notFoundIds }, 'Systems not found in Inventory, filtering them out');
+
+            // No remaining systems to fetch
+            if (remainingIds.length === 0) {
+                return {};
+            }
+
+            return await this.getSystemDetailsBatch(remainingIds, refresh);
+        }
+    }
+
     async getSystemProfileBatch (ids = [], refresh = false, retries = 2) {
         if (ids.length === 0) {
             return {};
@@ -229,6 +269,11 @@ module.exports = new class extends Connector {
                 e.error.details.message = 'Access to inventory service denied. You don\'t have the required \'inventory:hosts:read\' permission. Please check your RBAC permissions.';
                 throw e;
             }
+
+            // UNKNOWN_SYSTEM errors (from 404) bubble up directly
+            if (e.notFoundIds) {
+                throw e;
+            }
 
             if (retries > 0) {
                 log.warn({ error: e, ids, retries }, 'Inventory fetch failed. Retrying');
@@ -238,8 +283,7 @@ module.exports = new class extends Connector {
             throw e;
         }
 
-        // Inventory returns 404 with message "One or more hosts not found." when one or more requested hosts don't exist
-        // doHttp returns null when the response status is 404
+        // Handle 404 without not_found_ids (unexpected case, but be defensive)
         if (response === null) {
             throw new errors.BadRequest('UNKNOWN_SYSTEM', 'One or more requested systems do not exist in Inventory');
         }

src/connectors/inventory/mock.js

@@ -3,13 +3,35 @@
 const _ = require('lodash');
 const Connector = require('../Connector');
 const generator = require('./systemGenerator');
+const errors = require('../../errors');
+
+// Helper to create UNKNOWN_SYSTEM error with notFoundIds (matches http.js behavior)
+function createUnknownSystemError(notFoundIds) {
+    const err = new errors.BadRequest('UNKNOWN_SYSTEM', `Unknown system identifier "${notFoundIds.join(', ')}"`);
+    err.notFoundIds = notFoundIds;
+    return err;
+}
 
 module.exports = new class extends Connector {
     constructor () {
         super(module);
     }
 
     getSystemDetailsBatch (systems) {
+        // Check for non-existent systems first (new Inventory behavior)
+        const notFoundIds = systems.filter(id => generator.generateSystem(id) === null);
+        if (notFoundIds.length > 0) {
+            return Promise.reject(createUnknownSystemError(notFoundIds));
+        }
+
+        return Promise.resolve(_(systems)
+        .keyBy()
+        .mapValues(generator.generateSystem)
+        .value());
+    }
+
+    getSystemDetailsBatchPartial (systems) {
+        // Returns partial results - filters out missing systems instead of throwing
         return Promise.resolve(_(systems)
         .keyBy()
         .mapValues(generator.generateSystem)
@@ -18,18 +40,28 @@ module.exports = new class extends Connector {
     }
 
     getSystemInfoBatch (systems) {
+        // Check for non-existent systems first (new Inventory behavior)
+        const notFoundIds = systems.filter(id => generator.generateSystemInfo(id) === null);
+        if (notFoundIds.length > 0) {
+            return Promise.reject(createUnknownSystemError(notFoundIds));
+        }
+
         return Promise.resolve(_(systems)
             .keyBy()
             .mapValues(generator.generateSystemInfo)
-            .pickBy()
             .value());
     }
 
     getSystemProfileBatch (systems) {
+        // Check for non-existent systems first (new Inventory behavior)
+        const notFoundIds = systems.filter(id => generator.generateSystemProfile(id) === null);
+        if (notFoundIds.length > 0) {
+            return Promise.reject(createUnknownSystemError(notFoundIds));
+        }
+
         return Promise.resolve(_(systems)
         .keyBy()
         .mapValues(generator.generateSystemProfile)
-        .pickBy()
         .value());
     }
 

src/generator/generator.controller.js

@@ -5,7 +5,9 @@ const P = require('bluebird');
 const etag = require('etag');
 
 const errors = require('../errors');
+const queries = require('../remediations/remediations.queries');
 const inventory = require('../connectors/inventory');
+const { storeSystemDetails } = require('../remediations/controller.write');
 const templates = require('../templates/static');
 const SpecialPlay = require('./plays/SpecialPlay');
 const format = require('./format');
@@ -133,43 +135,40 @@ exports.resolveSystems = async function (issues, strict = true) {
         trace.event(`System IDs: ${JSON.stringify(systemIds)}`);
     }
 
-    // bypass cache as ansible_host may change so we want to grab the latest one
-    trace.event('Get system details...');
-    const systems = await inventory.getSystemDetailsBatch(systemIds, true);
-
-    // If strict=false and there are systems that don't exist in Inventory, remove them from the issues
-    if (!strict) {
-        trace.event('Remove systems for which we have no inventory entry...');
-        _.forEach(issues, issue => issue.systems = issue.systems.filter((id) => {
-            // eslint-disable-next-line security/detect-object-injection
-            return (systems.hasOwnProperty(id));
-        }));
-    }
-
-    // Map system IDs to hostnames and verify all systems exist in Inventory
-    // With strict=false: missing systems were already filtered out above, so this should pass
-    // With strict=true: no filtering happened, so throw an error if any system is missing
-    trace.event('Verify that there are no systems for which we have no inventory entry...');
-    _.forEach(issues, issue => issue.hosts = issue.systems.map(id => {
-        if (!systems.hasOwnProperty(id)) {
-            trace.event(`Found no data for system: ${id}`);
-            probes.failedGeneration(issue.id);
-            throw errors.unknownSystem(id);
+    trace.event('Get system details from local DB...');
+    let systems = await queries.getSystemDetailsForPlaybook(systemIds);
+
+    // Fallback: if any systems missing from local table, fetch from Inventory and store
+    const missingIds = systemIds.filter(id => !(id in systems));
+    if (missingIds.length > 0) {
+        trace.event(`Fetching ${missingIds.length} missing systems from Inventory...`);
+
+        let inventorySystems;
+        if (strict) {
+            // strict=true: throw UNKNOWN_SYSTEM error if any systems are missing from Inventory
+            inventorySystems = await inventory.getSystemDetailsBatch(missingIds, true);
+        } else {
+            // strict=false: gracefully handle missing systems, return partial results
+            inventorySystems = await inventory.getSystemDetailsBatchPartial(missingIds, true);
         }
 
-        // validated by openapi middleware and also above
-        // eslint-disable-next-line security/detect-object-injection
-        const system = systems[id];
-        return exports.systemToHost(system);
-    }));
-    trace.event('All systems verified!');
-
-    // If strict=false, filter out issues with no systems (systems that were removed because they don't exist in Inventory)
-    if (!strict) {
-        trace.event('Remove issues with no systems...')
-        issues = _.filter(issues, (issue) => (issue.systems.length > 0));
+        storeSystemDetails(inventorySystems).catch(err => log.warn({ err }, 'Failed to store system details'));
+        systems = { ...systems, ...inventorySystems };
     }
 
+    // Filter out systems not found in local systems table or Inventory, then map to hosts
+    // For strict=true this is a no-op since getSystemDetailsBatch throws if any systems are missing
+    trace.event('Filter systems and map to hosts...');
+    _.forEach(issues, issue => {
+        issue.systems = issue.systems.filter(id => id in systems);
+        // eslint-disable-next-line security/detect-object-injection
+        issue.hosts = issue.systems.map(id => exports.systemToHost(systems[id]));
+    });
+
+    // Remove issues that have no systems left after filtering
+    // For strict=true this is a no-op since all systems should exist
+    issues = _.filter(issues, issue => issue.systems.length > 0);
+
     trace.leave();
     return issues;
 };

src/remediations/controller.write.js

@@ -511,3 +511,6 @@ exports.removeSystem = errors.async(async function (req, res) {
 
     return res.status(404).end();
 });
+
+// Export for reuse in generator.controller.js (backfill systems not in local DB)
+exports.storeSystemDetails = storeSystemDetails;

src/remediations/remediations.queries.js

@@ -909,3 +909,25 @@ exports.getPlaybookRunsWithDispatcherCounts = async function (playbookRunIds) {
         raw: true
     });
 };
+
+exports.getSystemDetailsForPlaybook = async function (systemIds) {
+    if (!systemIds || systemIds.length === 0) {
+        return {};
+    }
+
+    const systems = await db.systems.findAll({
+        attributes: ['id', 'hostname', 'ansible_hostname', 'display_name'],
+        where: { id: systemIds },
+        raw: true
+    });
+
+    return _(systems)
+        .keyBy('id')
+        .mapValues(system => ({
+            id: system.id,
+            hostname: system.hostname,
+            ansible_host: system.ansible_hostname,
+            display_name: system.display_name
+        }))
+        .value();
+};