🏗 add amp check-json-schemas task to validate JSON files in PRs (#3…

…6350) Co-authored-by: Raghu Simha <rsimha@amp.dev>
ampproject · Oct 19, 2021 · f57e57f · f57e57f
1 parent 7fb31fe
commit f57e57f
Show file tree

Hide file tree

Showing 14 changed files with 405 additions and 72 deletions.
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -20,5 +20,35 @@
   "[yaml]": {"editor.formatOnSave": true},
   "[json]": {"editor.formatOnSave": true},
   "[json5]": {"editor.formatOnSave": true},
-  "[markdown]": {"editor.formatOnSave": true}
+  "[markdown]": {"editor.formatOnSave": true},
+
+  // Validate JSON schemas on the fly. Also used by the `amp check-json-schemas`
+  // task. Do not use URLs from the web, they are unsupported by this task.
+  "json.schemas": [
+    {
+      "fileMatch": [
+        "build-system/global-configs/canary-config.json",
+        "build-system/global-configs/prod-config.json"
+      ],
+      "url": "./build-system/json-schemas/amp-config.json"
+    },
+    {
+      "fileMatch": ["build-system/tasks/bundle-size/APPROVERS.json"],
+      "url": "./build-system/json-schemas/APPROVERS.json"
+    },
+    {
+      "fileMatch": ["build-system/global-configs/caches.json"],
+      "url": "./build-system/json-schemas/caches.json"
+    },
+    {
+      "fileMatch": [
+        "build-system/global-configs/client-side-experiments-config.json"
+      ],
+      "url": "./build-system/json-schemas/client-side-experiments-config.json"
+    },
+    {
+      "fileMatch": ["build-system/tasks/bundle-size/filesize.json"],
+      "url": "./build-system/json-schemas/filesize.json"
+    }
+  ]
 }
diff --git a/amp.js b/amp.js
@@ -23,13 +23,13 @@ createTask('ava');
 createTask('babel-plugin-tests', 'babelPluginTests');
 createTask('build');
 createTask('bundle-size', 'bundleSize');
-createTask('caches-json', 'cachesJson');
 createTask('check-analytics-vendors-list', 'checkAnalyticsVendorsList');
 createTask('check-asserts', 'checkAsserts');
 createTask('check-build-system', 'checkBuildSystem');
 createTask('check-exact-versions', 'checkExactVersions');
 createTask('check-ignore-lists', 'checkIgnoreLists');
 createTask('check-invalid-whitespaces', 'checkInvalidWhitespaces');
+createTask('check-json-schemas', 'checkJsonSchemas');
 createTask('check-links', 'checkLinks');
 createTask('check-owners', 'checkOwners');
 createTask('check-renovate-config', 'checkRenovateConfig');

