Standardize config files

[cob-web-corner]

Hello,

This is a rather large ask but would make things much easier for parsing, IaC, clustering, dynamic setup, provisioning from config files, Kubernetes config map, docker configuration, etc. Currently the config files located in /etc/dns are of an arbitrary nature. It would be nice to have these in some sort of standardized format that could be statically typed and parsed to classes (i.e. yaml/json). It would additionally significantly reduce your config file writers/readers and map to classes that are already defined.

Some of the apps are already standardized (dnsApp.config for example)

[ShreyasZare]

Thanks for the post. I have had this thought already but the task will need a lot of changes since entire code base is designed to serialize/de-serialize data in binary formats. From development perspective, the readers/writers are not much of an issue since changes are incremental. The original idea to have binary format was to discourage manual editing of the config so that the HTTP API is preferred instead. That was opinionated design decision so that the DNS server can do proper validation with HTTP API and also apply those changes immediately without need to reload/restart like with usual daemons.

[cob-web-corner]

I have had this thought already but the task will need a lot of changes since entire code base is designed to serialize/de-serialize data in binary formats

Yeah when I took a quick look through the code on the config read/writer side it definitely seemed like it would be an effort

That was opinionated design decision so that the DNS server can do proper validation with HTTP API and also apply those changes immediately without need to reload/restart like with usual daemons.

In this case those classes that do the validation could be abstracted to also work when loading the config files

Totally understood overall it would be a large effort. My worker around right now is to load a scratch instance, inject everything through the API and then grab the generated config files. It's an init container that runs, a python script runs everything in a yaml file through the API and then the config gets generated that I can copy/backup/use for other purposes

[ShreyasZare]

In this case those classes that do the validation could be abstracted to also work when loading the config files

With such a design, if there is validation failure then the server fails to start. This is what the current design is trying to prevent by discouraging manual config edits.

My worker around right now is to load a scratch instance, inject everything through the API and then grab the generated config files. It's an init container that runs, a python script runs everything in a yaml file through the API and then the config gets generated that I can copy/backup/use for other purposes

If you can describe your use-case in details then it will help me understand it better so that I can think of some solution to make it easier.

[cob-web-corner]

With such a design, if there is validation failure then the server fails to start. This is what the current design is trying to prevent by discouraging manual config edits.

Such is the case with any software really, but you cannot guard rail the stove because someone might burn their hand

If you can describe your use-case in details then it will help me understand it better so that I can think of some solution to make it easier.

In general the use case is deploying a ready to go system with it's configuration, basically IaC with provisioning after. Think dynamic failovers not from backups, deploying the server using pipelines, running it in kubernetes, etc

[ShreyasZare]

With such a design, if there is validation failure then the server fails to start. This is what the current design is trying to prevent by discouraging manual config edits.

Such is the case with any software really, but you cannot guard rail the stove because someone might burn their hand

That's true. The concern here was due to DNS being critical service, when it fails, it affects the entire system. So, this was to make sure that the DNS server always starts with correct config.

If you can describe your use-case in details then it will help me understand it better so that I can think of some solution to make it easier.

In general the use case is deploying a ready to go system with it's configuration, basically IaC with provisioning after. Think dynamic failovers not from backups, deploying the server using pipelines, running it in kubernetes, etc

Thanks for the description. Will see if something can be done to make it better.

[cob-web-corner]

With such a design, if there is validation failure then the server fails to start. This is what the current design is trying to prevent by discouraging manual config edits.

Such is the case with any software really, but you cannot guard rail the stove because someone might burn their hand

That's true. The concern here was due to DNS being critical service, when it fails, it affects the entire system. So, this was to make sure that the DNS server always starts with correct config.

I understand the idea

If you can describe your use-case in details then it will help me understand it better so that I can think of some solution to make it easier.

In general the use case is deploying a ready to go system with it's configuration, basically IaC with provisioning after. Think dynamic failovers not from backups, deploying the server using pipelines, running it in kubernetes, etc

Thanks for the description. Will see if something can be done to make it better.

Alright thanks for considering it.

