feat: consolidate dns name and ips for cloudflare source #716
Conversation
WalkthroughThe DNS provider now creates unified resource objects per record type with conditional IP fields, while the schema layer implements combined deduplication for DNS+IP entries before falling back to per-field processing. This consolidates resource creation and deduplication logic across two layers. Changes
Sequence DiagramsequenceDiagram
participant Provider as DNS Provider
participant Schema as Schema Layer
participant Dedup as Deduplicator
Note over Provider,Schema: Previous Flow
Provider->>Provider: Create resource per record type<br/>(with DNSName + IP fields)
Provider->>Schema: Pass resource object
Schema->>Schema: Split into individual fields<br/>(DNSName, PublicIPv4, PublicIPv6, etc.)
Schema->>Dedup: Check each field separately
Note over Provider,Schema: New Flow
Provider->>Provider: Create single resource<br/>(DNSName + conditional IP fields)
Provider->>Schema: Pass unified resource object
Schema->>Schema: Check if DNS + IP combined entry
alt DNS Service with IP
Schema->>Dedup: Check combined entry
Dedup-->>Schema: New values?
alt Has New Values
Schema->>Schema: Append resource
else No New Values
Schema->>Schema: Skip append
end
else Other Cases
Schema->>Schema: Split into individual fields<br/>(original behavior)
Schema->>Dedup: Check each field separately
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
pkg/schema/schema.go (2)
95-127: Document the DNS deduplication strategy.The special-case logic correctly handles DNS resources by appending them when any field (DNS name or IP) is new, allowing resources with partial overlaps (e.g., same DNS name with different IPs for load balancing). However, this non-obvious deduplication strategy lacks explanation.
Consider adding a comment like:
// Special handling for DNS providers: if resource has BOTH DNS name and IP, keep them together // Deduplication strategy: append the resource if it introduces at least one new value (DNS or IP) // This allows multiple records with the same DNS name but different IPs (e.g., round-robin DNS) // while preventing exact duplicate resources from being added multiple times.
99-99: Consider extracting the service name constant.The hardcoded service name
"dns"appears inline. Consider defining a constant if this pattern will be used for other services in the future.+const ( + ServiceDNS = "dns" +) + - if hasDNS && hasIP && resource.Service == "dns" { + if hasDNS && hasIP && resource.Service == ServiceDNS {pkg/providers/cloudflare/dns.go (1)
63-67: Consider documenting CNAME behavior more explicitly.While the comment on line 68 mentions that CNAME records only include the DNS name, it might be helpful to clarify that for CNAME records, the
record.Contentfield (which contains the CNAME target) is intentionally not captured in the resource fields. If this information is valuable, consider storing it in metadata.For example, if CNAME targets should be tracked:
if record.Type == "A" && record.Content != "" { resource.PublicIPv4 = record.Content } else if record.Type == "AAAA" && record.Content != "" { resource.PublicIPv6 = record.Content +} else if record.Type == "CNAME" && record.Content != "" && d.extendedMetadata { + // Store CNAME target in metadata for reference + if metadata == nil { + metadata = make(map[string]string) + } + metadata["cname_target"] = record.Content + resource.Metadata = metadata }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/providers/cloudflare/dns.go(1 hunks)pkg/schema/schema.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
pkg/providers/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
pkg/providers/**/*.go: All providers must implement the schema.Provider interface with methods: Name(), ID(), Resources(ctx), Services()
For complex providers, keep service-specific logic in separate files (e.g., instances.go, dns.go, route53.go, s3.go, eks.go) and return *schema.Resources merged by the main Resources()
Parse provider configuration via block.GetMetadata(key); support environment variables using $VAR syntax; return &schema.ErrNoSuchKey{Name: key} for missing required config
Respect service filtering: Services() must list supported services, and providers should honor -s filters when gathering resources
Always pass and honor context.Context in Resources() and long-running API calls to support cancellation
Do not manually deduplicate resources; use Resources.Append() to rely on schema’s ResourceDeduplicator
Each emitted resource must have at least one of IP or DNS populated; empty resources are invalid
Treat provider config id as a user-defined identifier for filtering, not a cloud resource ID
Files:
pkg/providers/cloudflare/dns.go
pkg/providers/*/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Each provider directory must include a main file named .go defining New(block schema.OptionBlock) and Resources(ctx) orchestration
Files:
pkg/providers/cloudflare/dns.go
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: projectdiscovery/cloudlist PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T18:34:55.853Z
Learning: Applies to pkg/providers/**/*.go : Each emitted resource must have at least one of IP or DNS populated; empty resources are invalid
📚 Learning: 2025-10-14T18:34:55.853Z
Learnt from: CR
Repo: projectdiscovery/cloudlist PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T18:34:55.853Z
Learning: Applies to pkg/providers/**/*.go : Each emitted resource must have at least one of IP or DNS populated; empty resources are invalid
Applied to files:
pkg/providers/cloudflare/dns.gopkg/schema/schema.go
📚 Learning: 2025-10-14T18:34:55.853Z
Learnt from: CR
Repo: projectdiscovery/cloudlist PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T18:34:55.853Z
Learning: Applies to pkg/providers/**/*.go : Do not manually deduplicate resources; use Resources.Append() to rely on schema’s ResourceDeduplicator
Applied to files:
pkg/providers/cloudflare/dns.gopkg/schema/schema.go
📚 Learning: 2025-10-14T18:34:55.853Z
Learnt from: CR
Repo: projectdiscovery/cloudlist PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T18:34:55.853Z
Learning: Applies to pkg/providers/**/*.go : For complex providers, keep service-specific logic in separate files (e.g., instances.go, dns.go, route53.go, s3.go, eks.go) and return *schema.Resources merged by the main Resources()
Applied to files:
pkg/providers/cloudflare/dns.gopkg/schema/schema.go
📚 Learning: 2025-10-14T18:34:55.853Z
Learnt from: CR
Repo: projectdiscovery/cloudlist PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T18:34:55.853Z
Learning: Applies to pkg/providers/**/*.go : Treat provider config id as a user-defined identifier for filtering, not a cloud resource ID
Applied to files:
pkg/providers/cloudflare/dns.go
📚 Learning: 2025-10-14T18:34:55.853Z
Learnt from: CR
Repo: projectdiscovery/cloudlist PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T18:34:55.853Z
Learning: Applies to pkg/providers/**/*.go : All providers must implement the schema.Provider interface with methods: Name(), ID(), Resources(ctx), Services()
Applied to files:
pkg/schema/schema.go
📚 Learning: 2025-10-14T18:34:55.853Z
Learnt from: CR
Repo: projectdiscovery/cloudlist PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T18:34:55.853Z
Learning: Applies to pkg/inventory/inventory.go : When adding a provider, also update the Providers map with the provider and its supported services
Applied to files:
pkg/schema/schema.go
🧬 Code graph analysis (2)
pkg/providers/cloudflare/dns.go (2)
pkg/schema/schema.go (2)
Resource(177-198)Provider(18-28)pkg/schema/validate/validate.go (3)
DNSName(72-72)PublicIPv4(73-73)PublicIPv6(74-74)
pkg/schema/schema.go (1)
pkg/schema/validate/validate.go (3)
DNSName(72-72)PublicIPv4(73-73)PublicIPv6(74-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Lint Test
- GitHub Check: Analyze (go)
- GitHub Check: release-test
- GitHub Check: Test Builds (1.22.x, ubuntu-latest)
- GitHub Check: Test Builds (1.22.x, macOS-latest)
- GitHub Check: Test Builds (1.22.x, windows-latest)
🔇 Additional comments (1)
pkg/providers/cloudflare/dns.go (1)
50-71: LGTM! Resource consolidation is implemented correctly.The implementation correctly creates a single unified resource per DNS record with DNSName always populated and conditional IP fields based on record type. This aligns well with the schema layer's special DNS service handling and satisfies the requirement that each resource must have at least one of IP or DNS populated.
Summary by CodeRabbit