Use parsed YAML structure to better detect valid Skaffold files. (Goo…

…gleContainerTools#111)
JetDC · Dec 19, 2018 · 4d03a66 · 4d03a66
1 parent fde682b
commit 4d03a66
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 19 deletions.
diff --git a/skaffold/src/main/kotlin/com/google/container/tools/skaffold/SkaffoldFileService.kt b/skaffold/src/main/kotlin/com/google/container/tools/skaffold/SkaffoldFileService.kt
@@ -22,16 +22,9 @@ import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.psi.search.FileTypeIndex
 import com.intellij.psi.search.GlobalSearchScope
 import org.jetbrains.yaml.YAMLFileType
-import java.io.ByteArrayInputStream
-import java.io.InputStream
-import java.util.Scanner
-import java.util.regex.Pattern
 
 // see https://github.com/GoogleContainerTools/skaffold/blob/master/examples/annotated-skaffold.yaml
-private const val SKAFFOLD_API_HEADER_REGEX = """\s*apiVersion:\s*skaffold/v"""
-private val SKAFFOLD_API_HEADER_PATTERN: Pattern by lazy {
-    Pattern.compile(SKAFFOLD_API_HEADER_REGEX)
-}
+private const val SKAFFOLD_API_HEADER = "skaffold/v"
 
 /** IDE service for finding Skaffold files in the given IDE project. */
 class SkaffoldFileService {
@@ -47,12 +40,13 @@ class SkaffoldFileService {
     fun isSkaffoldFile(file: VirtualFile): Boolean {
         with(file) {
             if (!isDirectory && fileType is YAMLFileType && isValid) {
-                val inputStream: InputStream = ByteArrayInputStream(contentsToByteArray())
-                inputStream.use {
-                    val scanner = Scanner(it)
-                    // consider this YAML file as Skaffold when first line contains proper API version
-                    return scanner.hasNextLine() &&
-                        SKAFFOLD_API_HEADER_PATTERN.matcher(scanner.nextLine()).find()
+                try {
+                    val skaffoldYaml = SkaffoldYamlConfiguration(this)
+                    return skaffoldYaml.apiVersion?.startsWith(SKAFFOLD_API_HEADER) == true
+                } catch (ex: Exception) {
+                    // We don't care about I/O or scan exceptions here since we only need to know if
+                    // the YAML file was in the proper format.
+                    return false
                 }
             }
         }

diff --git a/skaffold/src/main/kotlin/com/google/container/tools/skaffold/SkaffoldYamlConfiguration.kt b/skaffold/src/main/kotlin/com/google/container/tools/skaffold/SkaffoldYamlConfiguration.kt
@@ -37,6 +37,9 @@ class SkaffoldYamlConfiguration(skaffoldYamlFile: VirtualFile) {
         )
     }
 
+    /** apiVersion of Skaffold configuration file, in the form of skaffold/v{number} */
+    val apiVersion: String? = skaffoldYamlMap["apiVersion"]?.toString()
+
     /**
      * Skaffold profiles: map of profile name to a list of profile objects, each represented by
      * a map. If there are no profiles, empty map is returned.

diff --git a/skaffold/src/test/kotlin/com/google/container/tools/skaffold/SkaffoldFileServiceTest.kt b/skaffold/src/test/kotlin/com/google/container/tools/skaffold/SkaffoldFileServiceTest.kt
@@ -47,14 +47,18 @@ class SkaffoldFileServiceTest {
     }
 
     @Test
-    fun `extra spaces and tabs in header skaffold file is accepted`() {
+    fun `multiple fields and values with a header skaffold file is accepted`() {
         val skaffoldFile = MockVirtualFile.file("skaffold.yaml")
         skaffoldFile.setText(
-            """ apiVersion:    skaffold/v1alpha3
-                  kind: Config
-                  build:
-
             """
+            apiVersion: skaffold/v1beta1
+            kind: Config
+            profiles:
+              - name: gcb
+                build:
+                  googleCloudBuild:
+                    projectId: k8s-skaffold
+        """
         )
 
         assertThat(skaffoldFileService.isSkaffoldFile(skaffoldFile)).isTrue()