Merge pull request #3531 from balopat/init_refactor/k8smanifest_parsing

init refactor/clearer k8s manifest parsing

pkg/skaffold/deploy/kubectl.go

@@ -161,7 +161,7 @@ func (k *KubectlDeployer) manifestFiles(manifests []string) ([]string, error) {
 
 	var filteredManifests []string
 	for _, f := range list {
-		if !util.IsSupportedKubernetesFormat(f) {
+		if !util.HasKubernetesFileExtension(f) {
 			if !util.StrSliceContains(manifests, f) {
 				logrus.Infof("refusing to deploy/delete non {json, yaml} file %s", f)
 				logrus.Info("If you still wish to deploy this file, please specify it directly, outside a glob pattern.")

pkg/skaffold/initializer/kubectl/kubectl.go

@@ -62,11 +62,11 @@ func New(potentialConfigs []string) (*Kubectl, error) {
 
 // IsKubernetesManifest is for determining if a file is a valid Kubernetes manifest
 func IsKubernetesManifest(file string) bool {
-	if !util.IsSupportedKubernetesFormat(file) {
+	if !util.HasKubernetesFileExtension(file) {
 		return false
 	}
 
-	_, err := parseImagesFromKubernetesYaml(file)
+	_, err := parseKubernetesObjects(file)
 	return err == nil
 }
 
@@ -88,20 +88,35 @@ func (k *Kubectl) GetImages() []string {
 	return k.images
 }
 
-// parseImagesFromKubernetesYaml uses required fields from the k8s spec
+type yamlObject map[interface{}]interface{}
+
+// parseImagesFromKubernetesYaml parses the kubernetes yamls, and if it finds at least one
+// valid Kubernetes object, it will return the images referenced in them.
+func parseImagesFromKubernetesYaml(filepath string) ([]string, error) {
+	k8sObjects, err := parseKubernetesObjects(filepath)
+	if err != nil {
+		return nil, err
+	}
+
+	images := []string{}
+	for _, k8sObject := range k8sObjects {
+		images = append(images, parseImagesFromYaml(k8sObject)...)
+	}
+	return images, nil
+}
+
+// parseKubernetesObjects uses required fields from the k8s spec
 // to determine if a provided yaml file is a valid k8s manifest, as detailed in
 // https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/#required-fields.
-// if so, it will return the images referenced in the k8s config
-// so they can be built by the generated skaffold yaml
-func parseImagesFromKubernetesYaml(filepath string) ([]string, error) {
+// If so, it will return the parsed objects.
+func parseKubernetesObjects(filepath string) ([]yamlObject, error) {
 	f, err := os.Open(filepath)
 	if err != nil {
 		return nil, errors.Wrap(err, "opening config file")
 	}
 	r := k8syaml.NewYAMLReader(bufio.NewReader(f))
 
-	yamlsFound := 0
-	images := []string{}
+	var k8sObjects []yamlObject
 
 	for {
 		doc, err := r.Read()
@@ -112,26 +127,24 @@ func parseImagesFromKubernetesYaml(filepath string) ([]string, error) {
 			return nil, errors.Wrap(err, "reading config file")
 		}
 
-		m := make(map[interface{}]interface{})
-		if err := yaml.Unmarshal(doc, &m); err != nil {
+		obj := make(yamlObject)
+		if err := yaml.Unmarshal(doc, &obj); err != nil {
 			return nil, errors.Wrap(err, "reading Kubernetes YAML")
 		}
 
-		if !isKubernetesYaml(m) {
+		if !hasRequiredK8sManifestFields(obj) {
 			continue
 		}
 
-		yamlsFound++
-
-		images = append(images, parseImagesFromYaml(m)...)
+		k8sObjects = append(k8sObjects, obj)
 	}
-	if yamlsFound == 0 {
+	if len(k8sObjects) == 0 {
 		return nil, errors.New("no valid Kubernetes objects decoded")
 	}
-	return images, nil
+	return k8sObjects, nil
 }
 
-func isKubernetesYaml(doc map[interface{}]interface{}) bool {
+func hasRequiredK8sManifestFields(doc map[interface{}]interface{}) bool {
 	for _, field := range requiredFields {
 		if _, ok := doc[field]; !ok {
 			logrus.Debugf("%s not present in yaml, continuing", field)
@@ -142,14 +155,14 @@ func isKubernetesYaml(doc map[interface{}]interface{}) bool {
 }
 
 // adapted from pkg/skaffold/deploy/kubectl/recursiveReplaceImage()
-func parseImagesFromYaml(doc interface{}) []string {
+func parseImagesFromYaml(obj interface{}) []string {
 	images := []string{}
-	switch t := doc.(type) {
+	switch t := obj.(type) {
 	case []interface{}:
 		for _, v := range t {
 			images = append(images, parseImagesFromYaml(v)...)
 		}
-	case map[interface{}]interface{}:
+	case yamlObject:
 		for k, v := range t {
 			if k.(string) != "image" {
 				images = append(images, parseImagesFromYaml(v)...)

pkg/skaffold/util/util.go

@@ -51,10 +51,10 @@ func RandomID() string {
 // These are the supported file formats for Kubernetes manifests
 var validSuffixes = []string{".yml", ".yaml", ".json"}
 
-// IsSupportedKubernetesFormat is for determining if a file under a glob pattern
+// HasKubernetesFileExtension is for determining if a file under a glob pattern
 // is deployable file format. It makes no attempt to check whether or not the file
 // is actually deployable or has the correct contents.
-func IsSupportedKubernetesFormat(n string) bool {
+func HasKubernetesFileExtension(n string) bool {
 	for _, s := range validSuffixes {
 		if strings.HasSuffix(n, s) {
 			return true

pkg/skaffold/util/util_test.go

@@ -53,7 +53,7 @@ func TestSupportedKubernetesFormats(t *testing.T) {
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
-			actual := IsSupportedKubernetesFormat(test.in)
+			actual := HasKubernetesFileExtension(test.in)
 
 			t.CheckDeepEqual(test.out, actual)
 		})