diff --git a/tools/spectral/ipa/__tests__/IPA102CollectionIdentifierCamelCase.test.js b/tools/spectral/ipa/__tests__/IPA102CollectionIdentifierCamelCase.test.js index a69dca5302..2c10eccb1e 100644 --- a/tools/spectral/ipa/__tests__/IPA102CollectionIdentifierCamelCase.test.js +++ b/tools/spectral/ipa/__tests__/IPA102CollectionIdentifierCamelCase.test.js @@ -124,14 +124,67 @@ testRule('xgen-IPA-102-collection-identifier-camelCase', [ document: { paths: { '/resource_groups': { - 'x-xgen-IPA-exception': { - 'xgen-IPA-102-collection-identifier-camelCase': 'Legacy API path that cannot be changed', + get: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-102-collection-identifier-camelCase': 'Legacy API path that cannot be changed', + }, + }, + delete: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-102-collection-identifier-camelCase': 'Legacy API path that cannot be changed', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'valid with all methods having exceptions (every method needs exception)', + document: { + paths: { + '/resource_groups': { + get: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-102-collection-identifier-camelCase': 'Legacy API path that cannot be changed', + }, + }, + delete: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-102-collection-identifier-camelCase': 'Legacy API path that cannot be changed', + }, }, }, }, }, errors: [], }, + { + name: 'invalid when not all methods have exceptions', + document: { + paths: { + '/resource_groups': { + get: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-102-collection-identifier-camelCase': 'Legacy API path that cannot be changed', + }, + }, + post: { + description: 'This method has no exception', + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-102-collection-identifier-camelCase', + message: + "Collection identifiers must be in camelCase. Path segment 'resource_groups' in path '/resource_groups' is not in camelCase.", + path: ['paths', '/resource_groups'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, { name: 'reports violations for paths with double slashes', document: { diff --git a/tools/spectral/ipa/rulesets/IPA-102.yaml b/tools/spectral/ipa/rulesets/IPA-102.yaml index 358cd73801..8ac7c570b5 100644 --- a/tools/spectral/ipa/rulesets/IPA-102.yaml +++ b/tools/spectral/ipa/rulesets/IPA-102.yaml @@ -14,7 +14,7 @@ rules: - Path parameters should also follow camelCase naming - Certain values can be exempted via the ignoredValues configuration that can be supplied as `ignoredValues` argument to the rule - - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation + - Each operation within path with `x-xgen-IPA-exception` are excluded from validation - Double slashes (//) are not allowed in paths message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-102-collection-identifier-camelCase' diff --git a/tools/spectral/ipa/rulesets/functions/IPA102CollectionIdentifierCamelCase.js b/tools/spectral/ipa/rulesets/functions/IPA102CollectionIdentifierCamelCase.js index 83deaa033c..c9f88b8027 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA102CollectionIdentifierCamelCase.js +++ b/tools/spectral/ipa/rulesets/functions/IPA102CollectionIdentifierCamelCase.js @@ -4,7 +4,7 @@ import { collectException, handleInternalError, } from './utils/collectionUtils.js'; -import { hasException } from './utils/exceptions.js'; +import { hasExceptionInEveryHttpMethod } from './utils/exceptions.js'; import { isPathParam } from './utils/componentUtils.js'; import { casing } from '@stoplight/spectral-functions'; @@ -22,10 +22,12 @@ export default (input, options, { path, documentInventory }) => { const oas = documentInventory.resolved; const pathKey = input; - // Check for exception at the path level - if (hasException(oas.paths[input], RULE_NAME)) { - collectException(oas.paths[input], RULE_NAME, path); - return; + if (oas.paths[input]) { + const pathItem = oas.paths[input]; + if (hasExceptionInEveryHttpMethod(pathItem, RULE_NAME)) { + collectException(pathItem, RULE_NAME, path); + return; + } } // Extract ignored values from options diff --git a/tools/spectral/ipa/rulesets/functions/utils/exceptions.js b/tools/spectral/ipa/rulesets/functions/utils/exceptions.js index 256cab89dc..9c0f7ccc8f 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/exceptions.js +++ b/tools/spectral/ipa/rulesets/functions/utils/exceptions.js @@ -13,3 +13,23 @@ export function hasException(object, ruleName) { } return false; } + +/** + * Checks if every HTTP method in the path item has an exception for the specified rule + * Only considers methods that actually exist in the path item + * + * @param {object} pathItem - The path item object containing HTTP methods + * @param {string} ruleName - The name of the rule to check for exceptions + * @returns {boolean} true if every HTTP method has an exception for the rule, otherwise false + */ +export function hasExceptionInEveryHttpMethod(pathItem, ruleName) { + if (!pathItem) return false; + const httpMethods = ['get', 'post', 'put', 'delete', 'patch', 'options', 'head']; + const existingMethods = httpMethods.filter((method) => pathItem[method]); + + // If no HTTP methods exist, return false + if (existingMethods.length === 0) return false; + + // Check if every existing HTTP method has the exception + return existingMethods.every((method) => hasException(pathItem[method], ruleName)); +} diff --git a/tools/spectral/ipa/scripts/filter-ipa-violations.js b/tools/spectral/ipa/scripts/filter-ipa-violations.js index 28ab003cb2..b31a7e010b 100644 --- a/tools/spectral/ipa/scripts/filter-ipa-violations.js +++ b/tools/spectral/ipa/scripts/filter-ipa-violations.js @@ -1,27 +1,47 @@ import fs from 'node:fs/promises'; import { execSync } from 'child_process'; import path from 'path'; +import http from 'http'; +import https from 'https'; async function filterIpaViolations() { try { // Check if rule ID is provided const ruleId = process.argv[2]; if (!ruleId) { - console.error('Usage: node filter-ipa-violations.js '); + console.error('Usage: node filter-ipa-violations.js [remote-openapi-url]'); console.error('Example: node filter-ipa-violations.js xgen-IPA-102-collection-identifier-camelCase'); + console.error( + 'Example with remote file: node filter-ipa-violations.js xgen-IPA-102-collection-identifier-camelCase https://raw.githubusercontent.com/mongodb/openapi/refs/heads/dev/openapi/.raw/v2.yaml' + ); process.exit(1); } + // Check if a remote OpenAPI file URL is provided + const remoteUrl = process.argv[3]; const outputFile = path.join(process.cwd(), `${ruleId}-violations.md`); console.log(`Filtering violations for rule ID: ${ruleId}`); console.log('Running IPA validation...'); + // If remote URL provided, download it to a temp file + let openapiFilePath = './openapi/.raw/v2.yaml'; // Default local file + let tempFile = null; + + if (remoteUrl) { + console.log(`Using remote OpenAPI file: ${remoteUrl}`); + tempFile = path.join(process.cwd(), 'temp_openapi_file.yaml'); + await downloadFile(remoteUrl, tempFile); + openapiFilePath = tempFile; + } else { + console.log('Using local OpenAPI file'); + } + let validationOutput; try { // Run IPA validation and get output as JSON execSync( - 'spectral lint --format=json -o results.json ./openapi/.raw/v2.yaml --ruleset=./tools/spectral/ipa/ipa-spectral.yaml', + `spectral lint --format=json -o results.json ${openapiFilePath} --ruleset=./tools/spectral/ipa/ipa-spectral.yaml`, { encoding: 'utf-8', timeout: 4000, @@ -30,6 +50,15 @@ async function filterIpaViolations() { ); } catch (error) { console.error('Error (expected):', error.message); + } finally { + // Clean up temp file if it exists + if (tempFile) { + try { + await fs.unlink(tempFile); + } catch (err) { + console.error('Error removing temporary file:', err.message); + } + } } // Read the JSON output @@ -59,12 +88,11 @@ async function filterIpaViolations() { // Generate markdown content let markdownContent = `# ${ruleId} Violations Checklist\n\n`; markdownContent += `Generated on: ${new Date().toLocaleString()}\n\n`; - Object.keys(groupedBySource).forEach((source) => { const violations = groupedBySource[source]; violations.forEach((violation) => { - markdownContent += `## ${violation.source}\n\n`; + markdownContent += `## ${violation.message}\n\n`; markdownContent += `Path: \`${violation.path.join('/')}\`\n\n`; markdownContent += `- [ ] Fixed\n\n`; }); @@ -82,4 +110,34 @@ async function filterIpaViolations() { } } +// Function to download a file from a URL +function downloadFile(url, outputPath) { + return new Promise((resolve, reject) => { + const client = url.startsWith('https') ? https : http; + + console.log(`Downloading OpenAPI file from ${url}...`); + + const request = client.get(url, (response) => { + if (response.statusCode < 200 || response.statusCode >= 300) { + return reject(new Error(`Failed to download file, status code: ${response.statusCode}`)); + } + + const file = fs.open(outputPath, 'w').then((fileHandle) => fileHandle.createWriteStream()); + file + .then((fileStream) => { + response.pipe(fileStream); + response.on('end', () => { + console.log('Download complete'); + resolve(); + }); + }) + .catch(reject); + }); + + request.on('error', (err) => { + reject(err); + }); + }); +} + filterIpaViolations();