feat: initial Spectral integration #195

jorge-ibm · 2020-09-09T20:03:11Z

Initial integration of Spectral into the IBM Validator.
Changes:

Spectral is now ran as part of the validator execution. Any results found through spectral are then merged with the results found through the IBM Validator.
A default Spectral ruleset .defaultsForSpectral.yaml was created and includes all the Spectral rules for Swagger/OAS3 that are recommended by default, and don't overlap with any of the Validator's rules. Any future rules can be easily added from this file.
Added logic for Spectral to use a .spectral.yaml file if it's found in the cwd to give users the ability to specify their own custom Spectral ruleset.
Added several unit tests to ensure none of the enabled Spectral rules overlap with our validator's rules.
Fixed minor typo in an existing rule: securitySchemas -> securitySchemes

Given that the results from Spectral are just "fed in" to the validator, and the current validator logic handles the logic from there, I did not find any issues running any of the validator's cmd line options with this new integration.

CLAassistant · 2020-09-09T20:03:15Z

All committers have signed the CLA.

jorge-ibm · 2020-09-09T20:19:32Z

src/spectral/rulesets/.defaultsForSpectral.json

@@ -0,0 +1,27 @@
+{


If we want to start with a smaller ruleset, or no rules at all, we can also do that to minimize the blast radius of this change. Either option would still give users the ability to write their own rules.

src/spectral/utils/spectral-validator.js

jorge-ibm · 2020-09-10T18:28:05Z

src/plugins/validation/2and3/semantic-validators/parameters-ibm.js

@@ -92,7 +92,7 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) {
        let messageAuth =
          'Parameters must not explicitly define `Authorization`.';
        messageAuth = isOAS3
-          ? `${messageAuth} Rely on the \`securitySchemas\` and \`security\` fields to specify authorization methods.`
+          ? `${messageAuth} Rely on the \`securitySchemes\` and \`security\` fields to specify authorization methods.`


Minor typo Phil pointed out in our existing rule. This should be securitySchemes in OAS3

codecov · 2020-09-10T18:30:57Z

Codecov Report

❗ No coverage uploaded for pull request head (beta/integrate-spectral@0aeaadc). Click here to learn what that means.
The diff coverage is n/a.

dpopp07

Leaving an initial review of some things to clean up in the code while others are looking at the PR. It looks great so far but I'm going to keep looking at it

src/cli-validator/runValidator.js

dpopp07 · 2020-09-10T20:48:55Z

src/spectral/rulesets/.defaultsForSpectral.json

+		"no-eval-in-markdown": true,
+		"no-script-tags-in-markdown": true,
+		"openapi-tags": true,
+		"operation-description": true,


Do any of these conflict with our rules? Some of these look like things we might throw an error for - I wouldn't want there to be any duplicate warning/error messages

They shouldn't. I took care to try to test each rule I was enabling and making sure there wasn't duplicate output. Though if there's a rule we think is a duplicate, then it's possible I might've overlooked it.

Cool, that sounds good. I just wanted to double check

src/spectral/rulesets/.defaultsForSpectral.json

src/spectral/utils/spectral-validator.js

test/spectral/tests/disabled-rules.test.js

test/spectral/tests/enabled-rules.test.js

jorge-ibm · 2020-09-11T21:31:02Z

I realized that I missed updating the code to also include Spectral in api invocations of the validator. I'll submit some changes soon to make sure that's accounted for.

jorge-ibm · 2020-09-16T15:06:47Z

@mkistler @dpopp07 this is ready for re-review

src/spectral/rulesets/.defaultsForSpectral.json

mkistler

This looks good but I recommend we allow users to provide their Spectral config in yaml format.

src/cli-validator/utils/processConfiguration.js

jorge-ibm · 2020-09-18T18:24:45Z

@mkistler I just pushed a commit to change the default ruleset to yaml, and allow for custom ruleset files to be either yaml or json.

dpopp07

Looking good but I have a few more comments to be addressed before merging

dpopp07 · 2020-09-15T19:38:50Z

src/lib/index.js

+      const readFile = util.promisify(fs.readFile);
+      input = await readFile(input, 'utf8');
+      input = preprocessFile(input);
+    }


I'm unsure about this section - we've never supported file input through the JS API and I don't really think we should start unless specifically requested. It's the job of the user to load their files as JSON to pass to the validator

I guess I'm confused about the documentation for the api here, is openApiDoc just an object in that example and not a path to a file ?

Edit: I guess I should've payed more attention to the documentation:

openApiDoc Type: Object An object that represents an OpenAPI document.

Though I do want to point out that when I ran it against a file without my changes it worked fine which is way I guess I made that assumption 😄

That's interesting that it worked fine - I think it's not "technically" supported though. So I don't think we need explicit code here for it

src/cli-validator/utils/processConfiguration.js

dpopp07 · 2020-09-18T19:07:00Z

src/cli-validator/utils/processConfiguration.js

+        ruleSetFile = await findUp(file);
+      }
+    }
+  } catch (err) {


This is an extremely minor comment but since you aren't using the error at all, I think it might be better style to just use catch ()

I had to change this to catch { }

dpopp07 · 2020-09-18T19:32:26Z

src/spectral/utils/spectral-validator.js

+              JSON.stringify(validationResult)
+            );
+          }
+          continue;


Correct me if I'm wrong but this continue statement looks superfluous - there's no more code after it

Yeah I don't think that's doing anything. I'll remove it.

test/spectral/tests/spectral-validator.test.js

dpopp07

Looks good! 👍

dpopp07 · 2020-09-23T14:20:44Z

