From b84e88d99bef03dcbe7547800f050236e658c30f Mon Sep 17 00:00:00 2001 From: DinoChiesa Date: Thu, 14 Nov 2024 21:44:00 +0000 Subject: [PATCH] feat: TD016 check that HealthMonitor present only when applicable --- README.md | 1 + .../TD016-healthmon-but-no-loadbalancer.js | 81 ++++++++++++++++ ...no-LoadBalancer-but-with-HealthMonitor.xml | 33 +++++++ ...t1-with-LoadBalancer-and-HealthMonitor.xml | 34 +++++++ ...with-LoadBalancer-and-no-HealthMonitor.xml | 11 +++ .../t3-no-LoadBalancer-no-HealthMonitor.xml | 11 +++ test/specs/TD016-test.js | 93 +++++++++++++++++++ 7 files changed, 264 insertions(+) create mode 100644 lib/package/plugins/TD016-healthmon-but-no-loadbalancer.js create mode 100644 test/fixtures/resources/TD016/fail/t1-no-LoadBalancer-but-with-HealthMonitor.xml create mode 100644 test/fixtures/resources/TD016/pass/t1-with-LoadBalancer-and-HealthMonitor.xml create mode 100644 test/fixtures/resources/TD016/pass/t2-with-LoadBalancer-and-no-HealthMonitor.xml create mode 100644 test/fixtures/resources/TD016/pass/t3-no-LoadBalancer-no-HealthMonitor.xml create mode 100644 test/specs/TD016-test.js diff --git a/README.md b/README.md index 3680e7f..1a461e3 100644 --- a/README.md +++ b/README.md @@ -343,6 +343,7 @@ This is the current list: |   |:white_check_mark:| TD013 | TargetEndpoint SSLInfo | TargetEndpoint HTTPTargetConnection should correctly configure ClientAuthEnbled. | |   |:white_check_mark:| TD014 | TargetEndpoint SSLInfo | TargetEndpoint HTTPTargetConnection should use exctly one of URL, LoadBalancer. | |   |:white_check_mark:| TD015 | TargetEndpoint LoadBalancer | if TargetEndpoint HTTPTargetConnection uses a LoadBalancer, it should specify MaxFailures. | +|   |:white_check_mark:| TD016 | TargetEndpoint HealthMonitor | TargetEndpoint HTTPTargetConnection must use a HealthMonitor only with a LoadBalancer. | | 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/TD016-healthmon-but-no-loadbalancer.js b/lib/package/plugins/TD016-healthmon-but-no-loadbalancer.js new file mode 100644 index 0000000..76e1feb --- /dev/null +++ b/lib/package/plugins/TD016-healthmon-but-no-loadbalancer.js @@ -0,0 +1,81 @@ +/* + 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("../lintUtil.js").getRuleId(), + xpath = require("xpath"); + +const plugin = { + ruleId, + name: "TargetEndpoint HTTPTargetConnection HealthMonitor without LoadBalancer.", + message: + "TargetEndpoint HTTPTargetConnection should not use HealthMonitor without LoadBalancer.", + 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 == 0) { + const healthMonitors = htc.select("HealthMonitor"); + debug(`healthMonitors(${healthMonitors.length})`); + if (healthMonitors.length > 0) { + const healthMonitor = healthMonitors[0]; + endpoint.addMessage({ + plugin, + line: healthMonitor.lineNumber, + column: healthMonitor.columnNumber + }); + 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/test/fixtures/resources/TD016/fail/t1-no-LoadBalancer-but-with-HealthMonitor.xml b/test/fixtures/resources/TD016/fail/t1-no-LoadBalancer-but-with-HealthMonitor.xml new file mode 100644 index 0000000..132a88e --- /dev/null +++ b/test/fixtures/resources/TD016/fail/t1-no-LoadBalancer-but-with-HealthMonitor.xml @@ -0,0 +1,33 @@ + + + + true + false + truststore1 + + https://example.com/foo + + + true + 5 + + + true + 10 + 30 + 80 + GET + /healthcheck +
Basic 12e98yfw87etf
+ true + + + 200 +
YourOK
+ + + + + + + diff --git a/test/fixtures/resources/TD016/pass/t1-with-LoadBalancer-and-HealthMonitor.xml b/test/fixtures/resources/TD016/pass/t1-with-LoadBalancer-and-HealthMonitor.xml new file mode 100644 index 0000000..cfd5070 --- /dev/null +++ b/test/fixtures/resources/TD016/pass/t1-with-LoadBalancer-and-HealthMonitor.xml @@ -0,0 +1,34 @@ + + + + + + + 3 + + + + true + 5 + + + + true + 10 + 30 + 80 + GET + /healthcheck +
Basic 12e98yfw87etf
+ true + + + 200 +
YoureOK
+ + + + + + + diff --git a/test/fixtures/resources/TD016/pass/t2-with-LoadBalancer-and-no-HealthMonitor.xml b/test/fixtures/resources/TD016/pass/t2-with-LoadBalancer-and-no-HealthMonitor.xml new file mode 100644 index 0000000..7660e16 --- /dev/null +++ b/test/fixtures/resources/TD016/pass/t2-with-LoadBalancer-and-no-HealthMonitor.xml @@ -0,0 +1,11 @@ + + + + + + + 13 + + + + diff --git a/test/fixtures/resources/TD016/pass/t3-no-LoadBalancer-no-HealthMonitor.xml b/test/fixtures/resources/TD016/pass/t3-no-LoadBalancer-no-HealthMonitor.xml new file mode 100644 index 0000000..cfbf7e0 --- /dev/null +++ b/test/fixtures/resources/TD016/pass/t3-no-LoadBalancer-no-HealthMonitor.xml @@ -0,0 +1,11 @@ + + + + true + false + truststore1 + + https://example.com/foo + + + diff --git a/test/specs/TD016-test.js b/test/specs/TD016-test.js new file mode 100644 index 0000000..be150c7 --- /dev/null +++ b/test/specs/TD016-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 = "TD016", + 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/TD016"), + debug = require("debug")(`apigeelint:${testID}-test`); + +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 HealthMonitor with no LoadBalancer 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 > 0, "tests should exist"); + }); + + candidates.forEach(testOne); +}); + +describe(`${testID} - endpoint does not pass HealthMonitor with no LoadBalancer 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 > 0, "tests should exist"); + }); + + candidates.forEach(testOne); +});