[6/6] fff-dhcp: Add configuration scripts for dns

Submitted by Fabian Blaese on May 25, 2019, 6:48 p.m.

Details

Message ID 20190525184833.20820-7-fabian@blaese.de
State Accepted
Headers show

Commit Message

Fabian Blaese May 25, 2019, 6:48 p.m.
Because DNS Forwarding is done by dnsmasq which we configure
inside the fff-dhcp package, the configuration scripts for dns
are placed in this package.
---
 .../fff/fff-dhcp/files/etc/gateway.d/35-dns   | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns

Patch hide | download patch | download mbox

diff --git a/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns b/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
new file mode 100644
index 0000000..8ffd440
--- /dev/null
+++ b/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
@@ -0,0 +1,21 @@ 
+configure() {
+	## dns
+	uci -q del dhcp.@dnsmasq[0].server
+	if uci -q get gateway.@dns[0].server > /dev/null; then
+		for f in $(uci get gateway.@dns[0].server); do
+			uci add_list dhcp.@dnsmasq[0].server=$f
+			uci add_list dhcp.@dnsmasq[0].server="/in-addr.arpa/$f"
+			uci add_list dhcp.@dnsmasq[0].server="/ip6.arpa/$f"
+		done
+	else
+		echo "WARNING: No DNS servers set!"
+	fi
+}
+
+commit() {
+	uci commit dhcp
+}
+
+revert() {
+	uci revert dhcp
+}

Comments

Adrian Schmutzler May 25, 2019, 7:23 p.m.
Hallo Fabian,

Reviewed-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>

mit ein paar Vorschlägen unten.

Ich habe die Datei bei mir 40-dns genannt, wenn du Lust hast das anzugleichen wäre das praktisch für mich.


> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> Of Fabian Bläse
> Sent: Samstag, 25. Mai 2019 20:49
> To: franken-dev@freifunk.net
> Subject: [PATCH 6/6] fff-dhcp: Add configuration scripts for dns
> 
> Because DNS Forwarding is done by dnsmasq which we configure inside the
> fff-dhcp package, the configuration scripts for dns are placed in this package.
> ---
>  .../fff/fff-dhcp/files/etc/gateway.d/35-dns   | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
> 
> diff --git a/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
> b/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
> new file mode 100644
> index 0000000..8ffd440
> --- /dev/null
> +++ b/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
> @@ -0,0 +1,21 @@
> +configure() {
> +	## dns
> +	uci -q del dhcp.@dnsmasq[0].server

Das habe ich ins if gezogen, dann bleibt alles beim alten, wenn der Block in der config fehlt. Ist aber reine Geschmackssache.

> +	if uci -q get gateway.@dns[0].server > /dev/null; then
> +		for f in $(uci get gateway.@dns[0].server); do

Das kann man schön zusammenfassen:

	if dnsservers=$(uci -q get gateway.@dns[0].server); then
		for f in $dnsservers; do

Grüße

Adrian

> +			uci add_list dhcp.@dnsmasq[0].server=$f
> +			uci add_list dhcp.@dnsmasq[0].server="/in-
> addr.arpa/$f"
> +			uci add_list dhcp.@dnsmasq[0].server="/ip6.arpa/$f"
> +		done
> +	else
> +		echo "WARNING: No DNS servers set!"
> +	fi
> +}
> +
> +commit() {
> +	uci commit dhcp
> +}
> +
> +revert() {
> +	uci revert dhcp
> +}
> --
> 2.21.0
Robert Langhammer May 30, 2019, 11:11 p.m.
Hi Fabian,

Reviewed-by: Robert Langhammer <rlanghammer@web.de>

Am 25.05.19 um 20:48 schrieb Fabian Bläse:
> Because DNS Forwarding is done by dnsmasq which we configure
> inside the fff-dhcp package, the configuration scripts for dns
> are placed in this package.
> ---
>  .../fff/fff-dhcp/files/etc/gateway.d/35-dns   | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
>
> diff --git a/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns b/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
> new file mode 100644
> index 0000000..8ffd440
> --- /dev/null
> +++ b/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
> @@ -0,0 +1,21 @@
> +configure() {
> +	## dns
> +	uci -q del dhcp.@dnsmasq[0].server
> +	if uci -q get gateway.@dns[0].server > /dev/null; then
> +		for f in $(uci get gateway.@dns[0].server); do
> +			uci add_list dhcp.@dnsmasq[0].server=$f
> +			uci add_list dhcp.@dnsmasq[0].server="/in-addr.arpa/$f"
> +			uci add_list dhcp.@dnsmasq[0].server="/ip6.arpa/$f"
> +		done
> +	else
> +		echo "WARNING: No DNS servers set!"
> +	fi
> +}
> +
> +commit() {
> +	uci commit dhcp
> +}
> +
> +revert() {
> +	uci revert dhcp
> +}
Fabian Blaese May 31, 2019, 4:27 p.m.
Hallo Adrian,

