Added doNotSkipInvalidMacroForFunctions query parameter to throw erro…

…r in case of invalid oauth macro
cdapio · Dec 18, 2024 · 04bf38c · 04bf38c
1 parent d620d1c
commit 04bf38c
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 17 deletions.
diff --git a/cdap-api/src/main/java/io/cdap/cdap/api/macro/MacroParserOptions.java b/cdap-api/src/main/java/io/cdap/cdap/api/macro/MacroParserOptions.java
@@ -33,16 +33,18 @@ public class MacroParserOptions {
   private final boolean skipInvalid;
   private final int maxRecurseDepth;
   private final Set<String> functionWhitelist;
+  private final Set<String> doNotSkipInvalidMacroForFunctions;
 
   private MacroParserOptions(boolean evaluateLookups, boolean evaluateFunctions,
       boolean escapingEnabled, boolean skipInvalid,
-      int maxRecurseDepth, Set<String> functionWhitelist) {
+      int maxRecurseDepth, Set<String> functionWhitelist, Set<String> doNotSkipInvalidMacroForFunctions) {
     this.evaluateLookups = evaluateLookups;
     this.evaluateFunctions = evaluateFunctions;
     this.escapingEnabled = escapingEnabled;
     this.maxRecurseDepth = maxRecurseDepth;
     this.skipInvalid = skipInvalid;
     this.functionWhitelist = functionWhitelist;
+    this.doNotSkipInvalidMacroForFunctions = doNotSkipInvalidMacroForFunctions;
   }
 
   public boolean shouldEvaluateLookups() {
@@ -69,6 +71,10 @@ public Set<String> getFunctionWhitelist() {
     return functionWhitelist;
   }
 
+  public Set<String> getDoNotSkipInvalidMacroForFunctions() {
+    return doNotSkipInvalidMacroForFunctions;
+  }
+
   /**
    * @return Builder to create options
    */
@@ -88,6 +94,8 @@ public static class Builder {
     private int maxRecurseDepth = 10;
     private final Set<String> functionWhitelist = new HashSet<>();
 
+    private final Set<String> doNotSkipInvalidMacroForFunctions = new HashSet<>();
+
     public Builder disableLookups() {
       evaluateLookups = false;
       return this;
@@ -113,6 +121,16 @@ public Builder setMaxRecurseDepth(int maxRecurseDepth) {
       return this;
     }
 
+    public Builder setDoNotSkipInvalidMacroForFunctions(Collection<String> whitelist) {
+      doNotSkipInvalidMacroForFunctions.clear();
+      doNotSkipInvalidMacroForFunctions.addAll(whitelist);
+      return this;
+    }
+
+    public Builder setDoNotSkipInvalidMacroForFunctions(String... whitelist) {
+      return setDoNotSkipInvalidMacroForFunctions(Arrays.asList(whitelist));
+    }
+
     public Builder setFunctionWhitelist(Collection<String> whitelist) {
       functionWhitelist.clear();
       functionWhitelist.addAll(whitelist);
@@ -122,10 +140,9 @@ public Builder setFunctionWhitelist(Collection<String> whitelist) {
     public Builder setFunctionWhitelist(String... whitelist) {
       return setFunctionWhitelist(Arrays.asList(whitelist));
     }
-
     public MacroParserOptions build() {
       return new MacroParserOptions(evaluateLookups, evaluateFunctions, escapingEnabled,
-          skipInvalid, maxRecurseDepth, functionWhitelist);
+          skipInvalid, maxRecurseDepth, functionWhitelist, doNotSkipInvalidMacroForFunctions);
     }
   }
 }
diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/runtime/plugin/MacroParser.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/runtime/plugin/MacroParser.java
@@ -43,6 +43,7 @@ public class MacroParser {
   private final boolean functionsEnabled;
   private final boolean skipInvalid;
   private final Set<String> functionWhitelist;
+  private final Set<String> doNotSkipInvalidMacroForFunctions;
 
   public MacroParser(MacroEvaluator macroEvaluator) {
     this(macroEvaluator, MacroParserOptions.DEFAULT);
@@ -55,6 +56,7 @@ public MacroParser(MacroEvaluator macroEvaluator, MacroParserOptions options) {
     this.functionsEnabled = options.shouldEvaluateFunctions();
     this.functionWhitelist = options.getFunctionWhitelist();
     this.skipInvalid = options.shouldSkipInvalid();
+    this.doNotSkipInvalidMacroForFunctions = options.getDoNotSkipInvalidMacroForFunctions();
   }
 
   /**
@@ -242,7 +244,7 @@ private MacroMetadata getMacroFunctionMetadata(int startIndex, int endIndex, Str
         }
 
       } catch (InvalidMacroException e) {
-        if (!skipInvalid) {
+        if (!skipInvalid || doNotSkipInvalidMacroForFunctions.contains(macroFunction)) {
           throw e;
         }
       }

diff --git a/...cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java b/...cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java
@@ -185,7 +185,7 @@ public void putOAuthCredential(HttpServiceRequest request, HttpServiceResponder
 
       if (response.getResponseCode() != 200) {
         throw new OAuthServiceException(
-            HttpURLConnection.HTTP_INTERNAL_ERROR,
+          response.getResponseCode(),
             "Request for refresh token did not return 200. Response code: "
                 + response.getResponseCode()
                 + " , response message: "
@@ -243,7 +243,7 @@ public void getOAuthCredential(HttpServiceRequest request, HttpServiceResponder
 
       if (response.getResponseCode() != 200) {
         throw new OAuthServiceException(
-            HttpURLConnection.HTTP_INTERNAL_ERROR,
+          response.getResponseCode(),
             "Request for refresh token did not return 200. Response code: "
                 + response.getResponseCode()
                 + " , response message: "

diff --git a/...ipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/RemoteValidationRequest.java b/...ipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/RemoteValidationRequest.java
@@ -16,6 +16,8 @@
 
 package io.cdap.cdap.datapipeline.service;
 
+import java.util.Set;
+
 /**
  * RemoteValidationRequest
  */
@@ -24,10 +26,12 @@ public class RemoteValidationRequest {
   private final String namespace;
   //The original request string
   private final String request;
+  private final Set<String> doNotSkipInvalidMacroForFunctions;
 
-  public RemoteValidationRequest(String namespace, String request) {
+  public RemoteValidationRequest(String namespace, String request, Set<String> doNotSkipInvalidMacroForFunctions) {
     this.namespace = namespace;
     this.request = request;
+    this.doNotSkipInvalidMacroForFunctions = doNotSkipInvalidMacroForFunctions;
   }
 
   public String getNamespace() {
@@ -37,4 +41,8 @@ public String getNamespace() {
   public String getRequest() {
     return request;
   }
+
+  public Set<String> getDoNotSkipInvalidMacroForFunctions() {
+    return doNotSkipInvalidMacroForFunctions;
+  }
 }
diff --git a/...a-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/RemoteValidationTask.java b/...a-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/RemoteValidationTask.java
@@ -43,6 +43,7 @@
 
 import java.util.Collections;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.Function;
 
 /**
@@ -63,6 +64,7 @@ public void run(RunnableTaskContext context) throws Exception {
     RemoteValidationRequest remoteValidationRequest = GSON.fromJson(context.getParam(), RemoteValidationRequest.class);
     String namespace = remoteValidationRequest.getNamespace();
     String originalRequest = remoteValidationRequest.getRequest();
+    Set<String> doNotSkipInvalidMacroForFunctions = remoteValidationRequest.getDoNotSkipInvalidMacroForFunctions();
     StageValidationRequest validationRequest;
     try {
       validationRequest = GSON.fromJson(originalRequest,
@@ -96,11 +98,13 @@ OAuthAccessTokenMacroEvaluator.FUNCTION_NAME, new OAuthAccessTokenMacroEvaluator
     );
     MacroEvaluator macroEvaluator = new DefaultMacroEvaluator(new BasicArguments(arguments), evaluators,
                                                               DefaultMacroEvaluator.MAP_FUNCTIONS);
+
     MacroParserOptions macroParserOptions = MacroParserOptions.builder()
-      .skipInvalidMacros()
       .setEscaping(false)
       .setFunctionWhitelist(evaluators.keySet())
-      .build();
+      .skipInvalidMacros()
+      .setDoNotSkipInvalidMacroForFunctions(doNotSkipInvalidMacroForFunctions).build();
+
     Function<Map<String, String>, Map<String, String>> macroFn =
       macroProperties -> systemAppContext
         .evaluateMacros(namespace, macroProperties, macroEvaluator, macroParserOptions);

diff --git a/...data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/ValidationHandler.java b/...data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/ValidationHandler.java
@@ -59,13 +59,19 @@
 import java.lang.reflect.Type;
 import java.net.HttpURLConnection;
 import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.Function;
+import java.util.stream.Collectors;
+import javax.ws.rs.DefaultValue;
 import javax.ws.rs.GET;
 import javax.ws.rs.POST;
 import javax.ws.rs.Path;
 import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
 
 /**
  * Handles validation logic for pipelines.
@@ -88,25 +94,35 @@ public void healthCheck(HttpServiceRequest request, HttpServiceResponder respond
   @POST
   @Path("v1/contexts/{context}/validations/stage")
   public void validateStage(HttpServiceRequest request, HttpServiceResponder responder,
-                            @PathParam("context") String namespace) throws IOException, AccessException {
+                            @PathParam("context") String namespace,
+                            @QueryParam("doNotSkipInvalidMacroForFunctions")
+                              String doNotSkipInvalidMacroForFunctions)
+    throws IOException, AccessException {
     if (!getContext().getAdmin().namespaceExists(namespace)) {
       responder.sendError(HttpURLConnection.HTTP_NOT_FOUND, String.format("Namespace '%s' does not exist", namespace));
       return;
     }
+    Set<String> doNotSkipInvalidMacroForFunctionsSet = new HashSet<>();
+    if (doNotSkipInvalidMacroForFunctions != null && !doNotSkipInvalidMacroForFunctions.isEmpty()) {
+      doNotSkipInvalidMacroForFunctionsSet.addAll(Arrays.stream(doNotSkipInvalidMacroForFunctions.split(","))
+                                                     .map(String::trim)
+                                                     .filter(s -> !s.isEmpty())
+                                                     .collect(Collectors.toSet()));
+    }
 
     //Validate remotely if remote execution is enabled
     if (getContext().isRemoteTaskEnabled()) {
-      validateRemotely(request, responder, namespace);
+      validateRemotely(request, responder, namespace, doNotSkipInvalidMacroForFunctionsSet);
       return;
     }
-    validateLocally(request, responder, namespace);
+    validateLocally(request, responder, namespace, doNotSkipInvalidMacroForFunctionsSet);
   }
 
   private void validateRemotely(HttpServiceRequest request, HttpServiceResponder responder,
-                                String namespace) throws IOException {
+                                String namespace, Set<String> doNotSkipInvalidMacroForFunctions) throws IOException {
     String validationRequestString = StandardCharsets.UTF_8.decode(request.getContent()).toString();
     RemoteValidationRequest remoteValidationRequest =
-        new RemoteValidationRequest(namespace, validationRequestString);
+        new RemoteValidationRequest(namespace, validationRequestString, doNotSkipInvalidMacroForFunctions);
     RunnableTaskRequest runnableTaskRequest =
         RunnableTaskRequest.getBuilder(RemoteValidationTask.class.getName())
             .withParam(GSON.toJson(remoteValidationRequest)).withNamespace(namespace).build();
@@ -132,7 +148,7 @@ private int getExceptionCode(String exceptionClass, String exceptionMessage, Str
   }
 
   private void validateLocally(HttpServiceRequest request, HttpServiceResponder responder,
-                               String namespace) throws IOException {
+                               String namespace, Set<String> doNotSkipInvalidMacroForFunctions) throws IOException {
     StageValidationRequest validationRequest;
     try {
       validationRequest = GSON.fromJson(StandardCharsets.UTF_8.decode(request.getContent()).toString(),
@@ -170,10 +186,11 @@ OAuthAccessTokenMacroEvaluator.FUNCTION_NAME, new OAuthAccessTokenMacroEvaluator
     MacroEvaluator macroEvaluator = new DefaultMacroEvaluator(new BasicArguments(arguments), evaluators,
                                                               DefaultMacroEvaluator.MAP_FUNCTIONS);
     MacroParserOptions macroParserOptions = MacroParserOptions.builder()
-      .skipInvalidMacros()
       .setEscaping(false)
       .setFunctionWhitelist(evaluators.keySet())
-      .build();
+      .skipInvalidMacros()
+      .setDoNotSkipInvalidMacroForFunctions(doNotSkipInvalidMacroForFunctions).build();
+
     Function<Map<String, String>, Map<String, String>> macroFn =
       macroProperties -> getContext().evaluateMacros(namespace, macroProperties, macroEvaluator, macroParserOptions);
     String validationResponse = GSON.toJson(ValidationUtils.validate(