diff --git a/PO039-test.js b/PO039-test.js new file mode 100644 index 0000000..67c0961 --- /dev/null +++ b/PO039-test.js @@ -0,0 +1,105 @@ +/* + Copyright 2019-2024 Google LLC + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +const testID = "PO039", + assert = require("assert"), + fs = require("fs"), + path = require("path"), + bl = require("../../lib/package/bundleLinter.js"), + plugin = require(bl.resolvePlugin(testID)), + debug = require("debug")("apigeelint:" + testID), + Policy = require("../../lib/package/Policy.js"), + Dom = require("@xmldom/xmldom").DOMParser, + rootDir = path.resolve(__dirname, "../fixtures/resources/PO039"); + +const test = (suffix, cb) => { + const filename = `ML-test-${suffix}.xml`; + it(`should correctly process ${filename}`, () => { + const fqfname = path.join(rootDir, filename), + policyXml = fs.readFileSync(fqfname, "utf-8"), + doc = new Dom().parseFromString(policyXml), + p = new Policy(rootDir, filename, this, doc); + + p.getElement = () => doc.documentElement; + + //plugin.onBundle({ profile: "apigee" }); + + plugin.onPolicy(p, (e, foundIssues) => { + assert.equal(e, undefined, "should be undefined"); + cb(p, foundIssues); + }); + }); +}; + +describe(`${testID} - MessageLogging RessourceType element`, () => { + // test all the valid cases + fs.readdirSync(rootDir) + .map((shortFileName) => { + let m = shortFileName.match("^.+-(valid.+)\\.xml$"); + if (m) { + return m[1]; + } + }) + .filter((suffix) => suffix) + .forEach((suffix) => { + test(suffix, (p, foundIssues) => { + const messages = p.getReport().messages; + assert.ok(messages, "messages undefined"); + debug(messages); + assert.equal(foundIssues, false); + }); + }); + + test("invalid1", (p, foundIssues) => { + assert.equal(foundIssues, true); + const messages = p.getReport().messages; + assert.ok(messages, "messages undefined"); + debug(messages); + assert.equal(messages.length, 1, "unexpected number of messages"); + assert.ok(messages[0].message, "did not find message 0"); + assert.equal( + messages[0].message, + "The value 'gce_instance' should not be used here. ResourceType should be 'api'", + ); + }); + + test("invalid2", (p, foundIssues) => { + assert.equal(foundIssues, true); + const messages = p.getReport().messages; + assert.ok(messages, "messages undefined"); + debug(messages); + assert.equal(messages.length, 1, "unexpected number of messages"); + assert.ok(messages[0].message, "did not find message 0"); + assert.equal( + messages[0].message, + "The value 'apigee.googleapis.com/Environment' should not be used here. ResourceType should be 'api'", + ); + }); + + test("invalid3", (p, foundIssues) => { + assert.equal(foundIssues, true); + const messages = p.getReport().messages; + assert.ok(messages, "messages undefined"); + debug(messages); + assert.equal(messages.length, 2, "unexpected number of messages"); + assert.ok(messages[0].message, "did not find message 0"); + assert.equal(messages[0].message, "Unsupported element 'NotKey'"); + assert.equal( + messages[1].message, + "Label is missing a required Element: Key.", + ); + }); +}); diff --git a/externalPlugins/EX-001-CheckForPoliciesWhileStreaming.js b/externalPlugins/EX-001-CheckForPoliciesWhileStreaming.js deleted file mode 100644 index ce05e4d..0000000 --- a/externalPlugins/EX-001-CheckForPoliciesWhileStreaming.js +++ /dev/null @@ -1,53 +0,0 @@ -const plugin = { - ruleId: "EX-001", - name: "Streaming", - message: "Check for policies while streaming is enabled", - fatal: false, - severity: 2, // 1 = warn, 2 = error - nodeType: "Bundle", - enabled: true -}; - -const onBundle = function(bundle, cb) { - let hadWarnErr=false, isProxyStreamingEnabled=false, isTargetStreamingEnabled=false; - const proxies = bundle.getProxyEndpoints(); - proxies.forEach((proxyEndpoint, _p) => { - const httpProxyConnection = proxyEndpoint.getHTTPProxyConnection(); - if(httpProxyConnection){ - let properties = httpProxyConnection.getProperties(); - if(properties!=null && (properties["request.streaming.enabled"]=="true" || properties["response.streaming.enabled"]=="true")){ - isProxyStreamingEnabled = true; - } - } - }); - const targets = bundle.getTargetEndpoints(); - targets.forEach((targetEndpoint, _t) => { - const httpTargetConnection = targetEndpoint.getHTTPTargetConnection(); - if(httpTargetConnection){ - let properties = httpTargetConnection.getProperties(); - if(properties!=null && (properties["request.streaming.enabled"]=="true" || properties["response.streaming.enabled"]=="true")){ - isTargetStreamingEnabled = true; - } - } - }); - - if(isProxyStreamingEnabled || isTargetStreamingEnabled){ - bundle.getPolicies().forEach(function(policy) { - if ((policy.getType() === "AssignMessage" || policy.getType() === "ExtractVariables") && policy.getSteps().length > 0) { - bundle.addMessage({ - plugin, - source: policy.getSource(), - line: policy.getElement().lineNumber, - column: policy.getElement().columnNumber, - message: "ExtractVariables/AssignMessage policies not allowed when streaming is enabled" - }); - hadWarnErr = true; - } - }); - } -}; - -module.exports = { - plugin, - onBundle -}; \ No newline at end of file diff --git a/externalPlugins/EX-PO001-CheckForPoliciesWhileStreaming.js b/externalPlugins/EX-PO001-CheckForPoliciesWhileStreaming.js new file mode 100644 index 0000000..4658fd0 --- /dev/null +++ b/externalPlugins/EX-PO001-CheckForPoliciesWhileStreaming.js @@ -0,0 +1,87 @@ +/* + Copyright 2019-2024 Google LLC + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +const plugin = { + ruleId: "EX-PO001", + name: "Streaming", + message: "Check for policies while streaming is enabled", + fatal: false, + severity: 2, // 1 = warn, 2 = error + nodeType: "Bundle", + enabled: true, +}; + +const onBundle = function (bundle, cb) { + let flagged = false, + isProxyStreamingEnabled = false, + isTargetStreamingEnabled = false; + const proxies = bundle.getProxyEndpoints(); + proxies.forEach((proxyEndpoint, _p) => { + const httpProxyConnection = proxyEndpoint.getHTTPProxyConnection(); + if (httpProxyConnection) { + let properties = httpProxyConnection.getProperties(); + if ( + properties && + (properties["request.streaming.enabled"] == "true" || + properties["response.streaming.enabled"] == "true") + ) { + isProxyStreamingEnabled = true; + } + } + }); + const targets = bundle.getTargetEndpoints(); + targets.forEach((targetEndpoint, _t) => { + const httpTargetConnection = targetEndpoint.getHTTPTargetConnection(); + if (httpTargetConnection) { + let properties = httpTargetConnection.getProperties(); + if ( + properties && + (properties["request.streaming.enabled"] == "true" || + properties["response.streaming.enabled"] == "true") + ) { + isTargetStreamingEnabled = true; + } + } + }); + + if (isProxyStreamingEnabled || isTargetStreamingEnabled) { + bundle.getPolicies().forEach(function (policy) { + if ( + (policy.getType() === "AssignMessage" || + policy.getType() === "ExtractVariables") && + policy.getSteps().length > 0 + ) { + bundle.addMessage({ + plugin, + source: policy.getSource(), + line: policy.getElement().lineNumber, + column: policy.getElement().columnNumber, + message: + "ExtractVariables/AssignMessage policies not allowed when streaming is enabled", + }); + flagged = true; + } + }); + } + if (typeof cb == "function") { + cb(null, flagged); + } +}; + +module.exports = { + plugin, + onBundle, +}; diff --git a/lib/package/Endpoint.js b/lib/package/Endpoint.js index 269545b..b86d3e0 100644 --- a/lib/package/Endpoint.js +++ b/lib/package/Endpoint.js @@ -41,7 +41,6 @@ function Endpoint(element, bundle, fname, bundletype) { this.httpProxyConnection = null; this.httpTargetConnection = null; this.report = { - //filePath: path.join(bundle.sourcePath, path.basename(fname)), filePath: lintUtil.effectivePath(bundle, fname), errorCount: 0, warningCount: 0, diff --git a/lib/package/HTTPProxyConnection.js b/lib/package/HTTPProxyConnection.js index bb8f859..238a552 100644 --- a/lib/package/HTTPProxyConnection.js +++ b/lib/package/HTTPProxyConnection.js @@ -1,5 +1,5 @@ /* - Copyright 2019 Google LLC + Copyright 2019,2024 Google LLC Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -21,7 +21,7 @@ function HTTPProxyConnection(element, parent) { this.element = element; } -HTTPProxyConnection.prototype.getName = function() { +HTTPProxyConnection.prototype.getName = function () { if (!this.name) { var attr = xpath.select("//@name", this.element); this.name = (attr[0] && attr[0].value) || ""; @@ -29,15 +29,15 @@ HTTPProxyConnection.prototype.getName = function() { return this.name; }; -HTTPProxyConnection.prototype.getMessages = function() { +HTTPProxyConnection.prototype.getMessages = function () { return this.parent.getMessages(); }; -HTTPProxyConnection.prototype.getLines = function(start, stop) { +HTTPProxyConnection.prototype.getLines = function (start, stop) { return this.parent.getLines(start, stop); }; -HTTPProxyConnection.prototype.getSource = function() { +HTTPProxyConnection.prototype.getSource = function () { if (!this.source) { var start = this.element.lineNumber - 1, stop = this.element.nextSibling.lineNumber - 1; @@ -46,57 +46,57 @@ HTTPProxyConnection.prototype.getSource = function() { return this.source; }; -HTTPProxyConnection.prototype.getType = function() { +HTTPProxyConnection.prototype.getType = function () { return this.element.tagName; }; -HTTPProxyConnection.prototype.getBasePath = function() { +HTTPProxyConnection.prototype.getBasePath = function () { if (!this.basePath) { - //find the preflow tag var doc = xpath.select("./BasePath", this.element); if (doc && doc[0]) { - this.basePath = - (doc && doc[0] && doc[0].childNodes[0].nodeValue) || ""; + this.basePath = (doc && doc[0] && doc[0].childNodes[0].nodeValue) || ""; } } return this.basePath; }; - - -HTTPProxyConnection.prototype.getElement = function() { +HTTPProxyConnection.prototype.getElement = function () { return this.element; }; -HTTPProxyConnection.prototype.getParent = function() { +HTTPProxyConnection.prototype.getParent = function () { return this.parent; }; -HTTPProxyConnection.prototype.addMessage = function(msg) { +HTTPProxyConnection.prototype.addMessage = function (msg) { if (!msg.hasOwnProperty("entity")) { msg.entity = this; } this.parent.addMessage(msg); }; -HTTPProxyConnection.prototype.getProperties = function() { - var props = new Map(); - if (!this.properties) { - var propsNodeList = xpath.select("./Properties", this.element)[0].childNodes; - Array.from(propsNodeList).forEach(function(prop) { - if (prop.childNodes){ - props[prop.attributes[0].nodeValue]=prop.childNodes[0].nodeValue; +HTTPProxyConnection.prototype.getProperties = function () { + if (this.properties == undefined) { + let props = {}; + this.properties = props; + let propsNodeList = xpath.select("./Properties", this.element); + if (propsNodeList && propsNodeList[0]) { + Array.from(propsNodeList[0].childNodes).forEach((prop) => { + if (prop.childNodes && prop.attributes[0] && prop.childNodes[0]) { + props[prop.attributes[0].nodeValue] = prop.childNodes[0].nodeValue; } - }); + }); + } } - return props; + return this.properties; }; -HTTPProxyConnection.prototype.summarize = function() { - var summary = {}; - summary.name = this.getName(); - summary.basePath = this.getBasePath(); - return summary; +HTTPProxyConnection.prototype.summarize = function () { + // not sure this is ever used + return { + name: this.getName(), + basePath: this.getBasePath(), + }; }; //Public diff --git a/lib/package/Policy.js b/lib/package/Policy.js index a5a458a..7a5ac50 100644 --- a/lib/package/Policy.js +++ b/lib/package/Policy.js @@ -37,12 +37,12 @@ function Policy(realpath, fname, bundle, doc) { fixableWarningCount: 0, messages: [], }; + this.name = null; } Policy.prototype.getName = function () { - if (!this.name) { - let attr = xpath.select("//@name", this.getElement()); - this.name = (attr[0] && attr[0].value) || ""; + if (this.name == null) { + this.name = xpath.select("string(/*/@name)", this.getElement()); } return this.name; }; diff --git a/lib/package/bundleLinter.js b/lib/package/bundleLinter.js index 538e4e1..ad2908c 100644 --- a/lib/package/bundleLinter.js +++ b/lib/package/bundleLinter.js @@ -57,17 +57,55 @@ function exportReport(providedPath, stringifiedReport) { }); } -const getPluginPath = () => path.resolve(path.join(__dirname, "plugins")); +const internalPluginIdRe = new RegExp("^[A-Z]{2}[0-9]{3}$"); +const internalPluginFileRe = new RegExp("^[A-Z]{2}[0-9]{3}-.+\\.js$"); +const externalPluginIdRe = new RegExp("^EX-[A-Z]{2}[0-9]{3}$"); +const externalPluginFileRe = new RegExp("^EX-[A-Z]{2}[0-9]{3}-.+\\.js$"); +const nonlibrarySourceRe = new RegExp("^_.+\\.js$"); -const listPlugins = () => fs.readdirSync(getPluginPath()).filter(resolvePlugin); +/* exposed for testing */ +const getPluginResolver = (d, isInternal) => (idOrFilename) => { + if (idOrFilename.endsWith(".js")) { + if (idOrFilename.indexOf("/") < 0) { + if (!nonlibrarySourceRe.test(idOrFilename)) { + debug(`resolvePlugin file(${idOrFilename}) , prepending path...`); + return path.resolve(d, idOrFilename); + } else return null; + } else { + return idOrFilename; + } + } else { + let re = isInternal ? internalPluginIdRe : externalPluginIdRe; + if (re.test(idOrFilename)) { + let p = fs + .readdirSync(path.resolve(d)) + .filter((p) => p.startsWith(idOrFilename) && p.endsWith(".js")); + if (p.length > 1) { + throw new Error("plugin conflict: " + JSON.stringify(p)); + } + return p.length ? path.resolve(d, p[0]) : null; + } + } + return null; +}; -const listRuleIds = () => listPlugins().map((s) => s.substring(0, 5)); +const getInternalPluginsPath = () => + path.resolve(path.join(__dirname, "plugins")); + +const resolveInternalPlugin = getPluginResolver(getInternalPluginsPath(), true); -const listExternalRuleIds = (externalDir) => +const listPlugins = () => fs - .readdirSync(externalDir) - .filter(resolvePlugin) - .map((s) => s.substring(0, 8)); + .readdirSync(getInternalPluginsPath()) + .filter((fname) => internalPluginFileRe.test(fname)); + +const listExternalPlugins = (d) => + fs.readdirSync(d).filter((fname) => externalPluginFileRe.test(fname)); + +const listRuleIds = () => listPlugins().map((s) => s.substring(0, 5)); + +const listExternalRuleIds = (d) => + listExternalPlugins(d).map((s) => s.substring(0, 8)); const listFormatters = () => fs @@ -75,37 +113,30 @@ const listFormatters = () => .filter((s) => s.endsWith(".js")); const lint = function (config, done) { - return new Bundle(config, function (bundle, err) { - if (err) { - return done ? done(null, err) : console.log(err); + return new Bundle(config, function (bundle, error) { + if (error) { + return done ? done(null, error) : console.log(error); } - // for each builtin plugin - const normalizedPath = getPluginPath(); - fs.readdirSync(normalizedPath).forEach(function (file) { - if (!config.plugins || contains(config.plugins, file)) { - try { - executePlugin(normalizedPath + "/" + file, bundle); - } catch (e) { - debug("plugin error: " + file + " " + e); - } - } - }); - - // for each external plugin - if (config.externalPluginsDirectory) { - fs.readdirSync(config.externalPluginsDirectory).forEach(function (file) { - if (!config.plugins || contains(config.plugins, file)) { + const executePlugins = (list, pluginsPath) => { + list.forEach(function (filename) { + if (!config.plugins || contains(config.plugins, filename)) { try { - executePlugin( - path.resolve(config.externalPluginsDirectory) + "/" + file, - bundle - ); + executePlugin(path.resolve(pluginsPath, filename), bundle); } catch (e) { - debug("plugin error: " + file + " " + e); + console.error("plugin error: " + filename + " " + e.stack); } } }); + }; + + executePlugins(listPlugins(), getInternalPluginsPath()); + + if (config.externalPluginsDirectory) { + executePlugins( + listExternalPlugins(config.externalPluginsDirectory), + config.externalPluginsDirectory, + ); } const formatter = config.formatter || "json.js", @@ -124,31 +155,25 @@ const lint = function (config, done) { exportReport(config.writePath, formattedReport); } - if (done) { - done(bundle, null); - } - - // Exit code should return 1 when there are errors + // Exit code should return 1 when there are errors or too many warnings if (typeof config.setExitCode == "undefined" || config.setExitCode) { - bundle.getReport().some(function (value) { - if (value.errorCount > 0) { - process.exitCode = 1; - return; - } - }); + process.exitCode = bundle + .getReport() + .find((value) => value.errorCount > 0) + ? 1 + : 0; - // Exit code should return 1 when more than maximum number of warnings allowed - if (config.maxWarnings >= 0) { - let warningCount = 0; - bundle + if (!process.exitCode && config.maxWarnings >= 0) { + let warningCount = bundle .getReport() - .forEach((report) => (warningCount += report.warningCount)); + .reduce((a, item) => a + item.warningCount, 0); if (warningCount > config.maxWarnings) { process.exitCode = 1; - return; } } } + + return done ? done(bundle, null) : null; }); }; @@ -172,7 +197,7 @@ var getFormatter = function (format) { } catch (ex) { throw new Error( `There was a problem loading formatter: ${formatterPath}`, - { cause: ex } + { cause: ex }, ); } } else { @@ -183,34 +208,9 @@ var getFormatter = function (format) { const bfnName = (term) => term == "Bundle" ? "onBundle" : pluralize("on" + term, 2); -const pluginIdRe1 = new RegExp("^[A-Z]{2}[0-9]{3}$"); -const pluginIdRe2 = new RegExp("^_.+.js$"); - -/* exposed for testing */ -const resolvePlugin = (idOrFilename) => { - if (idOrFilename.endsWith(".js")) { - if (idOrFilename.indexOf("/") < 0) { - if (!pluginIdRe2.test(idOrFilename)) { - debug(`resolvePlugin file(${idOrFilename}) , prepending path...`); - return path.resolve(getPluginPath(), idOrFilename); - } else return null; - } else { - return idOrFilename; - } - } else if (pluginIdRe1.test(idOrFilename)) { - let p = fs - .readdirSync(path.resolve(getPluginPath())) - .filter((p) => p.startsWith(idOrFilename) && p.endsWith(".js")); - if (p.length > 1) { - throw new Error("plugin conflict: " + JSON.stringify(p)); - } - return p.length ? path.resolve(getPluginPath(), p[0]) : null; - } - return null; -}; - const executePlugin = function (file, bundle) { - let pluginPath = resolvePlugin(file); + const stats = fs.existsSync(file) && fs.statSync(file); + let pluginPath = stats && stats.isFile() ? file : resolveInternalPlugin(file); if (pluginPath) { debug(`executePlugin file(${pluginPath})`); @@ -230,7 +230,7 @@ const executePlugin = function (file, bundle) { "Resource", "Policy", "FaultRule", - "DefaultFaultRule" + "DefaultFaultRule", ]; entityTypes.forEach((etype) => { @@ -247,12 +247,11 @@ const executePlugin = function (file, bundle) { module.exports = { lint, - getPluginPath, listPlugins, listRuleIds, listExternalRuleIds, listFormatters, executePlugin, - resolvePlugin, // for testing - getFormatter + resolvePlugin: resolveInternalPlugin, // for testing + getFormatter, }; diff --git a/lib/package/plugins/PD004-proxyNameAttr.js b/lib/package/plugins/PD004-proxyNameAttr.js index 40c5337..23acccd 100644 --- a/lib/package/plugins/PD004-proxyNameAttr.js +++ b/lib/package/plugins/PD004-proxyNameAttr.js @@ -48,7 +48,7 @@ const onProxyEndpoint = function (endpoint, cb) { }); flagged = true; } else if (basename !== nameFromAttribute) { - const nameAttr = endpoint.select("//@name"); + const nameAttr = endpoint.select("/*/@name"); endpoint.addMessage({ plugin, line: nameAttr[0].lineNumber, diff --git a/lib/package/plugins/PO006-checkPolicyNameAttribute.js b/lib/package/plugins/PO006-checkPolicyNameAttribute.js index 3f38959..f1574f5 100644 --- a/lib/package/plugins/PO006-checkPolicyNameAttribute.js +++ b/lib/package/plugins/PO006-checkPolicyNameAttribute.js @@ -1,5 +1,5 @@ /* - Copyright 2019-2020 Google LLC + Copyright 2019-2020,2024 Google LLC Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -32,7 +32,7 @@ const onPolicy = function(policy, cb) { flagged = false; if (shortFilename !== nameFromAttribute) { - let nameAttr = policy.select('//@name'); + let nameAttr = policy.select('/*/@name'); policy.addMessage({ plugin, line: nameAttr[0].lineNumber, diff --git a/lib/package/plugins/PO039-messageLogging-responseType.js b/lib/package/plugins/PO039-messageLogging-responseType.js index 4aec269..00898db 100644 --- a/lib/package/plugins/PO039-messageLogging-responseType.js +++ b/lib/package/plugins/PO039-messageLogging-responseType.js @@ -40,7 +40,7 @@ const onPolicy = function (policy, cb) { ); try { if (clElements && clElements[0]) { - debug(`found ${clElements.length} response element(s)`); + debug(`found ${clElements.length} CloudLogging element(s)`); if (clElements[1]) { hadWarning = true; policy.addMessage({ diff --git a/test/fixtures/resources/PO039/ML-test-invalid4.xml b/test/fixtures/resources/PO039/ML-test-invalid4.xml new file mode 100644 index 0000000..73a85aa --- /dev/null +++ b/test/fixtures/resources/PO039/ML-test-invalid4.xml @@ -0,0 +1,18 @@ + + + projects/{organization.name}/logs/{proxylog} + { "foo" : "bar"} + + + projects/{organization.name}/logs/{proxylog} + { + "organization": "{organization.name}", + "environment": "{environment.name}", + "apiproxy": "{apiproxy.name}", + "revision": "{apiproxy.revision}", + "messageid": "{messageid}", + "url": "{proxy.url}" + } + + + diff --git a/test/specs/testBundle.js b/test/specs/testBundle.js index dd86a9a..5915f1a 100644 --- a/test/specs/testBundle.js +++ b/test/specs/testBundle.js @@ -49,7 +49,7 @@ describe("addMessage", function () { bundle.addMessage({ plugin, message }); bundle.getReport((report) => { - console.log(JSON.stringify(report, null, 2)); + //console.log(JSON.stringify(report, null, 2)); let bundleResult = report.find( (element) => element.filePath === proxyPath,