feat: TD016 check that HealthMonitor present only when applicable

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 |   |   |   |   |

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
+};

test/fixtures/resources/TD016/fail/t1-no-LoadBalancer-but-with-HealthMonitor.xml

@@ -0,0 +1,33 @@
+<TargetEndpoint name="default">
+  <HTTPTargetConnection>
+    <SSLInfo>
+      <Enabled>true</Enabled>
+      <IgnoreValidationErrors>false</IgnoreValidationErrors>
+      <TrustStore>truststore1</TrustStore>
+    </SSLInfo>
+    <URL>https://example.com/foo</URL>
+
+    <HealthMonitor>
+      <IsEnabled>true</IsEnabled>
+      <IntervalInSec>5</IntervalInSec>
+      <HTTPMonitor>
+        <Request>
+          <UseTargetServerSSLInfo>true</UseTargetServerSSLInfo>
+          <ConnectTimeoutInSec>10</ConnectTimeoutInSec>
+          <SocketReadTimeoutInSec>30</SocketReadTimeoutInSec>
+          <Port>80</Port>
+          <Verb>GET</Verb>
+          <Path>/healthcheck</Path>
+          <Header name="Authorization">Basic 12e98yfw87etf</Header>
+          <IncludeHealthCheckIdHeader>true</IncludeHealthCheckIdHeader>
+        </Request>
+        <SuccessResponse>
+          <ResponseCode>200</ResponseCode>
+          <Header name="ImOK">YourOK</Header>
+        </SuccessResponse>
+      </HTTPMonitor>
+    </HealthMonitor>
+
+  </HTTPTargetConnection>
+
+</TargetEndpoint>

test/fixtures/resources/TD016/pass/t1-with-LoadBalancer-and-HealthMonitor.xml

@@ -0,0 +1,34 @@
+<TargetEndpoint name="default">
+  <HTTPTargetConnection>
+
+    <LoadBalancer>
+      <Server name="server1"/>
+      <Server name="server2"/>
+      <MaxFailures>3</MaxFailures>
+    </LoadBalancer>
+
+    <HealthMonitor>
+      <IsEnabled>true</IsEnabled>
+      <IntervalInSec>5</IntervalInSec>
+
+      <HTTPMonitor>
+        <Request>
+          <UseTargetServerSSLInfo>true</UseTargetServerSSLInfo>
+          <ConnectTimeoutInSec>10</ConnectTimeoutInSec>
+          <SocketReadTimeoutInSec>30</SocketReadTimeoutInSec>
+          <Port>80</Port>
+          <Verb>GET</Verb>
+          <Path>/healthcheck</Path>
+          <Header name="Authorization">Basic 12e98yfw87etf</Header>
+          <IncludeHealthCheckIdHeader>true</IncludeHealthCheckIdHeader>
+        </Request>
+        <SuccessResponse>
+          <ResponseCode>200</ResponseCode>
+          <Header name="ImOK">YoureOK</Header>
+        </SuccessResponse>
+      </HTTPMonitor>
+
+    </HealthMonitor>
+
+  </HTTPTargetConnection>
+</TargetEndpoint>

test/fixtures/resources/TD016/pass/t2-with-LoadBalancer-and-no-HealthMonitor.xml

@@ -0,0 +1,11 @@
+<TargetEndpoint name="default">
+  <HTTPTargetConnection>
+
+    <LoadBalancer>
+      <Server name="server1"/>
+      <Server name="server2"/>
+      <MaxFailures>13</MaxFailures>
+    </LoadBalancer>
+
+  </HTTPTargetConnection>
+</TargetEndpoint>

test/fixtures/resources/TD016/pass/t3-no-LoadBalancer-no-HealthMonitor.xml

@@ -0,0 +1,11 @@
+<TargetEndpoint name="default">
+  <HTTPTargetConnection>
+    <SSLInfo>
+      <Enabled>true</Enabled>
+      <IgnoreValidationErrors>false</IgnoreValidationErrors>
+      <TrustStore>truststore1</TrustStore>
+    </SSLInfo>
+    <URL>https://example.com/foo</URL>
+  </HTTPTargetConnection>
+
+</TargetEndpoint>

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);
+});