Page MenuHomePhabricator

Extend puppet ipresolve() to support SRV records
Closed, DeclinedPublic

Description

Currently, the ipresolve function does only support A, AAAA and PTR records. It would be nice to be able to resolve SRV records as well to be able to add all of their entries to some ferm rule (T300966).

My context of this is the requirement to allow all etcd cluster nodes to talk to each other without having to hardcode a list of the nodes in hiera or to allow $DOMAIN_NETWORKS (which is that we currently do at least for the etcd peers port).

Not allowing DOMAIN_NETWORKS to access the etcd client port (basically implemented during T363307: Co-locate kube-apiserver and etcd on new staging control plane nodes) was a red herring in investigating T366094: k8s master capacity issues

Event Timeline

Does dnsquery::srv do what you need here?

The main hurdles I see in implementing this in the current ipresolve function are:

  1. It would mean changing the signature of the function from only returning one string representing an ip address to returning an array of structured data. This means basically a full rewrite of both the DNS caching class and of the function itself. Not undoable but a significant chunk of work. I would frankly prefer to get all data via a separate function for SRV records
  2. We need to decide now what data structure we want to return, and if we want higher level functions extracting data (like, a simple list of IP addresses) from that data structure. Getting both right from the start is important.

Having said that, we could do something relatively simplistic and just create a separate function for this that wraps around ipresolve. So the outer function gets the hostnames from the SRV record, which are then fed to ipresolve() to resolve them to IP addresses.

I also checked ferm's own @resolve function and it doesn't support SRV records although adding such support wouldn't be too hard either.

Change #1038283 had a related patch set uploaded (by JMeybohm; author: JMeybohm):

[operations/puppet@production] etcd::v3: Allow all nodes of an etcd cluster to connect to each other

https://gerrit.wikimedia.org/r/1038283

Does dnsquery::srv do what you need here?

I think it does indeed, yes. Thanks! ❤️

Does dnsquery::srv do what you need here?

It doesn't do everything @JMeybohm wants, but we can wire together the two function to actually get what we want as I suggested above.

I also checked ferm's own @resolve function and it doesn't support SRV records although adding such support wouldn't be too hard either.

AIUI @resolve() should no longer be used, as it has it's own drawbacks (T365687, T300966)

jijiki renamed this task from Extend puppte ipresolve() to support SRV records to Extend puppet ipresolve() to support SRV records.Jun 3 2024, 12:56 PM

Change #1038283 merged by JMeybohm:

[operations/puppet@production] etcd::v3: Allow all nodes of an etcd cluster to connect to each other

https://gerrit.wikimedia.org/r/1038283

Given an alternative solution was found for etcd, closing this one (after checking with Janis)