crdsdev / doc

Automatic documentation for CustomResourceDefinitions
https://doc.crds.dev
Apache License 2.0
233 stars 35 forks source link

Improve Yaml Parsing & CRD Selection #145

Open RichiCoder1 opened 3 years ago

RichiCoder1 commented 3 years ago

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 commented 3 years ago

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

RichiCoder1 commented 3 years ago

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 commented 3 years ago

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

RichiCoder1 commented 3 years ago

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

hasheddan commented 3 years ago

@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 :)