General cleanup and more godocs

3 files changed, 91 insertions, 69 deletions

diff --git a/modules/caddytls/acmemanager.go b/modules/caddytls/acmemanager.go
index a7a460a..40e2d24 100644
--- a/modules/caddytls/acmemanager.go
+++ b/modules/caddytls/acmemanager.go
@@ -3,6 +3,7 @@ package caddytls
 import (
 	"encoding/json"
 	"fmt"
+	"time"
 
 	"github.com/go-acme/lego/certcrypto"
 
@@ -18,11 +19,6 @@ func init() {
 	})
 }
 
-// ManagerMaker TODO: WIP...
-type ManagerMaker interface {
-	newManager(interactive bool) (certmagic.Manager, error)
-}
-
 // acmeManagerMaker makes an ACME manager
 // for managinig certificates using ACME.
 type acmeManagerMaker struct {
@@ -40,9 +36,11 @@ type acmeManagerMaker struct {
 	keyType certcrypto.KeyType
 }
 
-func (m *acmeManagerMaker) Provision() error {
-	m.setDefaults()
+func (m *acmeManagerMaker) newManager(interactive bool) (certmagic.Manager, error) {
+	return nil, nil
+}
 
+func (m *acmeManagerMaker) Provision() error {
 	// DNS providers
 	if m.Challenges.DNS != nil {
 		val, err := caddy2.LoadModuleInline("provider", "tls.dns", m.Challenges.DNS)
@@ -67,18 +65,71 @@ func (m *acmeManagerMaker) Provision() error {
 		m.Storage = nil // allow GC to deallocate - TODO: Does this help?
 	}
 
+	m.setDefaults()
+
 	return nil
 }
 
-// setDefaults indiscriminately sets all the default values in m.
+// setDefaults sets necessary values that are
+// currently empty to their default values.
 func (m *acmeManagerMaker) setDefaults() {
-	m.CA = certmagic.LetsEncryptStagingCA // certmagic.Default.CA // TODO: When not testing, switch to production CA
-	m.Email = certmagic.Default.Email
-	m.RenewAhead = caddy2.Duration(certmagic.Default.RenewDurationBefore)
-	m.keyType = certmagic.Default.KeyType
-	m.storage = certmagic.Default.Storage
+	if m.CA == "" {
+		m.CA = certmagic.LetsEncryptStagingCA // certmagic.Default.CA // TODO: When not testing, switch to production CA
+	}
+	if m.Email == "" {
+		m.Email = certmagic.Default.Email
+	}
+	if m.RenewAhead == 0 {
+		m.RenewAhead = caddy2.Duration(certmagic.Default.RenewDurationBefore)
+	}
+	if m.keyType == "" {
+		m.keyType = certmagic.Default.KeyType
+	}
+	if m.storage == nil {
+		m.storage = certmagic.Default.Storage
+	}
 }
 
-func (m *acmeManagerMaker) newManager(interactive bool) (certmagic.Manager, error) {
-	return nil, nil
+// makeCertMagicConfig converts m into a certmagic.Config, because
+// this is a special case where the default manager is the certmagic
+// Config and not a separate manager.
+func (m *acmeManagerMaker) makeCertMagicConfig() certmagic.Config {
+	storage := m.storage
+	if storage == nil {
+		storage = caddy2.GetStorage()
+	}
+
+	var ond *certmagic.OnDemandConfig
+	if m.OnDemand != nil {
+		ond = &certmagic.OnDemandConfig{
+			// TODO: fill this out
+		}
+	}
+
+	return certmagic.Config{
+		CA:                      certmagic.LetsEncryptStagingCA, //ap.CA, // TODO: Restore true value
+		Email:                   m.Email,
+		Agreed:                  true,
+		DisableHTTPChallenge:    m.Challenges.HTTP.Disabled,
+		DisableTLSALPNChallenge: m.Challenges.TLSALPN.Disabled,
+		RenewDurationBefore:     time.Duration(m.RenewAhead),
+		AltHTTPPort:             m.Challenges.HTTP.AlternatePort,
+		AltTLSALPNPort:          m.Challenges.TLSALPN.AlternatePort,
+		DNSProvider:             m.Challenges.dns,
+		KeyType:                 supportedCertKeyTypes[m.KeyType],
+		CertObtainTimeout:       time.Duration(m.ACMETimeout),
+		OnDemand:                ond,
+		MustStaple:              m.MustStaple,
+		Storage:                 storage,
+		// TODO: listenHost
+	}
+}
+
+// supportedCertKeyTypes is all the key types that are supported
+// for certificates that are obtained through ACME.
+var supportedCertKeyTypes = map[string]certcrypto.KeyType{
+	"RSA2048": certcrypto.RSA2048,
+	"RSA4096": certcrypto.RSA4096,
+	"P256":    certcrypto.EC256,
+	"P384":    certcrypto.EC384,
 }
diff --git a/modules/caddytls/matchers.go b/modules/caddytls/matchers.go
index c376f87..b308bd0 100644
--- a/modules/caddytls/matchers.go
+++ b/modules/caddytls/matchers.go
@@ -7,13 +7,22 @@ import (
 )
 
 type (
+	// MatchServerName matches based on SNI.
 	MatchServerName []string
 
 	// TODO: these others should be enterprise-only, probably
-	MatchProtocol   []string // TODO: version or protocol?
+
+	// MatchProtocol matches based on protocol.
+	MatchProtocol []string // TODO: Protocol or version?
+
+	// MatchClientCert matches based on client certificate / client auth?
 	MatchClientCert struct{} // TODO: client certificate options
-	MatchRemote     []string
-	MatchStarlark   string
+
+	// MatchRemote matches based on the remote address of the connection.
+	MatchRemote []string
+
+	// MatchStarlark matches based on a Starlark script.
+	MatchStarlark string
 )
 
 func init() {
@@ -39,6 +48,7 @@ func init() {
 	})
 }
 
+// Match matches hello based on SNI.
 func (m MatchServerName) Match(hello *tls.ClientHelloInfo) bool {
 	for _, name := range m {
 		// TODO: support wildcards (and regex?)
@@ -49,21 +59,25 @@ func (m MatchServerName) Match(hello *tls.ClientHelloInfo) bool {
 	return false
 }
 
+// Match matches hello based on protocol version.
 func (m MatchProtocol) Match(hello *tls.ClientHelloInfo) bool {
 	// TODO: not implemented
 	return false
 }
 
+// Match matches hello based on client certificate.
 func (m MatchClientCert) Match(hello *tls.ClientHelloInfo) bool {
 	// TODO: not implemented
 	return false
 }
 
+// Match matches hello based on remote address.
 func (m MatchRemote) Match(hello *tls.ClientHelloInfo) bool {
 	// TODO: not implemented
 	return false
 }
 
+// Match matches hello based on a Starlark script.
 func (m MatchStarlark) Match(hello *tls.ClientHelloInfo) bool {
 	// TODO: not implemented
 	return false
diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go
index 43ad957..fbc850c 100644
--- a/modules/caddytls/tls.go
+++ b/modules/caddytls/tls.go
@@ -5,10 +5,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"time"
 
 	"bitbucket.org/lightcodelabs/caddy2"
-	"github.com/go-acme/lego/certcrypto"
 	"github.com/go-acme/lego/challenge"
 	"github.com/klauspost/cpuid"
 	"github.com/mholt/certmagic"
@@ -30,8 +28,7 @@ type TLS struct {
 	certCache          *certmagic.Cache
 }
 
-// TODO: Finish stubbing out this two-phase setup process: prepare, then start...
-
+// Provision sets up the configuration for the TLS app.
 func (t *TLS) Provision() error {
 	// set up the certificate cache
 	// TODO: this makes a new cache every time; better to only make a new
@@ -97,15 +94,6 @@ func (t *TLS) Start(handle caddy2.Handle) error {
 		if err != nil {
 			return fmt.Errorf("automate: managing %v: %v", names, err)
 		}
-		// for _, name := range names {
-		// 	t.Manage([]string{name)
-		// 	ap := t.getAutomationPolicyForName(name)
-		// 	magic := certmagic.New(t.certCache, ap.makeCertMagicConfig())
-		// 	err := magic.Manage([]string{name})
-		// 	if err != nil {
-		// 		return fmt.Errorf("automate: manage %s: %v", name, err)
-		// 	}
-		// }
 	}
 	t.Certificates = nil // allow GC to deallocate - TODO: Does this help?
 
@@ -191,38 +179,11 @@ type AutomationPolicy struct {
 }
 
 func (ap AutomationPolicy) makeCertMagicConfig() certmagic.Config {
+	// default manager (ACME) is a special case because of how CertMagic is designed
+	// TODO: refactor certmagic so that ACME manager is not a special case by extracting
+	// its config fields out of the certmagic.Config struct, or something...
 	if acmeMgmt, ok := ap.management.(*acmeManagerMaker); ok {
-		// default, which is management via ACME
-
-		storage := acmeMgmt.storage
-		if storage == nil {
-			storage = caddy2.GetStorage()
-		}
-
-		var ond *certmagic.OnDemandConfig
-		if acmeMgmt.OnDemand != nil {
-			ond = &certmagic.OnDemandConfig{
-				// TODO: fill this out
-			}
-		}
-
-		return certmagic.Config{
-			CA:                      certmagic.LetsEncryptStagingCA, //ap.CA, // TODO: Restore true value
-			Email:                   acmeMgmt.Email,
-			Agreed:                  true,
-			DisableHTTPChallenge:    acmeMgmt.Challenges.HTTP.Disabled,
-			DisableTLSALPNChallenge: acmeMgmt.Challenges.TLSALPN.Disabled,
-			RenewDurationBefore:     time.Duration(acmeMgmt.RenewAhead),
-			AltHTTPPort:             acmeMgmt.Challenges.HTTP.AlternatePort,
-			AltTLSALPNPort:          acmeMgmt.Challenges.TLSALPN.AlternatePort,
-			DNSProvider:             acmeMgmt.Challenges.dns,
-			KeyType:                 supportedCertKeyTypes[acmeMgmt.KeyType],
-			CertObtainTimeout:       time.Duration(acmeMgmt.ACMETimeout),
-			OnDemand:                ond,
-			MustStaple:              acmeMgmt.MustStaple,
-			Storage:                 storage,
-			// TODO: listenHost
-		}
+		return acmeMgmt.makeCertMagicConfig()
 	}
 
 	return certmagic.Config{
@@ -260,13 +221,9 @@ type OnDemandConfig struct {
 	AskStarlark string `json:"ask_starlark,omitempty"`
 }
 
-// supportedCertKeyTypes is all the key types that are supported
-// for certificates that are obtained through ACME.
-var supportedCertKeyTypes = map[string]certcrypto.KeyType{
-	"RSA2048": certcrypto.RSA2048,
-	"RSA4096": certcrypto.RSA4096,
-	"P256":    certcrypto.EC256,
-	"P384":    certcrypto.EC384,
+// ManagerMaker makes a certificate manager.
+type ManagerMaker interface {
+	newManager(interactive bool) (certmagic.Manager, error)
 }
 
 // supportedCipherSuites is the unordered map of cipher suite