Merge pull request #1181 from pavel-krizskii/pkrizskii/validate-runne…

…rs-and-their-params Validate supported runners (gradle, maven, node-js, command-line, unity) and their param names
JetBrains · Oct 28, 2024 · 6ccfe5b · 6ccfe5b
2 parents 4deec2e + f34a3cc
commit 6ccfe5b
Show file tree

Hide file tree

Showing 8 changed files with 140 additions and 20 deletions.
diff --git a/...ain/kotlin/com/jetbrains/plugin/structure/teamcity/action/AllowedRunnerToAllowedParams.kt b/...ain/kotlin/com/jetbrains/plugin/structure/teamcity/action/AllowedRunnerToAllowedParams.kt
@@ -0,0 +1,44 @@
+package com.jetbrains.plugin.structure.teamcity.action
+
+val allowedRunnerToAllowedParams = mapOf(
+  "gradle" to setOf(
+    "tasks",
+    "build-file",
+    "incremental",
+    "working-directory",
+    "gradle-home",
+    "gradle-params",
+    "use-gradle-wrapper",
+    "gradle-wrapper-path",
+    "enable-debug",
+    "enable-stacktrace",
+    "jdk-home",
+    "jvm-args",
+  ),
+  "maven" to setOf(
+    "goals",
+    "pom-location",
+    "runner-arguments",
+    "working-directory",
+    "path",
+    "user-settings-selection",
+    "user-settings-path",
+    "use-own-local-repo",
+    "local-repo-scope",
+    "is-incremental",
+    "jdk-home",
+    "jvm-args",
+  ),
+  "node-js" to setOf(
+    "working-directory",
+    "shell-script",
+  ),
+  "command-line" to setOf(
+    "use-custom-script",
+    "script-content",
+    "working-directory",
+    "format-stderr-as-error",
+    "path",
+    "arguments",
+  ),
+)
diff --git a/.../src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionProblems.kt b/.../src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionProblems.kt
@@ -54,4 +54,31 @@ class TooShortValueProblem(
       "The current number of characters is $currentLength."
 }
 
+data class UnsupportedRunnerProblem(
+  val runnerName: String,
+  val supportedRunners: Collection<String>,
+) : InvalidPropertyProblem() {
+  override val message = "Unsupported runner $runnerName. Supported runners: ${supportedRunners.joinToString(", ")}"
+}
+
+data class UnsupportedRunnerParamsProblem(
+  val runnerName: String,
+  val unsupportedParams: Collection<String>,
+  val supportedParams: Collection<String>,
+) : InvalidPropertyProblem() {
+  override val message: String
+    get() {
+      check(supportedParams.isNotEmpty())
+      val paramsPrefix = when (unsupportedParams.size) {
+        1 -> """Parameter "${unsupportedParams.first()}" is"""
+        else -> "Parameters ${unsupportedParams.joinUsingDoubleQuotes()} are"
+      }
+      return "$paramsPrefix not supported by $runnerName runner. " +
+          "Supported parameters: ${supportedParams.joinUsingDoubleQuotes()}"
+    }
+
+  private fun Collection<String>.joinUsingDoubleQuotes() =
+    joinToString(prefix = "\"", separator = "\", \"", postfix = "\"")
+}
+
 class PropertiesCombinationProblem(override val message: String) : InvalidPropertyProblem()
