Skip to content
This repository was archived by the owner on Mar 31, 2025. It is now read-only.

Add support for selective config sync on v6 PiHoles#567

Open
dreary-ennui wants to merge 7 commits intomattwebbio:masterfrom
dreary-ennui:v6_selective_config
Open

Add support for selective config sync on v6 PiHoles#567
dreary-ennui wants to merge 7 commits intomattwebbio:masterfrom
dreary-ennui:v6_selective_config

Conversation

@dreary-ennui
Copy link

@dreary-ennui dreary-ennui commented Mar 14, 2025

Adds a couple functions to GET and PATCH specific portions of /api/config for v6 PiHoles that can be used when syncing the entirety of the config file is not preferred.

Currently supports syncing of:

  • Settings - Local DNS Records - Local DNS A records
  • Settings - Local DNS Records - Local DNS CNAME records
  • Settings - DNS - Upstream DNS Servers (built-in and custom)

Resolves #565
Desired functionality mentioned in #190, #576

Comment on lines 56 to 105
const clientVersion = client.getVersion();
if (clientVersion === 6 && success && enableSelectiveSync) {
const clientExistingConfig = JSON.parse(
await client.getExistingConfig()
);

if (
clientExistingConfig.config.webserver.api.app_pwhash === '' ||
(clientExistingConfig.config.webserver.api.app_pwhash !== '' &&
clientExistingConfig.config.webserver.api.app_sudo)
) {
let patchedConfigDNSRecords = {};
let patchedConfigCNAMERecords = {};

if (config.sync.v6.selective_LocalDnsRecords) {
patchedConfigDNSRecords = {
config: {
dns: {
hosts: configDataBackup.config.dns.hosts
}
}
};
}
if (config.sync.v6.selective_LocalCnameRecords) {
patchedConfigCNAMERecords = {
config: {
dns: {
cnameRecords: configDataBackup.config.dns.cnameRecords
}
}
};
}

const patchedConfig = deepMerge(
{},
patchedConfigDNSRecords,
patchedConfigCNAMERecords
);
if (patchedConfig) {
success = await client.uploadPatchedConfig(patchedConfig);
}
} else {
log.info(
chalk.red(
`⚠ Error: Secondary host ${host.baseUrl} has an app password configured but webserver.api.app_sudo is set to false in pihole.toml. Skipping selective config sync for this host. Set webserver.api.app_sudo to true to resolve, in either the pihole.toml file or from the pihole web interface by going to Settings -> All settings -> Webserver and API. You may need to click the Blue "Modified settings" slider if you do not see webserver.api.app_sudo in the UI.`
)
);
}
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will probably try to figure out a way to move this into downloadBackup and uploadBackup - I'd like to keep as much logic out of sync.ts as possible

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free! I was thinking of maybe doing the same; I figured since it was supplemental to the teleport functions and not exactly related to whether or not those succeed / fail that I'd make separate functions. I definitely agree that keeping sync.ts tidy is a good goal though

@mattwebbio
Copy link
Owner

mattwebbio commented Mar 14, 2025

Thanks so much for the submission, @dreary-ennui! I'll try to get some version of this merged over the weekend. I hope you don't mind if I refactor it a little bit (left a comment explaining what I'll probably do)

@codecov
Copy link

codecov bot commented Mar 14, 2025

Codecov Report

Attention: Patch coverage is 94.44444% with 4 lines in your changes missing coverage. Please review.

Project coverage is 97.57%. Comparing base (5e4bee8) to head (f3c5c80).

Files with missing lines Patch % Lines
src/sync.ts 86.66% 4 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #567      +/-   ##
==========================================
- Coverage   98.16%   97.57%   -0.60%     
==========================================
  Files          12       13       +1     
  Lines         381      453      +72     
  Branches      113      134      +21     
==========================================
+ Hits          374      442      +68     
- Misses          5        9       +4     
  Partials        2        2              
Flag Coverage Δ
integration 49.16% <0.00%> (-4.02%) ⬇️
unit 97.57% <94.44%> (-0.60%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dreary-ennui
Copy link
Author

Thanks so much for the submission, @dreary-ennui! I'll try to get some version of this merged over the weekend. I hope you don't mind if I refactor it a little bit (left a comment explaining what I'll probably do)

Not at all, go for it; makes sense to me.

@mattwebbio mattwebbio mentioned this pull request Mar 15, 2025
@dreary-ennui
Copy link
Author

dreary-ennui commented Mar 29, 2025

https://github.com/lovelaze/nebula-sync has a better method of selectively syncing the contents of the config backup JSON by having mutually exclusive include/exclude filters and recursive parsing, instead of hardcoding what the structure should be in the patched config.

lovelaze/nebula-sync@bd29573

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(V6) Local DNS Records Not Syncing

2 participants