Skip to content

fix: validate effective Access, DNS and endpoint configuration - #94

Merged
mannie-exe merged 6 commits into
mainfrom
dev
Oct 3, 2026
Merged

mannie-exe merged 6 commits into
mainfrom
dev

Conversation

@mannie-exe

@mannie-exe mannie-exe commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

This closes the concrete alpha.8 audit findings in Access validation, DNS TTL convergence and manager endpoint configuration. Access rules containing everyone: false or anyValidServiceToken: false fail schema and runtime validation before remote policy or token work. Proxied DNS records use Auto TTL; DNS-only records retain their configured TTL. Overlapping health and metrics bind addresses fail during configuration parsing, including an empty metrics address that resolves to the framework default.

Live validation found an additional schema defect: admission inserted ttl: 1 into explicit hostname entries, preventing inheritance of defaults.ttl. The hostname-level default is removed. Existing stored ttl: 1 values remain intentional Auto overrides until removed; the migration guide explains when to do that.

  • Regenerate the Access and DNS CRDs, with admission and runtime regressions.
  • Test proxy transitions, unchanged-record convergence, and recovery after cancelled DNS status or credential writes.
  • Separate controller-only removal from full decommissioning, retaining cleanup dependencies until finalization.

Validation at final head bd48a50:

  • Unit/race tests pass all 12 packages; formatting, lint, build and coverage CI are green. Offline checks pass.
  • Initial full local Colima/kind E2E: 107 passed, 1 failed, 14 skipped. The new TTL transition test exposed the schema defaulting defect; its ordered DNS group skipped the remaining cases.
  • After the correction, the entire DNS and CEL groups passed: 48 passed, 0 failed, with 75 outside the focus. This includes the failed test, all 14 skipped cases and the new TTL admission regression. All 123 current spec names have passing executions across the two runs; there was no additional single full-suite run at the final head.
  • Both suite teardowns completed. Owned clusters were removed, the three original clusters remain, and the temporary Colima setting was restored.
  • Greptile reviewed the final head with no actionable findings. Review-trigger comments were removed.

Capacity remains explicitly bounded evidence: the 1,000-entry ceiling is not a supported operating size. Existing synthetic tests measure full DNS operation counts and retry progress under shared windows, but do not certify mixed-controller quota fairness. Cancellation tests verify persisted recovery state, not process SIGKILL or leader failover.

Matching chart preparation is helm-chart PR #24, including port validation, configurable termination grace, schemas and migration/decommissioning guidance. It retains the published artifact pin until alpha.9 exists.

Do not merge or release this PR yet. The proposed next operator version is v0.2.0-alpha.9. Changelogs remain generated by the existing release workflow.

- api: require true boolean selectors
- controller: validate before token work
- test: cover every rule list and value
- docs: explain omission semantics
- dns: normalize effective provider TTL
- test: preserve DNS-only transitions
- test: reject repeated no-op writes
- docs: retain configured TTL intent
- startup: detect conflicting listeners
- test: cover flags and environment
- test: preserve disabled and port zero
- test: recover DNS after cancellation
- test: retain credential write intent
- docs: separate removal from cleanup
- docs: explain upcoming validation
- startup: match framework defaulting
- test: preserve empty health disablement
@greptile-apps

greptile-apps Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Adds validation rules for access policies and DNS configuration.

The changes reviewed since the previous review appear safe to merge.

Summary

The PR rejects false Access boolean selectors, uses Auto TTL for proxied DNS records while preserving configured TTL for DNS-only records, and detects overlapping manager listeners. The changes since the previous review remove the hostname-level TTL admission default, document migration for stored values, and extend DNS validation and transition tests.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Explicit hostname TTL omitted] --> B[Inherit resource default TTL]
  C[Explicit hostname TTL set] --> D[Use hostname TTL]
  B --> E{Effective record proxied?}
  D --> E
  E -->|Yes| F[Send Auto TTL 1]
  E -->|No| G[Send configured TTL]
Loading

Reviews (3) · Last reviewed commit: "fix(dns): preserve inherited ttl through..."

@codecov

codecov Bot commented Oct 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

- api: leave hostname TTL unset
- test: verify admission and inheritance
- docs: explain stored auto TTL migration
@mannie-exe
mannie-exe merged commit 5e0b559 into main Oct 3, 2026
9 checks passed
@mannie-exe
mannie-exe deleted the dev branch October 3, 2026 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant