Refactor Provider interface (Phase 1)

ROADMAP.md

@@ -0,0 +1,81 @@
+# ZoneKit Long-Term Roadmap
+
+This document outlines the strategic vision and development roadmap for ZoneKit over the next 5 years.
+
+## 🟢 0-3 Months: Foundation & Stability (v0.x - v1.0)
+
+**Theme: "Safe, Reliable, and Correct"**
+
+The immediate focus is on eliminating technical debt, ensuring data safety, and finalizing the core provider contract to reach a stable v1.0 release.
+
+*   **Safety & Correctness (Priority #1)**
+    *   [ ] **Atomic Operations**: Eliminate non-atomic bulk updates. Implement intelligent diffing in `Service.BulkUpdate` to minimize API calls and prevent data loss.
+    *   [ ] **Context Propagation**: Ensure `context.Context` is threaded through every layer of the application for proper timeout and cancellation handling.
+    *   [ ] **Validation**: Implement strict schema validation for all provider configurations and DNS records.
+
+*   **Provider Ecosystem**
+    *   [ ] **Conformance Suite**: Expand the conformance test harness to cover 100% of the `Provider` interface (CRUD, Edge Cases).
+    *   [ ] **Core Providers**: Fully support Cloudflare, Namecheap, AWS Route53, and Google Cloud DNS with production-grade reliability.
+
+*   **Developer Experience**
+    *   [ ] **Structured Logging**: Replace ad-hoc logging with `log/slog` for structured, machine-readable output.
+    *   [ ] **Error Handling**: Standardize error types across all providers (e.g., `ErrRecordNotFound`, `ErrAuthenticationFailed`).
+
+---
+
+## 🟡 3-6 Months: Advanced Features & Ecosystem (v1.x)
+
+**Theme: "Power User & Automation"**
+
+Once the core is stable, we shift focus to enabling complex workflows, automation, and broader integrations.
+
+*   **Advanced DNS Management**
+    *   [ ] **Zone Sync**: One-way synchronization between providers (e.g., "Primary: Cloudflare" -> "Backup: Route53").
+    *   [ ] **Dry Run**: Reliable "what-if" analysis for all operations, showing exactly what records will be created, updated, or deleted.
+    *   [ ] **Record Templates**: Support for templated zones (e.g., "Standard Mail Setup", "Web Server Basic") for rapid provisioning.
+
+*   **Infrastructure as Code (IaC)**
+    *   [ ] **Terraform Provider**: Release an official Terraform provider wrapping ZoneKit logic.
+    *   [ ] **GitOps Integration**: Native support for managing DNS configuration via Git repositories (YAML/JSON definitions).
+
+*   **Observability**
+    *   [ ] **Metrics**: Expose Prometheus metrics for API calls, latencies, and error rates.
+    *   [ ] **Audit Logs**: Comprehensive audit logging for all changes made via the tool.
+
+---
+
+## 🔵 6-12 Months: Enterprise & Scale (v2.x)
+
+**Theme: "Enterprise Ready"**
+
+Focus on multi-tenancy, team management, and handling massive scale.
+
+*   **Enterprise Security**
+    *   [ ] **SSO/OIDC**: Support for retrieving provider credentials via enterprise identity providers.
+    *   [ ] **RBAC**: Granular permissions for API keys (e.g., "Read Only", "Zone Specific Write").
+    *   [ ] **Vault Integration**: Native integration with HashiCorp Vault for secret management.
+
+*   **Performance**
+    *   [ ] **Parallel Execution**: Concurrent processing of multi-zone operations for high-performance updates.
+    *   [ ] **Caching**: Intelligent caching layer to reduce API costs and improve latency for read operations.
+
+---
+
+## 🟣 1-5 Years: Platform & Intelligence (v3.x+)
+
+**Theme: "The DNS Platform"**
+
+Long-term evolution from a CLI tool to a comprehensive DNS management platform.
+
+*   **SaaS Evolution**
+    *   [ ] **ZoneKit Cloud**: A managed SaaS offering providing a web UI and unified API over all your DNS providers.
+    *   [ ] **Global API**: A single, normalized API endpoint that routes to any underlying provider (Cloudflare, AWS, etc.).
+
+*   **Intelligent Automation (AI)**
+    *   [ ] **AI-Driven Optimization**: Automatic suggestions for DNS misconfigurations (e.g., missing SPF/DMARC, dangling CNAMEs).
+    *   [ ] **Smart Routing**: Dynamic updates to DNS records based on real-time latency or uptime monitoring of endpoints.
+    *   [ ] **Anomaly Detection**: Alerts for unusual DNS record changes or query patterns.
+
+*   **Global Ecosystem**
+    *   [ ] **Marketplace**: A community marketplace for custom provider plugins and automation scripts.
+    *   [ ] **Standardization**: Work towards establishing the "ZoneKit Schema" as an industry-standard format for vendor-agnostic DNS definition.

TODO.md

@@ -0,0 +1,53 @@
+# ZoneKit TODO List
+
+This list identifies gaps, technical debt, and areas for improvement in the codebase, prioritized by safety and production readiness.
+
+## 🚨 High Priority (Safety & Correctness)
+
+- [ ] **Refactor `Service.BulkUpdate` Strategy**
+  - **Issue**: Currently, `Service.BulkUpdate` builds a new record list and calls `SetRecords`. For providers like `RESTProvider` (which implements `SetRecords` via "delete all then create all"), this is **non-atomic and unsafe**. If creation fails, data is lost.
+  - **Task**: Update `Service.BulkUpdate` to:
+    1. Check `Provider.Capabilities()`.
+    2. If the provider supports **Atomic Bulk Replace**, use `SetRecords`.
+    3. Otherwise, orchestrate the update using granular `CreateRecord`, `UpdateRecord`, and `DeleteRecord` calls to minimize risk.
+
+- [ ] **Enhance `ProviderCapabilities`**
+  - **Issue**: `CanBulkReplace` is ambiguous. It doesn't distinguish between a safe, atomic API call and a dangerous client-side loop.
+  - **Task**: Add `IsBulkReplaceAtomic bool` to `ProviderCapabilities`.
+
+- [ ] **Fix `context.Context` Propagation**
+  - **Issue**: `Service` methods (e.g., `GetRecords`) create a new `context.Background()` instead of accepting a context from the caller. This prevents cancellation and timeout propagation from the CLI or API layer.
+  - **Task**: Update all `Service` methods to accept `ctx context.Context` as the first argument.
+
+- [ ] **Harden `RESTProvider` Error Handling**
+  - **Issue**: While `BulkReplaceRecords` now checks errors, it's still a "stop the world" failure.
+  - **Task**: Implement rollback attempts or "continue on error" policies where appropriate (configurable).
+
+## 🧪 Medium Priority (Testing & QA)
+
+- [ ] **Expand Conformance Test Suite**
+  - **Issue**: `pkg/dns/provider/conformance` only tests `ListZones` and `GetZone`.
+  - **Task**: Add tests for:
+    - `CreateRecord`: Verify record is created and ID is returned.
+    - `UpdateRecord`: Verify record is updated.
+    - `DeleteRecord`: Verify record is gone.
+    - `ListRecords`: Verify filtering and pagination (if applicable).
+    - `BulkReplaceRecords`: Verify state transitions.
+
+- [ ] **Add Integration Tests**
+  - **Issue**: Tests primarily rely on mocks.
+  - **Task**: Add integration tests that spin up a local HTTP server (mocking Cloudflare/DigitalOcean APIs) to verify the full `RESTProvider` -> `Mapper` -> `HTTP` stack.
+
+## 🧹 Low Priority (Cleanup & Features)
+
+- [ ] **Implement `BatchUpdate` Interface**
+  - **Issue**: Some providers support batch operations (e.g., "create 10 records") which is more efficient than 10 separate calls but less drastic than "replace zone".
+  - **Task**: Add `BatchUpdate(ctx, operations)` to `Provider` interface.
+
+- [ ] **Structured Logging**
+  - **Issue**: Logging is likely ad-hoc (using `fmt` or basic `log`).
+  - **Task**: Integrate a structured logger (like `log/slog`) to provide consistent, machine-readable logs for debugging production issues.
+
+- [ ] **Configuration Validation**
+  - **Issue**: Configuration loading could be stricter.
+  - **Task**: Use a validation library to ensure all required fields (auth, endpoints) are present and well-formed at startup.

pkg/dns/provider/builder/builder.go

@@ -87,7 +87,22 @@ func buildMappings(configMappings *dnsprovider.FieldMappings) mapper.Mappings {
 	}
 
 	m := mapper.Mappings{
-		ListPath: configMappings.ListPath,
+		ListPath:     configMappings.ListPath,
+		ResponsePath: configMappings.ResponsePath,
+		ZoneListPath: configMappings.ZoneListPath,
+		ZoneID:       configMappings.ZoneID,
+		ZoneName:     configMappings.ZoneName,
+	}
+
+	// Set defaults for zone mappings if empty
+	if m.ZoneListPath == "" {
+		m.ZoneListPath = "zones"
+	}
+	if m.ZoneID == "" {
+		m.ZoneID = "id"
+	}
+	if m.ZoneName == "" {
+		m.ZoneName = "name"
 	}
 
 	// Request mappings

pkg/dns/provider/cloudflare/openapi.yaml

@@ -7,6 +7,19 @@ servers:
   - url: https://api.cloudflare.com/client/v4
     description: Cloudflare API v4
 paths:
+  /zones:
+    get:
+      summary: List Zones
+      operationId: listZones
+      tags:
+        - Zones
+      responses:
+        '200':
+          description: List of Zones
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ZoneResponse'
   /zones/{zone_id}/dns_records:
     get:
       summary: List DNS records
@@ -223,6 +236,24 @@ components:
       properties:
         result:
           $ref: '#/components/schemas/DNSRecord'
+    Zone:
+      type: object
+      properties:
+        id:
+          type: string
+          description: Zone identifier
+        name:
+          type: string
+          description: Zone name
+        status:
+          type: string
+    ZoneResponse:
+      type: object
+      properties:
+        result:
+          type: array
+          items:
+            $ref: '#/components/schemas/Zone'
 security:
   - ApiKeyAuth: []
   - EmailAuth: []

pkg/dns/provider/conformance/conformance_test.go

@@ -0,0 +1,14 @@
+package conformance
+
+import (
+	"testing"
+
+	"zonekit/pkg/dns/provider"
+)
+
+func TestMockProviderConformance(t *testing.T) {
+	p := NewMockProvider()
+	p.Zones["zone-1"] = provider.Zone{ID: "zone-1", Name: "example.com"}
+
+	RunConformanceTests(t, p)
+}

pkg/dns/provider/conformance/mock_provider.go

@@ -0,0 +1,124 @@
+package conformance
+
+import (
+	"context"
+	"fmt"
+
+	"zonekit/pkg/dns/provider"
+	"zonekit/pkg/dnsrecord"
+)
+
+// MockProvider implements the Provider interface for testing
+type MockProvider struct {
+	Zones   map[string]provider.Zone
+	Records map[string]map[string]dnsrecord.Record // zoneID -> recordID -> Record
+}
+
+// NewMockProvider creates a new mock provider
+func NewMockProvider() *MockProvider {
+	return &MockProvider{
+		Zones:   make(map[string]provider.Zone),
+		Records: make(map[string]map[string]dnsrecord.Record),
+	}
+}
+
+func (m *MockProvider) Name() string {
+	return "mock"
+}
+
+func (m *MockProvider) ListZones(ctx context.Context) ([]provider.Zone, error) {
+	var zones []provider.Zone
+	for _, z := range m.Zones {
+		zones = append(zones, z)
+	}
+	return zones, nil
+}
+
+func (m *MockProvider) GetZone(ctx context.Context, zoneID string) (provider.Zone, error) {
+	z, ok := m.Zones[zoneID]
+	if !ok {
+		return provider.Zone{}, fmt.Errorf("zone not found")
+	}
+	return z, nil
+}
+
+func (m *MockProvider) ListRecords(ctx context.Context, zoneID string) ([]dnsrecord.Record, error) {
+	if _, ok := m.Zones[zoneID]; !ok {
+		return nil, fmt.Errorf("zone not found")
+	}
+	var records []dnsrecord.Record
+	if zoneRecords, ok := m.Records[zoneID]; ok {
+		for _, r := range zoneRecords {
+			records = append(records, r)
+		}
+	}
+	return records, nil
+}
+
+func (m *MockProvider) CreateRecord(ctx context.Context, zoneID string, record dnsrecord.Record) (dnsrecord.Record, error) {
+	if _, ok := m.Zones[zoneID]; !ok {
+		return dnsrecord.Record{}, fmt.Errorf("zone not found")
+	}
+	if m.Records[zoneID] == nil {
+		m.Records[zoneID] = make(map[string]dnsrecord.Record)
+	}
+	if record.ID == "" {
+		record.ID = fmt.Sprintf("rec-%d", len(m.Records[zoneID])+1)
+	}
+	m.Records[zoneID][record.ID] = record
+	return record, nil
+}
+
+func (m *MockProvider) UpdateRecord(ctx context.Context, zoneID string, recordID string, record dnsrecord.Record) (dnsrecord.Record, error) {
+	if _, ok := m.Zones[zoneID]; !ok {
+		return dnsrecord.Record{}, fmt.Errorf("zone not found")
+	}
+	if _, ok := m.Records[zoneID][recordID]; !ok {
+		return dnsrecord.Record{}, fmt.Errorf("record not found")
+	}
+	record.ID = recordID
+	m.Records[zoneID][recordID] = record
+	return record, nil
+}
+
+func (m *MockProvider) DeleteRecord(ctx context.Context, zoneID string, recordID string) error {
+	if _, ok := m.Zones[zoneID]; !ok {
+		return fmt.Errorf("zone not found")
+	}
+	if _, ok := m.Records[zoneID][recordID]; !ok {
+		return fmt.Errorf("record not found")
+	}
+	delete(m.Records[zoneID], recordID)
+	return nil
+}
+
+func (m *MockProvider) BulkReplaceRecords(ctx context.Context, zoneID string, records []dnsrecord.Record) error {
+	if _, ok := m.Zones[zoneID]; !ok {
+		return fmt.Errorf("zone not found")
+	}
+	m.Records[zoneID] = make(map[string]dnsrecord.Record)
+	for _, r := range records {
+		if r.ID == "" {
+			r.ID = fmt.Sprintf("rec-%d", len(m.Records[zoneID])+1)
+		}
+		m.Records[zoneID][r.ID] = r
+	}
+	return nil
+}
+
+func (m *MockProvider) Capabilities() provider.ProviderCapabilities {
+	return provider.ProviderCapabilities{
+		CanListZones:    true,
+		CanGetZone:      true,
+		CanCreateRecord: true,
+		CanUpdateRecord: true,
+		CanDeleteRecord: true,
+		CanBulkReplace:  true,
+	}
+}
+
+func (m *MockProvider) Validate() error {
+	return nil
+}
+
+var _ provider.Provider = (*MockProvider)(nil)

pkg/dns/provider/conformance/suite.go

@@ -0,0 +1,35 @@
+package conformance
+
+import (
+	"context"
+	"testing"
+
+	"zonekit/pkg/dns/provider"
+
+	"github.com/stretchr/testify/require"
+)
+
+// RunConformanceTests runs a set of tests to verify provider compliance
+func RunConformanceTests(t *testing.T, p provider.Provider) {
+	ctx := context.Background()
+
+	t.Run("Capabilities", func(t *testing.T) {
+		caps := p.Capabilities()
+		t.Logf("Provider %s capabilities: %+v", p.Name(), caps)
+	})
+
+	t.Run("ZoneOperations", func(t *testing.T) {
+		if !p.Capabilities().CanListZones {
+			t.Skip("Provider does not support listing zones")
+		}
+
+		zones, err := p.ListZones(ctx)
+		require.NoError(t, err)
+
+		if len(zones) > 0 && p.Capabilities().CanGetZone {
+			zone, err := p.GetZone(ctx, zones[0].ID)
+			require.NoError(t, err)
+			require.Equal(t, zones[0].ID, zone.ID)
+		}
+	})
+}