diff --git a/build-system/json-schemas/APPROVERS.json b/build-system/json-schemas/APPROVERS.json
@@ -0,0 +1,31 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "description": "@ampproject/filesize JSON config",
+  "type": "object",
+  "patternProperties": {
+    "^dist.+$": {
+      "type": "object",
+      "required": [
+        "approvers",
+        "threshold"
+      ],
+      "properties": {
+        "approvers": {
+          "type": "array",
+          "uniqueItems": true,
+          "minItems": 1,
+          "items": {
+            "type": "string",
+            "pattern": "^ampproject/wg-[\\w_-]+$"
+          }
+        },
+        "threshold": {
+          "type": "number",
+          "exclusiveMinimum": 0
+        }
+      },
+      "additionalProperties": false
+    }
+  },
+  "additionalProperties": false
+}
diff --git a/build-system/json-schemas/amp-config.json b/build-system/json-schemas/amp-config.json
@@ -0,0 +1,40 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "description": "AMP_CONFIG for canary/prod configurations",
+  "type": "object",
+  "required": [
+    "allow-doc-opt-in",
+    "allow-url-opt-in",
+    "canary"
+  ],
+  "properties": {
+    "allow-doc-opt-in": {
+      "type": "array",
+      "items": {
+        "type": "string",
+        "pattern": "^[\\w-]+$"
+      }
+    },
+    "allow-url-opt-in": {
+      "type": "array",
+      "items": {
+        "type": "string",
+        "pattern": "^[\\w-]+$"
+      }
+    },
+    "canary": {
+      "type": "number",
+      "enum": [
+        0,
+        1
+      ]
+    }
+  },
+  "patternProperties": {
+    "^(?!allow-doc-opt-in|allow-url-opt-in|canary)[\\w-]+$": {
+      "type": "number",
+      "minimum": 0,
+      "maximum": 1
+    }
+  }
+}
diff --git a/build-system/json-schemas/caches.json b/build-system/json-schemas/caches.json
@@ -0,0 +1,47 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "description": "List of known AMP caches",
+  "type": "object",
+  "required": [
+    "caches"
+  ],
+  "properties": {
+    "caches": {
+      "type": "array",
+      "items": {
+        "type": "object",
+        "required": [
+          "id",
+          "name",
+          "docs",
+          "cacheDomain",
+          "updateCacheApiDomainSuffix",
+          "thirdPartyFrameDomainSuffix"
+        ],
+        "properties": {
+          "id": {
+            "type": "string",
+            "pattern": "^[a-z0-9]+$"
+          },
+          "name": {
+            "type": "string"
+          },
+          "docs": {
+            "type": "string",
+            "format": "uri"
+          },
+          "cacheDomain": {
+            "type": "string"
+          },
+          "updateCacheApiDomainSuffix": {
+            "type": "string"
+          },
+          "thirdPartyFrameDomainSuffix": {
+            "type": "string"
+          }
+        }
+      }
+    }
+  },
+  "additionalProperties": false
+}
diff --git a/build-system/json-schemas/client-side-experiments-config.json b/build-system/json-schemas/client-side-experiments-config.json
@@ -0,0 +1,39 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "description": "Client-side (AMP_EXP) experiment definitions",
+  "type": "object",
+  "required": [
+    "experiments"
+  ],
+  "properties": {
+    "experiments": {
+      "type": "array",
+      "items": {
+        "type": "object",
+        "required": [
+          "name",
+          "percentage"
+        ],
+        "properties": {
+          "name": {
+            "type": "string",
+            "pattern": "^[A-Za-z][\\w-]*$"
+          },
+          "percentage": {
+            "type": "number",
+            "minimum": 0,
+            "maximum": 1
+          },
+          "rtvPrefixes": {
+            "type": "array",
+            "items": {
+              "type": "string",
+              "pattern": "^[\\d\\.]+$"
+            }
+          }
+        }
+      }
+    }
+  },
+  "additionalProperties": false
+}
diff --git a/build-system/json-schemas/filesize.json b/build-system/json-schemas/filesize.json
@@ -0,0 +1,39 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "description": "@ampproject/filesize JSON config",
+  "type": "object",
+  "required": [
+    "filesize"
+  ],
+  "properties": {
+    "filesize": {
+      "type": "object",
+      "required": [
+        "track"
+      ],
+      "properties": {
+        "track": {
+          "type": "array",
+          "uniqueItems": true,
+          "items": {
+            "type": "string"
+          }
+        },
+        "trackFormat": {
+          "type": "array",
+          "uniqueItems": true,
+          "items": {
+            "type": "string",
+            "enum": [
+              "brotli",
+              "gzip",
+              "none"
+            ]
+          }
+        }
+      },
+      "additionalProperties": false
+    }
+  },
+  "additionalProperties": false
+}
diff --git a/build-system/pr-check/build-targets.js b/build-system/pr-check/build-targets.js
@@ -7,6 +7,8 @@
  */
 const config = require('../test-configs/config');
 const fastGlob = require('fast-glob');
+const fs = require('fs');
+const json5 = require('json5');
 const minimatch = require('minimatch');
 const path = require('path');
 const {cyan} = require('kleur/colors');
@@ -25,6 +27,7 @@ let buildTargets;
  * Used to prevent the repeated expansion of globs during PR jobs.
  */
 const fileLists = {};
