|  | 
|  | 1 | +# Contributing to MongoDB Spectral Validations | 
|  | 2 | + | 
|  | 3 | +Thank you for your interest in contributing! We welcome contributions of all kinds—bug fixes, new features, documentation improvements, and more. | 
|  | 4 | + | 
|  | 5 | +**Note:** For MongoDB engineers, please review https://go/ipa-validation-internal-wiki for additional information. | 
|  | 6 | + | 
|  | 7 | +## Legacy Spectral Rule Implementation | 
|  | 8 | + | 
|  | 9 | +### Updating the .spectral.yaml Ruleset | 
|  | 10 | + | 
|  | 11 | +When adding new rules or updating the `.spectral.yaml` file, the validations will automatically update across the `mongodb/openapi` repository. Follow these steps: | 
|  | 12 | + | 
|  | 13 | +1. Open a pull request (PR) in the `mongodb/openapi` repository with changes to `tools/spectral/.spectral.yaml`. | 
|  | 14 | +2. Ensure that the new Spectral lint checks pass. | 
|  | 15 | +3. Review and merge the PR. | 
|  | 16 | + | 
|  | 17 | +## IPA Rule Implementation | 
|  | 18 | + | 
|  | 19 | +The rule validations are custom JS functions (see [/rulesets/functions](https://github.com/mongodb/openapi/tree/main/tools/spectral/ipa/rulesets/functions)). To learn more about custom functions, refer to the [Spectral Documentation](https://docs.stoplight.io/docs/spectral/a781e290eb9f9-custom-functions). | 
|  | 20 | + | 
|  | 21 | +The custom rule implementation allows for: | 
|  | 22 | + | 
|  | 23 | +- Advanced validations not available using the standard Spectral rules | 
|  | 24 | +- Custom exception handling | 
|  | 25 | +- Metrics collection | 
|  | 26 | + | 
|  | 27 | +### Exceptions | 
|  | 28 | + | 
|  | 29 | +Instead of using the [Spectral overrides approach](https://docs.stoplight.io/docs/spectral/293426e270fac-overrides), we use [custom OAS extensions](https://swagger.io/docs/specification/v3_0/openapi-extensions/) to handle exceptions to IPA validation rules. Exception extensions are added to the component which should be exempted, with the Spectral rule name and a reason. | 
|  | 30 | + | 
|  | 31 | +``` | 
|  | 32 | +"x-xgen-IPA-exception": { | 
|  | 33 | +    "xgen-IPA-104-resource-has-GET": "Legacy API, not used by infrastructure-as-code tooling", | 
|  | 34 | +} | 
|  | 35 | +``` | 
|  | 36 | + | 
|  | 37 | +## Testing | 
|  | 38 | + | 
|  | 39 | +- IPA Validation related code is tested using [Jest](https://jestjs.io/) | 
|  | 40 | +- Each custom validation function has tests, located in [/\_\_tests\_\_](https://github.com/mongodb/openapi/tree/main/tools/spectral/ipa/__tests__). They use the test hook [testRule.js](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/__tests__/__helpers__/testRule.js) as a common approach for Spectral rule testing | 
|  | 41 | +- Helper/util functions are tested as well, see [/\_\_tests\_\_/utils](https://github.com/mongodb/openapi/tree/main/tools/spectral/ipa/__tests__/utils) | 
|  | 42 | + | 
|  | 43 | +Install necessary dependencies with `npm install` if you haven't already. All Jest tests can be run with: | 
|  | 44 | + | 
|  | 45 | +``` | 
|  | 46 | +npm run test | 
|  | 47 | +``` | 
|  | 48 | + | 
|  | 49 | +To run a single test, in this case `singletonHasNoId.test.js`: | 
|  | 50 | + | 
|  | 51 | +``` | 
|  | 52 | +npm run test -- singletonHasNoId | 
|  | 53 | +``` | 
|  | 54 | + | 
|  | 55 | +## Code Style | 
|  | 56 | + | 
|  | 57 | +- Use [Prettier](https://prettier.io/) for code formatting | 
|  | 58 | + | 
|  | 59 | +``` | 
|  | 60 | +npx prettier . --write | 
|  | 61 | +``` | 
|  | 62 | + | 
|  | 63 | +- [ESLint](https://eslint.org/) is being used for linting | 
|  | 64 | + | 
|  | 65 | +## Pull Request Checklist | 
|  | 66 | + | 
|  | 67 | +- [ ] Ensure that code builds and CI tests pass | 
|  | 68 | +- [ ] Add or update unit tests as needed | 
|  | 69 | +- [ ] Update documentation (if applicable) | 
|  | 70 | + | 
|  | 71 | +``` | 
|  | 72 | +npm run gen-ipa-docs | 
|  | 73 | +``` | 
|  | 74 | + | 
|  | 75 | +- [ ] Reference related issues (e.g., Closes #123) | 
|  | 76 | + | 
|  | 77 | +## Technical Decisions | 
|  | 78 | + | 
|  | 79 | +### Resource & Singleton Evaluation | 
|  | 80 | + | 
|  | 81 | +In the IPA Spectral validation, a resource can be identified using a resource collection path. | 
|  | 82 | + | 
|  | 83 | +For example, resource collection path: `/resource` | 
|  | 84 | + | 
|  | 85 | +- To get all paths and the path objects for this resource, use [getResourcePathItems](https://github.com/mongodb/openapi/blob/99823b3dfd315f892c5f64f1db50f2124261929c/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js#L143) | 
|  | 86 | + | 
|  | 87 | +  - Will return path objects for paths (if present): | 
|  | 88 | +    - Resource collection path `/resource` | 
|  | 89 | +    - Single resource path `/resource + /{someId}` | 
|  | 90 | +    - Custom method path(s) | 
|  | 91 | +      - `/resource + /{someId} + :customMethod` | 
|  | 92 | +      - `/resource + :customMethod` | 
|  | 93 | + | 
|  | 94 | +- To check if a resource is a singleton, take the returned object from [getResourcePathItems](https://github.com/mongodb/openapi/blob/99823b3dfd315f892c5f64f1db50f2124261929c/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js#L143) and evaluate the resource using [isSingletonResource](https://github.com/mongodb/openapi/blob/99823b3dfd315f892c5f64f1db50f2124261929c/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js#L71) | 
|  | 95 | +- To check if a path belongs to a resource collection, use [isResourceCollectionIdentifier](https://github.com/mongodb/openapi/blob/99823b3dfd315f892c5f64f1db50f2124261929c/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js#L13) | 
|  | 96 | +- To check if a path belongs to a single resource, use [isSingleResourceIdentifier](https://github.com/mongodb/openapi/blob/99823b3dfd315f892c5f64f1db50f2124261929c/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js#L31) | 
|  | 97 | + | 
|  | 98 | + Note: Paths like `/resource/resource` or `/resource/{id}/{id}` are not evaluated as valid resource or single resource paths using isResourceCollectionIdentifier or isSingleResourceIdentifier. | 
|  | 99 | + | 
|  | 100 | +### Rule Implementation Guidelines | 
|  | 101 | + | 
|  | 102 | +#### How to Decide when to collect adoption and violation | 
|  | 103 | + | 
|  | 104 | +The collection of adoption, violation, and exemption should be at the same component level - i.e., the same jsonPath level. | 
|  | 105 | + | 
|  | 106 | +Use | 
|  | 107 | + | 
|  | 108 | +- [collectAndReturnViolation(jsonPath, ruleName, errorData)](https://github.com/mongodb/openapi/blob/cd4e085a68cb3bb6078e85dba85ad8ce1674f7da/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js#L14) for violation collection | 
|  | 109 | +- [collectAdoption(jsonPath,ruleName)](https://github.com/mongodb/openapi/blob/cd4e085a68cb3bb6078e85dba85ad8ce1674f7da/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js#L32) for adoption collection | 
|  | 110 | +- [collectException(object, ruleName, jsonPath)](https://github.com/mongodb/openapi/blob/cd4e085a68cb3bb6078e85dba85ad8ce1674f7da/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js#L32) for exception collection | 
|  | 111 | + | 
|  | 112 | +The rule developer should decide on which cases the rule will be considered as adopted and violated. | 
|  | 113 | + | 
|  | 114 | +- **Example**: IPA guideline - Enumeration values must be UPPER_SNAKE_CASE | 
|  | 115 | +- **Decision Process** | 
|  | 116 | + | 
|  | 117 | +Custom Spectral rule functions in the format of | 
|  | 118 | + | 
|  | 119 | +``` | 
|  | 120 | +export default (input, _, { path, documentInventory }) | 
|  | 121 | +``` | 
|  | 122 | + | 
|  | 123 | +- `input`: When the Spectral rule is processed according to the given and field parameters of the rule definition, input is the current component being processed. | 
|  | 124 | +- `path`: JSONPath to current input | 
|  | 125 | +- `documentInventory`: Whole OpenAPI spec (retrieve resolved or unresolved according to the need) | 
|  | 126 | + | 
|  | 127 | +When implementing a custom Spectral rule, please follow these conventions: | 
|  | 128 | + | 
|  | 129 | +- If adoption, violation, and exception data should be collected at the same component level, ensure all helper functions use the same `jsonPath`. | 
|  | 130 | +  This path is either passed directly as the Spectral rule function's parameter or derived from it. | 
|  | 131 | +- Use this `jsonPath` consistently in: | 
|  | 132 | + | 
|  | 133 | +  - `collectAndReturnViolation` | 
|  | 134 | +  - `collectAdoption` | 
|  | 135 | +  - `collectException` | 
|  | 136 | + | 
|  | 137 | +- Input assumptions: | 
|  | 138 | +  The custom rule function assumes its input is never undefined. You do not need to validate the presence or structure of the input parameter. | 
|  | 139 | + | 
|  | 140 | +- Expected behavior: A rule must collect exactly one of: | 
|  | 141 | + | 
|  | 142 | +  - an adoption, | 
|  | 143 | +  - a violation, or | 
|  | 144 | +  - an exception. | 
|  | 145 | + | 
|  | 146 | +- Violations can include multiple error messages, if needed. | 
|  | 147 | +  In that case, gather all messages into an array and pass it to `collectAndReturnViolation`, which is responsible for displaying messages to users. | 
|  | 148 | + | 
|  | 149 | +💡 Example: | 
|  | 150 | + | 
|  | 151 | +If you're validating an `enum` and want to highlight each invalid value, collect the individual error messages into an array and pass it to `collectAndReturnViolation`. | 
|  | 152 | + | 
|  | 153 | +``` | 
|  | 154 | +const errors = [] | 
|  | 155 | +... | 
|  | 156 | +
 | 
|  | 157 | +errors.push({ | 
|  | 158 | +path: <jsonPath array for violation>, | 
|  | 159 | +message: <custom error message> | 
|  | 160 | +}); | 
|  | 161 | +
 | 
|  | 162 | +.... | 
|  | 163 | +
 | 
|  | 164 | +if (errors.length === 0) { | 
|  | 165 | +collectAdoption(jsonPath, RULE_NAME); | 
|  | 166 | +} else { | 
|  | 167 | +return collectAndReturnViolation(jsonPath, RULE_NAME, errors); | 
|  | 168 | +} | 
|  | 169 | +``` | 
|  | 170 | + | 
|  | 171 | +#### How to Decide the component level at which the rule will be processed | 
|  | 172 | + | 
|  | 173 | +When designing a rule, think from the custom rule function’s perspective. | 
|  | 174 | +Consider which `input` and `jsonPath` values will be most helpful for accurately evaluating the rule and collecting adoption, violation, or exception data. | 
|  | 175 | + | 
|  | 176 | +High-level pseudocode for custom rule functions: | 
|  | 177 | + | 
|  | 178 | +``` | 
|  | 179 | +const RULE_NAME = 'xgen-IPA-xxx-rule-name' | 
|  | 180 | +
 | 
|  | 181 | +export default (input, opts, { path, documentInventory }) => { | 
|  | 182 | +
 | 
|  | 183 | +// TODO Optional filter cases that we do not want to handle | 
|  | 184 | +// Return no response for those cases. | 
|  | 185 | +
 | 
|  | 186 | +//Decide the jsonPath (component level) at which you want to collect exceptions, adoption, and violation | 
|  | 187 | +//It can be "path" parameter of custom rule function | 
|  | 188 | +//Or, a derived path from "path" parameter | 
|  | 189 | +if (hasException(input, RULE_NAME)) { | 
|  | 190 | +collectException(input, RULE_NAME, jsonPath); | 
|  | 191 | +return; | 
|  | 192 | +} | 
|  | 193 | +
 | 
|  | 194 | +errors = checkViolationsAndReturnErrors(...); | 
|  | 195 | +if (errors.length != 0) { | 
|  | 196 | +return collectAndReturnViolation(jsonPath, RULE_NAME, errors); | 
|  | 197 | +} | 
|  | 198 | +return collectAdoption(jsonPath, RULE_NAME); | 
|  | 199 | +}; | 
|  | 200 | +
 | 
|  | 201 | +//This function can accept "input", "path", "documentInventory", or other derived parameters | 
|  | 202 | +function checkViolationsAndReturnErrors(...){ | 
|  | 203 | +try { | 
|  | 204 | +const errors = [] | 
|  | 205 | +// TODO validate errors | 
|  | 206 | +return errors; | 
|  | 207 | +} catch(e) { | 
|  | 208 | +handleInternalError(RULE_NAME, jsonPathArray, e); | 
|  | 209 | +} | 
|  | 210 | +} | 
|  | 211 | +``` | 
|  | 212 | + | 
|  | 213 | +Each rule must specify the `given` and `then` fields to determine how the rule will traverse and evaluate the OpenAPI document. | 
|  | 214 | + | 
|  | 215 | +**Case 1**: The rule evaluates an object as a whole | 
|  | 216 | + | 
|  | 217 | +- If the given parameter targets a specific object (e.g., HTTP methods like get, post, etc.), and we want to pass that object in its entirety to the rule function: | 
|  | 218 | +  - The rule function parameters will be: | 
|  | 219 | +    - `input`: The object for the current `<pathKey>` the rule is processing | 
|  | 220 | +    - `path`: `[‘paths’, ‘<pathKey>’, ‘get’]` | 
|  | 221 | + | 
|  | 222 | +``` | 
|  | 223 | +xgen-IPA-xxx-rule-name: | 
|  | 224 | +description: "Rule description" | 
|  | 225 | +message: "{{error}} http:://go/ipa/x" | 
|  | 226 | +severity: warn | 
|  | 227 | +given: '$.paths[*].get' | 
|  | 228 | +then: | 
|  | 229 | +function: "customRuleFunction" | 
|  | 230 | +``` | 
|  | 231 | + | 
|  | 232 | +**Case 2**: The rule evaluates keys of an object | 
|  | 233 | + | 
|  | 234 | +If the given parameter refers to an object, and we want to iterate through its keys (e.g., top-level API paths), use `@key` to pass each key (string) as the input. | 
|  | 235 | + | 
|  | 236 | +- `input`: API endpoint path string such as `/api/atlas/v2/groups` | 
|  | 237 | +- `path`: `[‘paths’, ‘/api/atlas/v2/groups’]` | 
|  | 238 | + | 
|  | 239 | +``` | 
|  | 240 | +xgen-IPA-xxx-rule-name: | 
|  | 241 | +description: "Rule description" | 
|  | 242 | +message: "{{error}} http:://go/ipa/x" | 
|  | 243 | +severity: warn | 
|  | 244 | +given: '$.paths' | 
|  | 245 | +then: | 
|  | 246 | +field: @key | 
|  | 247 | +function: "customRuleFunction" | 
|  | 248 | +``` | 
0 commit comments