Added arguments for tests and some cleanup

Author: mariuskimmina

hook.go

@@ -1,8 +1,6 @@
 package tls
 
 import (
-	"fmt"
-
 	"github.com/coredns/caddy"
 )
 
@@ -30,11 +28,11 @@ func hook(event caddy.EventName, info interface{}) error {
 				}
 				_, err = instance.Restart(corefile)
 				if err != nil {
-					fmt.Printf("Error during Restart: %v, \n", err)
+					log.Errorf("Error during Restart: %v, \n", err)
 				}
 				return
 			case <-r.quit:
-				log.Info("Certificate renewal quit")
+				log.Debug("Received quit signal, stopping certificate renewal")
 				return
 			}
 		}

manager.go

@@ -4,14 +4,13 @@ import (
 	"context"
 	"errors"
 	"io/fs"
-	"strconv"
 	"time"
 
 	"crypto/tls"
 	"crypto/x509"
-	//"encoding/pem"
+	"encoding/pem"
 	"fmt"
-	//"os"
+	"os"
 
 	"github.com/caddyserver/certmagic"
 	"github.com/coredns/coredns/core/dnsserver"
@@ -24,46 +23,36 @@ type ACMEManager struct {
 }
 
 // NewACMEManager create a new ACMEManager
-func NewACMEManager(config *dnsserver.Config, zone string, ca string) *ACMEManager {
+func NewACMEManager(config *dnsserver.Config, zone string, ca string, caCert string, port int) *ACMEManager {
+    //TODO: change this
 	if ca == "" {
 		ca = "localhost:14001/dir" //pebble default
 	}
 
-	// TODO: this lets our  acme client trust the pebble cert
-	// this is only needed for testing and should not be in production
-	// figure out how to only do this in test cases
-	//certbytes, err := os.ReadFile("test/certs/pebble.minica.pem")
-	//if err != nil {
-	//fmt.Println(err.Error())
-	//panic("Failed to load Cert")
-	//}
-	//pemcert, _ := pem.Decode(certbytes)
-	//if pemcert == nil {
-	//fmt.Println("pemcert not found")
-	//}
-	//cert, err := x509.ParseCertificate(pemcert.Bytes)
-	//if err != nil {
-	//fmt.Println(err)
-	//panic("Failed to parse Cert")
-	//}
 	pool, err := x509.SystemCertPool()
 	if err != nil {
-		fmt.Println(err)
-		panic("Failed to get system Certpool")
+        log.Errorf("Failed to get system pool of trusted certificates: %v \n", err)
 	}
-	//pool.AddCert(cert)
 
-	portNumber, err := strconv.Atoi(config.Port)
-	if err != nil {
-		fmt.Println(err)
-		panic("Failed to convert config.Port to integer")
-	}
+    if caCert != "" {
+        certbytes, err := os.ReadFile(caCert)
+        if err != nil {
+            log.Errorf("Failed to read certificate provided by cacert option: %v \n", err)
+        }
+        pemcert, _ := pem.Decode(certbytes)
+        if pemcert == nil {
+            fmt.Println("pemcert not found")
+        }
+        cert, err := x509.ParseCertificate(pemcert.Bytes)
+        if err != nil {
+            log.Errorf("Failed to parse certificate provided by cacert option: %v \n", err)
+        }
+        pool.AddCert(cert)
+    }
 
-	//TODO: the address cannot be hardcoded
 	solver := &DNSSolver{
-		Port: portNumber,
-		Addr: "127.0.0.1:1053",
-	}
+        Port: port,
+    }
 
 	certmagic.DefaultACME.Email = "[email protected]"
 

setup.go

@@ -2,6 +2,7 @@ package tls
 
 import (
 	"context"
+	"strconv"
 	"sync"
 	"time"
 
@@ -37,7 +38,9 @@ var (
 const (
 	argDomain   = "domain"
 	argCa       = "ca"
+	argCaCert   = "cacert"
 	argCertPath = "certpath"
+	argPort     = "port"
 )
 
 func parseTLS(c *caddy.Controller) error {
@@ -65,6 +68,8 @@ func parseTLS(c *caddy.Controller) error {
 
 			var domainNameACME string
 			var ca string
+			var caCert string
+            var port string
 			//certPath := "/home/marius/.local/share/certmagic/certificates/example.com/example.com.crt"
 
 			for c.NextBlock() {
@@ -82,6 +87,18 @@ func parseTLS(c *caddy.Controller) error {
 						return plugin.Error("tls", c.Errf("To many arguments to ca"))
 					}
 					ca = caArgs[0]
+				case argCaCert:
+					caCertArgs := c.RemainingArgs()
+					if len(caCertArgs) > 1 {
+						return plugin.Error("tls", c.Errf("To many arguments to cacert"))
+					}
+					caCert = caCertArgs[0]
+				case argPort:
+					portArgs := c.RemainingArgs()
+					if len(portArgs) > 1 {
+						return plugin.Error("tls", c.Errf("To many arguments to port"))
+					}
+					port = portArgs[0]
 				case argCertPath:
 					certPathArgs := c.RemainingArgs()
 					if len(certPathArgs) > 1 {
@@ -93,7 +110,15 @@ func parseTLS(c *caddy.Controller) error {
 				}
 			}
 
-			manager := NewACMEManager(config, domainNameACME, ca)
+            portNumber := 53
+            if port != "" {
+                portNumber, err = strconv.Atoi(port)
+                if err != nil {
+                    log.Errorf("Failed to convert port argument to integer: %v \n", err)
+                }
+            }
+
+			manager := NewACMEManager(config, domainNameACME, ca, caCert, portNumber)
 
 			var names []string
 			names = append(names, manager.Zone)

solver.go

@@ -14,8 +14,7 @@ import (
 )
 
 type DNSSolver struct {
-	Port int
-	Addr string
+    Port    int 
 	DNS  *ACMEServer
 }
 
@@ -96,7 +95,7 @@ func (d *DNSSolver) Present(ctx context.Context, challenge acme.Challenge) error
 	d.DNS = acmeServer
 
 	addr := net.UDPAddr{
-		Port: 53,
+		Port: d.Port,
 		IP:   net.ParseIP("0.0.0.0"),
 	}
 

solver_test.go

@@ -9,17 +9,17 @@ import (
 	"github.com/miekg/dns"
 )
 
-func TestCheckDNSChallenge() {
-	_ = []struct {
-		name     string
-		question string
-	}{
-		{
-			name:     "ACME Challenge",
-			question: "_acme-challenge.example.com.",
-		},
-	}
-}
+//func TestCheckDNSChallenge() {
+//_ = []struct {
+//name     string
+//question string
+//}{
+//{
+//name:     "ACME Challenge",
+//question: "_acme-challenge.example.com.",
+//},
+//}
+//}
 
 func setupACME(readyChan chan string) {
 	acmeServer := &ACMEServer{

test/Corefile

@@ -2,6 +2,7 @@ tls://.:1053 {
     tls acme {
         domain example.com
         ca localhost:14001/dir
+        cacert test/certs/pebble.minica.pem
     }
     whoami
 }

test/pebble_test.go

@@ -174,6 +174,8 @@ func TestRenewal(t *testing.T) {
                 tls acme {
                     domain example.com
                     ca     localhost:14001/dir
+                    cacert test/certs/pebble.minica.pem
+                    port   1053
                 }
                 whoami  
             }`,
@@ -219,19 +221,19 @@ func TestRenewal(t *testing.T) {
 			// wait for certificate to expire
 			time.Sleep(80 * time.Second)
 
-			_, _, err = client.Exchange(m, tcp)
+            r, _, err := client.Exchange(m, tcp)
 
 			if err != nil {
 				if err.Error() == "x509: cannot validate certificate for :: because it doesn't contain any IP SANs" {
-					fmt.Println("This errror is expected")
+					fmt.Println("Ignoring certificate error")
 				} else {
 					fmt.Println(err)
 				}
 			}
 
-			//if n := len(r.Answer); n != tc.AnswerLength {
-			//t.Errorf("Expected %v answers, got %v", tc.AnswerLength, n)
-			//}
+			if n := len(r.Answer); n != tc.AnswerLength {
+                t.Errorf("Expected %v answers, got %v", tc.AnswerLength, n)
+			}
 
 			//if tc.AnswerLength > 0 {
 			//if r.Answer[0].(*dns.A).A.String() != tc.ExpectedIP {
@@ -243,6 +245,5 @@ func TestRenewal(t *testing.T) {
 			//t.Errorf("Expected 2 RRs in additional section, but got %d", n)
 			//}
 			//}
-		})
-	}
+		}) }
 }