WX-1420 Fix GCP Batch label regex restriction (#7355)

Co-authored-by: Beibei Chen <beibei@formbio.com>
Co-authored-by: Janet Gainer-Dewar <jdewar@broadinstitute.org>

supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/models/GcpLabel.scala

@@ -18,13 +18,15 @@ final case class GcpLabel(key: String, value: String)
 object GcpLabel {
 
   val MaxLabelLength = 63
-  val GoogleLabelRegexPattern = "[a-z]([-a-z0-9]*[a-z0-9])?"
-  val GoogleLabelRegex = GoogleLabelRegexPattern.r
+  val GoogleLabelKeyRegexPattern = "[a-z]([-a-z_0-9]*[a-z0-9])?" // label key must start with letter
+  val GoogleLabelValueRegexPattern = "[a-z0-9]([-a-z_0-9]*[a-z0-9])?"
+  val GoogleLabelKeyRegex = GoogleLabelKeyRegexPattern.r
+  val GoogleLabelValueRegex = GoogleLabelValueRegexPattern.r
 
-  // This function is used to coerce a string into one that meets the requirements for a label submission to Google Pipelines API.
-  // See 'labels' in https://cloud.google.com/genomics/reference/rpc/google.genomics.v1alpha2#google.genomics.v1alpha2.RunPipelineArgs
+  // This function is used to coerce a string into one that meets the requirements for a label submission to Google Batch API.
+  // https://cloud.google.com/compute/docs/labeling-resources#requirements
   def safeGoogleName(mainText: String, emptyAllowed: Boolean = false): String =
-    validateLabelRegex(mainText) match {
+    validateLabelRegex(mainText, true) match {
       case Valid(labelText) => labelText
       case invalid @ _ if mainText.equals("") && emptyAllowed => mainText
       case invalid @ _ =>
@@ -57,15 +59,19 @@ object GcpLabel {
         }
     }
 
-  def validateLabelRegex(s: String): ErrorOr[String] =
-    (GoogleLabelRegex.pattern.matcher(s).matches, s.length <= MaxLabelLength) match {
+  def validateLabelRegex(s: String, isKey: Boolean): ErrorOr[String] = {
+    val regexPattern = if (isKey) GoogleLabelKeyRegex.pattern else GoogleLabelValueRegex.pattern
+    val field = if (isKey) "label key" else "label value"
+
+    (regexPattern.matcher(s).matches, s.length <= MaxLabelLength) match {
       case (true, true) => s.validNel
       case (false, false) =>
-        s"Invalid label field: `$s` did not match regex '$GoogleLabelRegexPattern' and it is ${s.length} characters. The maximum is $MaxLabelLength.".invalidNel
-      case (false, _) => s"Invalid label field: `$s` did not match the regex '$GoogleLabelRegexPattern'".invalidNel
+        s"Invalid $field field: `$s` did not match regex '$regexPattern' and it is ${s.length} characters. The maximum is $MaxLabelLength.".invalidNel
+      case (false, _) => s"Invalid $field field: `$s` did not match the regex '$regexPattern'".invalidNel
       case (_, false) =>
-        s"Invalid label field: `$s` is ${s.length} characters. The maximum is $MaxLabelLength.".invalidNel
+        s"Invalid $field field: `$s` is ${s.length} characters. The maximum is $MaxLabelLength.".invalidNel
     }
+  }
 
   def safeLabels(values: (String, String)*): Seq[GcpLabel] = {
     def safeGoogleLabel(kvp: (String, String)): GcpLabel =
@@ -74,7 +80,7 @@ object GcpLabel {
   }
 
   def validateLabel(key: String, value: String): ErrorOr[GcpLabel] =
-    (validateLabelRegex(key), validateLabelRegex(value)).mapN { (validKey, validValue) =>
+    (validateLabelRegex(key, true), validateLabelRegex(value, false)).mapN { (validKey, validValue) =>
       GcpLabel(validKey, validValue)
     }
 

supportedBackends/google/batch/src/test/scala/cromwell/backend/google/batch/models/GcpBatchLabelSpec.scala

@@ -18,14 +18,13 @@ class GcpBatchLabelSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matche
     "cromwell-root-workflow-id-" -> "cromwell-root-workflow-id---x",
     "0-cromwell-root-workflow-id-" -> "x--0-cromwell-root-workflow-id---x",
     "Cromwell-root-workflow-id" -> "cromwell-root-workflow-id",
-    "cromwell_root_workflow_id" -> "cromwell-root-workflow-id",
     "too-long-too-long-too-long-too-long-too-long-too-long-too-long-t" -> "too-long-too-long-too-long-too---g-too-long-too-long-too-long-t",
     "0-too-long-and-invalid-too-long-and-invalid-too-long-and-invali+" -> "x--0-too-long-and-invalid-too----nvalid-too-long-and-invali---x"
   )
 
   googleLabelConversions foreach { case (label: String, conversion: String) =>
     it should s"not validate the bad label key '$label'" in {
-      GcpLabel.validateLabelRegex(label) match {
+      GcpLabel.validateLabelRegex(label, true) match {
         case Invalid(_) => // Good!
         case Valid(_) => fail(s"Label validation succeeded but should have failed.")
       }