coredns / fallback

The fallback plugin allows sending queries to alternate upstreams based on rcodes returned from the plugin chain.
Apache License 2.0
12 stars 17 forks source link

Move Lookup logic from current Proxy to Forward #9

Closed fturib closed 3 years ago

fturib commented 6 years ago

Right now, fixes for fallback most likely induce fix in proxy (in fact in the Lookup part) It would be better to improve plugin Forward instead of Proxy.

However, that needs some adaptation in plugin Forward to implement reusable Lookup and ensure the Lookup can be defined from Setup parameters.

ekleiner commented 6 years ago

I am wondering. Should we keep support of both: fallback and proxy or it is better not to overcomplicate and just switch to forward?

ekleiner commented 6 years ago

I would propose the next syntax for fallback plugin with proxy by default for now and making forward default and it in future:

{
    fallback [original] [proxy|forward] RCODE PROXY_PARAMS
}
fturib commented 6 years ago

@miekg , @johnbelamaric : what are the rules for backward incompatibility or deprecation for an external plugin such as Fallback ?

if @ekleiner switch implementation of inner proxy of Fallback from proxy.Proxy to forward.Proxy, most likely the parameters PROXY_PARAMS will change, which will create a backward incompatible path.

Example, right now:

. {
    ...
    fallback NXDOMAIN . 192.168.1.1:53 192.168.1.2:53 {
        protocol dns force_tcp
    }
}

will become easily:

. {
    ...
    fallback NXDOMAIN . 192.168.1.1:53 192.168.1.2:53 {
        force_tcp
    }
}
miekg commented 6 years ago

[ Quoting notifications@github.com in "Re: [coredns/fallback] Move Lookup ..." ]

@miekg , @johnbelamaric : what are the rules for backward incompatibility or deprecation for an external plugin such as Fallback ?

The rules are:

version 1.x.y: announce deprecation version 1.x.y+1: allow config, but noop the functionality version 1.x+1.0: deprecate and remove the code

fturib commented 6 years ago

Maybe the wise way is to follow that path : deprecate current "fallback" and create a new one based on the proxy ability of forward plugin.

ekleiner commented 6 years ago

I like suggestion from Francois a lot more then adding temporary parameters to current plugin. Good idea to create new plugin based on current and make changes there.

fturib commented 6 years ago

can we name "ffallback" for "forward-fallback" ?

fturib commented 6 years ago

@miekg : is your reply about deprecation about external plugin ? NOTE: as an external plugin it is not in the released image of CoreDNS ...

hum.. but how do we warn the change of stanza in that case ? need to version the external plugins ? (at least the one hosted on coreDNS repo ?)

chrisohaver commented 6 years ago

If we do change the name (if we actually need to), how about... "default" or "alternate"

fturib commented 6 years ago

The plan is to deprecate this Fallback plugin (which is based on Proxy plugin capability) and create a similar plugin named "Alternate" that will be based on Forward plugin (using proxy capability of Forward).

Current users of this Fallback plugin will have the deprecation time to move to the other Alternate plugin and adapt to the new syntax.

@miekg : are you ok with this solution ?

if Yes, can you create the repo "alternate" : https://github.com/coredns/alternate. (I guess anyone can create it, but I am not sure what should be the setting and if anyone can do those settings once it is created).

Thanks

miekg commented 6 years ago

[ Quoting notifications@github.com in "Re: [coredns/fallback] Move Lookup ..." ]

The plan is to deprecated this Fallback plugin based on Proxy plugin as proxy itself will be deprecated soon) and create a similar plugin named "Alternate" that will be based on Forward plugin (using proxy capability of Forward).

Current users of this Fallback plugin will have the deprecation time to move to the other Alternate plugin and adapt to the new syntax.

@miekg : are you o with this solution ?

This is outside coredns/coredns, so being a bit blunt I don't care that much.

if Yes, can you create the repo "alternate" : https://github.com/coredns/altenate. (I guess anyone can create it, but I am not sure what should be the setting and if anyone can do those settings once it is created).

you should be able to create it.

fturib commented 6 years ago

you should be able to create it.

You are right. I just did it.

@ekleiner : the repo is now available. I am able to define the settings for this repo (I guess because I am the creator).

fturib commented 6 years ago

Seems related to wrong link. the right one is : https://github.com/coredns/alternate