src/spectral/utils/spectral-validator.js

+            //warning
+            messages.addMessage(path, message, 'warning');
+          } else if (severity === 0) {
+            //error


Very picky style comment, comments should have a space after the "//" here and on line 28. It doesn't really matter but I'm surprised eslint didn't complain

# [0.30.0](v0.29.4...v0.30.0) (2020-09-23) ### Features * initial Spectral integration ([#195](#195)) ([ccbc7a6](ccbc7a6))

dpopp07 · 2020-09-23T15:59:26Z

🎉 This PR is included in version 0.30.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

jorge-ibm force-pushed the beta/integrate-spectral branch from d0c1a1a to ee2201f Compare September 9, 2020 20:04

jorge-ibm requested review from dpopp07, mkistler, hudlow, padamstx and rmkeezer September 9, 2020 20:16

jorge-ibm commented Sep 9, 2020

View reviewed changes

src/spectral/utils/spectral-validator.js Outdated Show resolved Hide resolved

jorge-ibm force-pushed the beta/integrate-spectral branch from ee2201f to 074523b Compare September 10, 2020 18:26

jorge-ibm commented Sep 10, 2020

View reviewed changes

dpopp07 requested changes Sep 10, 2020

View reviewed changes

jorge-ibm requested review from dpopp07, mkistler, hudlow and padamstx and removed request for rmkeezer, mkistler, hudlow and padamstx September 11, 2020 22:31

jorge-ibm added the ready-for-review label Sep 14, 2020

mkistler reviewed Sep 18, 2020

View reviewed changes

src/spectral/rulesets/.defaultsForSpectral.json Outdated Show resolved Hide resolved

mkistler approved these changes Sep 18, 2020

View reviewed changes

src/cli-validator/utils/processConfiguration.js Outdated Show resolved Hide resolved

jorge-ibm force-pushed the beta/integrate-spectral branch from b6c99f8 to 6bfc910 Compare September 18, 2020 18:26

dpopp07 reviewed Sep 18, 2020

View reviewed changes

jorge-ibm removed the ready-for-review label Sep 22, 2020

jorge-ibm requested a review from dpopp07 September 22, 2020 19:15

jorge-ibm added the ready-for-review label Sep 22, 2020

dpopp07 approved these changes Sep 23, 2020

View reviewed changes

jorge-ibm added 4 commits September 23, 2020 09:46

feat: initial Spectral integration

4bed5cd

PR feedback, test enhancements, & including spectral in api invocation

bf56014

use default yml ruleset file & support yml custom rulesets

f67b774

More PR feedback. Improve unit test performance

0aeaadc

jorge-ibm force-pushed the beta/integrate-spectral branch from c256fbe to 0aeaadc Compare September 23, 2020 14:55

jorge-ibm merged commit ccbc7a6 into master Sep 23, 2020

dpopp07 pushed a commit that referenced this pull request Sep 23, 2020

chore(release): 0.30.0 [skip ci]

de621f6

# [0.30.0](v0.29.4...v0.30.0) (2020-09-23) ### Features * initial Spectral integration ([#195](#195)) ([ccbc7a6](ccbc7a6))

dpopp07 added the released label Sep 23, 2020

dpopp07 deleted the beta/integrate-spectral branch September 23, 2020 16:15

rmkeezer mentioned this pull request Oct 2, 2020

feat: allow spectral rules to be defined in the validaterc #202

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: initial Spectral integration #195

feat: initial Spectral integration #195

jorge-ibm commented Sep 9, 2020 •

edited

CLAassistant commented Sep 9, 2020 •

edited

jorge-ibm Sep 9, 2020

jorge-ibm Sep 10, 2020 •

edited

codecov bot commented Sep 10, 2020 •

edited

dpopp07 left a comment

dpopp07 Sep 10, 2020

jorge-ibm Sep 10, 2020

dpopp07 Sep 10, 2020

jorge-ibm commented Sep 11, 2020

jorge-ibm commented Sep 16, 2020

mkistler left a comment

jorge-ibm commented Sep 18, 2020 •

edited

dpopp07 left a comment

dpopp07 Sep 15, 2020

jorge-ibm Sep 18, 2020 •

edited

dpopp07 Sep 18, 2020

dpopp07 Sep 18, 2020

jorge-ibm Sep 22, 2020

dpopp07 Sep 18, 2020

jorge-ibm Sep 22, 2020

dpopp07 left a comment

dpopp07 Sep 23, 2020

dpopp07 commented Sep 23, 2020

feat: initial Spectral integration #195

feat: initial Spectral integration #195

Conversation

jorge-ibm commented Sep 9, 2020 • edited

CLAassistant commented Sep 9, 2020 • edited

Choose a reason for hiding this comment

jorge-ibm Sep 10, 2020 • edited

Choose a reason for hiding this comment

codecov bot commented Sep 10, 2020 • edited

Codecov Report

dpopp07 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorge-ibm commented Sep 11, 2020

jorge-ibm commented Sep 16, 2020

mkistler left a comment

Choose a reason for hiding this comment

jorge-ibm commented Sep 18, 2020 • edited

dpopp07 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorge-ibm Sep 18, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpopp07 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpopp07 commented Sep 23, 2020

jorge-ibm commented Sep 9, 2020 •

edited

CLAassistant commented Sep 9, 2020 •

edited

jorge-ibm Sep 10, 2020 •

edited

codecov bot commented Sep 10, 2020 •

edited

jorge-ibm commented Sep 18, 2020 •

edited

jorge-ibm Sep 18, 2020 •

edited