Merge pull request #442 from DinoChiesa/pd002-fix

fix: correct PD002 so that it handles sharedflows correctly

lib/package/plugins/PD002-emptyRouteRules.js

@@ -14,7 +14,9 @@
   limitations under the License.
 */
 
-const ruleId = require("../myUtil.js").getRuleId();
+const ruleId = require("../myUtil.js").getRuleId(),
+  util = require("util"),
+  debug = require("debug")("apigeelint:" + ruleId);
 
 const plugin = {
   ruleId,
@@ -29,33 +31,60 @@ const plugin = {
 
 const onProxyEndpoint = function (ep, cb) {
   const routeRules = ep.getRouteRules();
-  let hadError = false;
+  const parent = ep.getParent();
+  debug(`parent: ${util.format(parent)}`);
+  let flagged = false;
 
-  if (routeRules) {
-    const blankRR = routeRules.filter(
-      (rr) => !rr.getCondition() || rr.getCondition().getExpression() === ""
-    );
-    if (blankRR.length > 1) {
-      blankRR.slice(1).forEach((rr) =>
+  if (routeRules && routeRules.length) {
+    if (parent.bundleTypeName == "apiproxy") {
+      const blankRR = routeRules.filter(
+        (rr) => !rr.getCondition() || rr.getCondition().getExpression() === ""
+      );
+      if (blankRR.length > 1) {
+        blankRR.slice(1).forEach((rr) =>
+          ep.addMessage({
+            plugin,
+            source: rr.getSource(),
+            line: rr.getElement().lineNumber,
+            column: rr.getElement().columnNumber,
+            severity: 2, //2=error
+            message: `Multiple RouteRules with no Condition. Only the first is evaluated.`
+          })
+        );
+        flagged = true;
+      } else if (blankRR.length == 0) {
+        ep.addMessage({
+          plugin,
+          severity: 1, //1=warning
+          message: `There is no RouteRule with no Condition. Your proxy may not operate correctly.`
+        });
+        flagged = true;
+      }
+    } else {
+      routeRules.forEach((rr) =>
         ep.addMessage({
           plugin,
           source: rr.getSource(),
           line: rr.getElement().lineNumber,
           column: rr.getElement().columnNumber,
-          message: `Multiple RouteRules with no Condition. Only the first is evaluated.`
+          severity: 1, //1=warning
+          message: `Misplaced RouteRule in a SharedFlow.`
         })
       );
-      hadError = true;
-    } else if (blankRR.length == 0) {
+      flagged = true;
+    }
+  } else {
+    if (parent.bundleTypeName == "apiproxy") {
       ep.addMessage({
         plugin,
-        severity: 1, //1=warning
-        message: `There is no RouteRule with no Condition. Your proxy may not operate correctly.`
+        severity: 2, //2=error
+        message: `There is no RouteRule.`
       });
+      flagged = true;
     }
   }
   if (typeof cb == "function") {
-    cb(null, hadError);
+    cb(null, flagged);
   }
 };
 

test/fixtures/resources/PD002-no-unconditional-routerule/proxy2/apiproxy/no-route-rules.xml

@@ -0,0 +1,4 @@
+<APIProxy revision="1" name="no-route-rules">
+  <ConfigurationVersion minorVersion="0" majorVersion="4"/>
+  <DisplayName>no-route-rules</DisplayName>
+</APIProxy>

test/fixtures/resources/PD002-no-unconditional-routerule/proxy2/apiproxy/policies/AM-Clean-Request-Headers-From-Response.xml

@@ -0,0 +1,21 @@
+<AssignMessage name='AM-Clean-Request-Headers-From-Response'>
+  <Remove>
+    <Headers>
+      <Header name='Host'/>
+      <Header name='Accept'/>
+      <Header name='Authorization'/>
+      <Header name='Origin'/>
+      <Header name='user-agent'/>
+      <Header name='X-Request-id'/>
+      <Header name='x-client-data'/>
+      <Header name='x-cloud-trace-context'/>
+      <Header name='X-Powered-By'/>
+      <Header name='X-b3-traceid'/>
+      <Header name='X-b3-spanid'/>
+      <Header name='X-b3-sampled'/>
+      <Header name='X-Forwarded-For'/>
+      <Header name='X-Forwarded-Port'/>
+      <Header name='X-Forwarded-Proto'/>
+    </Headers>
+  </Remove>
+</AssignMessage>

test/fixtures/resources/PD002-no-unconditional-routerule/proxy2/apiproxy/policies/AM-Inject-Proxy-Revision-Header.xml

@@ -0,0 +1,7 @@
+<AssignMessage name='AM-Inject-Proxy-Revision-Header'>
+  <Set>
+    <Headers>
+      <Header name='APIProxy'>{apiproxy.name} r{apiproxy.revision}</Header>
+    </Headers>
+  </Set>
+</AssignMessage>

test/fixtures/resources/PD002-no-unconditional-routerule/proxy2/apiproxy/policies/AM-Response.xml

@@ -0,0 +1,12 @@
+<AssignMessage name='AM-Response'>
+  <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+  <Set>
+    <Payload contentType='application/json'>{
+    "status" : "ok"
+}
+</Payload>
+    <ReasonPhrase>OK</ReasonPhrase>
+    <StatusCode>200</StatusCode>
+  </Set>
+  <AssignTo>response</AssignTo>
+</AssignMessage>

test/fixtures/resources/PD002-no-unconditional-routerule/proxy2/apiproxy/policies/RF-Unknown-Request.xml

@@ -0,0 +1,16 @@
+<RaiseFault name='RF-Unknown-Request'>
+  <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+  <FaultResponse>
+    <Set>
+      <Payload contentType='application/json'>{
+  "error" : {
+    "code" : 404.01,
+    "message" : "that request was unknown; try a different request."
+  }
+}
+</Payload>
+      <StatusCode>404</StatusCode>
+      <ReasonPhrase>Not Found</ReasonPhrase>
+    </Set>
+  </FaultResponse>
+</RaiseFault>

test/fixtures/resources/PD002-no-unconditional-routerule/proxy2/apiproxy/proxies/endpoint1.xml

@@ -0,0 +1,76 @@
+<ProxyEndpoint name="endpoint1">
+  <Description>Proxy Endpoint 1</Description>
+  <HTTPProxyConnection>
+    <BasePath>/no-route-rules</BasePath>
+  </HTTPProxyConnection>
+
+  <FaultRules/>
+  <DefaultFaultRule name="default-fault-rule">
+    <Step>
+      <Name>AM-Inject-Proxy-Revision-Header</Name>
+    </Step>
+    <AlwaysEnforce>true</AlwaysEnforce>
+  </DefaultFaultRule>
+
+  <PreFlow name="PreFlow">
+    <Request>
+    </Request>
+    <Response>
+      <Step>
+        <Name>AM-Clean-Request-Headers-From-Response</Name>
+      </Step>
+    </Response>
+  </PreFlow>
+
+  <PostFlow name="PostFlow">
+    <Request>
+    </Request>
+    <Response>
+      <Step>
+        <Name>AM-Inject-Proxy-Revision-Header</Name>
+      </Step>
+    </Response>
+  </PostFlow>
+
+  <PostClientFlow name="PostClientFlow">
+    <Request>
+    </Request>
+    <Response>
+    </Response>
+  </PostClientFlow>
+
+  <Flows>
+
+    <Flow name="flow1">
+      <Request>
+      </Request>
+      <Response>
+        <Step>
+          <Name>AM-Response</Name>
+        </Step>
+      </Response>
+      <Condition>(proxy.pathsuffix MatchesPath "/t1") and (request.verb = "GET")</Condition>
+    </Flow>
+
+    <Flow name="unknown request">
+      <Request>
+        <Step>
+          <Name>RF-Unknown-Request</Name>
+        </Step>
+      </Request>
+      <Response>
+      </Response>
+    </Flow>
+
+  </Flows>
+
+  <!-- There is no RouteRule.
+
+  <RouteRule name="BrokenRouteRule">
+    <TargetEndpoint>target-1</TargetEndpoint>
+    <Condition>proxy.pathsuffix = "/foobar"</Condition>
+  </RouteRule>
+
+  -->
+
+</ProxyEndpoint>

test/fixtures/resources/PD002-no-unconditional-routerule/proxy2/apiproxy/targets/target-1.xml

@@ -0,0 +1,22 @@
+<TargetEndpoint name="target-1">
+  <Description/>
+  <FaultRules/>
+  <PreFlow name="PreFlow">
+    <Request/>
+    <Response/>
+  </PreFlow>
+  <PostFlow name="PostFlow">
+    <Request/>
+    <Response/>
+  </PostFlow>
+  <Flows/>
+  <HTTPTargetConnection>
+    <SSLInfo>
+      <Enabled>true</Enabled>
+      <Enforce>true</Enforce>
+      <IgnoreValidationErrors>false</IgnoreValidationErrors>
+    </SSLInfo>
+    <Properties/>
+    <URL>https://echo.dchiesa.demo.altostrat.com</URL>
+  </HTTPTargetConnection>
+</TargetEndpoint>

test/fixtures/resources/PD002-no-unconditional-routerule/sharedflow1/sharedflowbundle/authorize-idp-access-tokens.xml

@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Copyright 2023 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.
+
+-->
+<SharedFlowBundle name="sample-hello-cicd-sf">
+    <Description>Authorize IdP Access Tokens Shared Flow</Description>
+</SharedFlowBundle>

test/fixtures/resources/PD002-no-unconditional-routerule/sharedflow1/sharedflowbundle/policies/JWT-VerifyIdPToken.xml

@@ -0,0 +1,9 @@
+<VerifyJWT async="false" continueOnError="false" enabled="true" name="JWT-VerifyIdPToken">
+    <DisplayName>JWT-VerifyIdPToken</DisplayName>
+    <Algorithm>RS256</Algorithm>
+    <PublicKey>
+        <JWKS uriRef="propertyset.idp_configuration.jwks_uri"/>
+    </PublicKey>
+    <Issuer ref="propertyset.idp_configuration.issuer"/>
+    <Audience ref="propertyset.idp_configuration.audience"/>
+</VerifyJWT>

test/fixtures/resources/PD002-no-unconditional-routerule/sharedflow1/sharedflowbundle/policies/VA-IdentifyClientApp.xml

@@ -0,0 +1,4 @@
+<VerifyAPIKey async="false" continueOnError="false" enabled="true" name="VA-IdentifyClientApp">
+    <DisplayName>VA-IdentifyClientApp</DisplayName>
+    <APIKey ref="jwt.JWT-VerifyIdPToken.claim.REPLACEWITHIDPCLIENTIDCLAIM"/>
+</VerifyAPIKey>

test/fixtures/resources/PD002-no-unconditional-routerule/sharedflow1/sharedflowbundle/sharedflows/default.xml

@@ -0,0 +1,8 @@
+<SharedFlow name="default">
+    <Step>
+        <Name>JWT-VerifyIdPToken</Name>
+    </Step>
+    <Step>
+        <Name>VA-IdentifyClientApp</Name>
+    </Step>
+</SharedFlow>

test/fixtures/resources/PD002-no-unconditional-routerule/sharedflow2/sharedflowbundle/authorize-idp-access-tokens.xml

@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Copyright 2023 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.
+
+-->
+<SharedFlowBundle name="sample-hello-cicd-sf">
+    <Description>Authorize IdP Access Tokens Shared Flow</Description>
+</SharedFlowBundle>

test/fixtures/resources/PD002-no-unconditional-routerule/sharedflow2/sharedflowbundle/policies/JWT-VerifyIdPToken.xml

@@ -0,0 +1,9 @@
+<VerifyJWT async="false" continueOnError="false" enabled="true" name="JWT-VerifyIdPToken">
+    <DisplayName>JWT-VerifyIdPToken</DisplayName>
+    <Algorithm>RS256</Algorithm>
+    <PublicKey>
+        <JWKS uriRef="propertyset.idp_configuration.jwks_uri"/>
+    </PublicKey>
+    <Issuer ref="propertyset.idp_configuration.issuer"/>
+    <Audience ref="propertyset.idp_configuration.audience"/>
+</VerifyJWT>

test/fixtures/resources/PD002-no-unconditional-routerule/sharedflow2/sharedflowbundle/policies/VA-IdentifyClientApp.xml

@@ -0,0 +1,4 @@
+<VerifyAPIKey async="false" continueOnError="false" enabled="true" name="VA-IdentifyClientApp">
+    <DisplayName>VA-IdentifyClientApp</DisplayName>
+    <APIKey ref="jwt.JWT-VerifyIdPToken.claim.REPLACEWITHIDPCLIENTIDCLAIM"/>
+</VerifyAPIKey>

test/fixtures/resources/PD002-no-unconditional-routerule/sharedflow2/sharedflowbundle/sharedflows/default.xml

@@ -0,0 +1,15 @@
+<SharedFlow name="default">
+    <Step>
+        <Name>JWT-VerifyIdPToken</Name>
+    </Step>
+    <Step>
+        <Name>VA-IdentifyClientApp</Name>
+    </Step>
+
+    <!-- this does not belong here -->
+  <RouteRule name="BrokenRouteRule">
+    <TargetEndpoint>target-1</TargetEndpoint>
+    <Condition>proxy.pathsuffix = "/foobar"</Condition>
+  </RouteRule>
+
+</SharedFlow>