Merge pull request #472 from DinoChiesa/target-lb-checks

feat: TD008 and TD009 to perform checks on LoadBalancer
apigee · Sep 27, 2024 · 0e85491 · 0e85491
2 parents e21e568 + 0c88e64
commit 0e85491
Show file tree

Hide file tree

Showing 17 changed files with 520 additions and 0 deletions.
diff --git a/README.md b/README.md
@@ -335,6 +335,8 @@ This is the current list:
 | &nbsp; |:white_check_mark:| TD005 | TargetEndpoint SSLInfo references | TargetEndpoint SSLInfo should use references for KeyStore and TrustStore. |
 | &nbsp; |:white_check_mark:| TD006 | TargetEndpoint SSLInfo | When using a LoadBalancer, the SSLInfo should not be configured under HTTPTargetConnection. |
 | &nbsp; |:white_check_mark:| TD007 | TargetEndpoint SSLInfo | TargetEndpoint HTTPTargetConnection SSLInfo should use TrustStore. |
+| &nbsp; |:white_check_mark:| TD008 | TargetEndpoint LoadBalancer Servers | LoadBalancer should not have duplicate Server entries or multiple Fallbacks. |
+| &nbsp; |:white_check_mark:| TD009 | TargetEndpoint LoadBalancer | TargetEndpoint HTTPTargetConnection should have at most one LoadBalancer. |
 | Flow | &nbsp; | &nbsp; | &nbsp; | &nbsp; |
 | &nbsp; |:white_check_mark:| FL001 | Unconditional Flows |  Only one unconditional flow will get executed. Error if more than one was detected. |
 | Step | &nbsp; | &nbsp; | &nbsp; | &nbsp; |

