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

Improve Yaml Parsing & CRD Selection #145

Open
RichiCoder1 opened this issue Apr 13, 2021 · 5 comments
Open

Improve Yaml Parsing & CRD Selection #145

RichiCoder1 opened this issue Apr 13, 2021 · 5 comments

Comments

@RichiCoder1
Copy link
Contributor

Random idea I had while thinking about some issues was to subsititute some of the current regex'd logic with some more robust code based on yaml and yqlib.

This code supports both multi docs and uses yq for selecting CRD Nodes.

Psuedocode:

import (
    /* ...snip... */
	"github.com/mikefarah/yq/v4/pkg/yqlib"
	yaml "gopkg.in/yaml.v3"
)

/* ...snip... */

func splitYAML(greps []git.GrepResult, dir string) map[string][]*yaml.Node {
	allCRDs := map[string][]yaml.Node
	for _, res := range greps {
		b, err := ioutil.ReadFile(dir + "/" + res.FileName)
		if err != nil {
			log.Printf("failed to read yaml file: %s", res.FileName)
			continue
		}
        
        nodes, err := GetCrdNodes(b)
		if err != nil {
			log.Printf("failed to parse yaml file: %s", res.FileName)
			continue
		}
        allCRDS = append(allCRDS, nodes)
	}
	return allCRDs
}

func GetCrdNodes(rawFile []byte) ([]yaml.Node, error) {
	var candidates []*yaml.Node
	dec := yaml.NewDecoder(bytes.NewReader(rawFile))
	for {
		var node yaml.Node
		err := dec.Decode(&node)
		if err == io.EOF {
			break
		}
		if err != nil {
			return nil, err
		}
		candidates = append(candidates, &node)
	}

	evaluator := yqlib.NewAllAtOnceEvaluator()
	matches, err := evaluator.EvaluateNodes(`select(.kind == "CustomResourceDefinition")`, candidates...)
	if err != nil {
		fmt.Println(fmt.Errorf("Failed to parse nodes: %s", err))
	}

	nodes := make([]yaml.Node, 0)
	for match := matches.Front(); match != nil; match = match.Next() {
		candidate := match.Value.(*yqlib.CandidateNode)
		nodes = append(nodes, *candidate.Node)
	}

	return nodes, nil
}

Downside is YQ is not a light dependency

@RichiCoder1
Copy link
Contributor Author

Related to #144, #126, #137, and potentially #134 if we wanted to allow tweaking yq expressions

@RichiCoder1
Copy link
Contributor Author

RichiCoder1 commented Apr 13, 2021

Follow up: Could actually probably still get the benefit of multi-doc and selection without yqlib as we could just check for it ourselves on yaml.Node

@hasheddan
Copy link
Member

As an update, I have observed panics on invalid YAMLs in yaml.v3 in situations where they are templated with {{ as described here.

@RichiCoder1
Copy link
Contributor Author

Hmm. I'll add some panic handling to that method

@hasheddan
Copy link
Member

@RichiCoder1 it isn't too big of a problem because the pod just gets restarted. Feel free to leave as is if you don't want to invest the time because we should have improved parsing soon :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants