From 56c46c6d5026755b8fc67fad298dc69e98562402 Mon Sep 17 00:00:00 2001
From: Drew Stinnett <drew@drewlink.com>
Date: Mon, 11 Oct 2021 08:50:43 -0400
Subject: [PATCH] feat: enforce packager defaults (#372)

* feat: apply defaults to required deb values

* feat: Adding default info to config documentation

* fix: cleanup comments

* refactor: implementing refactor from PR discussion

* fix: making deprecation warning more clear
---
 deb/deb.go                | 23 ++++++++++++++++++++++-
 deb/deb_test.go           | 15 +++++++++++++++
 www/docs/configuration.md |  7 ++++++-
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/deb/deb.go b/deb/deb.go
index c804968..3e1f70b 100644
--- a/deb/deb.go
+++ b/deb/deb.go
@@ -82,7 +82,7 @@ func (*Deb) ConventionalFileName(info *nfpm.Info) string {
 var ErrInvalidSignatureType = errors.New("invalid signature type")
 
 // Package writes a new deb package to the given writer using the given info.
-func (*Deb) Package(info *nfpm.Info, deb io.Writer) (err error) { // nolint: funlen
+func (d *Deb) Package(info *nfpm.Info, deb io.Writer) (err error) { // nolint: funlen
 	arch, ok := archToDebian[info.Arch]
 	if ok {
 		info.Arch = arch
@@ -91,6 +91,9 @@ func (*Deb) Package(info *nfpm.Info, deb io.Writer) (err error) { // nolint: fun
 		return err
 	}
 
+	// Set up some deb specific defaults
+	d.SetPackagerDefaults(info)
+
 	dataTarball, md5sums, instSize, dataTarballName, err := createDataTarball(info)
 	if err != nil {
 		return err
@@ -151,6 +154,24 @@ func (*Deb) Package(info *nfpm.Info, deb io.Writer) (err error) { // nolint: fun
 	return nil
 }
 
+func (*Deb) SetPackagerDefaults(info *nfpm.Info) {
+	// Priority should be set on all packages per:
+	//   https://www.debian.org/doc/debian-policy/ch-archive.html#priorities
+	// "optional" seems to be the safe/sane default here
+	if info.Priority == "" {
+		info.Priority = "optional"
+	}
+
+	// The safe thing here feels like defaulting to something like below.
+	// That will prevent existing configs from breaking anyway...  Wondering
+	// if in the long run we should be more strict about this and error when
+	// not set?
+	if info.Maintainer == "" {
+		log.Println("DEPRECATION WARNING: Leaving the 'maintainer' field unset will not be allowed in a future version")
+		info.Maintainer = "Unset Maintainer <unset@localhost>"
+	}
+}
+
 func addArFile(w *ar.Writer, name string, body []byte) error {
 	header := ar.Header{
 		Name:    files.ToNixPath(name),
diff --git a/deb/deb_test.go b/deb/deb_test.go
index 0db155d..8fec9ee 100644
--- a/deb/deb_test.go
+++ b/deb/deb_test.go
@@ -1071,3 +1071,18 @@ func extractFileFromAr(tb testing.TB, arFile []byte, filename string) []byte {
 
 	return nil
 }
+
+func TestEmptyButRequiredDebFields(t *testing.T) {
+	item := nfpm.WithDefaults(&nfpm.Info{
+		Name:    "foo",
+		Version: "v1.0.0",
+	})
+	Default.SetPackagerDefaults(item)
+
+	require.Equal(t, "optional", item.Priority)
+	require.Equal(t, "Unset Maintainer <unset@localhost>", item.Maintainer)
+
+	var deb bytes.Buffer
+	err := Default.Package(item, &deb)
+	require.NoError(t, err)
+}
diff --git a/www/docs/configuration.md b/www/docs/configuration.md
index 890928e..c621455 100644
--- a/www/docs/configuration.md
+++ b/www/docs/configuration.md
@@ -46,9 +46,14 @@ release: 1
 section: default
 
 # Priority.
+# Defaults to `optional` on deb
+# Defaults to empty on rpm and apk
 priority: extra
 
-# Maintaner.
+# Maintainer.
+# Defaults to empty on rpm and apk
+# Leaving this field empty on 'deb' packages is deprecated, and will be removed
+# in a future release
 maintainer: Carlos Alexandro Becker <root@carlosbecker.com>
 
 # Description.