From 50565c7a9b4d5f9ab96ee279dd92e2ae4f5e31d6 Mon Sep 17 00:00:00 2001 From: balopat Date: Sun, 19 Jan 2020 10:16:29 -0800 Subject: [PATCH 1/2] init refactor/clearer k8s manifest parsing --- pkg/skaffold/deploy/kubectl.go | 2 +- pkg/skaffold/initializer/kubectl/kubectl.go | 52 +++++++++++++-------- pkg/skaffold/util/util.go | 4 +- pkg/skaffold/util/util_test.go | 2 +- 4 files changed, 37 insertions(+), 23 deletions(-) diff --git a/pkg/skaffold/deploy/kubectl.go b/pkg/skaffold/deploy/kubectl.go index 11106456cc0..68c316c024a 100644 --- a/pkg/skaffold/deploy/kubectl.go +++ b/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.") diff --git a/pkg/skaffold/initializer/kubectl/kubectl.go b/pkg/skaffold/initializer/kubectl/kubectl.go index 68306034f5f..53221642758 100644 --- a/pkg/skaffold/initializer/kubectl/kubectl.go +++ b/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,25 @@ 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++ + k8sObjects = append(k8sObjects, obj) - images = append(images, parseImagesFromYaml(m)...) } - 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 +156,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)...) diff --git a/pkg/skaffold/util/util.go b/pkg/skaffold/util/util.go index cdae2ccafa9..bbc2a257773 100644 --- a/pkg/skaffold/util/util.go +++ b/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 diff --git a/pkg/skaffold/util/util_test.go b/pkg/skaffold/util/util_test.go index 956a02631af..42559014b97 100644 --- a/pkg/skaffold/util/util_test.go +++ b/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) }) From c8aacd4d51965354094a4fdfab9838af76cfe2a1 Mon Sep 17 00:00:00 2001 From: balopat Date: Sun, 19 Jan 2020 12:38:36 -0800 Subject: [PATCH 2/2] lint --- pkg/skaffold/initializer/kubectl/kubectl.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/skaffold/initializer/kubectl/kubectl.go b/pkg/skaffold/initializer/kubectl/kubectl.go index 53221642758..a7f275f5461 100644 --- a/pkg/skaffold/initializer/kubectl/kubectl.go +++ b/pkg/skaffold/initializer/kubectl/kubectl.go @@ -137,7 +137,6 @@ func parseKubernetesObjects(filepath string) ([]yamlObject, error) { } k8sObjects = append(k8sObjects, obj) - } if len(k8sObjects) == 0 { return nil, errors.New("no valid Kubernetes objects decoded")