simon/acme-dns
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Refactoring - improving coverage (#371)

Browse Source 
* Increase code coverage in acmedns

* More testing of ReadConfig() and its fallback mechanism

* Found that if someone put a '"' double quote into the filename that we configure zap to log to, it would cause the the JSON created to be invalid. I have replaced the JSON string with proper config

* Better handling of config options for api.TLS - we now error on an invalid value instead of silently failing.

added a basic test for api.setupTLS() (to increase test coverage)

* testing nameserver isOwnChallenge and isAuthoritative methods

* add a unit test for nameserver answerOwnChallenge

* fix linting errors

* bump go and golangci-lint versions in github actions

* Update golangci-lint.yml

Bumping github-actions workflow versions to accommodate some changes in upstream golanci-lint

* Bump Golang version to 1.23 (currently the oldest supported version)

Bump golanglint-ci to 2.0.2 and migrate the config file.

This should resolve the math/rand/v2 issue

* bump golanglint-ci action version

* Fixing up new golanglint-ci warnings and errors

---------

Co-authored-by: Joona Hoikkala <5235109+joohoi@users.noreply.github.com>
This commit is contained in:
Jason Playne
2025-05-06 22:20:59 +03:00
committed by GitHub
co-authored by Joona Hoikkala
parent d20fae37c9
commit e0f9745182
16 changed files with 511 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/acmedns/acmetxt.go
+2 -2
View File
@@ -6,7 +6,7 @@ import (
"github.com/google/uuid"
)
// Check if IP belongs to an allowed net
// AllowedFrom Check if IP belongs to an allowed net
func (a ACMETxt) AllowedFrom(ip string) bool {
remoteIP := net.ParseIP(ip)
// Range not limited
@@ -22,7 +22,7 @@ func (a ACMETxt) AllowedFrom(ip string) bool {
return false
}
// Go through list (most likely from headers) to check for the IP.
// AllowedFromList Go through list (most likely from headers) to check for the IP.
// Reason for this is that some setups use reverse proxy in front of acme-dns
func (a ACMETxt) AllowedFromList(ips []string) bool {
if len(ips) == 0 {
pkg/acmedns/config.go
+19 -5
View File
@@ -8,6 +8,13 @@ import (
"github.com/BurntSushi/toml"
)
const (
ApiTlsProviderNone = "none"
ApiTlsProviderLetsEncrypt = "letsencrypt"
ApiTlsProviderLetsEncryptStaging = "letsencryptstaging"
ApiTlsProviderCert = "cert"
)
func FileIsAccessible(fname string) bool {
_, err := os.Stat(fname)
if err != nil {
@@ -45,24 +52,31 @@ func prepareConfig(conf AcmeDnsConfig) (AcmeDnsConfig, error) {
conf.API.ACMECacheDir = "api-certs"
}
switch conf.API.TLS {
case ApiTlsProviderCert, ApiTlsProviderLetsEncrypt, ApiTlsProviderLetsEncryptStaging, ApiTlsProviderNone:
// we have a good value
default:
return conf, fmt.Errorf("invalid value for api.tls, expected one of [%s, %s, %s, %s]", ApiTlsProviderCert, ApiTlsProviderLetsEncrypt, ApiTlsProviderLetsEncryptStaging, ApiTlsProviderNone)
}
return conf, nil
}
func ReadConfig(configFile string) (AcmeDnsConfig, string, error) {
func ReadConfig(configFile, fallback string) (AcmeDnsConfig, string, error) {
var usedConfigFile string
var config AcmeDnsConfig
var err error
if FileIsAccessible(configFile) {
usedConfigFile = configFile
config, err = readTomlConfig(configFile)
} else if FileIsAccessible("./config.cfg") {
usedConfigFile = "./config.cfg"
config, err = readTomlConfig("./config.cfg")
} else if FileIsAccessible(fallback) {
usedConfigFile = fallback
config, err = readTomlConfig(fallback)
} else {
err = fmt.Errorf("configuration file not found")
}
if err != nil {
err = fmt.Errorf("encountered an error while trying to read configuration file: %s\n", err)
err = fmt.Errorf("encountered an error while trying to read configuration file: %w", err)
}
return config, usedConfigFile, err
}
pkg/acmedns/logging.go
+22 -19
View File
@@ -1,14 +1,18 @@
package acmedns
import (
"encoding/json"
"fmt"
"go.uber.org/zap/zapcore"
"go.uber.org/zap"
)
func SetupLogging(config AcmeDnsConfig) (*zap.Logger, error) {
var logger *zap.Logger
var (
logger *zap.Logger
zapCfg zap.Config
err error
)
logformat := "console"
if config.Logconfig.Format == "json" {
logformat = "json"
@@ -21,23 +25,22 @@ func SetupLogging(config AcmeDnsConfig) (*zap.Logger, error) {
if config.Logconfig.Logtype == "file" {
errorPath = config.Logconfig.File
}
zapConfigJson := fmt.Sprintf(`{
"level": "%s",
"encoding": "%s",
"outputPaths": ["%s"],
"errorOutputPaths": ["%s"],
"encoderConfig": {
"timeKey": "time",
"messageKey": "msg",
"levelKey": "level",
"levelEncoder": "lowercase",
"timeEncoder": "iso8601"
}
}`, config.Logconfig.Level, logformat, outputPath, errorPath)
var zapCfg zap.Config
if err := json.Unmarshal([]byte(zapConfigJson), &zapCfg); err != nil {
zapCfg.Level, err = zap.ParseAtomicLevel(config.Logconfig.Level)
if err != nil {
return logger, err
}
logger, err := zapCfg.Build()
zapCfg.Encoding = logformat
zapCfg.OutputPaths = []string{outputPath}
zapCfg.ErrorOutputPaths = []string{errorPath}
zapCfg.EncoderConfig = zapcore.EncoderConfig{
TimeKey: "time",
MessageKey: "msg",
LevelKey: "level",
EncodeLevel: zapcore.LowercaseLevelEncoder,
EncodeTime: zapcore.ISO8601TimeEncoder,
}
logger, err = zapCfg.Build()
return logger, err
}
pkg/acmedns/testdata/test_read_fallback_config.toml
Vendored
+36
View File
@@ -0,0 +1,36 @@
[general]
listen = "127.0.0.1:53"
protocol = "both"
domain = "test.example.org"
nsname = "test.example.org"
nsadmin = "test.example.org"
records = [
"test.example.org. A 127.0.0.1",
"test.example.org. NS test.example.org.",
]
debug = true
[database]
engine = "dinosaur"
connection = "roar"
[api]
ip = "0.0.0.0"
disable_registration = false
port = "443"
tls = "none"
tls_cert_privkey = "/etc/tls/example.org/privkey.pem"
tls_cert_fullchain = "/etc/tls/example.org/fullchain.pem"
acme_cache_dir = "api-certs"
notification_email = ""
corsorigins = [
"*"
]
use_header = true
header_name = "X-is-gonna-give-it-to-ya"
[logconfig]
loglevel = "info"
logtype = "stdout"
logfile = "./acme-dns.log"
logformat = "json"
pkg/acmedns/types.go
+1 -1
View File
@@ -8,7 +8,7 @@ type Account struct {
Subdomain string
}
// DNSConfig holds the config structure
// AcmeDnsConfig holds the config structure
type AcmeDnsConfig struct {
General general
Database dbsettings
pkg/acmedns/util_test.go
+142 -12
View File
@@ -1,7 +1,10 @@
package acmedns
import (
"fmt"
"math/rand/v2"
"os"
"reflect"
"syscall"
"testing"
@@ -46,19 +49,33 @@ func TestSetupLoggingError(t *testing.T) {
for _, test := range []struct {
format string
level string
file string
errexpected bool
}{
{"text", "warn", false},
{"json", "debug", false},
{"text", "info", false},
{"json", "error", false},
{"text", "something", true},
{"text", "warn", "", false},
{"json", "debug", "", false},
{"text", "info", "", false},
{"json", "error", "", false},
{"text", "something", "", true},
{"text", "info", "a path with\" in its name.txt", false},
} {
conf.Logconfig.Format = test.format
conf.Logconfig.Level = test.level
if test.file != "" {
conf.Logconfig.File = test.file
conf.Logconfig.Logtype = "file"
}
_, err := SetupLogging(conf)
if test.errexpected && err == nil {
t.Errorf("Expected error but did not get one for loglevel: %s", err)
} else if !test.errexpected && err != nil {
t.Errorf("Unexpected error: %s", err)
}
// clean up the file zap creates
if test.file != "" {
_ = os.Remove(test.file)
}
}
}
@@ -88,7 +105,7 @@ func TestReadConfig(t *testing.T) {
} {
tmpfile, err := os.CreateTemp("", "acmedns")
if err != nil {
t.Error("Could not create temporary file")
t.Fatalf("Could not create temporary file: %s", err)
}
defer os.Remove(tmpfile.Name())
@@ -99,7 +116,7 @@ func TestReadConfig(t *testing.T) {
if err := tmpfile.Close(); err != nil {
t.Error("Could not close temporary file")
}
ret, _, _ := ReadConfig(tmpfile.Name())
ret, _, _ := ReadConfig(tmpfile.Name(), "")
if ret.General.Listen != test.output.General.Listen {
t.Errorf("Test %d: Expected listen value %s, but got %s", i, test.output.General.Listen, ret.General.Listen)
}
@@ -109,10 +126,112 @@ func TestReadConfig(t *testing.T) {
}
}
func TestReadConfigFallback(t *testing.T) {
var (
path string
err error
)
testPath := "testdata/test_read_fallback_config.toml"
path, err = getNonExistentPath()
if err != nil {
t.Errorf("failed getting non existant path: %s", err)
}
cfg, used, err := ReadConfig(path, testPath)
if err != nil {
t.Fatalf("failed to read a config file when we should have: %s", err)
}
if used != testPath {
t.Fatalf("we read from the wrong file. got: %s, want: %s", used, testPath)
}
expected := AcmeDnsConfig{
General: general{
Listen: "127.0.0.1:53",
Proto: "both",
Domain: "test.example.org",
Nsname: "test.example.org",
Nsadmin: "test.example.org",
Debug: true,
StaticRecords: []string{
"test.example.org. A 127.0.0.1",
"test.example.org. NS test.example.org.",
},
},
Database: dbsettings{
Engine: "dinosaur",
Connection: "roar",
},
API: httpapi{
Domain: "",
IP: "0.0.0.0",
DisableRegistration: false,
AutocertPort: "",
Port: "443",
TLS: "none",
TLSCertPrivkey: "/etc/tls/example.org/privkey.pem",
TLSCertFullchain: "/etc/tls/example.org/fullchain.pem",
ACMECacheDir: "api-certs",
NotificationEmail: "",
CorsOrigins: []string{"*"},
UseHeader: true,
HeaderName: "X-is-gonna-give-it-to-ya",
},
Logconfig: logconfig{
Level: "info",
Logtype: "stdout",
File: "./acme-dns.log",
Format: "json",
},
}
if !reflect.DeepEqual(cfg, expected) {
t.Errorf("Did not read the config correctly: got %+v, want: %+v", cfg, expected)
}
}
func getNonExistentPath() (string, error) {
path := fmt.Sprintf("/some/path/that/should/not/exist/on/any/filesystem/%10d.cfg", rand.Int())
if _, err := os.Stat(path); os.IsNotExist(err) {
return path, nil
}
return "", fmt.Errorf("attempted non existant file exists!?: %s", path)
}
// TestReadConfigFallbackError makes sure we error when we do not have a fallback config file
func TestReadConfigFallbackError(t *testing.T) {
var (
badPaths []string
i int
)
for len(badPaths) < 2 && i < 10 {
i++
if path, err := getNonExistentPath(); err == nil {
badPaths = append(badPaths, path)
}
}
if len(badPaths) != 2 {
t.Fatalf("did not create exactly 2 bad paths")
}
_, _, err := ReadConfig(badPaths[0], badPaths[1])
if err == nil {
t.Errorf("Should have failed reading non existant file: %s", err)
}
}
func TestFileCheckPermissionDenied(t *testing.T) {
tmpfile, err := os.CreateTemp("", "acmedns")
if err != nil {
t.Error("Could not create temporary file")
t.Fatalf("Could not create temporary file: %s", err)
}
defer os.Remove(tmpfile.Name())
_ = syscall.Chmod(tmpfile.Name(), 0000)
@@ -131,7 +250,7 @@ func TestFileCheckNotExists(t *testing.T) {
func TestFileCheckOK(t *testing.T) {
tmpfile, err := os.CreateTemp("", "acmedns")
if err != nil {
t.Error("Could not create temporary file")
t.Fatalf("Could not create temporary file: %s", err)
}
defer os.Remove(tmpfile.Name())
if !FileIsAccessible(tmpfile.Name()) {
@@ -144,9 +263,20 @@ func TestPrepareConfig(t *testing.T) {
input AcmeDnsConfig
shoulderror bool
}{
{AcmeDnsConfig{Database: dbsettings{Engine: "whatever", Connection: "whatever_too"}}, false},
{AcmeDnsConfig{Database: dbsettings{Engine: "", Connection: "whatever_too"}}, true},
{AcmeDnsConfig{Database: dbsettings{Engine: "whatever", Connection: ""}}, true},
{AcmeDnsConfig{
Database: dbsettings{Engine: "whatever", Connection: "whatever_too"},
API: httpapi{TLS: ApiTlsProviderNone},
}, false},
{AcmeDnsConfig{Database: dbsettings{Engine: "", Connection: "whatever_too"},
API: httpapi{TLS: ApiTlsProviderNone},
}, true},
{AcmeDnsConfig{Database: dbsettings{Engine: "whatever", Connection: ""},
API: httpapi{TLS: ApiTlsProviderNone},
}, true},
{AcmeDnsConfig{
Database: dbsettings{Engine: "whatever", Connection: "whatever_too"},
API: httpapi{TLS: "whatever"},
}, true},
} {
_, err := prepareConfig(test.input)
if test.shoulderror {