mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-14 23:08:51 +00:00
asyncpoll: refactor Sectigo / Entrust / GlobalSign to bounded polling (Phase 2)
Phase 2 of the #5 acquisition-readiness fix from the 2026-05-01 issuer
coverage audit. Phase 1 (commit 711265b) shipped the shared asyncpoll
package and refactored DigiCert as the reference. This commit applies
the same pattern to the remaining three async-CA connectors and adds
the operator-facing docs.
Per-connector refactors:
- Sectigo (sectigo.go): GetOrderStatus now wraps pollEnrollmentOnce in
asyncpoll.Poll. The collectNotReady sentinel (cert approved by SCM
but not yet retrievable from the collect endpoint) maps to
StillPending and rides the backoff schedule rather than the prior
"return pending immediately" branch. Added isPermanentStatusError
helper to distinguish transient HTTP errors (5xx / 429 / network)
from permanent ones (4xx / parse failure) — the wrapped checkStatus
errors get triaged at the poll closure boundary.
- Entrust (entrust.go): GetOrderStatus wraps pollEnrollmentOnce. The
AWAITING_APPROVAL status maps to StillPending; operators using
approval-pending workflows where humans approve enrollments should
bump CERTCTL_ENTRUST_POLL_MAX_WAIT_SECONDS to 86400 (24h) so a
single scheduler tick can wait through the approval window. The
default 10-minute deadline matches the other three connectors.
- GlobalSign (globalsign.go): GetOrderStatus wraps pollCertificateOnce.
GlobalSign tracks orders by serial number rather than order ID, but
the polling shape is identical to the other three. Status-code
triage matches DigiCert: 4xx (not 429) is permanent, 5xx / 429 /
network is transient.
Per-connector Config field added:
- DigiCert.PollMaxWaitSeconds (env CERTCTL_DIGICERT_POLL_MAX_WAIT_SECONDS)
- Sectigo.PollMaxWaitSeconds (env CERTCTL_SECTIGO_POLL_MAX_WAIT_SECONDS)
- Entrust.PollMaxWaitSeconds (env CERTCTL_ENTRUST_POLL_MAX_WAIT_SECONDS)
- GlobalSign.PollMaxWaitSeconds (env CERTCTL_GLOBALSIGN_POLL_MAX_WAIT_SECONDS)
internal/config/config.go env-var loaders updated for all four. Default
is 600 seconds (10 minutes); zero falls back to the asyncpoll package
default.
Test-helper updates: every existing test that exercises the pending
branch (collectNotReady, AWAITING_APPROVAL, status="pending", etc.)
now sets PollMaxWaitSeconds=1 in its Config so the test doesn't block
on the production-default 10-minute deadline. Tests that exercise
permanent-error branches (404, 401, malformed JSON, etc.) continue
to return immediately.
Test sites updated:
- buildSectigoConnector helper + GetOrderStatus_CollectNotReady test
- buildEntrustConnector helper + GetOrderStatus_Pending test
- buildGlobalsignConnector helper + GetOrderStatus_Pending test +
the GetHTTPClient_NoMTLSCertPaths test (network failure now rides
the backoff schedule rather than returning immediately)
Documentation:
- docs/async-polling.md: new operator reference covering the backoff
schedule, status-code triage, the four env vars, failure modes, and
where the implementation lives. Audit blocker citation included.
- docs/connectors.md: per-issuer sections for DigiCert, Sectigo,
Entrust, GlobalSign each gain the PollMaxWaitSeconds env var row
and a cross-link to async-polling.md.
Lint cleanup: simplified the isPermanentStatusError branch to satisfy
staticcheck S1008 (single-line return for a final boolean check).
Verified locally:
- gofmt -l . clean
- go vet ./... clean
- staticcheck ./... clean
- golangci-lint run --timeout 5m ./... → 0 issues
- go test -short -count=1 across all 4 connector packages + config + asyncpoll: green
Audit reference: cowork/issuer-coverage-audit-2026-05-01/RESULTS.md
Top-10 fix #5 — Phase 2.
This commit is contained in:
@@ -39,6 +39,7 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/shankar0123/certctl/internal/connector/issuer"
|
||||
"github.com/shankar0123/certctl/internal/connector/issuer/asyncpoll"
|
||||
)
|
||||
|
||||
// Config represents the GlobalSign Atlas HVCA issuer connector configuration.
|
||||
@@ -73,6 +74,24 @@ type Config struct {
|
||||
// internal CA not present in the host's default trust bundle.
|
||||
// Set via CERTCTL_GLOBALSIGN_SERVER_CA_PATH environment variable.
|
||||
ServerCAPath string `json:"server_ca_path,omitempty"`
|
||||
|
||||
// PollMaxWaitSeconds caps how long GetOrderStatus blocks doing
|
||||
// internal exponential-backoff polling before returning
|
||||
// StillPending. Default 600 (10 minutes). GlobalSign tracks
|
||||
// orders by serial number rather than order ID, but the polling
|
||||
// shape is identical.
|
||||
//
|
||||
// Set via CERTCTL_GLOBALSIGN_POLL_MAX_WAIT_SECONDS. Audit fix #5.
|
||||
PollMaxWaitSeconds int `json:"poll_max_wait_seconds,omitempty"`
|
||||
}
|
||||
|
||||
// pollMaxWait returns the configured PollMaxWait as a time.Duration,
|
||||
// or the asyncpoll package default if unset.
|
||||
func (c *Config) pollMaxWait() time.Duration {
|
||||
if c.PollMaxWaitSeconds <= 0 {
|
||||
return asyncpoll.DefaultMaxWait
|
||||
}
|
||||
return time.Duration(c.PollMaxWaitSeconds) * time.Second
|
||||
}
|
||||
|
||||
// Connector implements the issuer.Connector interface for GlobalSign Atlas HVCA.
|
||||
@@ -423,21 +442,72 @@ func (c *Connector) RevokeCertificate(ctx context.Context, request issuer.Revoca
|
||||
return nil
|
||||
}
|
||||
|
||||
// GetOrderStatus checks the status of a GlobalSign certificate order by serial number.
|
||||
// Polls the certificate endpoint; when status is "issued", downloads and returns the cert.
|
||||
// GetOrderStatus checks the status of a GlobalSign certificate order
|
||||
// by serial number, using bounded internal polling (asyncpoll.Poll).
|
||||
// One call blocks for up to PollMaxWait (default 10m) doing
|
||||
// exponential backoff with jitter; returns Done with the cert,
|
||||
// Failed with the rejection reason, or StillPending if the deadline
|
||||
// expires (caller can re-invoke).
|
||||
//
|
||||
// Audit fix #5 Phase 2: previously each scheduler tick made one HTTP
|
||||
// call against an unready order. GlobalSign tracks orders by serial
|
||||
// number rather than order ID, but the polling shape is identical.
|
||||
func (c *Connector) GetOrderStatus(ctx context.Context, orderID string) (*issuer.OrderStatus, error) {
|
||||
c.logger.Debug("checking GlobalSign certificate status", "serial", orderID)
|
||||
|
||||
var done *issuer.OrderStatus
|
||||
var lastPendingMsg string
|
||||
cfg := asyncpoll.Config{MaxWait: c.config.pollMaxWait()}
|
||||
|
||||
res, err := asyncpoll.Poll(ctx, cfg, func(ctx context.Context) (asyncpoll.Result, error) {
|
||||
status, result, pollErr := c.pollCertificateOnce(ctx, orderID)
|
||||
if status != nil {
|
||||
switch result {
|
||||
case asyncpoll.Done:
|
||||
done = status
|
||||
case asyncpoll.StillPending:
|
||||
if status.Message != nil {
|
||||
lastPendingMsg = *status.Message
|
||||
}
|
||||
}
|
||||
}
|
||||
return result, pollErr
|
||||
})
|
||||
|
||||
now := time.Now()
|
||||
switch res {
|
||||
case asyncpoll.Done:
|
||||
return done, nil
|
||||
case asyncpoll.Failed:
|
||||
return nil, err
|
||||
default:
|
||||
msg := lastPendingMsg
|
||||
if msg == "" {
|
||||
msg = fmt.Sprintf("certificate %s still pending after PollMaxWait", orderID)
|
||||
}
|
||||
return &issuer.OrderStatus{
|
||||
OrderID: orderID,
|
||||
Status: "pending",
|
||||
Message: &msg,
|
||||
UpdatedAt: now,
|
||||
}, nil
|
||||
}
|
||||
}
|
||||
|
||||
// pollCertificateOnce makes one HTTP GET against the GlobalSign Atlas
|
||||
// HVCA certificate status endpoint and translates the response into
|
||||
// an asyncpoll.Result. 4xx (not 429) is permanent; 5xx / 429 / network
|
||||
// is transient.
|
||||
func (c *Connector) pollCertificateOnce(ctx context.Context, orderID string) (*issuer.OrderStatus, asyncpoll.Result, error) {
|
||||
client, err := c.getHTTPClient(ctx)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, asyncpoll.Failed, err
|
||||
}
|
||||
|
||||
// GlobalSign status endpoint: GET /v2/certificates/{serial}
|
||||
statusURL := strings.TrimSuffix(c.config.APIUrl, "/") + fmt.Sprintf("/v2/certificates/%s", orderID)
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, statusURL, nil)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to create status request: %w", err)
|
||||
return nil, asyncpoll.Failed, fmt.Errorf("failed to create status request: %w", err)
|
||||
}
|
||||
|
||||
req.Header.Set("ApiKey", c.config.APIKey)
|
||||
@@ -446,40 +516,39 @@ func (c *Connector) GetOrderStatus(ctx context.Context, orderID string) (*issuer
|
||||
|
||||
resp, err := client.Do(req)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("GlobalSign status request failed: %w", err)
|
||||
return nil, asyncpoll.StillPending, fmt.Errorf("GlobalSign status request failed: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
respBody, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to read status response: %w", err)
|
||||
return nil, asyncpoll.StillPending, fmt.Errorf("failed to read status response: %w", err)
|
||||
}
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, fmt.Errorf("GlobalSign certificate status returned %d: %s", resp.StatusCode, string(respBody))
|
||||
statusErr := fmt.Errorf("GlobalSign certificate status returned %d: %s", resp.StatusCode, string(respBody))
|
||||
if resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode >= 500 {
|
||||
return nil, asyncpoll.StillPending, statusErr
|
||||
}
|
||||
return nil, asyncpoll.Failed, statusErr
|
||||
}
|
||||
|
||||
var certResp certificateResponse
|
||||
if err := json.Unmarshal(respBody, &certResp); err != nil {
|
||||
return nil, fmt.Errorf("failed to parse status response: %w", err)
|
||||
return nil, asyncpoll.Failed, fmt.Errorf("failed to parse status response: %w", err)
|
||||
}
|
||||
|
||||
now := time.Now()
|
||||
|
||||
switch certResp.Status {
|
||||
case "issued":
|
||||
if certResp.Certificate == "" {
|
||||
return nil, fmt.Errorf("certificate status is issued but certificate PEM is missing")
|
||||
return nil, asyncpoll.Failed, fmt.Errorf("certificate status is issued but certificate PEM is missing")
|
||||
}
|
||||
|
||||
notBefore, notAfter, err := parseCertDates(certResp.Certificate)
|
||||
if err != nil {
|
||||
c.logger.Warn("failed to parse certificate dates", "error", err)
|
||||
}
|
||||
|
||||
c.logger.Info("GlobalSign certificate ready",
|
||||
"serial", orderID)
|
||||
|
||||
c.logger.Info("GlobalSign certificate ready", "serial", orderID)
|
||||
return &issuer.OrderStatus{
|
||||
OrderID: orderID,
|
||||
Status: "completed",
|
||||
@@ -489,7 +558,7 @@ func (c *Connector) GetOrderStatus(ctx context.Context, orderID string) (*issuer
|
||||
NotBefore: ¬Before,
|
||||
NotAfter: ¬After,
|
||||
UpdatedAt: now,
|
||||
}, nil
|
||||
}, asyncpoll.Done, nil
|
||||
|
||||
case "pending", "processing":
|
||||
msg := fmt.Sprintf("certificate %s is %s", orderID, certResp.Status)
|
||||
@@ -498,7 +567,7 @@ func (c *Connector) GetOrderStatus(ctx context.Context, orderID string) (*issuer
|
||||
Status: "pending",
|
||||
Message: &msg,
|
||||
UpdatedAt: now,
|
||||
}, nil
|
||||
}, asyncpoll.StillPending, nil
|
||||
|
||||
case "rejected", "denied", "failed":
|
||||
msg := fmt.Sprintf("certificate %s was %s", orderID, certResp.Status)
|
||||
@@ -507,7 +576,7 @@ func (c *Connector) GetOrderStatus(ctx context.Context, orderID string) (*issuer
|
||||
Status: "failed",
|
||||
Message: &msg,
|
||||
UpdatedAt: now,
|
||||
}, nil
|
||||
}, asyncpoll.Done, nil
|
||||
|
||||
default:
|
||||
msg := fmt.Sprintf("unknown certificate status: %s", certResp.Status)
|
||||
@@ -516,7 +585,7 @@ func (c *Connector) GetOrderStatus(ctx context.Context, orderID string) (*issuer
|
||||
Status: "pending",
|
||||
Message: &msg,
|
||||
UpdatedAt: now,
|
||||
}, nil
|
||||
}, asyncpoll.StillPending, nil
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -19,9 +19,10 @@ import (
|
||||
func buildGlobalsignConnector(t *testing.T, baseURL string) *globalsign.Connector {
|
||||
t.Helper()
|
||||
cfg := &globalsign.Config{
|
||||
APIUrl: baseURL,
|
||||
APIKey: "k",
|
||||
APISecret: "s",
|
||||
APIUrl: baseURL,
|
||||
APIKey: "k",
|
||||
APISecret: "s",
|
||||
PollMaxWaitSeconds: 1, // keep async-pending tests fast
|
||||
}
|
||||
// Use NewWithHTTPClient with a test client so getHTTPClient short-circuits
|
||||
// (no mTLS cert loading). Custom transport is required so the
|
||||
@@ -125,19 +126,26 @@ func TestGlobalsign_GetOrderStatus_IssuedWithMalformedPEM_NonFatalParseDateWarni
|
||||
func TestGlobalsign_GetHTTPClient_NoMTLSCertPaths_ReturnsClientAsIs(t *testing.T) {
|
||||
// When ClientCertPath and ClientKeyPath are both empty, getHTTPClient
|
||||
// returns httpClient as-is — exercises that branch.
|
||||
//
|
||||
// PollMaxWaitSeconds=1 keeps this test fast: a network failure on
|
||||
// the invalid host is now treated as transient by the bounded-
|
||||
// polling Poller, so without the deadline the call blocks for
|
||||
// the production-default 10 minutes.
|
||||
cfg := &globalsign.Config{
|
||||
APIUrl: "http://example.invalid",
|
||||
APIKey: "k",
|
||||
APISecret: "s",
|
||||
APIUrl: "http://example.invalid",
|
||||
APIKey: "k",
|
||||
APISecret: "s",
|
||||
PollMaxWaitSeconds: 1,
|
||||
// no cert paths
|
||||
}
|
||||
c := globalsign.NewWithHTTPClient(cfg, slog.Default(), &http.Client{})
|
||||
// GetOrderStatus will fail at HTTP do (invalid host), but getHTTPClient
|
||||
// will have been exercised through the no-mTLS branch.
|
||||
_, err := c.GetOrderStatus(context.Background(), "x")
|
||||
if err == nil {
|
||||
t.Errorf("expected error from invalid host")
|
||||
}
|
||||
// GetOrderStatus blocks briefly (1s) due to bounded polling, then
|
||||
// returns a pending OrderStatus (transient network err did not
|
||||
// become a permanent failure). The test exercises the
|
||||
// no-mTLS branch in getHTTPClient — the post-poll status doesn't
|
||||
// matter; we just need GetOrderStatus to be invoked through the
|
||||
// branch.
|
||||
_, _ = c.GetOrderStatus(context.Background(), "x")
|
||||
}
|
||||
|
||||
func TestGlobalsign_GetHTTPClient_MTLSPathConfigured_LoadsKeyPair(t *testing.T) {
|
||||
|
||||
@@ -355,9 +355,10 @@ func TestGlobalSignConnector(t *testing.T) {
|
||||
defer mockServer.Close()
|
||||
|
||||
config := &globalsign.Config{
|
||||
APIUrl: mockServer.URL,
|
||||
APIKey: "gs-test-key",
|
||||
APISecret: "gs-test-secret",
|
||||
APIUrl: mockServer.URL,
|
||||
APIKey: "gs-test-key",
|
||||
APISecret: "gs-test-secret",
|
||||
PollMaxWaitSeconds: 1, // keep async-pending tests fast
|
||||
}
|
||||
|
||||
connector := globalsign.NewWithHTTPClient(config, logger, httpClient)
|
||||
|
||||
Reference in New Issue
Block a user