An outside the box idea would be a terraform provider, but that's a whole project/effort on its own. Terraform providers basically wrap existing APIs

[IngmarStein]

I'd like to add one more argument in favor of human-readable config files: many people like to put configuration files under version control (e.g. as an audit trail, for changelogs, or easy rollbacks). This is arguably a lot more useful with human-readable diffs.

[timhae]

I realize it would probably take quite some time to implement this so I have a suggestion: what do you think of a "sidecar application" that translates a config file into api calls? This would give you the flexibility to fine tune the config format and make changes under the hood that implement the config file one setting at a time. This could also live in this repo and can be written in any language, also c# if you prefer that.

[ShreyasZare]

I realize it would probably take quite some time to implement this so I have a suggestion: what do you think of a "sidecar application" that translates a config file into api calls? This would give you the flexibility to fine tune the config format and make changes under the hood that implement the config file one setting at a time. This could also live in this repo and can be written in any language, also c# if you prefer that.

Thanks for the suggestion. Such a tool would be redundant. This is since you can just install the DNS server on any laptop, use the GUI to set a desired config and then use the Backup Settings option to export it for use.

[timhae]

The purpose of that would be to allow you not having to do any configuration through the webui but instead have a potentially scm tracked file as mentioned earlier in this issue and other issues regarding configuration files. If you aren't interested, please let me know then I will just implement that downstream in nixpkgs/nixos

[ShreyasZare]

The purpose of that would be to allow you not having to do any configuration through the webui but instead have a potentially scm tracked file as mentioned earlier in this issue and other issues regarding configuration files.

If you are looking for some kind of text config to API call then the API can be updated to accept the settings in json format, the same format the get settings api returns. You can then keep the json locally and track it.

If you aren't interested, please let me know then I will just implement that downstream in nixpkgs/nixos

I can update the API as described above when I have some time available. Creating and maintaining a set of separate tools for the conversion will not be feasible for me.

[timhae]

You mean this endpoint? If it would accept json instead of parameters that would get me 90% there. If you want I can also prepare a PR. Thanks for your time and effort :)

[ShreyasZare]

You mean this endpoint? If it would accept json instead of parameters that would get me 90% there. If you want I can also prepare a PR. Thanks for your time and effort :)

Yes, the Set DNS Settings can be changed to accept json for POST requests. The json can be exact same format as that in the Get DNS Settings call.

[timhae]

This is what I have so far:

From a6648c2e2322f7aca1777a62183979091fde4b80 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20H=C3=A4ring?= <[email protected]>
Date: Thu, 21 Nov 2024 21:53:03 +0100
Subject: [PATCH 1/1] feat: accept json body in SetDnsSettings endpoint

---
 DnsServerCore/WebServiceSettingsApi.cs | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/DnsServerCore/WebServiceSettingsApi.cs b/DnsServerCore/WebServiceSettingsApi.cs
index d99d1d75..b9d8646f 100644
--- a/DnsServerCore/WebServiceSettingsApi.cs
+++ b/DnsServerCore/WebServiceSettingsApi.cs
@@ -31,6 +31,7 @@ using System.Net.Mail;
 using System.Net.Sockets;
 using System.Text;
 using System.Text.Json;
+using System.Text.Json.Nodes;
 using System.Threading;
 using System.Threading.Tasks;
 using TechnitiumLibrary;
@@ -570,7 +571,7 @@ namespace DnsServerCore
             WriteDnsSettings(jsonWriter);
         }

