feat(spel): Add feature flag for DoNotEval SpEL. (#981) (#987)

* feat(spel): Add feature flag for DoNotEval SpEL. * refactor: Move Feature Flag to class. * fix: dependencies. * chore(config): Add final statement, replace Getter and Setter with Data annotation. Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit e743ea3) Co-authored-by: armory-abedonik <[email protected]>
spinnaker · Oct 19, 2022 · 8cd72b8 · 8cd72b8
1 parent 3334ae8
commit 8cd72b8
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 11 deletions.
diff --git a/kork-expressions/kork-expressions.gradle b/kork-expressions/kork-expressions.gradle
@@ -16,6 +16,8 @@ dependencies {
   api "org.springframework:spring-context"
   api "org.slf4j:slf4j-api"
 
+  implementation "org.springframework.boot:spring-boot"
+
   testImplementation "org.assertj:assertj-core"
   testImplementation "org.junit.jupiter:junit-jupiter-api"
   testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine"

diff --git a/...-expressions/src/main/java/com/netflix/spinnaker/kork/expressions/ExpressionsSupport.java b/...-expressions/src/main/java/com/netflix/spinnaker/kork/expressions/ExpressionsSupport.java
@@ -25,6 +25,7 @@
 import com.netflix.spinnaker.kork.expressions.allowlist.FilteredPropertyAccessor;
 import com.netflix.spinnaker.kork.expressions.allowlist.MapPropertyAccessor;
 import com.netflix.spinnaker.kork.expressions.allowlist.ReturnTypeRestrictor;
+import com.netflix.spinnaker.kork.expressions.config.ExpressionProperties;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -58,15 +59,19 @@ public class ExpressionsSupport {
 
   private final Set<Class<?>> allowedReturnTypes;
   private final List<ExpressionFunctionProvider> expressionFunctionProviders;
+  private final ExpressionProperties expressionProperties;
 
-  public ExpressionsSupport(Class<?> extraAllowedReturnType) {
-    this(new Class[] {extraAllowedReturnType}, null, null);
+  public ExpressionsSupport(
+      Class<?> extraAllowedReturnType, ExpressionProperties expressionProperties) {
+    this(new Class[] {extraAllowedReturnType}, null, null, expressionProperties);
   }
 
   public ExpressionsSupport(
       Class<?>[] extraAllowedReturnTypes,
       List<ExpressionFunctionProvider> extraExpressionFunctionProviders,
-      PluginManager pluginManager) {
+      PluginManager pluginManager,
+      ExpressionProperties expressionProperties) {
+    this.expressionProperties = expressionProperties;
 
     allowedReturnTypes =
         new HashSet<>(
@@ -84,16 +89,13 @@ public ExpressionsSupport(
                 HashMap.class,
                 LinkedHashMap.class,
                 TreeMap.class,
-                TreeSet.class,
-                NotEvaluableExpression.class));
+                TreeSet.class));
     Collections.addAll(allowedReturnTypes, extraAllowedReturnTypes);
 
     expressionFunctionProviders =
         new ArrayList<>(
             Arrays.asList(
-                new FlowExpressionFunctionProvider(),
-                new JsonExpressionFunctionProvider(),
-                new StringExpressionFunctionProvider()));
+                new JsonExpressionFunctionProvider(), new StringExpressionFunctionProvider()));
 
     if (extraExpressionFunctionProviders != null) {
       expressionFunctionProviders.addAll(extraExpressionFunctionProviders);
@@ -105,6 +107,11 @@ public ExpressionsSupport(
       expressionFunctionProviders.addAll(
           pluginManager.getExtensions(ExpressionFunctionProvider.class));
     }
+
+    if (expressionProperties.getDoNotEvalSpel().isEnabled()) {
+      allowedReturnTypes.add(NotEvaluableExpression.class);
+      expressionFunctionProviders.add(new FlowExpressionFunctionProvider());
+    }
   }
 
   public List<ExpressionFunctionProvider> getExpressionFunctionProviders() {

diff --git a/...ons/src/main/java/com/netflix/spinnaker/kork/expressions/config/ExpressionProperties.java b/...ons/src/main/java/com/netflix/spinnaker/kork/expressions/config/ExpressionProperties.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2022 Armory, Inc.
+ *
+ * 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
+ *
+ *   http://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.
+ */
+
+package com.netflix.spinnaker.kork.expressions.config;
+
+import lombok.Data;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+
+@Data
+@ConfigurationProperties(prefix = "expression")
+public class ExpressionProperties {
+
+  private final FeatureFlag doNotEvalSpel = new FeatureFlag();
+
+  @Data
+  public static class FeatureFlag {
+    private boolean enabled;
+  }
+}
diff --git a/...essions/src/test/java/com/netflix/spinnaker/kork/expressions/ExpressionTransformTest.java b/...essions/src/test/java/com/netflix/spinnaker/kork/expressions/ExpressionTransformTest.java
@@ -2,6 +2,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import com.netflix.spinnaker.kork.expressions.config.ExpressionProperties;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -21,9 +22,11 @@ class ExpressionTransformTest {
 
   @Test
   void evaluateCompositeExpression() {
+    ExpressionProperties expressionProperties = new ExpressionProperties();
+
     ExpressionEvaluationSummary summary = new ExpressionEvaluationSummary();
     StandardEvaluationContext evaluationContext =
-        new ExpressionsSupport(Trigger.class)
+        new ExpressionsSupport(Trigger.class, expressionProperties)
             .buildEvaluationContext(new Pipeline(new Trigger(100)), true);
 
     String evaluated =

diff --git a/...ressions/src/test/java/com/netflix/spinnaker/kork/expressions/ExpressionsSupportTest.java b/...ressions/src/test/java/com/netflix/spinnaker/kork/expressions/ExpressionsSupportTest.java
@@ -20,6 +20,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import com.netflix.spinnaker.kork.expressions.config.ExpressionProperties;
 import java.util.Collections;
 import java.util.Map;
 import java.util.function.Function;
@@ -56,6 +57,9 @@ public void testToJsonWhenNotEvaluableExpression() {
 
   @Test
   public void testToJsonWhenExpressionAndEvaluationContext() {
+    ExpressionProperties expressionProperties = new ExpressionProperties();
+    expressionProperties.getDoNotEvalSpel().setEnabled(true);
+
     Map<String, Object> testContext =
         Collections.singletonMap(
             "file_json", Collections.singletonMap("owner", "managed-by-${team}"));
@@ -65,14 +69,18 @@ public void testToJsonWhenExpressionAndEvaluationContext() {
         new ExpressionTransform(parserContext, parser, Function.identity())
             .transformString(
                 testInput,
-                new ExpressionsSupport(null).buildEvaluationContext(testContext, true),
+                new ExpressionsSupport(null, expressionProperties)
+                    .buildEvaluationContext(testContext, true),
                 new ExpressionEvaluationSummary());
 
     assertThat(evaluated).isEqualTo("{\"owner\":\"managed-by-${team}\"}");
   }
 
   @Test
   public void testToJsonWhenComposedExpressionAndEvaluationContext() {
+    ExpressionProperties expressionProperties = new ExpressionProperties();
+    expressionProperties.getDoNotEvalSpel().setEnabled(true);
+
     Map<String, Object> testContext =
         Collections.singletonMap(
             "file_json",
@@ -83,7 +91,8 @@ public void testToJsonWhenComposedExpressionAndEvaluationContext() {
         new ExpressionTransform(parserContext, parser, Function.identity())
             .transformString(
                 testInput,
-                new ExpressionsSupport(null).buildEvaluationContext(testContext, true),
+                new ExpressionsSupport(null, expressionProperties)
+                    .buildEvaluationContext(testContext, true),
                 new ExpressionEvaluationSummary());
 
     assertThat(evaluated).isEqualTo("{\"json_file\":\"${#toJson(#doNotEval(file_json))}\"}");