diff --git a/lib/package/plugins/TD008-target-LB-servers.js b/lib/package/plugins/TD008-target-LB-servers.js
@@ -0,0 +1,138 @@
+/*
+  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 duplicate Server elements",
+  message: "Duplicate Server elements found within 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 == 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: "Multiple Server entries with IsFallback=true"
+          });
+          flagged = true;
+        }
+
+        const servers = xpath.select("Server", loadBalancer);
+        if (servers.length > 1) {
+          const previouslyDetected = [];
+          servers.slice(0, -1).forEach((item, i) => {
+            if (!previouslyDetected.includes(i)) {
+              const dupesForI = [];
+              const compares = servers.slice(i + 1);
+              compares.forEach((c, j) => {
+                const indexOfCompared = j + i + 1;
+                if (!previouslyDetected.includes(indexOfCompared)) {
+                  const attrsA = xpath.select("@*", item);
+                  const attrsB = xpath.select("@*", c);
+                  if (!attrsA || !attrsA.find((attr) => attr.name == "name")) {
+                    endpoint.addMessage({
+                      plugin,
+                      line: item.lineNumber,
+                      column: item.columnNumber,
+                      message:
+                        "Missing name attribute for server within LoadBalancer"
+                    });
+                    flagged = true;
+                  } else {
+                    const nameAttrA = attrsA.find(
+                      (attr) => attr.name == "name"
+                    );
+
+                    if (attrsB) {
+                      const nameAttrB = attrsB.find(
+                        (attr) => attr.name == "name"
+                      );
+                      if (nameAttrB && nameAttrB.value == nameAttrA.value) {
+                        debug(`duplicate found at index ${indexOfCompared}`);
+                        dupesForI.push(indexOfCompared);
+                      }
+                    }
+                  }
+                }
+              });
+
+              if (dupesForI.length) {
+                dupesForI.forEach((ix) =>
+                  endpoint.addMessage({
+                    plugin,
+                    line: servers[ix].lineNumber,
+                    column: servers[ix].columnNumber,
+                    message: plugin.message
+                  })
+                );
+                flagged = true;
+                previouslyDetected.push(...dupesForI);
+              }
+            }
+          });
+        }
+      }
+    } 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/TD009-target-LB-multiple-LB.js b/lib/package/plugins/TD009-target-LB-multiple-LB.js
@@ -0,0 +1,70 @@
+/*
+  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();
+
+const plugin = {
+  ruleId,
+  name: "TargetEndpoint HTTPTargetConnection should have at most one LoadBalancer",
+  message:
+    "TargetEndpoint HTTPTargetConnection should not have more than one 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 > 1) {
+        endpoint.addMessage({
+          plugin,
+          line: loadBalancers[1].lineNumber,
+          column: loadBalancers[1].columnNumber,
+          message: plugin.message
+        });
+        debug(`onTargetEndpoint set flagged`);
+        flagged = true;
+      }
+    } catch (exc1) {
+      console.error("exception: " + exc1);
+      debug(`onTargetEndpoint exc(${exc1})`);
+      debug(`${exc1.stack}`);
+    }
+  }
+
+  if (typeof cb == "function") {
+    debug(`onTargetEndpoint isFlagged(${flagged})`);
+    cb(null, flagged);
+  }
+};
+
+module.exports = {
+  plugin,
+  onTargetEndpoint
+};
diff --git a/test/fixtures/resources/TD008/fail/t1.xml b/test/fixtures/resources/TD008/fail/t1.xml
@@ -0,0 +1,8 @@
+<TargetEndpoint name="default">
+    <HTTPTargetConnection>
+      <LoadBalancer>
+         <Server name="server1"/>
+         <Server name="server1"/>
+      </LoadBalancer>
+    </HTTPTargetConnection>
+</TargetEndpoint>
diff --git a/test/fixtures/resources/TD008/fail/t2.xml b/test/fixtures/resources/TD008/fail/t2.xml
@@ -0,0 +1,9 @@
+<TargetEndpoint name="default">
+    <HTTPTargetConnection>
+      <LoadBalancer>
+         <Server name="server1"/>
+         <Server name="server2"/>
+         <Server name="server1"/>
+      </LoadBalancer>
+    </HTTPTargetConnection>
+</TargetEndpoint>
diff --git a/test/fixtures/resources/TD008/fail/t3.xml b/test/fixtures/resources/TD008/fail/t3.xml
@@ -0,0 +1,12 @@
+<TargetEndpoint name="default">
+    <HTTPTargetConnection>
+      <LoadBalancer>
+         <Server name="server1"/>
+         <Server name="server2"/>
+         <Server name="server3"/>
+         <Server name="server2"/>
+         <Server name="server4"/>
+         <Server name="server2"/>
+      </LoadBalancer>
+    </HTTPTargetConnection>
+</TargetEndpoint>
diff --git a/test/fixtures/resources/TD008/fail/t4.xml b/test/fixtures/resources/TD008/fail/t4.xml
@@ -0,0 +1,14 @@
+<TargetEndpoint name="default">
+    <HTTPTargetConnection>
+      <LoadBalancer>
+         <Server name="server1"/>
+         <!-- two servers marked fallback will fail the TD008 check -->
+         <Server name="server2">
+           <IsFallback>true</IsFallback>
+         </Server>
+         <Server name="server3">
+           <IsFallback>true</IsFallback>
+         </Server>
+      </LoadBalancer>
+    </HTTPTargetConnection>
+</TargetEndpoint>
diff --git a/test/fixtures/resources/TD008/fail/t5.xml b/test/fixtures/resources/TD008/fail/t5.xml
@@ -0,0 +1,9 @@
+<TargetEndpoint name="default">
+    <HTTPTargetConnection>
+      <LoadBalancer>
+         <Server name="server1"/>
+         <Server />
+         <Server name="server3"/>
+      </LoadBalancer>
+    </HTTPTargetConnection>
+</TargetEndpoint>
diff --git a/test/fixtures/resources/TD008/pass/t1.xml b/test/fixtures/resources/TD008/pass/t1.xml
@@ -0,0 +1,8 @@
+<TargetEndpoint name="default">
+    <HTTPTargetConnection>
+      <LoadBalancer>
+         <Server name="server1"/>
+         <Server name="server2"/>
+      </LoadBalancer>
+    </HTTPTargetConnection>
+</TargetEndpoint>
diff --git a/test/fixtures/resources/TD008/pass/t2.xml b/test/fixtures/resources/TD008/pass/t2.xml
@@ -0,0 +1,9 @@
+<TargetEndpoint name="default">
+    <HTTPTargetConnection>
+      <LoadBalancer>
+         <Server name="server1"/>
+         <Server name="server2"/>
+         <Server name="server3"/>
+      </LoadBalancer>
+    </HTTPTargetConnection>
+</TargetEndpoint>
diff --git a/test/fixtures/resources/TD008/pass/t3.xml b/test/fixtures/resources/TD008/pass/t3.xml
@@ -0,0 +1,7 @@
+<TargetEndpoint name="default">
+    <HTTPTargetConnection>
+      <LoadBalancer>
+         <Server name="server1"/>
+      </LoadBalancer>
+    </HTTPTargetConnection>
+</TargetEndpoint>
diff --git a/test/fixtures/resources/TD008/pass/t4.xml b/test/fixtures/resources/TD008/pass/t4.xml
@@ -0,0 +1,12 @@
+<TargetEndpoint name="default">
+    <HTTPTargetConnection>
+      <LoadBalancer>
+         <Server name="server1"/>
+         <Server name="server2"/>
+         <!-- single fallback is OK -->
+         <Server name="server3">
+           <IsFallback>true</IsFallback>
+         </Server>
+      </LoadBalancer>
+    </HTTPTargetConnection>
+</TargetEndpoint>
diff --git a/test/fixtures/resources/TD009/fail/t1.xml b/test/fixtures/resources/TD009/fail/t1.xml
@@ -0,0 +1,11 @@
+<TargetEndpoint name="default">
+  <HTTPTargetConnection>
+    <!-- multiple LoadBalancer elements will fail the TD009 check -->
+      <LoadBalancer>
+         <Server name="server1"/>
+      </LoadBalancer>
+      <LoadBalancer>
+         <Server name="server2"/>
+      </LoadBalancer>
+    </HTTPTargetConnection>
+</TargetEndpoint>
diff --git a/test/fixtures/resources/TD009/pass/t1.xml b/test/fixtures/resources/TD009/pass/t1.xml
@@ -0,0 +1,9 @@
+<TargetEndpoint name="default">
+  <HTTPTargetConnection>
+    <!-- a single LoadBalancer element will pass the TD009 check -->
+      <LoadBalancer>
+         <Server name="server1"/>
+         <Server name="server2"/>
+      </LoadBalancer>
+    </HTTPTargetConnection>
+</TargetEndpoint>
diff --git a/test/fixtures/resources/TD009/pass/t2.xml b/test/fixtures/resources/TD009/pass/t2.xml
@@ -0,0 +1,16 @@
+<TargetEndpoint name="t2">
+    <HTTPTargetConnection>
+
+      <!-- zero LoadBalancer elements will also pass the TD009 check -->
+
+      <SSLInfo>
+        <Enabled>true</Enabled>
+        <Enforce>true</Enforce>
+        <CommonName
+            wildcardMatch="false">echo.dchiesa.demo.altostrat.com</CommonName>
+        <TrustStore>ref://myTrustStoreRef</TrustStore>
+      </SSLInfo>
+
+      <URL>https://echo.dchiesa.demo.altostrat.com</URL>
+    </HTTPTargetConnection>
+</TargetEndpoint>