--check-config breaks boostrap dns

[jumpsmm7]

Using the Lastest Stable Release when --check-config is run for the first time on a fresh meaning "new" .yaml file bootstrap resolver that are listed in

- 8.8.8.8
- 9.9.9.9

format gets changed to

-'[8.8.8.8 9.9.9.9]'

which the webui reports as an incorrect format.

Here is the output

2022/01/03 01:39:20.322742 [info] AdGuard Home, version v0.107.2
2022/01/03 01:39:20.442423 [info] home.upgradeSchema0to1(): called
2022/01/03 01:39:20.442484 [info] deleting /tmp/mnt/My_Part/entware/etc/AdGuardHome/dnsfilter.txt as we don't need it anymore
2022/01/03 01:39:20.442568 [info] home.upgradeSchema1to2(): called
2022/01/03 01:39:20.442586 [info] deleting /tmp/mnt/My_Part/entware/etc/AdGuardHome/Corefile as we don't need it anymore
2022/01/03 01:39:20.442624 [info] home.upgradeSchema2to3(): called
2022/01/03 01:39:20.442679 [info] home.upgradeSchema3to4(): called
2022/01/03 01:39:20.442705 [info] home.upgradeSchema4to5(): called
2022/01/03 01:39:20.442729 [info] home.upgradeSchema5to6(): called
2022/01/03 01:39:20.442744 [info] Upgrade yaml: 6 to 7
2022/01/03 01:39:20.442758 [info] Upgrade yaml: 7 to 8
2022/01/03 01:39:20.442777 [info] Upgrade yaml: 8 to 9
2022/01/03 01:39:20.442792 [info] Upgrade yaml: 9 to 10
2022/01/03 01:39:20.442820 [info] Upgrade yaml: 10 to 11
2022/01/03 01:39:20.442838 [info] Upgrade yaml: 11 to 12
2022/01/03 01:39:20.449652 [info] configuration file is ok

In side the .yaml file, the bootstrap resolvers have been changed from

- 8.8.8.8
- 9.9.9.9

to

-'[8.8.8.8 9.9.9.9]'

[ainar-g]

Hello. We cannot reproduce this. Judging by the logs, the configuration file isn't exactly “new”, as it tries to upgrade it all the way from schema version zero.

Which is also weird. @ameshkov, I don't think that --check-config should overwrite the configuration file with a migrated version? In fact, I don't think it should do anything except reading it.

[jumpsmm7]

Hello. We cannot reproduce this. Judging by the logs, the configuration file isn't exactly “new”, as it tries to upgrade it all the way from schema version zero.

Which is also weird. @ameshkov, I don't think that --check-config should overwrite the configuration file with a migrated version? In fact, I don't think it should do anything except reading it.

So I am starting out with a new .yaml file with the initial username, password hash generated with bcrypt-tools, bind host and port, dns bind host and port, upstream dns and the boostrap dns defined. I then run check config on it which then upgrades it to hold all the defaults required by AdGuardHome. The problem is that it out puts the boostrap DNS with [ ] brackets around it after upgrading to the defaults. it goes from

- 8.8.8.8
- 9.9.9.9

format to -'[8.8.8.8 9.9.9.9]' after upgrading.

to reproduce the issue you would have to do the same.

Start off with a blank .yaml add in the username and password hash. Then add the dns bind host and port and add the upstream dns, and the bootstrap dns. I added them in the correct format they would appear in the .yaml file. I then ran --check-config on the same config. It added all the rest of the default parameters, except it changed the bootstrap dns format.

[jumpsmm7]

The biggest issue here is if I rerun --check-config afterwards, AdGuardHome views

- '[8.8.8.8 9.9.9.9]'

to be a valid entry for bootstrap dns.

[ainar-g]

Sorry for the late response. The current behaviour is definitely invalid, since --check-config really shouldn't rewrite the configuration file.

As for the original use case, we'll probably consider adding a mechanism for setting some default values on the first run, including user information and addresses, in the future. But that will require a refactoring of the configuration handling, which should happen around v0.108.

[jumpsmm7]

Sorry for the late response. The current behaviour is definitely invalid, since --check-config really shouldn't rewrite the configuration file.

As for the original use case, we'll probably consider adding a mechanism for setting some default values on the first run, including user information and addresses, in the future. But that will require a refactoring of the configuration handling, which should happen around v0.108.

