From 8e83f024016987ba8a9cbebe5e680ec8a30c326e Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 30 Apr 2026 15:16:11 +0000 Subject: [PATCH] feat(f5,iis): explicit ValidateOnly + leverage existing transactional rollback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 8 of the deploy-hardening I master bundle. F5 + IIS already have transactional / explicit-backup-restore rollback semantics in their DeployCertificate paths. Phase 8 adds the explicit ValidateOnly dry-run probe that operators use to preview a deploy without touching the live cert. F5 (validate_only.go): - ValidateOnly probes the iControl REST API via Authenticate. Cheap (no F5 transaction created) + cached after first success. Failure surfaces as a wrapped error so operators see the actual cause (auth provider down, invalid creds, BIG-IP unreachable, etc.). nil client returns ErrValidateOnlyNotSupported. - A true cert-bind dry-run requires F5's no-commit transaction mode (v17.5+); V3-Pro can add per-version dispatch. V2 ships the reachability probe as the load-bearing safety check. - 5 new tests in validate_only_test.go covering: auth-success, auth-fail wrapped, nil-client sentinel, error-message contains BIG-IP context, recoverable auth-fail surfaces provider info. IIS (validate_only.go): - ValidateOnly runs `Get-WebSite -Name ` via the injected PowerShellExecutor. Confirms the IIS PS module is loaded AND the site exists AND the agent has admin privileges. Failure here surfaces the actual PowerShell stderr (site not found / module missing / access denied). - A true cert-bind dry-run would need IIS to expose a no-commit New-WebBinding (it doesn't); V3-Pro can extend with a temp-install + immediate-remove. V2 ships the permission + module probe as the load-bearing check. - 5 new tests in validate_only_test.go covering: get-website succeeds, get-website fails, nil-executor sentinel, site-name quoting (handles spaces in 'Default Web Site'), output-context in error. Smoke test connectorsAtPhase3 list shrunk from 10 to 7 entries (f5 + iis + postfix removed). Caddy stays in (file-mode returns sentinel; api-mode is real-impl). Envoy + Traefik stay in (no validate-with-target command exists for either). javakeystore + k8ssecret + ssh + wincertstore stay in pending Phase 9. Coverage: F5 holds at ≥85%; IIS holds at ≥85%. Race detector clean. golangci-lint v2.11.4 clean. Phase 9 next: SSH + WinCertStore + JavaKeystore + K8s — the non-file-server connectors. --- internal/connector/target/f5/validate_only.go | 38 ++++-- .../connector/target/f5/validate_only_test.go | 115 ++++++++++++++++++ .../connector/target/iis/validate_only.go | 38 ++++-- .../target/iis/validate_only_test.go | 96 +++++++++++++++ .../target/validate_only_smoke_test.go | 25 ++-- 5 files changed, 284 insertions(+), 28 deletions(-) create mode 100644 internal/connector/target/f5/validate_only_test.go create mode 100644 internal/connector/target/iis/validate_only_test.go diff --git a/internal/connector/target/f5/validate_only.go b/internal/connector/target/f5/validate_only.go index 7e6d720..b3cfd29 100644 --- a/internal/connector/target/f5/validate_only.go +++ b/internal/connector/target/f5/validate_only.go @@ -2,17 +2,39 @@ package f5 import ( "context" + "fmt" "github.com/shankar0123/certctl/internal/connector/target" ) -// ValidateOnly is the default Phase 3 stub for the deploy-hardening -// I master bundle: returns ErrValidateOnlyNotSupported so existing -// connectors compile against the extended target.Connector interface -// without changing behavior. Phase f5 dry-run support arrives when -// the connector's atomic-deploy implementation lands (NGINX in -// Phase 4, Apache in Phase 5, etc.); each phase replaces this stub -// with a real validate-with-the-target implementation. +// ValidateOnly — Phase 8 of the deploy-hardening I master bundle. +// F5 already has full transactional rollback semantics in +// DeployCertificate (the iControl REST API is transactional — +// `mgmt/tm/transaction` wraps the install + bind together; on +// failure the whole transaction aborts atomically with no live +// VS impact). Phase 8 makes the dry-run explicit by probing the +// BIG-IP control plane health: if the API is reachable and +// authenticated, ValidateOnly returns nil; otherwise it returns +// the wrapped client error so operators can preview a deploy +// without touching the live SSL profile. +// +// Note: a full dry-run that simulates the cert install + bind +// without commit would require F5 to expose a no-commit transaction +// mode (it does not in v15.x; it does in v17.5+ — V3-Pro will add +// per-version dispatch). For V2 the reachability probe is the +// load-bearing safety check. func (c *Connector) ValidateOnly(ctx context.Context, request target.DeploymentRequest) error { - return target.ErrValidateOnlyNotSupported + if c.client == nil { + return target.ErrValidateOnlyNotSupported + } + // Probe by attempting authentication. The F5 client caches + // the token after first success, so subsequent ValidateOnly + // calls are cheap. Failure here means the BIG-IP is + // unreachable, the operator credentials are wrong, or the + // auth provider (TACACS+, RADIUS) is down — all reasons to + // abort a deploy preview. + if err := c.client.Authenticate(ctx); err != nil { + return fmt.Errorf("F5 ValidateOnly: BIG-IP control plane probe failed: %w", err) + } + return nil } diff --git a/internal/connector/target/f5/validate_only_test.go b/internal/connector/target/f5/validate_only_test.go new file mode 100644 index 0000000..ca2ece7 --- /dev/null +++ b/internal/connector/target/f5/validate_only_test.go @@ -0,0 +1,115 @@ +package f5 + +import ( + "context" + "errors" + "log/slog" + "os" + "testing" + + "github.com/shankar0123/certctl/internal/connector/target" +) + +// Phase 8 of the deploy-hardening I master bundle: F5 ValidateOnly +// real implementation tests. F5 already has full transactional +// rollback via the iControl REST `mgmt/tm/transaction` endpoint; +// the new bit is the explicit dry-run probe via Authenticate. + +type stubF5Authenticator struct { + authErr error +} + +func (s *stubF5Authenticator) Authenticate(_ context.Context) error { + return s.authErr +} + +// implement the rest of the F5Client interface as no-ops so the +// stub satisfies the interface. +func (s *stubF5Authenticator) UploadFile(context.Context, string, []byte) error { + return nil +} +func (s *stubF5Authenticator) InstallCert(context.Context, string, string) error { return nil } +func (s *stubF5Authenticator) InstallKey(context.Context, string, string) error { return nil } +func (s *stubF5Authenticator) CreateTransaction(context.Context) (string, error) { + return "", nil +} +func (s *stubF5Authenticator) CommitTransaction(context.Context, string) error { + return nil +} +func (s *stubF5Authenticator) UpdateSSLProfile(context.Context, string, string, string, string, string, string) error { + return nil +} +func (s *stubF5Authenticator) GetSSLProfile(context.Context, string, string) (*SSLProfileInfo, error) { + return nil, nil +} +func (s *stubF5Authenticator) DeleteCert(context.Context, string, string) error { return nil } +func (s *stubF5Authenticator) DeleteKey(context.Context, string, string) error { return nil } + +func quietLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(os.NewFile(0, os.DevNull), &slog.HandlerOptions{Level: slog.LevelError})) +} + +func TestF5_ValidateOnly_Auth_Succeeds_ReturnsNil(t *testing.T) { + c := NewWithClient(&Config{Host: "f5.example", Username: "admin"}, quietLogger(), &stubF5Authenticator{authErr: nil}) + if err := c.ValidateOnly(context.Background(), target.DeploymentRequest{}); err != nil { + t.Errorf("got %v, want nil", err) + } +} + +func TestF5_ValidateOnly_AuthFails_ReturnsWrappedError(t *testing.T) { + c := NewWithClient(&Config{Host: "f5.example", Username: "admin"}, quietLogger(), &stubF5Authenticator{authErr: errors.New("invalid credentials")}) + err := c.ValidateOnly(context.Background(), target.DeploymentRequest{}) + if err == nil { + t.Fatal("expected error") + } + if errors.Is(err, target.ErrValidateOnlyNotSupported) { + t.Errorf("got sentinel, want wrapped auth error: %v", err) + } +} + +func TestF5_ValidateOnly_NilClient_ReturnsSentinel(t *testing.T) { + c := &Connector{config: &Config{Host: "f5.example"}, logger: quietLogger()} + // Don't inject a client. + if err := c.ValidateOnly(context.Background(), target.DeploymentRequest{}); !errors.Is(err, target.ErrValidateOnlyNotSupported) { + t.Errorf("got %v, want sentinel", err) + } +} + +func TestF5_ValidateOnly_AuthFailureMessageMentionsBIGIP(t *testing.T) { + c := NewWithClient(&Config{Host: "f5.example"}, quietLogger(), &stubF5Authenticator{authErr: errors.New("conn refused")}) + err := c.ValidateOnly(context.Background(), target.DeploymentRequest{}) + if err == nil { + t.Fatal("expected error") + } + if !contains(err.Error(), "BIG-IP") { + t.Errorf("error missing BIG-IP context: %v", err) + } +} + +func TestF5_ValidateOnly_RecoverableAuthErrIsActionable(t *testing.T) { + // Auth-fail variant that simulates a one-time TACACS+ outage — + // the operator is meant to see this as actionable. + c := NewWithClient(&Config{Host: "f5.example"}, quietLogger(), &stubF5Authenticator{authErr: errors.New("TACACS+ auth provider unreachable")}) + err := c.ValidateOnly(context.Background(), target.DeploymentRequest{}) + if err == nil { + t.Fatal("expected error") + } + if !contains(err.Error(), "TACACS+") { + t.Errorf("error doesn't surface auth provider info: %v", err) + } +} + +func contains(haystack, needle string) bool { + return len(haystack) > 0 && len(needle) > 0 && + (len(haystack) >= len(needle)) && + (indexOf(haystack, needle) >= 0) +} + +func indexOf(s, substr string) int { + for i := 0; i+len(substr) <= len(s); i++ { + if s[i:i+len(substr)] == substr { + return i + } + } + return -1 +} diff --git a/internal/connector/target/iis/validate_only.go b/internal/connector/target/iis/validate_only.go index cbcaad4..72ff6a7 100644 --- a/internal/connector/target/iis/validate_only.go +++ b/internal/connector/target/iis/validate_only.go @@ -2,17 +2,39 @@ package iis import ( "context" + "fmt" "github.com/shankar0123/certctl/internal/connector/target" ) -// ValidateOnly is the default Phase 3 stub for the deploy-hardening -// I master bundle: returns ErrValidateOnlyNotSupported so existing -// connectors compile against the extended target.Connector interface -// without changing behavior. Phase iis dry-run support arrives when -// the connector's atomic-deploy implementation lands (NGINX in -// Phase 4, Apache in Phase 5, etc.); each phase replaces this stub -// with a real validate-with-the-target implementation. +// ValidateOnly — Phase 8 of the deploy-hardening I master bundle. +// IIS already has explicit pre-deploy backup + post-rollback +// re-import semantics in DeployCertificate. Phase 8 adds an +// explicit dry-run via a PowerShell health probe: if the agent +// can run a `Get-WebSite` cmdlet, the IIS PowerShell module is +// loaded and the agent has the right permissions; ValidateOnly +// returns nil. Otherwise it returns the wrapped script error so +// operators can preview a deploy without touching the live cert +// store. +// +// Note: a true cert-bind dry-run would require IIS to expose a +// no-commit `New-WebBinding` mode (it does not). For V2 the +// permission + module probe is the load-bearing safety check. +// V3-Pro can extend this with a temporary cert install + immediate +// remove. func (c *Connector) ValidateOnly(ctx context.Context, request target.DeploymentRequest) error { - return target.ErrValidateOnlyNotSupported + if c.executor == nil { + return target.ErrValidateOnlyNotSupported + } + // Probe `Get-WebSite -Name ` to confirm the IIS + // PowerShell module is loaded AND the configured site exists. + // Failure here means the agent isn't on a Windows host with + // IIS installed, the site name is wrong, or the agent is + // running as a user without IIS administration privileges. + script := fmt.Sprintf(`Get-WebSite -Name %q | Select-Object -ExpandProperty Name`, c.config.SiteName) + out, err := c.executor.Execute(ctx, script) + if err != nil { + return fmt.Errorf("IIS ValidateOnly: %w (output: %s)", err, out) + } + return nil } diff --git a/internal/connector/target/iis/validate_only_test.go b/internal/connector/target/iis/validate_only_test.go new file mode 100644 index 0000000..f400051 --- /dev/null +++ b/internal/connector/target/iis/validate_only_test.go @@ -0,0 +1,96 @@ +package iis + +import ( + "context" + "errors" + "log/slog" + "os" + "strings" + "testing" + + "github.com/shankar0123/certctl/internal/connector/target" +) + +// Phase 8 of the deploy-hardening I master bundle: IIS ValidateOnly +// real implementation tests. IIS already has explicit pre-deploy +// backup + post-rollback re-import semantics; the new bit is the +// PowerShell health probe via Get-WebSite. + +type stubExecutor struct { + out string + err error +} + +func (s *stubExecutor) Execute(_ context.Context, _ string) (string, error) { + return s.out, s.err +} + +func quietLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(os.NewFile(0, os.DevNull), &slog.HandlerOptions{Level: slog.LevelError})) +} + +func TestIIS_ValidateOnly_GetWebSite_Succeeds(t *testing.T) { + c := NewWithExecutor(&Config{SiteName: "Default Web Site"}, quietLogger(), &stubExecutor{out: "Default Web Site"}) + if err := c.ValidateOnly(context.Background(), target.DeploymentRequest{}); err != nil { + t.Errorf("got %v, want nil", err) + } +} + +func TestIIS_ValidateOnly_GetWebSite_Fails(t *testing.T) { + c := NewWithExecutor(&Config{SiteName: "Missing"}, quietLogger(), &stubExecutor{ + out: "Get-WebSite : Cannot find a Web site with name 'Missing'", + err: errors.New("PowerShell exit 1"), + }) + err := c.ValidateOnly(context.Background(), target.DeploymentRequest{}) + if err == nil { + t.Fatal("expected error") + } + if errors.Is(err, target.ErrValidateOnlyNotSupported) { + t.Errorf("got sentinel, want wrapped error: %v", err) + } + if !strings.Contains(err.Error(), "Cannot find") { + t.Errorf("error missing PowerShell stderr: %v", err) + } +} + +func TestIIS_ValidateOnly_NilExecutor_ReturnsSentinel(t *testing.T) { + c := &Connector{config: &Config{SiteName: "x"}, logger: quietLogger()} + if err := c.ValidateOnly(context.Background(), target.DeploymentRequest{}); !errors.Is(err, target.ErrValidateOnlyNotSupported) { + t.Errorf("got %v, want sentinel", err) + } +} + +func TestIIS_ValidateOnly_SiteNameQuoted(t *testing.T) { + // Verify the script PROPERLY quotes site names with spaces (a common + // IIS site name pattern). + captured := "" + exec := &stubExecutor{out: "Default Web Site"} + c := NewWithExecutor(&Config{SiteName: "Default Web Site"}, quietLogger(), exec) + // Wrap exec to capture script. + c.executor = captureExec{wrapped: exec, captured: &captured} + c.ValidateOnly(context.Background(), target.DeploymentRequest{}) + if !strings.Contains(captured, `"Default Web Site"`) { + t.Errorf("script missing quoted site name: %q", captured) + } +} + +type captureExec struct { + wrapped PowerShellExecutor + captured *string +} + +func (c captureExec) Execute(ctx context.Context, script string) (string, error) { + *c.captured = script + return c.wrapped.Execute(ctx, script) +} + +func TestIIS_ValidateOnly_OutputContextInError(t *testing.T) { + c := NewWithExecutor(&Config{SiteName: "DWS"}, quietLogger(), &stubExecutor{ + out: "WARNING: This site is in stopped state", + err: errors.New("exit 1"), + }) + err := c.ValidateOnly(context.Background(), target.DeploymentRequest{}) + if err == nil || !strings.Contains(err.Error(), "stopped state") { + t.Errorf("got %v", err) + } +} diff --git a/internal/connector/target/validate_only_smoke_test.go b/internal/connector/target/validate_only_smoke_test.go index 07058d4..37bca57 100644 --- a/internal/connector/target/validate_only_smoke_test.go +++ b/internal/connector/target/validate_only_smoke_test.go @@ -24,13 +24,13 @@ import ( // apache removed Phase 5 — real ValidateOnly implementation now in apache.go. "github.com/shankar0123/certctl/internal/connector/target/caddy" "github.com/shankar0123/certctl/internal/connector/target/envoy" - "github.com/shankar0123/certctl/internal/connector/target/f5" + // f5 removed Phase 8 — real ValidateOnly implementation now in validate_only.go. // haproxy removed Phase 6 — real ValidateOnly implementation now in haproxy.go. - "github.com/shankar0123/certctl/internal/connector/target/iis" + // iis removed Phase 8 — real ValidateOnly implementation now in validate_only.go. "github.com/shankar0123/certctl/internal/connector/target/javakeystore" "github.com/shankar0123/certctl/internal/connector/target/k8ssecret" // nginx removed Phase 4 — real ValidateOnly implementation now in nginx.go. - "github.com/shankar0123/certctl/internal/connector/target/postfix" + // postfix removed Phase 7 — real ValidateOnly implementation now in postfix.go. "github.com/shankar0123/certctl/internal/connector/target/ssh" "github.com/shankar0123/certctl/internal/connector/target/traefik" "github.com/shankar0123/certctl/internal/connector/target/wincertstore" @@ -66,19 +66,20 @@ var connectorsAtPhase3 = []struct { }{ // apache removed Phase 5 — its ValidateOnly is now the real // implementation; tested directly in apache/apache_atomic_test.go. + // caddy: file mode returns sentinel (no validate-with-target); + // api mode is real-impl. Empty Connector hits the file-mode path. {"caddy", func() target.Connector { return &caddy.Connector{} }}, + // envoy: no validate-with-target command exists; always sentinel. {"envoy", func() target.Connector { return &envoy.Connector{} }}, - {"f5", func() target.Connector { return &f5.Connector{} }}, - // haproxy removed Phase 6 — its ValidateOnly is now real; - // tested in haproxy/haproxy_atomic_test.go. - {"iis", func() target.Connector { return &iis.Connector{} }}, + // f5 removed Phase 8 — Authenticate-probe real impl. + // haproxy removed Phase 6 — `haproxy -c -f` real impl. + // iis removed Phase 8 — Get-WebSite probe real impl. {"javakeystore", func() target.Connector { return &javakeystore.Connector{} }}, {"k8ssecret", func() target.Connector { return &k8ssecret.Connector{} }}, - // nginx removed Phase 4 — its ValidateOnly is now the real - // implementation; tested directly in - // internal/connector/target/nginx/nginx_test.go. - {"postfix", func() target.Connector { return &postfix.Connector{} }}, + // nginx removed Phase 4 — `nginx -t` real impl. + // postfix removed Phase 7 — `postfix check` / `doveconf -n` real impl. {"ssh", func() target.Connector { return &ssh.Connector{} }}, + // traefik: no validate-with-target command exists; always sentinel. {"traefik", func() target.Connector { return &traefik.Connector{} }}, {"wincertstore", func() target.Connector { return &wincertstore.Connector{} }}, } @@ -86,7 +87,7 @@ var connectorsAtPhase3 = []struct { func TestEveryConnectorDefaultsToSentinel(t *testing.T) { // Expected list size shrinks as Phases 4-9 land their real // ValidateOnly implementations. Phase 4 removed nginx. - const expectedAtCurrentPhase = 10 + const expectedAtCurrentPhase = 7 if len(connectorsAtPhase3) != expectedAtCurrentPhase { t.Fatalf("connectors-at-phase list = %d entries, want %d (drift in the 13-connector inventory)", len(connectorsAtPhase3), expectedAtCurrentPhase) }