Skip to content

Commit

Permalink
Review DNS providers (#580)
Browse files Browse the repository at this point in the history 
* refactor: create log.Infof and log.Warnf
* refactor: review DNS providers.
    - use one `http.Client` by provider instead of one client by request
    - use the same receiver name `d` for all `DNSProvider`
    - use `http.MethodXXX`
* refactor: logger init.
  • Loading branch information
@ldez
ldez committed Jun 21, 2018
1 parent 57782ac commit a1585a7
Show file tree
Hide file tree
Showing 42 changed files with 573 additions and 549 deletions.
44 changes: 22 additions & 22 deletions acme/client.go
View file
Expand Up @@ -164,7 +164,7 @@ func (c *Client) Register(tosAgreed bool) (*RegistrationResource, error) {
if c == nil || c.user == nil {
return nil, errors.New("acme: cannot register a nil client or user")
}
log.Printf("[INFO] acme: Registering account for %s", c.user.GetEmail())
log.Infof("acme: Registering account for %s", c.user.GetEmail())

accMsg := accountMessage{}
if c.user.GetEmail() != "" {
Expand Down Expand Up @@ -198,7 +198,7 @@ func (c *Client) RegisterWithExternalAccountBinding(tosAgreed bool, kid string,
if c == nil || c.user == nil {
return nil, errors.New("acme: cannot register a nil client or user")
}
log.Printf("[INFO] acme: Registering account (EAB) for %s", c.user.GetEmail())
log.Infof("acme: Registering account (EAB) for %s", c.user.GetEmail())

accMsg := accountMessage{}
if c.user.GetEmail() != "" {
Expand Down Expand Up @@ -244,7 +244,7 @@ func (c *Client) RegisterWithExternalAccountBinding(tosAgreed bool, kid string,
// ResolveAccountByKey will attempt to look up an account using the given account key
// and return its registration resource.
func (c *Client) ResolveAccountByKey() (*RegistrationResource, error) {
log.Printf("[INFO] acme: Trying to resolve account by key")
log.Infof("acme: Trying to resolve account by key")

acc := accountMessage{OnlyReturnExisting: true}
hdr, err := postJSON(c.jws, c.directory.NewAccountURL, acc, nil)
Expand Down Expand Up @@ -273,7 +273,7 @@ func (c *Client) DeleteRegistration() error {
if c == nil || c.user == nil {
return errors.New("acme: cannot unregister a nil client or user")
}
log.Printf("[INFO] acme: Deleting account for %s", c.user.GetEmail())
log.Infof("acme: Deleting account for %s", c.user.GetEmail())

accMsg := accountMessage{
Status: "deactivated",
Expand All @@ -293,7 +293,7 @@ func (c *Client) QueryRegistration() (*RegistrationResource, error) {
return nil, errors.New("acme: cannot query the registration of a nil client or user")
}
// Log the URL here instead of the email as the email may not be set
log.Printf("[INFO] acme: Querying account for %s", c.user.GetRegistration().URI)
log.Infof("acme: Querying account for %s", c.user.GetRegistration().URI)

accMsg := accountMessage{}

Expand Down Expand Up @@ -339,9 +339,9 @@ DNSNames:
}

if bundle {
log.Printf("[INFO][%s] acme: Obtaining bundled SAN certificate given a CSR", strings.Join(domains, ", "))
log.Infof("[%s] acme: Obtaining bundled SAN certificate given a CSR", strings.Join(domains, ", "))
} else {
log.Printf("[INFO][%s] acme: Obtaining SAN certificate given a CSR", strings.Join(domains, ", "))
log.Infof("[%s] acme: Obtaining SAN certificate given a CSR", strings.Join(domains, ", "))
}

order, err := c.createOrderForIdentifiers(domains)
Expand All @@ -363,7 +363,7 @@ DNSNames:
return nil, err
}

log.Printf("[INFO][%s] acme: Validations succeeded; requesting certificates", strings.Join(domains, ", "))
log.Infof("[%s] acme: Validations succeeded; requesting certificates", strings.Join(domains, ", "))

failures := make(ObtainError)
cert, err := c.requestCertificateForCsr(order, bundle, csr.Raw, nil)
Expand Down Expand Up @@ -399,9 +399,9 @@ func (c *Client) ObtainCertificate(domains []string, bundle bool, privKey crypto
}

if bundle {
log.Printf("[INFO][%s] acme: Obtaining bundled SAN certificate", strings.Join(domains, ", "))
log.Infof("[%s] acme: Obtaining bundled SAN certificate", strings.Join(domains, ", "))
} else {
log.Printf("[INFO][%s] acme: Obtaining SAN certificate", strings.Join(domains, ", "))
log.Infof("[%s] acme: Obtaining SAN certificate", strings.Join(domains, ", "))
}

order, err := c.createOrderForIdentifiers(domains)
Expand All @@ -423,7 +423,7 @@ func (c *Client) ObtainCertificate(domains []string, bundle bool, privKey crypto
return nil, err
}

log.Printf("[INFO][%s] acme: Validations succeeded; requesting certificates", strings.Join(domains, ", "))
log.Infof("[%s] acme: Validations succeeded; requesting certificates", strings.Join(domains, ", "))

failures := make(ObtainError)
cert, err := c.requestCertificateForOrder(order, bundle, privKey, mustStaple)
Expand Down Expand Up @@ -482,7 +482,7 @@ func (c *Client) RenewCertificate(cert CertificateResource, bundle, mustStaple b

// This is just meant to be informal for the user.
timeLeft := x509Cert.NotAfter.Sub(time.Now().UTC())
log.Printf("[INFO][%s] acme: Trying renewal with %d hours remaining", cert.Domain, int(timeLeft.Hours()))
log.Infof("[%s] acme: Trying renewal with %d hours remaining", cert.Domain, int(timeLeft.Hours()))

// We always need to request a new certificate to renew.
// Start by checking to see if the certificate was based off a CSR, and
Expand Down Expand Up @@ -556,7 +556,7 @@ func (c *Client) solveChallengeForAuthz(authorizations []authorization) error {
for _, authz := range authorizations {
if authz.Status == "valid" {
// Boulder might recycle recent validated authz (see issue #267)
log.Printf("[INFO][%s] acme: Authorization already valid; skipping challenge", authz.Identifier.Value)
log.Infof("[%s] acme: Authorization already valid; skipping challenge", authz.Identifier.Value)
continue
}

Expand Down Expand Up @@ -587,7 +587,7 @@ func (c *Client) chooseSolver(auth authorization, domain string) (int, solver) {
if solver, ok := c.solvers[Challenge(challenge.Type)]; ok {
return i, solver
}
log.Printf("[INFO][%s] acme: Could not find solver for: %s", domain, challenge.Type)
log.Infof("[%s] acme: Could not find solver for: %s", domain, challenge.Type)
}
return 0, nil
}
Expand Down Expand Up @@ -639,7 +639,7 @@ func (c *Client) getAuthzForOrder(order orderResource) ([]authorization, error)

func logAuthz(order orderResource) {
for i, auth := range order.Authorizations {
log.Printf("[INFO][%s] AuthURL: %s", order.Identifiers[i].Value, auth)
log.Infof("[%s] AuthURL: %s", order.Identifiers[i].Value, auth)
}
}

Expand Down Expand Up @@ -756,7 +756,7 @@ func (c *Client) checkCertResponse(order orderMessage, certRes *CertificateResou

if err != nil {
// If we fail to acquire the issuer cert, return the issued certificate - do not fail.
log.Printf("[WARNING][%s] acme: Could not bundle issuer certificate: %v", certRes.Domain, err)
log.Warnf("[%s] acme: Could not bundle issuer certificate: %v", certRes.Domain, err)
} else {
issuerCert = pemEncode(derCertificateBytes(issuerCert))

Expand All @@ -773,21 +773,21 @@ func (c *Client) checkCertResponse(order orderMessage, certRes *CertificateResou
certRes.Certificate = cert
certRes.CertURL = order.Certificate
certRes.CertStableURL = order.Certificate
log.Printf("[INFO][%s] Server responded with a certificate.", certRes.Domain)
log.Infof("[%s] Server responded with a certificate.", certRes.Domain)
return true, nil

case "processing":
return false, nil
case "invalid":
return false, errors.New("Order has invalid state: invalid")
return false, errors.New("order has invalid state: invalid")
default:
return false, nil
}

return false, nil
}

// getIssuerCertificate requests the issuer certificate
func (c *Client) getIssuerCertificate(url string) ([]byte, error) {
log.Printf("[INFO] acme: Requesting issuer cert from %s", url)
log.Infof("acme: Requesting issuer cert from %s", url)
resp, err := httpGet(url)
if err != nil {
return nil, err
Expand Down Expand Up @@ -841,7 +841,7 @@ func validate(j *jws, domain, uri string, c challenge) error {
for {
switch chlng.Status {
case "valid":
log.Printf("[INFO][%s] The server validated our request", domain)
log.Infof("[%s] The server validated our request", domain)
return nil
case "pending":
case "processing":
Expand Down
10 changes: 5 additions & 5 deletions acme/client_test.go
View file
Expand Up @@ -152,13 +152,13 @@ func TestValidate(t *testing.T) {
w.Header().Add("Replay-Nonce", "12345")
w.Header().Add("Retry-After", "0")
switch r.Method {
case "HEAD":
case "POST":
case http.MethodHead:
case http.MethodPost:
st := statuses[0]
statuses = statuses[1:]
writeJSONResponse(w, &challenge{Type: "http-01", Status: st, URL: "http://example.com/", Token: "token"})

case "GET":
case http.MethodGet:
st := statuses[0]
statuses = statuses[1:]
writeJSONResponse(w, &challenge{Type: "http-01", Status: st, URL: "http://example.com/", Token: "token"})
Expand Down Expand Up @@ -199,7 +199,7 @@ func TestGetChallenges(t *testing.T) {
var ts *httptest.Server
ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "GET", "HEAD":
case http.MethodGet, http.MethodHead:
w.Header().Add("Replay-Nonce", "12345")
w.Header().Add("Retry-After", "0")
writeJSONResponse(w, directory{
Expand All @@ -209,7 +209,7 @@ func TestGetChallenges(t *testing.T) {
RevokeCertURL: ts.URL,
KeyChangeURL: ts.URL,
})
case "POST":
case http.MethodPost:
writeJSONResponse(w, orderMessage{})
}
}))
Expand Down
10 changes: 5 additions & 5 deletions acme/dns_challenge.go
View file
Expand Up @@ -72,10 +72,10 @@ type dnsChallenge struct {
}

func (s *dnsChallenge) Solve(chlng challenge, domain string) error {
log.Printf("[INFO][%s] acme: Trying to solve DNS-01", domain)
log.Infof("[%s] acme: Trying to solve DNS-01", domain)

if s.provider == nil {
return errors.New("No DNS Provider configured")
return errors.New("no DNS Provider configured")
}

// Generate the Key Authorization for the challenge
Expand All @@ -86,18 +86,18 @@ func (s *dnsChallenge) Solve(chlng challenge, domain string) error {

err = s.provider.Present(domain, chlng.Token, keyAuth)
if err != nil {
return fmt.Errorf("Error presenting token: %s", err)
return fmt.Errorf("error presenting token: %s", err)
}
defer func() {
err := s.provider.CleanUp(domain, chlng.Token, keyAuth)
if err != nil {
log.Printf("Error cleaning up %s: %v ", domain, err)
log.Warnf("Error cleaning up %s: %v ", domain, err)
}
}()

fqdn, value, _ := DNS01Record(domain, keyAuth)

log.Printf("[INFO][%s] Checking DNS record propagation using %+v", domain, RecursiveNameservers)
log.Infof("[%s] Checking DNS record propagation using %+v", domain, RecursiveNameservers)

var timeout, interval time.Duration
switch provider := s.provider.(type) {
Expand Down
10 changes: 5 additions & 5 deletions acme/dns_challenge_manual.go
View file
Expand Up @@ -30,9 +30,9 @@ func (*DNSProviderManual) Present(domain, token, keyAuth string) error {
return err
}

log.Printf("[INFO] acme: Please create the following TXT record in your %s zone:", authZone)
log.Printf("[INFO] acme: %s", dnsRecord)
log.Printf("[INFO] acme: Press 'Enter' when you are done")
log.Infof("acme: Please create the following TXT record in your %s zone:", authZone)
log.Infof("acme: %s", dnsRecord)
log.Infof("acme: Press 'Enter' when you are done")

reader := bufio.NewReader(os.Stdin)
_, _ = reader.ReadString('\n')
Expand All @@ -49,7 +49,7 @@ func (*DNSProviderManual) CleanUp(domain, token, keyAuth string) error {
return err
}

log.Printf("[INFO] acme: You can now remove this TXT record from your %s zone:", authZone)
log.Printf("[INFO] acme: %s", dnsRecord)
log.Infof("acme: You can now remove this TXT record from your %s zone:", authZone)
log.Infof("acme: %s", dnsRecord)
return nil
}
6 changes: 3 additions & 3 deletions acme/http.go
View file
Expand Up @@ -80,7 +80,7 @@ func initCertPool() *x509.CertPool {
// httpHead performs a HEAD request with a proper User-Agent string.
// The response body (resp.Body) is already closed when this function returns.
func httpHead(url string) (resp *http.Response, err error) {
req, err := http.NewRequest("HEAD", url, nil)
req, err := http.NewRequest(http.MethodHead, url, nil)
if err != nil {
return nil, fmt.Errorf("failed to head %q: %v", url, err)
}
Expand All @@ -98,7 +98,7 @@ func httpHead(url string) (resp *http.Response, err error) {
// httpPost performs a POST request with a proper User-Agent string.
// Callers should close resp.Body when done reading from it.
func httpPost(url string, bodyType string, body io.Reader) (resp *http.Response, err error) {
req, err := http.NewRequest("POST", url, body)
req, err := http.NewRequest(http.MethodPost, url, body)
if err != nil {
return nil, fmt.Errorf("failed to post %q: %v", url, err)
}
Expand All @@ -111,7 +111,7 @@ func httpPost(url string, bodyType string, body io.Reader) (resp *http.Response,
// httpGet performs a GET request with a proper User-Agent string.
// Callers should close resp.Body when done reading from it.
func httpGet(url string) (resp *http.Response, err error) {
req, err := http.NewRequest("GET", url, nil)
req, err := http.NewRequest(http.MethodGet, url, nil)
if err != nil {
return nil, fmt.Errorf("failed to get %q: %v", url, err)
}
Expand Down
4 changes: 2 additions & 2 deletions acme/http_challenge.go
View file
Expand Up @@ -19,7 +19,7 @@ func HTTP01ChallengePath(token string) string {

func (s *httpChallenge) Solve(chlng challenge, domain string) error {

log.Printf("[INFO][%s] acme: Trying to solve HTTP-01", domain)
log.Infof("[%s] acme: Trying to solve HTTP-01", domain)

// Generate the Key Authorization for the challenge
keyAuth, err := getKeyAuthorization(chlng.Token, s.jws.privKey)
Expand All @@ -34,7 +34,7 @@ func (s *httpChallenge) Solve(chlng challenge, domain string) error {
defer func() {
err := s.provider.CleanUp(domain, chlng.Token, keyAuth)
if err != nil {
log.Printf("[%s] error cleaning up: %v", domain, err)
log.Warnf("[%s] error cleaning up: %v", domain, err)
}
}()

Expand Down
6 changes: 3 additions & 3 deletions acme/http_challenge_server.go
View file
Expand Up @@ -60,12 +60,12 @@ func (s *HTTPProviderServer) serve(domain, token, keyAuth string) {
// For validation it then writes the token the server returned with the challenge
mux := http.NewServeMux()
mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) {
if strings.HasPrefix(r.Host, domain) && r.Method == "GET" {
if strings.HasPrefix(r.Host, domain) && r.Method == http.MethodGet {
w.Header().Add("Content-Type", "text/plain")
w.Write([]byte(keyAuth))
log.Printf("[INFO][%s] Served key authentication", domain)
log.Infof("[%s] Served key authentication", domain)
} else {
log.Printf("[WARN] Received request for domain %s with method %s but the domain did not match any challenge. Please ensure your are passing the HOST header properly.", r.Host, r.Method)
log.Warnf("Received request for domain %s with method %s but the domain did not match any challenge. Please ensure your are passing the HOST header properly.", r.Host, r.Method)
w.Write([]byte("TEST"))
}
})
Expand Down
6 changes: 3 additions & 3 deletions acme/http_test.go
View file
Expand Up @@ -22,7 +22,7 @@ func TestHTTPHeadUserAgent(t *testing.T) {
t.Fatal(err)
}

if method != "HEAD" {
if method != http.MethodHead {
t.Errorf("Expected method to be HEAD, got %s", method)
}
if !strings.Contains(ua, ourUserAgent) {
Expand All @@ -44,7 +44,7 @@ func TestHTTPGetUserAgent(t *testing.T) {
}
res.Body.Close()

if method != "GET" {
if method != http.MethodGet {
t.Errorf("Expected method to be GET, got %s", method)
}
if !strings.Contains(ua, ourUserAgent) {
Expand All @@ -66,7 +66,7 @@ func TestHTTPPostUserAgent(t *testing.T) {
}
res.Body.Close()

if method != "POST" {
if method != http.MethodPost {
t.Errorf("Expected method to be POST, got %s", method)
}
if !strings.Contains(ua, ourUserAgent) {
Expand Down
4 changes: 2 additions & 2 deletions acme/tls_alpn_challenge.go
View file
Expand Up @@ -23,7 +23,7 @@ type tlsALPNChallenge struct {

// Solve manages the provider to validate and solve the challenge.
func (t *tlsALPNChallenge) Solve(chlng challenge, domain string) error {
log.Printf("[INFO][%s] acme: Trying to solve TLS-ALPN-01", domain)
log.Infof("[%s] acme: Trying to solve TLS-ALPN-01", domain)

// Generate the Key Authorization for the challenge
keyAuth, err := getKeyAuthorization(chlng.Token, t.jws.privKey)
Expand All @@ -38,7 +38,7 @@ func (t *tlsALPNChallenge) Solve(chlng challenge, domain string) error {
defer func() {
err := t.provider.CleanUp(domain, chlng.Token, keyAuth)
if err != nil {
log.Printf("[%s] error cleaning up: %v", domain, err)
log.Warnf("[%s] error cleaning up: %v", domain, err)
}
}()

Expand Down

0 comments on commit a1585a7

Please sign in to comment.