diff --git a/README.md b/README.md index 86e047a..20b1fd6 100644 --- a/README.md +++ b/README.md @@ -335,8 +335,9 @@ This is the current list: |   |:white_check_mark:| TD005 | TargetEndpoint SSLInfo references | TargetEndpoint SSLInfo should use references for KeyStore and TrustStore. | |   |:white_check_mark:| TD006 | TargetEndpoint SSLInfo | When using a LoadBalancer, the SSLInfo should not be configured under HTTPTargetConnection. | |   |:white_check_mark:| TD007 | TargetEndpoint SSLInfo | TargetEndpoint HTTPTargetConnection SSLInfo should use TrustStore. | -|   |:white_check_mark:| TD008 | TargetEndpoint LoadBalancer Servers | LoadBalancer should not have duplicate Server entries or multiple Fallbacks. | +|   |:white_check_mark:| TD008 | TargetEndpoint LoadBalancer Servers | LoadBalancer should not have multiple IsFallback Server entries. | |   |:white_check_mark:| TD009 | TargetEndpoint LoadBalancer | TargetEndpoint HTTPTargetConnection should have at most one LoadBalancer. | +|   |:white_check_mark:| TD010 | TargetEndpoint LoadBalancer Servers | LoadBalancer should have at least one Server entry, and no duplicate Server entries. | | Flow |   |   |   |   | |   |:white_check_mark:| FL001 | Unconditional Flows | Only one unconditional flow will get executed. Error if more than one was detected. | | Step |   |   |   |   | diff --git a/lib/package/plugins/TD008-target-LB-multiple-fallbacks.js b/lib/package/plugins/TD008-target-LB-multiple-fallbacks.js new file mode 100644 index 0000000..4736f00 --- /dev/null +++ b/lib/package/plugins/TD008-target-LB-multiple-fallbacks.js @@ -0,0 +1,94 @@ +/* + 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 ruleId = require("../myUtil.js").getRuleId(), + xpath = require("xpath"); + +const plugin = { + ruleId, + name: "TargetEndpoint HTTPTargetConnection LoadBalancer with multiple fallback entries", + message: "Multiple Server entries with IsFallback=true", + fatal: false, + severity: 1, // 1 = warn, 2 = error + nodeType: "Endpoint", + enabled: true +}; + +const path = require("path"), + debug = require("debug")("apigeelint:" + ruleId); + +const onTargetEndpoint = function (endpoint, cb) { + const htc = endpoint.getHTTPTargetConnection(), + shortFilename = path.basename(endpoint.getFileName()); + let flagged = false; + + debug(`onTargetEndpoint shortfile(${shortFilename})`); + if (htc) { + try { + const loadBalancers = htc.select("LoadBalancer"); + debug(`loadBalancers(${loadBalancers.length})`); + + if (loadBalancers.length == 1) { + const loadBalancer = loadBalancers[0]; + // check multiple fallbacks + const fallbacks = xpath.select( + "Server[IsFallback = 'true']", + loadBalancer + ); + if (fallbacks.length > 1) { + endpoint.addMessage({ + plugin, + line: loadBalancers[0].lineNumber, + column: loadBalancers[0].columnNumber, + message: plugin.message + }); + flagged = true; + } + const servers = xpath.select("Server", loadBalancer); + if (servers.length == 1 && fallbacks.length == 1) { + endpoint.addMessage({ + plugin, + line: loadBalancers[0].lineNumber, + column: loadBalancers[0].columnNumber, + message: + "Only one server in a Load balancer; should not be marked IsFallback" + }); + flagged = true; + } + } + } catch (exc1) { + console.error("exception: " + exc1); + debug(`onTargetEndpoint exc(${exc1})`); + debug(`${exc1.stack}`); + endpoint.addMessage({ + plugin, + message: "Exception while processing: " + exc1 + }); + + flagged = true; + } + } + + if (typeof cb == "function") { + debug(`onTargetEndpoint isFlagged(${flagged})`); + cb(null, flagged); + } +}; + +module.exports = { + plugin, + onTargetEndpoint +}; diff --git a/lib/package/plugins/TD008-target-LB-servers.js b/lib/package/plugins/TD010-target-LB-duplicate-servers.js similarity index 91% rename from lib/package/plugins/TD008-target-LB-servers.js rename to lib/package/plugins/TD010-target-LB-duplicate-servers.js index dc82126..6ec7b68 100644 --- a/lib/package/plugins/TD008-target-LB-servers.js +++ b/lib/package/plugins/TD010-target-LB-duplicate-servers.js @@ -43,23 +43,16 @@ const onTargetEndpoint = function (endpoint, cb) { if (loadBalancers.length == 1) { const loadBalancer = loadBalancers[0]; - // check multiple fallbacks - const fallbacks = xpath.select( - "Server[IsFallback = 'true']", - loadBalancer - ); - if (fallbacks.length > 1) { + const servers = xpath.select("Server", loadBalancer); + if (servers.length == 0) { endpoint.addMessage({ plugin, - line: loadBalancers[0].lineNumber, - column: loadBalancers[0].columnNumber, - message: "Multiple Server entries with IsFallback=true" + line: loadBalancer.lineNumber, + column: loadBalancer.columnNumber, + message: "No Server elements within LoadBalancer" }); flagged = true; - } - - const servers = xpath.select("Server", loadBalancer); - if (servers.length > 1) { + } else if (servers.length > 1) { const previouslyDetected = []; servers.slice(0, -1).forEach((item, i) => { if (!previouslyDetected.includes(i)) { diff --git a/test/fixtures/resources/TD008/fail/t4.xml b/test/fixtures/resources/TD008/fail/t1-multiple-fallbacks.xml similarity index 100% rename from test/fixtures/resources/TD008/fail/t4.xml rename to test/fixtures/resources/TD008/fail/t1-multiple-fallbacks.xml diff --git a/test/fixtures/resources/TD008/fail/t1.xml b/test/fixtures/resources/TD008/fail/t1.xml deleted file mode 100644 index fdfa417..0000000 --- a/test/fixtures/resources/TD008/fail/t1.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - - - diff --git a/test/fixtures/resources/TD008/fail/t2-multiple-fallbacks-non-consecutive.xml b/test/fixtures/resources/TD008/fail/t2-multiple-fallbacks-non-consecutive.xml new file mode 100644 index 0000000..e86fff3 --- /dev/null +++ b/test/fixtures/resources/TD008/fail/t2-multiple-fallbacks-non-consecutive.xml @@ -0,0 +1,14 @@ + + + + + + true + + + + true + + + + diff --git a/test/fixtures/resources/TD008/fail/t3-one-server-marked-as-fallback.xml b/test/fixtures/resources/TD008/fail/t3-one-server-marked-as-fallback.xml new file mode 100644 index 0000000..284fea9 --- /dev/null +++ b/test/fixtures/resources/TD008/fail/t3-one-server-marked-as-fallback.xml @@ -0,0 +1,9 @@ + + + + + true + + + + diff --git a/test/fixtures/resources/TD008/fail/t3.xml b/test/fixtures/resources/TD008/fail/t3.xml deleted file mode 100644 index 04c8d18..0000000 --- a/test/fixtures/resources/TD008/fail/t3.xml +++ /dev/null @@ -1,12 +0,0 @@ - - - - - - - - - - - - diff --git a/test/fixtures/resources/TD008/pass/t1.xml b/test/fixtures/resources/TD008/pass/t1-two-servers-no-fallbacks.xml similarity index 100% rename from test/fixtures/resources/TD008/pass/t1.xml rename to test/fixtures/resources/TD008/pass/t1-two-servers-no-fallbacks.xml diff --git a/test/fixtures/resources/TD008/pass/t2.xml b/test/fixtures/resources/TD008/pass/t2-three-servers-no-fallbacks.xml similarity index 100% rename from test/fixtures/resources/TD008/pass/t2.xml rename to test/fixtures/resources/TD008/pass/t2-three-servers-no-fallbacks.xml diff --git a/test/fixtures/resources/TD008/pass/t3-one-server-no-fallbacks.xml b/test/fixtures/resources/TD008/pass/t3-one-server-no-fallbacks.xml new file mode 100644 index 0000000..c55e84d --- /dev/null +++ b/test/fixtures/resources/TD008/pass/t3-one-server-no-fallbacks.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/test/fixtures/resources/TD008/pass/t3.xml b/test/fixtures/resources/TD008/pass/t3.xml deleted file mode 100644 index 09b91c4..0000000 --- a/test/fixtures/resources/TD008/pass/t3.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - - - - diff --git a/test/fixtures/resources/TD008/pass/t4.xml b/test/fixtures/resources/TD008/pass/t4-three-servers-one-fallback.xml similarity index 100% rename from test/fixtures/resources/TD008/pass/t4.xml rename to test/fixtures/resources/TD008/pass/t4-three-servers-one-fallback.xml diff --git a/test/fixtures/resources/TD008/fail/t5.xml b/test/fixtures/resources/TD008/pass/t5-three-servers-no-fallbacks.xml similarity index 100% rename from test/fixtures/resources/TD008/fail/t5.xml rename to test/fixtures/resources/TD008/pass/t5-three-servers-no-fallbacks.xml diff --git a/test/fixtures/resources/TD008/pass/t6-many-servers-some-dupes-no-fallbacks.xml b/test/fixtures/resources/TD008/pass/t6-many-servers-some-dupes-no-fallbacks.xml new file mode 100644 index 0000000..29fb772 --- /dev/null +++ b/test/fixtures/resources/TD008/pass/t6-many-servers-some-dupes-no-fallbacks.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/test/fixtures/resources/TD008/pass/t7-three-servers-with-duplicate-no-fallbacks.xml b/test/fixtures/resources/TD008/pass/t7-three-servers-with-duplicate-no-fallbacks.xml new file mode 100644 index 0000000..0714aeb --- /dev/null +++ b/test/fixtures/resources/TD008/pass/t7-three-servers-with-duplicate-no-fallbacks.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/test/fixtures/resources/TD008/pass/t8-two-servers-duplicate-no-fallback.xml b/test/fixtures/resources/TD008/pass/t8-two-servers-duplicate-no-fallback.xml new file mode 100644 index 0000000..00ec265 --- /dev/null +++ b/test/fixtures/resources/TD008/pass/t8-two-servers-duplicate-no-fallback.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/test/fixtures/resources/TD010/fail/T5-no-servers.xml b/test/fixtures/resources/TD010/fail/T5-no-servers.xml new file mode 100644 index 0000000..b450040 --- /dev/null +++ b/test/fixtures/resources/TD010/fail/T5-no-servers.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/test/fixtures/resources/TD010/fail/t1-missing-name-attr.xml b/test/fixtures/resources/TD010/fail/t1-missing-name-attr.xml new file mode 100644 index 0000000..a81e7ef --- /dev/null +++ b/test/fixtures/resources/TD010/fail/t1-missing-name-attr.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/test/fixtures/resources/TD010/fail/t2-duplicate-server.xml b/test/fixtures/resources/TD010/fail/t2-duplicate-server.xml new file mode 100644 index 0000000..bd5080b --- /dev/null +++ b/test/fixtures/resources/TD010/fail/t2-duplicate-server.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/test/fixtures/resources/TD010/fail/t3-non-consecutive-duplicate.xml b/test/fixtures/resources/TD010/fail/t3-non-consecutive-duplicate.xml new file mode 100644 index 0000000..737e4fb --- /dev/null +++ b/test/fixtures/resources/TD010/fail/t3-non-consecutive-duplicate.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/test/fixtures/resources/TD010/fail/t4-dupes-in-random-order.xml b/test/fixtures/resources/TD010/fail/t4-dupes-in-random-order.xml new file mode 100644 index 0000000..29fb772 --- /dev/null +++ b/test/fixtures/resources/TD010/fail/t4-dupes-in-random-order.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/test/fixtures/resources/TD010/pass/t1-two-servers.xml b/test/fixtures/resources/TD010/pass/t1-two-servers.xml new file mode 100644 index 0000000..93cef39 --- /dev/null +++ b/test/fixtures/resources/TD010/pass/t1-two-servers.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/test/fixtures/resources/TD008/fail/t2.xml b/test/fixtures/resources/TD010/pass/t2-three-servers-no-fallback.xml similarity index 86% rename from test/fixtures/resources/TD008/fail/t2.xml rename to test/fixtures/resources/TD010/pass/t2-three-servers-no-fallback.xml index e6b3d6d..a2cc639 100644 --- a/test/fixtures/resources/TD008/fail/t2.xml +++ b/test/fixtures/resources/TD010/pass/t2-three-servers-no-fallback.xml @@ -3,7 +3,7 @@ - + diff --git a/test/fixtures/resources/TD010/pass/t3-one-server.xml b/test/fixtures/resources/TD010/pass/t3-one-server.xml new file mode 100644 index 0000000..7447b93 --- /dev/null +++ b/test/fixtures/resources/TD010/pass/t3-one-server.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/test/fixtures/resources/TD010/pass/t4-three-servers-with-fallback.xml b/test/fixtures/resources/TD010/pass/t4-three-servers-with-fallback.xml new file mode 100644 index 0000000..35c6f5e --- /dev/null +++ b/test/fixtures/resources/TD010/pass/t4-three-servers-with-fallback.xml @@ -0,0 +1,12 @@ + + + + + + + + true + + + + diff --git a/test/fixtures/resources/TD010/pass/t5-three-servers-multiple-fallbacks.xml b/test/fixtures/resources/TD010/pass/t5-three-servers-multiple-fallbacks.xml new file mode 100644 index 0000000..6802fb5 --- /dev/null +++ b/test/fixtures/resources/TD010/pass/t5-three-servers-multiple-fallbacks.xml @@ -0,0 +1,14 @@ + + + + + + + true + + + true + + + + diff --git a/test/fixtures/resources/TD010/pass/t6-three-servers-multiple-fallbacks.xml b/test/fixtures/resources/TD010/pass/t6-three-servers-multiple-fallbacks.xml new file mode 100644 index 0000000..d66e99e --- /dev/null +++ b/test/fixtures/resources/TD010/pass/t6-three-servers-multiple-fallbacks.xml @@ -0,0 +1,14 @@ + + + + + + true + + + + true + + + + diff --git a/test/specs/TD008-test.js b/test/specs/TD008-test.js index d1d9919..8528841 100644 --- a/test/specs/TD008-test.js +++ b/test/specs/TD008-test.js @@ -37,7 +37,7 @@ const loadEndpoint = (sourceDir, shortFileName) => { return endpoint; }; -describe(`${testID} - endpoint passes duplicate server check`, function () { +describe(`${testID} - endpoint passes multiple fallback server check`, function () { const sourceDir = path.join(rootDir, "pass"); const testOne = (shortFileName) => { const endpoint = loadEndpoint(sourceDir, shortFileName); @@ -59,13 +59,13 @@ describe(`${testID} - endpoint passes duplicate server check`, function () { .filter((shortFileName) => shortFileName.endsWith(".xml")); it(`checks that there are tests`, () => { - assert.ok(candidates.length > 0, "tests should exist"); + assert.ok(candidates.length > 1, "tests should exist"); }); candidates.forEach(testOne); }); -describe(`${testID} - endpoint does not pass duplicate server check`, () => { +describe(`${testID} - endpoint does not pass multiple fallback server check`, () => { const sourceDir = path.join(rootDir, "fail"); const testOne = (shortFileName) => { @@ -86,7 +86,7 @@ describe(`${testID} - endpoint does not pass duplicate server check`, () => { .filter((shortFileName) => shortFileName.endsWith(".xml")); it(`checks that there are tests`, () => { - assert.ok(candidates.length > 0, "tests should exist"); + assert.ok(candidates.length > 1, "tests should exist"); }); candidates.forEach(testOne); diff --git a/test/specs/TD010-test.js b/test/specs/TD010-test.js new file mode 100644 index 0000000..f62e456 --- /dev/null +++ b/test/specs/TD010-test.js @@ -0,0 +1,93 @@ +/* + 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. +*/ + +/* global describe, it */ + +const testID = "TD010", + assert = require("assert"), + fs = require("fs"), + util = require("util"), + path = require("path"), + bl = require("../../lib/package/bundleLinter.js"), + plugin = require(bl.resolvePlugin(testID)), + Endpoint = require("../../lib/package/Endpoint.js"), + Dom = require("@xmldom/xmldom").DOMParser, + rootDir = path.resolve(__dirname, "../fixtures/resources/TD010"), + debug = require("debug")("apigeelint:" + testID); + +const loadEndpoint = (sourceDir, shortFileName) => { + const fqPath = path.join(sourceDir, shortFileName), + xml = fs.readFileSync(fqPath).toString("utf-8"), + doc = new Dom().parseFromString(xml), + endpoint = new Endpoint(doc.documentElement, null, ""); + endpoint.getFileName = () => shortFileName; + return endpoint; +}; + +describe(`${testID} - endpoint passes duplicate server check`, function () { + const sourceDir = path.join(rootDir, "pass"); + const testOne = (shortFileName) => { + const endpoint = loadEndpoint(sourceDir, shortFileName); + + it(`check ${shortFileName} passes`, () => { + plugin.onTargetEndpoint(endpoint, (e, foundIssues) => { + assert.equal(e, undefined, "should be undefined"); + const messages = endpoint.getReport().messages; + debug(util.format(messages)); + assert.equal(foundIssues, false, "should be no issues"); + assert.ok(messages, "messages should exist"); + assert.equal(messages.length, 0, "unexpected number of messages"); + }); + }); + }; + + const candidates = fs + .readdirSync(sourceDir) + .filter((shortFileName) => shortFileName.endsWith(".xml")); + + it(`checks that there are tests`, () => { + assert.ok(candidates.length > 1, "tests should exist"); + }); + + candidates.forEach(testOne); +}); + +describe(`${testID} - endpoint does not pass duplicate server check`, () => { + const sourceDir = path.join(rootDir, "fail"); + + const testOne = (shortFileName) => { + const policy = loadEndpoint(sourceDir, shortFileName); + it(`check ${shortFileName} throws error`, () => { + plugin.onTargetEndpoint(policy, (e, foundIssues) => { + assert.equal(undefined, e, "should be undefined"); + assert.equal(true, foundIssues, "should be issues"); + const messages = policy.getReport().messages; + assert.ok(messages, "messages for issues should exist"); + debug(util.format(messages)); + }); + }); + }; + + const candidates = fs + .readdirSync(sourceDir) + .filter((shortFileName) => shortFileName.endsWith(".xml")); + + it(`checks that there are tests`, () => { + assert.ok(candidates.length > 1, "tests should exist"); + }); + + candidates.forEach(testOne); +});