add tooling to convert endpoints to a tree and back

[philbrookes]

add tooling to convert endpoints to a tree and back

[maleck13]

@maksymvavilov I think we should check for nils more aggressively and add some nil tests also as there are a lot of pointers and derefrencing pointers to get the value in here so could easilly end up with a panic

[maksymvavilov]

@maksymvavilov I think we should check for nils more aggressively and add some nil tests also as there are a lot of pointers and derefrencing pointers to get the value in here so could easilly end up with a panic

safeguarded all public calls from nil pointers

[maleck13]

so this looks fine but I am a bit confused as to why the data set isn't just attached to the node

[maksymvavilov]

We can have more datasets than nodes. For example, we can have 3 klb.testdomain.com records that target two (eu.klb.testdomain.com and us.klb.testdomain.com) and by having 3 datasets we know to create 3 enpoints of klb.tesdomain.com.

We could "push" datasets to the child (this will solve the need to search on removal) but we will end up with an intricate logic for converting them to endpoints: for each node, we will need to check each child for the number of datasets. And it doesn't look right to store in a child the data you need to convert the parent into the endpoint 🤔
here is what i mean:

endpoints := []externaldns.Endpoint{}

for _, child := range node.Children {

    ToEndpoints(child, endpoints)

    for _, dataSet := child.Datasets {
        endpoints = append(endpoints, externaldnsEnpoints{
            DNSName: node.Name
            Targets: dataSet.Targets
            .....
        }
    }
}

or something similar. Will have a Node that has dataset for it's parent.

[maleck13]

eh perhaps it is something we can do beyond this initial impl would rather get this in for now

[maksymvavilov]

it was suggested to use topology-like approach 🤔 we will definitely revisit it

[maleck13]

what if the index is 0? should this be >=0 ? looks like index will return -1 if not present

[maksymvavilov]

good catch. Thanks. Fixed

[maleck13]

I wonder would it read a little better if this check was done in a function name something like isFinalDNSEntry(node *DNSTreeNode)bool

[maksymvavilov]

added isALeafNode() func

[maleck13]

its not clear what node.DataSets being nil means here

[maksymvavilov]

it is a "prevent panic is the tree was created manually and with a mistake"

[maleck13]

some minor things to consider but generally looks good