+const jsonFilesWithSchemas = [];
 
 /***
  * All of AMP's build targets that can be tested during CI.
@@ -35,14 +38,14 @@ const Targets = {
   AVA: 'AVA',
   BABEL_PLUGIN: 'BABEL_PLUGIN',
   BUILD_SYSTEM: 'BUILD_SYSTEM',
-  CACHES_JSON: 'CACHES_JSON',
   DEV_DASHBOARD: 'DEV_DASHBOARD',
   DOCS: 'DOCS',
   E2E_TEST: 'E2E_TEST',
   HTML_FIXTURES: 'HTML_FIXTURES',
   IGNORE_LIST: 'IGNORE_LIST',
   INTEGRATION_TEST: 'INTEGRATION_TEST',
   INVALID_WHITESPACES: 'INVALID_WHITESPACES',
+  JSON_FILES: 'JSON_FILES',
   LINT: 'LINT',
   LINT_RULES: 'LINT_RULES',
   OWNERS: 'OWNERS',
@@ -65,7 +68,6 @@ const Targets = {
  */
 const nonRuntimeTargets = [
   Targets.AVA,
-  Targets.CACHES_JSON,
   Targets.DEV_DASHBOARD,
   Targets.DOCS,
   Targets.E2E_TEST,
@@ -146,15 +148,6 @@ const targetMatchers = {
           file.endsWith('.json')))
     );
   },
-  [Targets.CACHES_JSON]: (file) => {
-    if (isOwnersFile(file)) {
-      return false;
-    }
-    return (
-      file == 'build-system/tasks/caches-json.js' ||
-      file == 'build-system/global-configs/caches.json'
-    );
-  },
   [Targets.DOCS]: (file) => {
     if (isOwnersFile(file)) {
       return false;
@@ -210,6 +203,12 @@ const targetMatchers = {
       file.startsWith('build-system/test-configs')
     );
   },
+  [Targets.JSON_FILES]: (file) => {
+    return (
+      jsonFilesWithSchemas.includes(file) ||
+      file == 'build-system/tasks/check-json-schemas.js'
+    );
+  },
   [Targets.LINT]: (file) => {
     if (isOwnersFile(file)) {
       return false;
@@ -398,6 +397,22 @@ function expandFileLists() {
     const fileListName = globName.replace('Globs', 'Files');
     fileLists[fileListName] = fastGlob.sync(config[globName], {dot: true});
   }
+
+  const vscodeSettings = json5.parse(
+    fs.readFileSync('.vscode/settings.json', 'utf8')
+  );
+  /** @type {Array<{fileMatch: string[], url: string}>} */
+  const schemas = vscodeSettings['json.schemas'];
+  const jsonGlobs = schemas.flatMap(({fileMatch, url}) => [
+    ...fileMatch,
+    path.normalize(url),
+  ]);
+  jsonFilesWithSchemas.push(
+    fastGlob.sync(jsonGlobs, {
+      dot: true,
+      ignore: ['**/node_modules'],
+    })
+  );
 }
 
 module.exports = {

diff --git a/build-system/pr-check/checks.js b/build-system/pr-check/checks.js
@@ -19,11 +19,11 @@ function pushBuildWorkflow() {
   timedExecOrDie('amp validate-html-fixtures');
   timedExecOrDie('amp lint');
   timedExecOrDie('amp prettify');
+  timedExecOrDie('amp check-json-schemas');
   timedExecOrDie('amp ava');
   timedExecOrDie('amp check-build-system');
   timedExecOrDie('amp check-ignore-lists');
   timedExecOrDie('amp babel-plugin-tests');
-  timedExecOrDie('amp caches-json');
   timedExecOrDie('amp check-exact-versions');
   timedExecOrDie('amp check-renovate-config');
   timedExecOrDie('amp server-tests');
@@ -69,6 +69,10 @@ function prBuildWorkflow() {
     timedExecOrDie('amp prettify');
   }
 
+  if (buildTargetsInclude(Targets.JSON_FILES)) {
+    timedExecOrDie('amp check-json-schemas');
+  }
+
   if (buildTargetsInclude(Targets.AVA)) {
     timedExecOrDie('amp ava');
   }
@@ -81,10 +85,6 @@ function prBuildWorkflow() {
     timedExecOrDie('amp babel-plugin-tests');
   }
 
-  if (buildTargetsInclude(Targets.CACHES_JSON)) {
-    timedExecOrDie('amp caches-json');
-  }
-
   if (buildTargetsInclude(Targets.DOCS)) {
     timedExecOrDie('amp check-links --local_changes'); // only for PR builds
     timedExecOrDie('amp markdown-toc');