diff --git a/...ions/src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionSpec.kt b/...ions/src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionSpec.kt
@@ -134,8 +134,9 @@ object TeamCityActionSpec {
   object ActionStepWith {
     const val NAME = "with"
     const val DESCRIPTION = "runner or action reference"
-    const val MAX_LENGTH = 100
-    val allowedPrefixes = listOf("runner/", "action/")
+    const val RUNNER_PREFIX = "runner/"
+    private const val ACTION_PREFIX = "action/"
+    val allowedPrefixes = listOf(RUNNER_PREFIX, ACTION_PREFIX)
   }
 
   object ActionStepScript {

diff --git a/...mcity-actions/src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/Validator.kt b/...mcity-actions/src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/Validator.kt
@@ -16,6 +16,7 @@ import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionR
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionStepName
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionStepScript
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionStepWith
+import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionStepWith.RUNNER_PREFIX
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionSteps
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionVersion
 import com.vdurmont.semver4j.Semver
@@ -189,7 +190,6 @@ private suspend fun SequenceScope<PluginProblem>.validateActionStep(step: Action
       )
     )
   } else if (step.with != null) {
-    validateMaxLength(step.with, ActionStepWith.NAME, ActionStepWith.DESCRIPTION, ActionStepWith.MAX_LENGTH)
     if (ActionStepWith.allowedPrefixes.none { step.with.startsWith(it) }) {
       yield(
         InvalidPropertyValueProblem(
@@ -198,6 +198,18 @@ private suspend fun SequenceScope<PluginProblem>.validateActionStep(step: Action
         )
       )
     }
+    if (step.with.startsWith(RUNNER_PREFIX)) {
+      val runnerName = step.with.substringAfter(RUNNER_PREFIX)
+      val allowedParams = allowedRunnerToAllowedParams[runnerName]
+      if (allowedParams == null) {
+        yield(UnsupportedRunnerProblem(runnerName, allowedRunnerToAllowedParams.keys))
+      } else {
+        val unsupportedParams = step.parameters.filter { !allowedParams.contains(it.key) }.keys
+        if (unsupportedParams.isNotEmpty()) {
+          yield(UnsupportedRunnerParamsProblem(runnerName, unsupportedParams, allowedParams))
+        }
+      }
+    }
   } else {
     validateNotEmptyIfExists(step.script, ActionStepScript.NAME, ActionStepScript.DESCRIPTION)
     validateMaxLength(

diff --git a/...src/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/ParseInvalidActionTests.kt b/...src/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/ParseInvalidActionTests.kt
@@ -15,10 +15,11 @@ import com.jetbrains.plugin.structure.teamcity.action.Inputs.someSelectTextInput
 import com.jetbrains.plugin.structure.teamcity.action.Requirements.someExistsRequirement
 import com.jetbrains.plugin.structure.teamcity.action.Steps.someScriptStep
 import com.jetbrains.plugin.structure.teamcity.action.Steps.someWithStep
+import org.junit.Assert
 import org.junit.Test
 import java.nio.file.Files
 import java.nio.file.Path
-import java.util.UUID
+import java.util.*
 
 class ParseInvalidActionTests(
   fileSystemType: FileSystemType,
@@ -419,16 +420,6 @@ class ParseInvalidActionTests(
     )
   }
 
-  @Test
-  fun `action with too long 'with' property for action step`() {
-    assertProblematicPlugin(
-      prepareActionYaml(someAction.copy(steps = listOf(someWithStep.copy(with = "runner/" + randomAlphanumeric(94))))),
-      listOf(
-        TooLongValueProblem("with", "runner or action reference", 101, 100)
-      )
-    )
-  }
-
   @Test
   fun `action with too long 'script' property for action step`() {
     assertProblematicPlugin(
@@ -452,6 +443,46 @@ class ParseInvalidActionTests(
     )
   }
 
+  @Test
+  fun `action with unknown runner`() {
+    assertProblematicPlugin(
+      prepareActionYaml(someAction.copy(steps = listOf(someWithStep.copy(with = "runner/unknown-runner")))),
+      listOf(UnsupportedRunnerProblem("unknown-runner", allowedRunnerToAllowedParams.keys))
+    )
+  }
+
+  @Test
+  fun `action with one unknown runner parameter`() {
+    val runner = "maven"
+    val allowedParams = allowedRunnerToAllowedParams[runner]!!
+    val step = someWithStep.copy(with = "runner/$runner", params = mapOf("unknownParam" to "val", "path" to "somePath"))
+    val result = assertProblematicPlugin(
+      prepareActionYaml(someAction.copy(steps = listOf(step))),
+      listOf(UnsupportedRunnerParamsProblem(runner, listOf("unknownParam"), allowedParams))
+    )
+    Assert.assertEquals(
+      "Parameter \"unknownParam\" is not supported by $runner runner. " +
+          "Supported parameters: ${allowedParams.joinUsingDoubleQuotes()}",
+      result.errorsAndWarnings.first().message,
+    )
+  }
+
+  @Test
+  fun `action with multiple unknown runner parameters`() {
+    val runner = "gradle"
+    val allowedParams = allowedRunnerToAllowedParams[runner]!!
+    val step = someWithStep.copy(with = "runner/$runner", params = mapOf("unknown1" to "val", "unknown2" to "val"))
+    val result = assertProblematicPlugin(
+      prepareActionYaml(someAction.copy(steps = listOf(step))),
+      listOf(UnsupportedRunnerParamsProblem(runner, listOf("unknown1", "unknown2"), allowedParams))
+    )
+    Assert.assertEquals(
+      """Parameters "unknown1", "unknown2" are not supported by $runner runner. """ +
+          "Supported parameters: ${allowedParams.joinUsingDoubleQuotes()}",
+      result.errorsAndWarnings.first().message,
+    )
+  }
+
   @Test
   fun `action with empty 'script' property for action step`() {
     assertProblematicPlugin(
@@ -473,4 +504,7 @@ class ParseInvalidActionTests(
       file("action.yaml") { actionYaml }
     }
   }
+
+  private fun Collection<String>.joinUsingDoubleQuotes() =
+    joinToString(prefix = "\"", separator = "\", \"", postfix = "\"")
 }
diff --git a/...s/src/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/ParseValidActionTests.kt b/...s/src/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/ParseValidActionTests.kt
@@ -52,8 +52,11 @@ class ParseValidActionTests(
 
   @Test
   fun `parse action with runner-based step`() {
-    val step = someWithStep.copy(with = "runner/runnerName")
-    createPluginSuccessfully(prepareActionYaml(someAction.copy(steps = listOf(step))))
+    val runners = listOf("gradle", "maven", "node-js", "command-line")
+    runners.forEach { runnerName ->
+      val step = someWithStep.copy(with = "runner/$runnerName")
+      createPluginSuccessfully(prepareActionYaml(someAction.copy(steps = listOf(step))))
+    }
   }
 
   @Test

diff --git a/...rc/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/ParseValidFullActionTest.kt b/...rc/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/ParseValidFullActionTest.kt
@@ -88,9 +88,8 @@ class ParseValidFullActionTest(
       - name: step 1
         with: runner/maven
         params:
-          pomLocation: pom.xml
+          pom-location: pom.xml
           goals: build
-          one more param: one more value
       - script: echo "step 2 output"
         name: step 2
       - name: step 3

diff --git a/...tructure/tests/src/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/TestData.kt b/...tructure/tests/src/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/TestData.kt
@@ -28,7 +28,7 @@ object Actions {
     steps = listOf(
       ActionStepBuilder(
         stepName = "step name",
-        with = "runner/someRunnerName",
+        with = "runner/command-line",
       )
     ),
   )
@@ -58,7 +58,7 @@ object Requirements {
 object Steps {
   val someWithStep = ActionStepBuilder(
     stepName = "some step",
-    with = "runner/some_runner"
+    with = "runner/command-line"
   )
 
   val someScriptStep = ActionStepBuilder(