From e2405bda25f5524154631cf78f1ffc344c4c9aa0 Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Fri, 20 Mar 2026 17:47:59 -0300 Subject: [PATCH 1/2] feat: add DNS provider Validate method with early credential checking Addresses issues #111 and #137 with a unified design. ## What changed ### pkg/dnsprovider/provider.go - Add ValidateOptions struct (CheckCreds bool field) - Add Validate(ctx, domain, dnsConfig, opts) method to DNSProvider interface ### pkg/config/config.go - Add DNSConfig.Single() helper returning the sole provider name + config map ### pkg/dnsprovider/cloudflare/provider.go - Implement Validate with two phases: - Config (always): zone_name present, domain within zone - Creds (CheckCreds=true): token set, zone accessible via API ### cmd/nic/validate.go - Add --check-creds flag - Validate DNS config (and optionally credentials) when dns block is present ### cmd/nic/deploy.go - Add validateDNSCredentials shared helper - Call it early in runDeploy, before any tofu operations ### cmd/nic/destroy.go - Call validateDNSCredentials early in runDestroy, before any tofu operations ### pkg/dnsprovider/cloudflare/provider_test.go - Add TestValidate with 9 table-driven cases: missing zone_name, nil config, domain outside zone, domain equal to zone, missing token, inaccessible zone, valid config-only, valid with creds --- cmd/nic/deploy.go | 33 +++++ cmd/nic/destroy.go | 10 ++ cmd/nic/validate.go | 36 ++++++ pkg/config/config.go | 13 ++ pkg/dnsprovider/cloudflare/provider.go | 55 +++++++++ pkg/dnsprovider/cloudflare/provider_test.go | 127 ++++++++++++++++++++ pkg/dnsprovider/provider.go | 13 ++ 7 files changed, 287 insertions(+) diff --git a/cmd/nic/deploy.go b/cmd/nic/deploy.go index 75a2f30a..f35cc77e 100644 --- a/cmd/nic/deploy.go +++ b/cmd/nic/deploy.go @@ -15,6 +15,7 @@ import ( "github.com/nebari-dev/nebari-infrastructure-core/pkg/argocd" "github.com/nebari-dev/nebari-infrastructure-core/pkg/config" + "github.com/nebari-dev/nebari-infrastructure-core/pkg/dnsprovider" "github.com/nebari-dev/nebari-infrastructure-core/pkg/endpoint" "github.com/nebari-dev/nebari-infrastructure-core/pkg/git" providerPkg "github.com/nebari-dev/nebari-infrastructure-core/pkg/provider" @@ -128,6 +129,15 @@ func runDeploy(cmd *cobra.Command, args []string) error { slog.Info("Provider selected", "provider", provider.Name()) + // Validate DNS credentials early, before any infrastructure operations + if cfg.DNS != nil { + if err := validateDNSCredentials(ctx, cfg); err != nil { + span.RecordError(err) + slog.Error("DNS validation failed", "error", err) + return err + } + } + // Deploy infrastructure if err := provider.Deploy(ctx, cfg); err != nil { span.RecordError(err) @@ -225,6 +235,29 @@ func runDeploy(cmd *cobra.Command, args []string) error { return nil } +// validateDNSCredentials runs a full credential check for the configured DNS +// provider. Called early in deploy and destroy to surface errors before any +// tofu operations begin. +func validateDNSCredentials(ctx context.Context, cfg *config.NebariConfig) error { + providerName, dnsConfig, err := cfg.DNS.Single() + if err != nil { + return fmt.Errorf("DNS configuration error: %w", err) + } + + dp, err := dnsRegistry.Get(ctx, providerName) + if err != nil { + return err + } + + slog.Info("Validating DNS credentials", "dns_provider", providerName) + if err := dp.Validate(ctx, cfg.Domain, dnsConfig, dnsprovider.ValidateOptions{CheckCreds: true}); err != nil { + return fmt.Errorf("DNS validation failed: %w", err) + } + + slog.Info("DNS credentials validated successfully", "dns_provider", providerName) + return nil +} + // lookupEndpointAndProvisionDNS gets the load balancer endpoint from the cluster // and provisions DNS records if a DNS provider is configured. Returns the LB // endpoint for use in manual DNS guidance (may be nil if lookup failed). diff --git a/cmd/nic/destroy.go b/cmd/nic/destroy.go index b4811f60..3391b465 100644 --- a/cmd/nic/destroy.go +++ b/cmd/nic/destroy.go @@ -14,6 +14,7 @@ import ( "go.opentelemetry.io/otel/attribute" "github.com/nebari-dev/nebari-infrastructure-core/pkg/config" + "github.com/nebari-dev/nebari-infrastructure-core/pkg/provider" "github.com/nebari-dev/nebari-infrastructure-core/pkg/status" ) @@ -117,6 +118,15 @@ func runDestroy(cmd *cobra.Command, args []string) error { } } + // Validate DNS credentials early, before any infrastructure operations + if cfg.DNS != nil { + if err := validateDNSCredentials(ctx, cfg); err != nil { + span.RecordError(err) + slog.Error("DNS validation failed", "error", err) + return err + } + } + // Setup status handler for progress updates ctx, cleanupStatus := status.StartHandler(ctx, statusLogHandler()) defer cleanupStatus() diff --git a/cmd/nic/validate.go b/cmd/nic/validate.go index ea059620..ff0ffe83 100644 --- a/cmd/nic/validate.go +++ b/cmd/nic/validate.go @@ -9,10 +9,12 @@ import ( "go.opentelemetry.io/otel/attribute" "github.com/nebari-dev/nebari-infrastructure-core/pkg/config" + "github.com/nebari-dev/nebari-infrastructure-core/pkg/dnsprovider" ) var ( validateConfigFile string + validateCheckCreds bool validateCmd = &cobra.Command{ Use: "validate", @@ -26,6 +28,7 @@ all required fields.`, func init() { validateCmd.Flags().StringVarP(&validateConfigFile, "file", "f", "", "Path to nebari-config.yaml file (required)") + validateCmd.Flags().BoolVar(&validateCheckCreds, "check-creds", false, "Also verify DNS credentials against the live provider API (requires network access)") // Panic is appropriate in init() since we cannot return errors and this indicates a programming error if err := validateCmd.MarkFlagRequired("file"); err != nil { panic(err) @@ -63,9 +66,42 @@ func runValidate(cmd *cobra.Command, args []string) error { return err } + // Validate DNS configuration when present + if cfg.DNS != nil { + providerName, dnsConfig, err := cfg.DNS.Single() + if err != nil { + span.RecordError(err) + slog.Error("Invalid DNS configuration", "error", err) + return fmt.Errorf("DNS configuration error: %w", err) + } + + dnsProvider, err := dnsRegistry.Get(ctx, providerName) + if err != nil { + span.RecordError(err) + slog.Error("DNS provider not available", "error", err, "dns_provider", providerName) + return err + } + + opts := dnsprovider.ValidateOptions{CheckCreds: validateCheckCreds} + if err := dnsProvider.Validate(ctx, cfg.Domain, dnsConfig, opts); err != nil { + span.RecordError(err) + slog.Error("DNS validation failed", "error", err, "dns_provider", providerName) + return fmt.Errorf("DNS validation failed: %w", err) + } + + if validateCheckCreds { + slog.Info("DNS credentials validated", "dns_provider", providerName) + } else { + slog.Info("DNS configuration validated (use --check-creds to also verify credentials)", "dns_provider", providerName) + } + } + fmt.Printf("✓ Configuration file is valid\n") fmt.Printf(" Provider: %s\n", cfg.Provider) fmt.Printf(" Project: %s\n", cfg.ProjectName) + if cfg.DNS != nil { + fmt.Printf(" DNS: %s\n", cfg.DNS.ProviderName()) + } return nil } diff --git a/pkg/config/config.go b/pkg/config/config.go index 7cfa93e9..ec8fb4e0 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -102,6 +102,19 @@ func (d *DNSConfig) ProviderConfig() map[string]any { return nil } +// Single returns the sole configured DNS provider name and its config map. +// Returns an error if DNS is nil or does not contain exactly one provider. +// This is a convenience helper for use in CLI command handlers. +func (d *DNSConfig) Single() (string, map[string]any, error) { + if d == nil { + return "", nil, fmt.Errorf("no DNS configuration present") + } + if err := d.Validate(); err != nil { + return "", nil, err + } + return d.ProviderName(), d.ProviderConfig(), nil +} + // CertificateConfig holds TLS certificate configuration type CertificateConfig struct { // Type is the certificate type: "selfsigned" or "letsencrypt" diff --git a/pkg/dnsprovider/cloudflare/provider.go b/pkg/dnsprovider/cloudflare/provider.go index 83d4b3e6..8f6249fc 100644 --- a/pkg/dnsprovider/cloudflare/provider.go +++ b/pkg/dnsprovider/cloudflare/provider.go @@ -11,6 +11,7 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" + "github.com/nebari-dev/nebari-infrastructure-core/pkg/dnsprovider" "github.com/nebari-dev/nebari-infrastructure-core/pkg/status" ) @@ -41,6 +42,60 @@ func (p *Provider) Name() string { return "cloudflare" } +// Validate checks the Cloudflare DNS configuration. +// +// Config phase (always runs): +// - zone_name is present and non-empty +// - domain is a subdomain of (or equal to) zone_name +// +// Creds phase (only when opts.CheckCreds == true): +// - CLOUDFLARE_API_TOKEN environment variable is set +// - Token can resolve the zone ID from the Cloudflare API +func (p *Provider) Validate(ctx context.Context, domain string, dnsConfig map[string]any, opts dnsprovider.ValidateOptions) error { + tracer := otel.Tracer("nebari-infrastructure-core") + ctx, span := tracer.Start(ctx, "cloudflare.Validate") + defer span.End() + + span.SetAttributes( + attribute.String("domain", domain), + attribute.Bool("check_creds", opts.CheckCreds), + ) + + // --- Config phase --- + cfCfg, err := extractCloudflareConfig(domain, dnsConfig) + if err != nil { + span.RecordError(err) + return err + } + span.SetAttributes(attribute.String("zone_name", cfCfg.ZoneName)) + + if !opts.CheckCreds { + return nil + } + + // --- Creds phase --- + apiToken, err := getAPIToken() + if err != nil { + span.RecordError(err) + return err + } + + client, err := p.getClient(apiToken) + if err != nil { + span.RecordError(err) + return err + } + + zoneID, err := client.ResolveZoneID(ctx, cfCfg.ZoneName) + if err != nil { + span.RecordError(err) + return fmt.Errorf("DNS credential check failed: zone %q not accessible with provided token: %w", cfCfg.ZoneName, err) + } + span.SetAttributes(attribute.String("zone_id", zoneID)) + + return nil +} + // ProvisionRecords creates or updates DNS records for the deployment. // It creates a root domain record and wildcard record pointing to the // load balancer endpoint. The record type (A or CNAME) is determined diff --git a/pkg/dnsprovider/cloudflare/provider_test.go b/pkg/dnsprovider/cloudflare/provider_test.go index 11e4684d..d5e92993 100644 --- a/pkg/dnsprovider/cloudflare/provider_test.go +++ b/pkg/dnsprovider/cloudflare/provider_test.go @@ -5,6 +5,8 @@ import ( "fmt" "strings" "testing" + + "github.com/nebari-dev/nebari-infrastructure-core/pkg/dnsprovider" ) // mockClient implements CloudflareClient for testing. @@ -426,3 +428,128 @@ func wrapUpdate(original func(context.Context, string, string, string, string, s return nil } } + +func TestValidate(t *testing.T) { + baseDNS := map[string]any{"zone_name": "example.com"} + + tests := []struct { + name string + domain string + dnsConfig map[string]any + envToken string + checkCreds bool + mock *mockClient + wantErr bool + wantErrContain string + }{ + // --- Config-only phase (checkCreds: false) --- + { + name: "valid config, no creds check", + domain: "nebari.example.com", + dnsConfig: baseDNS, + checkCreds: false, + mock: &mockClient{}, + wantErr: false, + }, + { + name: "missing zone_name", + domain: "nebari.example.com", + dnsConfig: map[string]any{}, + checkCreds: false, + mock: &mockClient{}, + wantErr: true, + wantErrContain: "zone_name", + }, + { + name: "nil dns config", + domain: "nebari.example.com", + dnsConfig: nil, + checkCreds: false, + mock: &mockClient{}, + wantErr: true, + wantErrContain: "dns configuration is missing", + }, + { + name: "domain not within zone", + domain: "nebari.otherdomain.com", + dnsConfig: map[string]any{"zone_name": "example.com"}, + checkCreds: false, + mock: &mockClient{}, + wantErr: true, + wantErrContain: "not within zone", + }, + { + name: "domain equal to zone is valid", + domain: "example.com", + dnsConfig: map[string]any{"zone_name": "example.com"}, + checkCreds: false, + mock: &mockClient{}, + wantErr: false, + }, + // --- Creds phase (checkCreds: true) --- + { + name: "missing API token", + domain: "nebari.example.com", + dnsConfig: baseDNS, + envToken: "", // intentionally unset + checkCreds: true, + mock: &mockClient{}, + wantErr: true, + wantErrContain: "CLOUDFLARE_API_TOKEN", + }, + { + name: "zone not accessible with token", + domain: "nebari.example.com", + dnsConfig: baseDNS, + envToken: "bad-token", + checkCreds: true, + mock: &mockClient{ + resolveZoneIDFn: func(_ context.Context, _ string) (string, error) { + return "", fmt.Errorf("zone not found") + }, + }, + wantErr: true, + wantErrContain: "not accessible", + }, + { + name: "valid config and credentials", + domain: "nebari.example.com", + dnsConfig: baseDNS, + envToken: "valid-token", + checkCreds: true, + mock: &mockClient{ + resolveZoneIDFn: func(_ context.Context, _ string) (string, error) { + return "zone-abc123", nil + }, + }, + wantErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if tc.envToken != "" { + t.Setenv("CLOUDFLARE_API_TOKEN", tc.envToken) + } else { + t.Setenv("CLOUDFLARE_API_TOKEN", "") + } + + provider := NewProviderForTesting(tc.mock) + opts := dnsprovider.ValidateOptions{CheckCreds: tc.checkCreds} + err := provider.Validate(context.Background(), tc.domain, tc.dnsConfig, opts) + + if tc.wantErr { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tc.wantErrContain) + } + if !strings.Contains(err.Error(), tc.wantErrContain) { + t.Fatalf("expected error containing %q, got %q", tc.wantErrContain, err.Error()) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} diff --git a/pkg/dnsprovider/provider.go b/pkg/dnsprovider/provider.go index c86e2ae5..682ee14a 100644 --- a/pkg/dnsprovider/provider.go +++ b/pkg/dnsprovider/provider.go @@ -2,12 +2,25 @@ package dnsprovider import "context" +// ValidateOptions controls which checks Validate performs. +type ValidateOptions struct { + // CheckCreds, when true, also verifies credentials against the live provider + // API (e.g. resolves the zone ID). Requires network access. + CheckCreds bool +} + // DNSProvider defines the interface that all DNS providers must implement. // Providers are stateless - domain and DNS config are passed to each call. type DNSProvider interface { // Name returns the DNS provider name (cloudflare, route53, azure-dns, etc.) Name() string + // Validate checks the DNS configuration. It always verifies required config + // fields (e.g. zone_name) and that the domain falls within the zone. + // When opts.CheckCreds is true it additionally verifies that credentials are + // present and can reach the live provider API. + Validate(ctx context.Context, domain string, dnsConfig map[string]any, opts ValidateOptions) error + // ProvisionRecords creates or updates DNS records for the deployment. // It creates a root domain record and wildcard record pointing to the // load balancer endpoint. The provider determines the record type From c10be463ff03176cbab603e2466cbaf6afb0b8b5 Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Fri, 20 Mar 2026 17:51:47 -0300 Subject: [PATCH 2/2] fix: add Validate to mockDNSProvider in registry_test to satisfy interface --- pkg/dnsprovider/registry_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/dnsprovider/registry_test.go b/pkg/dnsprovider/registry_test.go index f5759c45..4031278c 100644 --- a/pkg/dnsprovider/registry_test.go +++ b/pkg/dnsprovider/registry_test.go @@ -22,6 +22,10 @@ func (m *mockDNSProvider) DestroyRecords(ctx context.Context, domain string, dns return nil } +func (m *mockDNSProvider) Validate(ctx context.Context, domain string, dnsConfig map[string]any, opts ValidateOptions) error { + return nil +} + func TestNewRegistry(t *testing.T) { registry := NewRegistry() if registry == nil || registry.providers == nil {