Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

init refactor/clearer k8s manifest parsing #3531

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
52 changes: 33 additions & 19 deletions pkg/skaffold/initializer/kubectl/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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)...)
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down