Thank you for your indeavors, I run a .sh installer for a sub-Linux setup (busy box) for asuswrt-merlin routers. I have configured it so entware can utilize AdGuardHome as a package using its package manager. Similar to how openwrt runs it. Basically users can utilize the installer via ssh terminal to install adguardhome for use with asuswrt merlin routers. The .sh installer uses the check config to verify that the config is in good standing.

[rickhull]

I have run into this issue on NixOS, though it is shaped a little differently for me, perhaps. Via Nix, the configuration YAML is generated. Then it is checked with --config-check. On 107.6, --check-config fails with this YAML:

dns:
  bind_hosts = [ '1.2.3.4', '5.6.7.8' ]

It is looking instead for dns.bind_host.

Later in my YAML, I have:

dns:
  bootstrap_dns: ['1.2.3.4', '5.6.7.8']

But I end up with:

dns:
  boostrap_dns:
    - "['1.2.3.4', '5.6.7.8']"

Also, I have:

dns:
  querylog_interval: 24h

And this is rejected by --check-config, saying it doesn't expect a string. I change it to 1:

dns:
  querylog_interval: 1

and then --check-config accepts it and appears to rewrites my yaml back to what I had initially:

dns:
  querylog_interval: 24h

For me, the rewriting is a concern. Also I see the schema upgrade process starting from 0, as mentioned. This is concerning. And maybe this is why --check-config is rejecting yamls using the newest schema.

[jumpsmm7]

@rickhull

I am in full accordance with your opinion on this matter. For me, I manage a shell script where users install AdGuardHome straight from the SSH terminal on to their Asus routers that runs RMerlin or " @RMerl " firmware.

Heres my actual installer: https://github.com/jumpsmm7/Asuswrt-Merlin-AdGuardHome-Installer/blob/master/installer

Check-configs upgrading the .yaml actually helps with setup process because it establishes default values for the .yaml. The problem is like you have pointed out, the process is all bugged out. If it is bugged out for us, imagine how bugged out it is behind the scenes.

This is a workaround I had to implement to prevent the behavior. Once the values are placed inside the .yaml file "correctly" and it no longer has to "upgrade the schema", then the values remain consistent. check_AdGuardHome_yaml () { [ ! -f "$1" ] && return 1 chmod 644 "$1" PTXT "$INFO Checking AdGuardHome configuration..." if [ "$1" = "$YAML_ORI" ] && [ ! -f "$YAML_BAK" ]; then $AGH_FILE --check-config -c "$1" --no-check-update -l "/dev/null" yaml_nvars_insert " bootstrap_dns:" "\ - $BOOTSTRAP2" "$1" yaml_nvars_insert " bootstrap_dns:" "\ - $BOOTSTRAP1" "$1" yaml_nvars_delete "[$BOOTSTRAP1 $BOOTSTRAP2]" "$1" fi yaml_nvars_replace 'log_localtime: false' 'log_localtime: true' "$1" if ! $AGH_FILE --check-config -c "$1" --no-check-update -l "/dev/null"; then PTXT "$INFO Move invalid configuration file to $YAML_ERR" \ "$INFO Operation will continue with clean config file." mv "$1" "$YAML_ERR" return 1 elif [ "$1" = "$YAML_ORI" ]; then cp -a "$1" "$YAML_FILE" fi }

What might help is if the AdGuardHome devs would actually put an Example.yaml included with their setup, this would allow other developers and do it yourselfers to leverage this for the actual installation process.

[rickhull]

deleted, irrelevant

[rickhull]

I think I got to the bottom of it. NixOS is not providing schema_version: 12 in the YAML to --check-config. Thus the schema is assumed to be 0 and goes through the upgrade process and fails early.

CONFIRMED: setting schema_version: 12 resolves the issue.

I created a NixOS issue for setting this as a default: https://github.com/NixOS/nixpkgs/issues/173938

[jumpsmm7]

@rickhull That definitely seems to fix this bug as well. So the solution is to make sure the correct schema_version is applied before attempting to run --check-config.

[jumpsmm7]

@ainar-g so a quick solution for this is to make sure the appropriate schema is present inside the .yaml before check-config is ran on it the first time. It appears check-configs bug may be inside old aspects of adguardhome check-config feature that may need to be revised. If the user (or package handlers) make sure the correct schema is inside the .yaml , then the crisis is adverted. Maybe some appropriate documentation should be added to the wiki, until an alternative fix is found for this bug.