Skip to content

docs: T9175: document dns forwarding options security-status-poll-domain - #2193

Open
ruben-herold wants to merge 3 commits into
vyos:rollingfrom
ruben-herold:T9175-security-poll-suffix
Open

docs: T9175: document dns forwarding options security-status-poll-domain#2193
ruben-herold wants to merge 3 commits into
vyos:rollingfrom
ruben-herold:T9175-security-poll-suffix

Conversation

@ruben-herold

@ruben-herold ruben-herold commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Change Summary

Document the new opt-in service dns forwarding options security-status-poll-domain setting that controls the PowerDNS recursor's
periodic security status poll, disabled by default (VyOS ships a
patched/backported pdns-recursor package that's never updated
in-place, so the poll result is meaningless — see T9175).

Related Task(s)

Related PR(s)

Backport

Checklist:

Document the new opt-in `service dns forwarding options
security-poll-suffix` setting that controls the PowerDNS recursor's
periodic security status poll, disabled by default.
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 50d8deac-4a8d-44a6-9c75-4ec8a138ee1c

📥 Commits

Reviewing files that changed from the base of the PR and between b60309e and f96e2e2.

📒 Files selected for processing (1)
  • docs/configuration/service/dns.md
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • ansible/ansible (manual)
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/configuration/service/dns.md
📜 Recent review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: Mergify Merge Queue
  • GitHub Check: Mergify Merge Protections
  • GitHub Check: Summary
⚠️ CI failures not shown inline (4)