-        public void SetDnsSettings(HttpContext context)
+        public async void SetDnsSettings(HttpContext context)
         {
             UserSession session = context.GetCurrentSession();

@@ -587,10 +588,25 @@ namespace DnsServerCore
             bool _webServiceEnablingTls = false;

             HttpRequest request = context.Request;
+            JsonDocument jsonDoc = null;
+            JsonElement jsonElem = default;
+
+            if (request.HasJsonContentType())
+            {
+                using (var reader = new StreamReader(request.Body))
+                {
+                    var body = await reader.ReadToEndAsync();
+                    jsonDoc = JsonDocument.Parse(body);
+                }
+            }

             //general
             if (request.TryGetQueryOrForm("dnsServerDomain", out string dnsServerDomain))
             {
+                if (jsonDoc != null && jsonDoc.RootElement.TryGetProperty("dnsServerDomain", out jsonElem))
+                {
+                    dnsServerDomain = jsonElem.GetString();
+                }
                 dnsServerDomain = dnsServerDomain.TrimEnd('.');

                 if (!_dnsWebService.DnsServer.ServerDomain.Equals(dnsServerDomain, StringComparison.OrdinalIgnoreCase))
@@ -601,6 +617,10 @@ namespace DnsServerCore
             }

             string dnsServerLocalEndPoints = request.QueryOrForm("dnsServerLocalEndPoints");
+            if (jsonDoc != null && jsonDoc.RootElement.TryGetProperty("dnsServerLocalEndPoints", out jsonElem))
+            {
+                dnsServerLocalEndPoints = jsonElem.GetString();
+            }
             if (dnsServerLocalEndPoints is not null)
             {
                 if (dnsServerLocalEndPoints.Length == 0)
@@ -636,6 +656,10 @@ namespace DnsServerCore
             }

             string dnsServerIPv4SourceAddresses = request.QueryOrForm("dnsServerIPv4SourceAddresses");
+            if (jsonDoc != null && jsonDoc.RootElement.TryGetProperty("dnsServerIPv4SourceAddresses", out jsonElem))
+            {
+                dnsServerIPv4SourceAddresses = jsonElem.GetString();
+            }
             if (dnsServerIPv4SourceAddresses is not null)
                 DnsClientConnection.IPv4SourceAddresses = ParseNetworkAddresses(dnsServerIPv4SourceAddresses);

--
2.47.0

A couple of notes:

• I decided to make the function async, not sure if that is a good idea since now you would basically need a lock to handle concurrent requests
• I don't parse the body json into a struct since you don't have that struct currently and I think the dynamic access is good enough for this and can be improved further down the road
• I always give the json settings predecence because that was easier to implement

If you think this is going in the right direction, I will continue for the rest of the many settings and then open a PR. Please let me know what you think.

[ShreyasZare]

Thanks for the details. I would suggest to define generic sub functions like you see in the Extentions class. These functions would read from the json if its available else they would call the intended extension method. Thus the code that reads the settings would almost remain the same.

[timhae]

I have opened #1123 , please let me know what you think

[ShreyasZare]

I have opened #1123 , please let me know what you think

Thanks for the PR. Wont be able to accept this since it has too many issues related to performance. I will get this implemented myself soon.

[timhae]

I understand. If you want to touch that part of the code anyways, would you mind parsing non-string arguments in the json as their respective type so we don't have to post "true" or "1234" but can do true and 1234? Thanks!

[ShreyasZare]

I understand. If you want to touch that part of the code anyways, would you mind parsing non-string arguments in the json as their respective type so we don't have to post "true" or "1234" but can do true and 1234? Thanks!

I did not get the parsing part. Is there any specific palace you noticed that occurring in the current code/api?

[timhae]

everything is currently parsed from string since forms/url parameters don't have types. JSON bodys do have types so this {"key": "true"} is something different than this {"key": true} and currently all parsing assumes strings as input and not already typed input.. But I can completely understand if you want to keep it like it is since that probably requires less changes.

[ShreyasZare]

everything is currently parsed from string since forms/url parameters don't have types. JSON bodys do have types so this {"key": "true"} is something different than this {"key": true} and currently all parsing assumes strings as input and not already typed input.. But I can completely understand if you want to keep it like it is since that probably requires less changes.

The current parsing which assumes strings as input does that since query strings and forms provides values as strings inherently as you too mentioned. JSON data is always parsed by the JSON library and the values are read using it in the native types in all places. I don't think there is any instance in code where JSON variables are being read as string and then parsed to different type. I have also mentioned that the JSON format for this API would use the same format being returned by the Get Settings API and that format does not use strings for any numbers or boolean.