On 25.05.19 21:23, mail@adrianschmutzler.de wrote:
> Hallo Fabian,
> 
> Reviewed-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> 
> mit ein paar Vorschlägen unten.
> 
> Ich habe die Datei bei mir 40-dns genannt, wenn du Lust hast das anzugleichen wäre das praktisch für mich.
Warum?
Zugegeben, ich hab mir bei der Nummerierung nicht allzu viele Gedanken gemacht, aber warum 40?

>> diff --git a/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
>> b/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
>> new file mode 100644
>> index 0000000..8ffd440
>> --- /dev/null
>> +++ b/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns
>> @@ -0,0 +1,21 @@
>> +configure() {
>> +	## dns
>> +	uci -q del dhcp.@dnsmasq[0].server
> 
> Das habe ich ins if gezogen, dann bleibt alles beim alten, wenn der Block in der config fehlt. Ist aber reine Geschmackssache.
Bei mir halt immer die Idee, dass alles weg kommt, was der Nutzer nicht explizit haben will.
Daher würde ich das so lassen.

>> +	if uci -q get gateway.@dns[0].server > /dev/null; then
>> +		for f in $(uci get gateway.@dns[0].server); do
> 
> Das kann man schön zusammenfassen:
> 
> 	if dnsservers=$(uci -q get gateway.@dns[0].server); then
> 		for f in $dnsservers; do
Jo. Ich mach dann ne v2.

Gruß
Fabian
Adrian Schmutzler May 31, 2019, 8:26 p.m.
Hallo,

 

das mit dem Namen ist bei mir auch reine Willkür, mir geht es nur darum dass sie dann gleich heißen und man einfacher diffen kann (bei mir ist die package schon in der FW). Ich kann meine dazu aber auch umbenennen.

 

Zwecks DNS löschen sehe ich halt keinen Grund, warum man alle DNS Server entfernen können wollte. Das war aber nur ein Vorschlag, ist mit hier total egal.

 

Grüße

 

Adrian

 

From: Fabian Bläse [mailto:fabian@blaese.de] 
Sent: Freitag, 31. Mai 2019 18:27
To: mail@adrianschmutzler.de; franken-dev@freifunk.net
Subject: Re: [PATCH 6/6] fff-dhcp: Add configuration scripts for dns

 

Hallo Adrian, 

On 25.05.19 21:23, mail@adrianschmutzler.de <mailto:mail@adrianschmutzler.de>  wrote: 
> Hallo Fabian, 
> 
> Reviewed-by: Adrian Schmutzler <freifunk@adrianschmutzler.de <mailto:freifunk@adrianschmutzler.de> > 
> 
> mit ein paar Vorschlägen unten. 
> 
> Ich habe die Datei bei mir 40-dns genannt, wenn du Lust hast das anzugleichen wäre das praktisch für mich. 
Warum? 
Zugegeben, ich hab mir bei der Nummerierung nicht allzu viele Gedanken gemacht, aber warum 40? 

>> diff --git a/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns 
>> b/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns 
>> new file mode 100644 
>> index 0000000..8ffd440 
>> --- /dev/null 
>> +++ b/src/packages/fff/fff-dhcp/files/etc/gateway.d/35-dns 
>> @@ -0,0 +1,21 @@ 
>> +configure() { 
>> +    ## dns 
>> +    uci -q del dhcp.@dnsmasq[0].server <mailto:dhcp.@dnsmasq[0].server>  
> 
> Das habe ich ins if gezogen, dann bleibt alles beim alten, wenn der Block in der config fehlt. Ist aber reine Geschmackssache.

Bei mir halt immer die Idee, dass alles weg kommt, was der Nutzer nicht explizit haben will. 
Daher würde ich das so lassen. 

>> +    if uci -q get gateway.@dns[0].server <mailto:gateway.@dns[0].server>  > /dev/null; then 
>> +            for f in $(uci get gateway.@dns[0].server <mailto:gateway.@dns[0].server> ); do 
> 
> Das kann man schön zusammenfassen: 
> 
>       if dnsservers=$(uci -q get gateway.@dns[0].server <mailto:gateway.@dns[0].server> ); then 
>               for f in $dnsservers; do 
Jo. Ich mach dann ne v2. 

Gruß 
Fabian
Fabian Blaese June 2, 2019, 11:27 a.m.
Hallo Adrian,

dann würde ich den Dateinamen so lassen. Bei dir musst du das wenn ich das richtig sehe sowieso noch zwischen Packages verschieben.

Gruß
Fabian

On 31.05.19 22:26, mail@adrianschmutzler.de wrote:
> Hallo,
> 
>  
> 
> das mit dem Namen ist bei mir auch reine Willkür, mir geht es nur darum dass sie dann gleich heißen und man einfacher diffen kann (bei mir ist die package schon in der FW). Ich kann meine dazu aber auch umbenennen.
> 
>  
> 
> Zwecks DNS löschen sehe ich halt keinen Grund, warum man alle DNS Server entfernen können wollte. Das war aber nur ein Vorschlag, ist mit hier total egal.
> 
>  
> 
> Grüße
> 
>  
> 
> Adrian
Fabian Blaese June 15, 2019, 11:12 a.m.
- Fehlendes Signed-off-by von mir ergänzt.
- Die Bedingung im if entsprechend Adrians Vorschlag angepasst.

Applied.