GitHub Actions: AI Validation / prepare: docs: T9175: document dns forwarding options security-poll-suffix

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m# Fetch the base branch explicitly by refname to avoid ambiguity with�[0m
 �[36;1m# same-named tags (e.g., a `rolling` tag), then diff against FETCH_HEAD.�[0m
 �[36;1mgit fetch --no-tags --depth=1 origin "refs/heads/rolling"�[0m
 �[36;1mBASE="FETCH_HEAD"�[0m
 �[36;1m# --diff-filter=ACMRT excludes Deleted entries so the bundling�[0m
 �[36;1m# loop below (`git show HEAD:<path>`) doesn't try to extract�[0m
 �[36;1m# blobs for files that no longer exist in the merge ref.�[0m
 �[36;1m# Deletions still appear in diff-md.patch (full diff) but not�[0m
 �[36;1m# in changed-md.txt (which drives the bundling step).�[0m
 �[36;1mgit diff "$BASE...HEAD" --name-only --diff-filter=ACMRT -z -- ':(glob)docs/**/*.md'  > changed-md.z�[0m
 �[36;1mgit diff "$BASE...HEAD" --name-only --diff-filter=ACMRT -z -- ':(glob)docs/**/*.rst' > changed-rst.z�[0m
 �[36;1m# Reject paths containing line-disrupting control bytes (LF, CR,�[0m
 �[36;1m# other 0x01-0x1F + 0x7F) before generating the newline-delimited�[0m
 �[36;1m# *.txt manifests. NUL itself can't appear in a git pathname�[0m
 �[36;1m# (it's the on-disk tree-entry terminator), so it stays out of�[0m
 �[36;1m# the rejection class and remains the legitimate record delimiter�[0m
 �[36;1m# for `git diff -z` — `grep -z` honors that contract.�[0m
 �[36;1m#�[0m
 �[36;1m# POSIX filesystems generally allow LF/CR in filenames and git�[0m
 �[36;1m# stores them fine; the hazard is purely in our line-delimited�[0m
 �[36;1m# downstream tooling. Without this guard, `tr '\0' '\n'` on a�[0m
 �[36;1m# path like `docs/foo\nbar.md` would split it into two logical�[0m
 �[36;1m# lines — downstream consumers reading line-by-line would miss�[0m
 �[36;1m# validation coverage on the real file (or worse, act on a�[0m
 �[36;1m# synthetic path). Fail fast at this seam.�[0m
 �[36;1m#�[0m
 �[36;1m# An earlier `tr -d '\0\n\r' | grep [\x00-\x1F\x7F]` form�[0m
 �[36;1m# stripped the very bytes it was m...

GitHub Actions: AI Validation / 1_prepare.txt: docs: T9175: document dns forwarding options security-poll-suffix

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m# Fetch the base branch explicitly by refname to avoid ambiguity with�[0m
 �[36;1m# same-named tags (e.g., a `rolling` tag), then diff against FETCH_HEAD.�[0m
 �[36;1mgit fetch --no-tags --depth=1 origin "refs/heads/rolling"�[0m
 �[36;1mBASE="FETCH_HEAD"�[0m
 �[36;1m# --diff-filter=ACMRT excludes Deleted entries so the bundling�[0m
 �[36;1m# loop below (`git show HEAD:<path>`) doesn't try to extract�[0m
 �[36;1m# blobs for files that no longer exist in the merge ref.�[0m
 �[36;1m# Deletions still appear in diff-md.patch (full diff) but not�[0m
 �[36;1m# in changed-md.txt (which drives the bundling step).�[0m
 �[36;1mgit diff "$BASE...HEAD" --name-only --diff-filter=ACMRT -z -- ':(glob)docs/**/*.md'  > changed-md.z�[0m
 �[36;1mgit diff "$BASE...HEAD" --name-only --diff-filter=ACMRT -z -- ':(glob)docs/**/*.rst' > changed-rst.z�[0m
 �[36;1m# Reject paths containing line-disrupting control bytes (LF, CR,�[0m
 �[36;1m# other 0x01-0x1F + 0x7F) before generating the newline-delimited�[0m
 �[36;1m# *.txt manifests. NUL itself can't appear in a git pathname�[0m
 �[36;1m# (it's the on-disk tree-entry terminator), so it stays out of�[0m
 �[36;1m# the rejection class and remains the legitimate record delimiter�[0m
 �[36;1m# for `git diff -z` — `grep -z` honors that contract.�[0m
 �[36;1m#�[0m
 �[36;1m# POSIX filesystems generally allow LF/CR in filenames and git�[0m
 �[36;1m# stores them fine; the hazard is purely in our line-delimited�[0m
 �[36;1m# downstream tooling. Without this guard, `tr '\0' '\n'` on a�[0m
 �[36;1m# path like `docs/foo\nbar.md` would split it into two logical�[0m
 �[36;1m# lines — downstream consumers reading line-by-line would miss�[0m
 �[36;1m# validation coverage on the real file (or worse, act on a�[0m
 �[36;1m# synthetic path). Fail fast at this seam.�[0m
 �[36;1m#�[0m
 �[36;1m# An earlier `tr -d '\0\n\r' | grep [\x00-\x1F\x7F]` form�[0m
 �[36;1m# stripped the very bytes it was m...

GitHub Actions: AI Validation / validate: docs: T9175: document dns forwarding options security-poll-suffix

Conclusion: failure

View job details

##[group]Removing auth
 Removing SSH command configuration
 [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand
 [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
 Removing HTTP extra header
 [command]/usr/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
 [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
 Removing includeIf entries pointing to credentials config files
 [command]/usr/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
 includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path
 includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path
 includeif.gitdir:/github/workspace/reviewer/.git.path
 includeif.gitdir:/github/workspace/reviewer/.git/worktrees/*.path
 [command]/usr/bin/git config --local --get-all includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path
 /home/runner/work/_temp/git-credentials-0db8eb42-e25a-416e-8cae-bbab8b6afc15.config
 [command]/usr/bin/git config --local --unset includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path /home/runner/work/_temp/git-credentials-0db8eb42-e25a-416e-8cae-bbab8b6afc15.config
 [command]/usr/bin/git config --local --get-all includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path
 /home/runner/work/_temp/git-credentials-0db8eb42-e25a-416e-8cae-bbab8b6afc15.config
 [command]/usr/bin/git config --local --unset includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path /home/runne...

GitHub Actions: AI Validation / 0_validate.txt: docs: T9175: document dns forwarding options security-poll-suffix

Conclusion: failure

View job details

##[group]Removing auth
 Removing SSH command configuration
 [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand
 [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
 Removing HTTP extra header
 [command]/usr/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
 [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
 Removing includeIf entries pointing to credentials config files
 [command]/usr/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
 includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path
 includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path
 includeif.gitdir:/github/workspace/reviewer/.git.path
 includeif.gitdir:/github/workspace/reviewer/.git/worktrees/*.path
 [command]/usr/bin/git config --local --get-all includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path
 /home/runner/work/_temp/git-credentials-0db8eb42-e25a-416e-8cae-bbab8b6afc15.config
 [command]/usr/bin/git config --local --unset includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path /home/runner/work/_temp/git-credentials-0db8eb42-e25a-416e-8cae-bbab8b6afc15.config
 [command]/usr/bin/git config --local --get-all includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path
 /home/runner/work/_temp/git-credentials-0db8eb42-e25a-416e-8cae-bbab8b6afc15.config
 [command]/usr/bin/git config --local --unset includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path /home/runne...
🧰 Additional context used
🔍 Remote MCP vyos.dev

Relevant review context

  • T9175 is “In progress” with Normal priority. It reports noisy PowerDNS security-poll warnings because VyOS’s packaged PowerDNS cannot be updated independently of the full VyOS release. The task proposes disabling the check via security-poll-suffix.
  • Task discussion explicitly favors opt-in behavior: polling should not occur unless configured.
  • The task links the related implementation PR vyos/vyos-1x#5389 and documentation PR vyos/vyos-documentation#2193.
  • Review the documentation for consistency with the renamed option: it should clearly communicate that the default is disabled, while configuring a domain enables polling and removing/emptying the value disables it.

📝 Walkthrough

Summary by CodeRabbit

  • Documentation
    • Added guidance for the security-status-poll-domain DNS forwarding option.
    • Documented that security status polling is disabled by default.
    • Explained how to configure PowerDNS or custom polling domains.
    • Included commands for enabling and disabling security status polling.

Walkthrough

The DNS service documentation now describes the security-status-poll-domain option, its disabled-by-default behavior, supported polling domains, and commands to enable or disable polling.

Changes

DNS configuration documentation

Layer / File(s) Summary
Security polling domain documentation
docs/configuration/service/dns.md:432-458
Documents security-status-poll-domain, its disabled-by-default behavior, supported PowerDNS or custom polling domains, and the command to disable polling.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the documentation change for the specific DNS forwarding option.
Description check ✅ Passed The description accurately explains the documented setting, its default behavior, rationale, and related tasks.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
✨ Simplify code
  • Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ruben-herold's task in 2m 26s —— View job


Review — T9175 security-poll-suffix docs

  • Read pass1-findings.json (empty — no deterministic findings)
  • Diff PR against origin/rolling
  • Cross-reference vyos-1x rolling + the linked vyos-1x PR #5389
  • Post review

Verification against source

Feature is not yet in vyos-1x rolling — the corresponding vyos/vyos-1x#5389 is still OPEN. Verified the doc against that PR's diff:

  • CLI path service dns forwarding options security-poll-suffix — ✅ matches the new <leafNode name="security-poll-suffix"> under options in interface-definitions/service_dns_forwarding.xml.in.
  • Default = disabled — ✅ matches XML help ((default: disabled)) and recursor.conf.j2 which renders an empty value when unset.
  • secpoll.powerdns.com as the canonical PowerDNS domain — ✅ matches XML valueHelp.
  • Custom domain suffix — ✅ matches XML valueHelp + fqdn validator.

No factual issues. Reminder: this docs PR must not merge before vyos-1x#5389, otherwise docs/rolling will describe a CLI that does not exist in the corresponding VyOS build.

Issues

INFO — Placement inside "Per-domain forwarding" section
docs/configuration/service/dns.md:493
The block is inserted after the service dns forwarding domain … recursion-desired example, inside the ### Per-domain forwarding section (line 432). But security-poll-suffix sits under service dns forwarding options, alongside ecs-add-for / ecs-ipv4-bits / edns-subnet-allow-list — the "EDNS Client Subnet" section (line 375) or a dedicated new ### Security poll subsection would fit the file's structure better. Readers browsing per-domain settings will get a non-sequitur, and readers looking for options settings will not find it.

INFO — Missing Example: block
docs/configuration/service/dns.md:493-503
Every other {cfgcmd} in this file is followed by Example: + a fenced ```none block showing an invocation (e.g. lines 394-399, 425-430, 486-490). This new block breaks that pattern. Suggested addition:

Example:

```none
set service dns forwarding options security-poll-suffix secpoll.powerdns.com

**INFO — Missing bold summary line**
`docs/configuration/service/dns.md:495`
Neighboring `{cfgcmd}` blocks lead with a one-line `**bold**` summary of the setting (e.g. line 379 `**Configure which client source address is sent…**`, line 482 `**Set the {abbr}\`RD (Recursion Desired)\` bit…**`). This block goes straight into explanatory prose. Adding a one-line bold summary matches the file's convention.

**INFO — Double vs single backticks for inline code**
`docs/configuration/service/dns.md:500`
`` ``secpoll.powerdns.com`` `` uses double backticks. The rest of this file uses single backticks for inline code (see `SERVFAIL` at line 320, 324, 471; `validate` at 324). Both render as code, but the file's convention is single backticks.

**INFO — Extra blank line before block**
`docs/configuration/service/dns.md:491-492`
Two blank lines precede the new `{cfgcmd}` block, where the rest of the file uses one. Minor.

### Needs Verification

None.

### Stats

Validated 1 MyST file. Skipped 0 RST files awaiting MyST migration. 1 CLI command checked. Branch reviewed: `rolling` (vyos-1x: `rolling`; feature source verified against unmerged PR vyos/vyos-1x#5389).
 · branch: `T9175-security-poll-suffix`

@mergify mergify Bot added the rolling label Aug 9, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
docs/configuration/service/dns.md (1)

500-500: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use single backticks for MyST inline code.

Line [500] uses double backticks. MyST reserves double backticks for embedded RST contexts. Write secpoll.powerdns.com with single backticks.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/configuration/service/dns.md` at line 500, Update the inline code
formatting in the DNS documentation sentence to use single backticks around
secpoll.powerdns.com instead of double backticks, preserving the surrounding
text.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/configuration/service/dns.md`:
- Around line 493-502: Update the security-poll-suffix documentation to show the
supported disable command using delete service dns forwarding options
security-poll-suffix. Clarify that only non-empty suffix values enable polling,
that polling is disabled when the setting is absent, and that
secpoll.powerdns.com is PowerDNS’s upstream domain rather than the VyOS default.

---

Nitpick comments:
In `@docs/configuration/service/dns.md`:
- Line 500: Update the inline code formatting in the DNS documentation sentence
to use single backticks around secpoll.powerdns.com instead of double backticks,
preserving the surrounding text.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 84c868ba-8be8-4446-87a5-3d6779571c65

📥 Commits

Reviewing files that changed from the base of the PR and between 3f6c16e and 6fae1b9.

📒 Files selected for processing (1)
  • docs/configuration/service/dns.md
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • ansible/ansible (manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: validate
  • GitHub Check: Mergify Merge Protections
  • GitHub Check: Summary
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/**/*.md: Canonical docs pages must be written as MyST Markdown (.md); edit existing pages in .md only and never use the old md- prefix for new pages.
Use {cfgcmd}, {opcmd}, and {cmdincludemd} fenced directives in MyST pages for VyOS command coverage; do not replace them with plain text or bash fences.
Use MyST ATX headings (#, ##, ###, etc.) in canonical pages; the RST heading hierarchy does not apply to .md sources.
In MyST pages, prefer single backticks for inline code; double backticks are reserved for embedded RST contexts.
Use % stop_vyoslinter and % start_vyoslinter comment markers in top-level MyST content to suppress real IPs or other allowed long-line exceptions, and keep them paired.
In MyST pages, write TODO markers as {todo} fenced directives.

Files:

  • docs/configuration/service/dns.md
docs/{_include/*.txt,**/*.md}

📄 CodeRabbit inference engine (AGENTS.md)

Keep documentation lines within 80 characters unless the content is inside a code block or fenced/preformatted block.

Files:

  • docs/configuration/service/dns.md
🧠 Learnings (13)
📚 Learning: 2026-05-06T20:48:49.689Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:71-71
Timestamp: 2026-05-06T20:48:49.689Z
Learning: In vyos/vyos-documentation, the 80-character line-length rule documented under Source conventions / Formatting applies only to documentation source files located under docs/ (e.g., docs/**/*.rst and docs/**/*.md). The rule is enforced by the vyoslinter (doc-linter.py from vyos/.github) when reviewing changed files via lint-doc.yml, and only for files within docs/**. Do not suggest hard-wrapping CLAUDE.md (repo-root documentation) because GitHub renders and reflows content. For CLAUDE.md, reviews should not enforce the 80-char wrapping; apply the rule only to files matching **/docs/**/*.{rst,md}.

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-05-06T20:48:57.970Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:91-93
Timestamp: 2026-05-06T20:48:57.970Z
Learning: The 80-character line limit applies only to documentation sources under docs/** (RST/MD). Do not enforce this limit on repo-root Markdown files like CLAUDE.md or README.md. The vyoslinter (doc-linter.py, run via lint-doc.yml from vyos/.github) lints only changed files within docs/**; root files are excluded. GitHub renders root Markdown with viewport-width reflow, so hard-wrapping these files reduces readability without tooling benefit.

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-05-06T20:48:50.446Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:64-64
Timestamp: 2026-05-06T20:48:50.446Z
Learning: Enforce the 80-character line-length limit only for documentation source files under docs/ (docs/**/*.md and docs/**/*.rst rendered by Sphinx and linted by vyoslinter via lint-doc.yml). Do not flag line-length issues in repository-root Markdown files such as CLAUDE.md or README.md, which GitHub renders with viewport-width reflow. This applies to all files within docs/ that are part of the documentation source.

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-05-06T20:48:54.578Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:80-84
Timestamp: 2026-05-06T20:48:54.578Z
Learning: Enforce the 80-character line-length limit only for documentation source files under docs/ (docs/**/*.rst and docs/**/*.md). Do not flag repo-root files like CLAUDE.md or README.md, since they are rendered by GitHub and not subject to this rule. The doc-linter (doc-linter.py via lint-doc.yml) only lints docs/**, so CI checks won't flag root files for line length.

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-05-06T20:49:00.044Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:108-108
Timestamp: 2026-05-06T20:49:00.044Z
Learning: Limit the 80-character line length check and vyoslinter (doc-linter.py) enforcement to documentation source files under docs/**/*.{rst,md}. Do not apply or flag line-length issues in repo-root files like CLAUDE.md or README.md, which are rendered directly by GitHub and are not linted by lint-doc.yml. This pattern narrows checks to Sphinx source docs and prevents false positives in non-doc files.

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-05-06T20:48:53.302Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:79-79
Timestamp: 2026-05-06T20:48:53.302Z
Learning: Limit line length to 80 characters only for documentation sources under the docs directory (docs/**/*.rst and docs/**/*.md). This is enforced by the vyoslinter doc-linter.py (from the vyos/.github repo) via lint-doc.yml on changed files under docs/**. Do not flag line-length violations in repository-root Markdown files like CLAUDE.md or README.md, as they are rendered by GitHub and reflow in the UI.

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-05-06T20:49:10.359Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:142-142
Timestamp: 2026-05-06T20:49:10.359Z
Learning: In vyos/vyos-documentation, the 80-character line limit and vyoslinter enforcement apply only to documentation source files under docs/**/*.rst and docs/**/*.md that Sphinx renders. Repo-root files such as CLAUDE.md and README.md are outside the linter's scope (lint-doc.yml runs on docs/**) and are rendered by GitHub with automatic paragraph reflow — do not flag line-length violations in these files.

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-05-06T20:49:15.361Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:163-163
Timestamp: 2026-05-06T20:49:15.361Z
Learning: In vyos/vyos-documentation, enforce the 80-character line-length limit (Source conventions / Formatting) only for Sphinx documentation source files under docs/**/*.rst and docs/**/*.md. The lint-doc.yml workflow runs the doc-linter (doc-linter.py) and checks only docs/** changed files. Files in the repository root (e.g., CLAUDE.md, README.md) are rendered by GitHub and are not subject to this rule; do not flag line-length violations in those files.

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-05-08T07:01:22.978Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1878
File: docs/troubleshooting/connectivity.rst:0-0
Timestamp: 2026-05-08T07:01:22.978Z
Learning: For the VyOS documentation (MyST-based docs), MyST directive opener lines must keep the entire directive arguments on a single line. This includes MyST fenced-directive openers like ```{opcmd} ... ``` and the RST-equivalent form .. opcmd:: ... when ported/used in MyST. Because the MyST parser does not support wrapped/continued directive arguments across multiple lines, do not raise/keep review warnings suggesting line wrapping for these directive opener lines due to line-length (even if they exceed 80 characters).

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-05-08T07:01:22.978Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1878
File: docs/troubleshooting/connectivity.rst:0-0
Timestamp: 2026-05-08T07:01:22.978Z
Learning: In vyos/vyos-documentation, do not raise line-length (>80 chars) review findings for MyST directive opener lines (the directive “opener” that uses MyST directive syntax such as `{cfgcmd}` / `{opcmd}` fence/openers). CI does not enforce the 80-character limit for these specific opener lines, and existing documentation contains longer opener lines that pass lint.

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-05-13T22:16:06.198Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 2021
File: docs/automation/terraform/terraformvyos.md:14-14
Timestamp: 2026-05-13T22:16:06.198Z
Learning: In the vyos/vyos-documentation repo, when a PR is a byte-for-byte documentation port of an existing file from the rolling branch to a release branch (e.g., circinus, sagitta), keep the port content identical to the production-tested rolling source. For these ports, do not raise new review findings for documentation issues that are already present in the rolling source (for example, markdownlint MD059 like non-descriptive link text such as `[link]`/`[install]`). Instead, defer those existing issues to a rolling-side cleanup PR (e.g., `#2024`) and then backport the cleanup via Mergify.

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-05-13T22:43:41.056Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 2024
File: docs/automation/terraform/terraformvyos.md:0-0
Timestamp: 2026-05-13T22:43:41.056Z
Learning: In docs/**/*.md, for Markdown reference definition lines of the form `[label]: <URL>`, if the line cannot be shortened to <= 80 characters (because the URL itself is near/at the limit), suppress the vyoslinter warning by wrapping only that reference definition with a `% stop_vyoslinter` / `% start_vyoslinter` block. If the reference definition can fit within 80 characters, leave it outside any suppression block.

Applied to files:

  • docs/configuration/service/dns.md
📚 Learning: 2026-06-05T19:21:44.474Z
Learnt from: LiudmylaNad
Repo: vyos/vyos-documentation PR: 2066
File: docs/configuration/protocols/traffic-engineering.md:0-0
Timestamp: 2026-06-05T19:21:44.474Z
Learning: In the vyos/vyos-documentation MyST documentation pages, when writing CLI example invocations directly under a `{cfgcmd}` directive, use `none` fenced code blocks for those examples. Do not change these example blocks to `{opcmd}` or `{cfgcmd}`—`{opcmd}` is reserved for operational-mode commands, and the surrounding `{cfgcmd}` directive already documents the target command. Plain `none` blocks for these CLI examples are intentional and correct.

Applied to files:

  • docs/configuration/service/dns.md
🔍 Remote MCP Context7, vyos.dev

Relevant review context

  • T9175 is an Open bug with priority “Requires assessment”, titled “PowerDNS periodically queries security-status.secpoll.powerdns.com and discards the result.” It reports warnings caused by a non-release Debian package version and proposes disabling the poll because VyOS packages are not updated independently.
  • Task comments explicitly favor an opt-in setting rather than unsolicited polling.
  • PowerDNS Recursor documents that security-poll-suffix defaults to secpoll.powerdns.com; setting it to an empty value disables polling, while a custom domain is supported.
  • Keyword searches found no separate related task beyond T9175.

Review implication: Ensure the VyOS documentation clearly distinguishes its opt-in/default-disabled behavior from PowerDNS’s upstream default, and documents that an empty suffix disables polling while non-empty values can target a custom security-poll domain.

Comment thread docs/configuration/service/dns.md Outdated
The security-poll-suffix cfgcmd was placed under "Per-domain
forwarding" even though it's a global forwarding option, and used the
"forwarding options" convention (bold summary + Example block) used
by ecs-add-for/ecs-ipv4-bits/edns-subnet-allow-list right above it.

Also documents how to disable polling again (delete the option),
clarifies it only takes effect with a non-empty suffix, and that
secpoll.powerdns.com is PowerDNS' own poll domain rather than a VyOS
default. Switches secpoll.powerdns.com from double to single
backticks, matching MyST inline-code convention.

Flagged by CodeRabbit and the Claude review bot on PR vyos#2193.
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ruben-herold's task in 1m 41s —— View job


Review — T9175 security-poll-suffix docs (re-review after b60309e)

  • Read pass1-findings.json (empty — no deterministic findings)
  • Diff current PR head against origin/rolling
  • Read current state of docs/configuration/service/dns.md around the new block
  • Cross-reference vyos-1x rolling + PR #5389
  • Post updated review

Verification against source

.vyos-1x/ HEAD is on rolling. security-poll-suffix / security_poll_suffix / secpoll are still not present in vyos-1x rolling (interface-definitions, service_dns_forwarding.py, or any Jinja template under data/templates/dns-forwarding/). The corresponding vyos/vyos-1x#5389 is still open. This docs PR must not merge before vyos-1x#5389 — otherwise docs/rolling will describe a CLI that doesn't exist in the corresponding VyOS build.

Response to previous findings

All six items from my prior review, plus the CodeRabbit comment, are addressed in b60309e:

  • Placement — Moved from inside ### Per-domain forwarding to after the other service dns forwarding options blocks (ecs-add-for, ecs-ipv4-bits, edns-subnet-allow-list), right before ### Per-domain forwarding. ✅ Fixed.
  • Missing Example: block — Added Example: + ```none block at dns.md:447-451. ✅ Fixed.
  • Missing bold summary line — Now leads with **Configure the domain suffix used by the PowerDNS recursor…** at dns.md:434-435. ✅ Fixed.
  • Double vs single backticks`secpoll.powerdns.com` now uses single backticks at dns.md:442. ✅ Fixed. (CodeRabbit nitpick resolved.)
  • Extra blank line — Normalized. ✅ Fixed.
  • CodeRabbit — disable/opt-in clarity — Added a "To disable polling again:" section with delete service dns forwarding options security-poll-suffix at dns.md:453-457, plus the clarifying sentence at dns.md:442-444 that secpoll.powerdns.com is PowerDNS' own domain, not a VyOS default, and that only a non-empty suffix opts in. ✅ Fixed.

Issues

INFO — Section placement
docs/configuration/service/dns.md:432
The block now lives at the tail of ### EDNS Client Subnet (heading at line 375). It is an options command like its ECS neighbours, so the grouping is defensible, but security-poll-suffix is unrelated to EDNS Client Subnet, and a reader who lands on the ECS heading and scrolls down may be confused. A dedicated ### Security polling subsection at the same nesting level would be the cleanest fix — but this is non-blocking and much better than the previous placement inside per-domain forwarding.

INFO — Punctuation nit
docs/configuration/service/dns.md:443
"not a VyOS default - set it explicitly" uses a hyphen-minus where an em- or en-dash would be idiomatic (e.g. — set it explicitly). Purely cosmetic.

Needs Verification

None.

Stats

Validated 1 MyST file. Skipped 0 RST files awaiting MyST migration. 1 CLI command checked. Branch reviewed: rolling (vyos-1x: rolling; feature source is unmerged PR vyos/vyos-1x#5389 — still open).
· branch: T9175-security-poll-suffix

…ll-domain

Follows vyos-1x PR #5389 rename per dmbaturin's review: the value is a
full FQDN, not a suffix, and the poll checks for known vulnerabilities.
@ruben-herold

Copy link
Copy Markdown
Contributor Author

Updated to match the option rename in vyos/vyos-1x#5389 (security-poll-suffixsecurity-status-poll-domain, per dmbaturin's review). Pushed.

@ruben-herold ruben-herold changed the title docs: T9175: document dns forwarding options security-poll-suffix docs: T9175: document dns forwarding options security-status-poll-domain Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

1 participant