fix: simplify code and improve test

Remove some paths from the code where you can shoot yourself in the
foot.
Improve tests to check for most cases.

pipeline/sign/sign.go

@@ -71,17 +71,7 @@ func signone(ctx *context.Context, artifact string) (string, error) {
 	env := map[string]string{
 		"artifact": artifact,
 	}
-
-	sig := expand(cfg.Signature, env)
-	if sig == "" {
-		return "", fmt.Errorf("sign: signature file cannot be empty")
-	}
-	if sig == artifact {
-		return "", fmt.Errorf("sign: artifact and signature cannot be the same")
-	}
-	env["signature"] = sig
-
-	// todo(fs): check if $out already exists
+	env["signature"] = expand(cfg.Signature, env)
 
 	var args []string
 	for _, a := range cfg.Args {
@@ -94,13 +84,10 @@ func signone(ctx *context.Context, artifact string) (string, error) {
 	// #nosec
 	cmd := exec.Command(cfg.Cmd, args...)
 	output, err := cmd.CombinedOutput()
-	if len(output) > 200 {
-		output = output[:200]
-	}
 	if err != nil {
 		return "", fmt.Errorf("sign: %s failed with %q", cfg.Cmd, string(output))
 	}
-	return sig, nil
+	return env["signature"], nil
 }
 
 func expand(s string, env map[string]string) string {

pipeline/sign/sign_test.go

@@ -2,11 +2,11 @@ package sign
 
 import (
 	"bytes"
-	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"sort"
 	"testing"
 
 	"github.com/goreleaser/goreleaser/config"
@@ -15,13 +15,80 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-const keyring = "testdata/gnupg"
-
 func TestDescription(t *testing.T) {
 	assert.NotEmpty(t, Pipe{}.String())
 }
 
-func TestSign(t *testing.T) {
+func TestSignDefault(t *testing.T) {
+	ctx := &context.Context{}
+	Pipe{}.Default(ctx)
+	assert.Equal(t, ctx.Config.Sign.Cmd, "gpg")
+	assert.Equal(t, ctx.Config.Sign.Signature, "${artifact}.sig")
+	assert.Equal(t, ctx.Config.Sign.Args, []string{"--output", "$signature", "--detach-sig", "$artifact"})
+	assert.Equal(t, ctx.Config.Sign.Artifacts, "none")
+}
+
+func TestSignDisabled(t *testing.T) {
+	ctx := &context.Context{}
+	ctx.Config.Sign.Artifacts = "none"
+	err := Pipe{}.Run(ctx)
+	assert.EqualError(t, err, "artifact signing disabled")
+}
+
+func TestSignInvalidArtifacts(t *testing.T) {
+	ctx := &context.Context{}
+	ctx.Config.Sign.Artifacts = "foo"
+	err := Pipe{}.Run(ctx)
+	assert.EqualError(t, err, "invalid list of artifacts to sign: foo")
+}
+
+func TestSignArtifacts(t *testing.T) {
+	// fix permission on keyring dir to suppress warning about insecure permissions
+	if err := os.Chmod(keyring, 0700); err != nil {
+		t.Fatal("Chmod: ", err)
+	}
+
+	tests := []struct {
+		desc       string
+		ctx        *context.Context
+		signatures []string
+		artifacts  []string
+	}{
+		{
+			desc: "sign all artifacts",
+			ctx: &context.Context{
+				Config: config.Project{
+					Sign: config.Sign{Artifacts: "all"},
+				},
+				Artifacts: []string{"artifact1", "artifact2", "checksum"},
+				Checksums: []string{"checksum"},
+			},
+			signatures: []string{"artifact1.sig", "artifact2.sig", "checksum.sig"},
+		},
+		{
+			desc: "sign only checksums",
+			ctx: &context.Context{
+				Config: config.Project{
+					Sign: config.Sign{Artifacts: "checksum"},
+				},
+				Artifacts: []string{"artifact1", "artifact2", "checksum"},
+				Checksums: []string{"checksum"},
+			},
+			signatures: []string{"checksum.sig"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			testSign(t, tt.ctx, tt.signatures)
+		})
+	}
+}
+
+const keyring = "testdata/gnupg"
+const user = "nopass"
+
+func testSign(t *testing.T, ctx *context.Context, signatures []string) {
 	// create temp dir for file and signature
 	tmpdir, err := ioutil.TempDir("", "goreleaser")
 	if err != nil {
@@ -29,68 +96,67 @@ func TestSign(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 
-	artifact := "foo.txt"
-	signature := artifact + ".sig"
+	ctx.Config.Dist = tmpdir
 
-	// create fake artifact
-	file := filepath.Join(tmpdir, artifact)
-	if err = ioutil.WriteFile(file, []byte("foo"), 0644); err != nil {
-		t.Fatal("WriteFile: ", err)
+	// create some fake artifacts
+	artifacts := ctx.Artifacts
+	for _, f := range artifacts {
+		file := filepath.Join(tmpdir, f)
+		if err := ioutil.WriteFile(file, []byte("foo"), 0644); err != nil {
+			t.Fatal("WriteFile: ", err)
+		}
 	}
 
-	// fix permission on keyring dir to suppress warning about insecure permissions
-	if err = os.Chmod(keyring, 0700); err != nil {
-		t.Fatal("Chmod: ", err)
-	}
-
-	// sign artifact
-	ctx := &context.Context{
-		Config: config.Project{
-			Dist: tmpdir,
-			Sign: config.Sign{
-				Artifacts: "all",
-			},
-		},
-	}
-	ctx.AddArtifact(artifact)
-
+	// configure the pipeline
+	// make sure we are using the test keyring
 	err = Pipe{}.Default(ctx)
 	if err != nil {
 		t.Fatal("Default: ", err)
 	}
-
-	// make sure we are using the test keyring
 	ctx.Config.Sign.Args = append([]string{"--homedir", keyring}, ctx.Config.Sign.Args...)
 
+	// run the pipeline
 	err = Pipe{}.Run(ctx)
 	if err != nil {
 		t.Fatal("Run: ", err)
 	}
 
-	// verify signature was made with key for usesr 'nopass'
-	if err := verifySig(t, keyring, file, filepath.Join(tmpdir, signature), "nopass"); err != nil {
-		t.Fatal("verify: ", err)
+	// verify that only the artifacts and the signatures are in the dist dir
+	files, err := ioutil.ReadDir(tmpdir)
+	if err != nil {
+		t.Fatal("ReadDir: ", err)
+	}
+	gotFiles := []string{}
+	for _, f := range files {
+		gotFiles = append(gotFiles, f.Name())
+	}
+
+	wantFiles := append(artifacts, signatures...)
+	sort.Strings(wantFiles)
+
+	assert.Equal(t, wantFiles, gotFiles)
+
+	// verify the signatures
+	for _, sig := range signatures {
+		artifact := sig[:len(sig)-len(".sig")]
+
+		// verify signature was made with key for usesr 'nopass'
+		cmd := exec.Command("gpg", "--homedir", keyring, "--verify", filepath.Join(ctx.Config.Dist, sig), filepath.Join(ctx.Config.Dist, artifact))
+		out, err := cmd.CombinedOutput()
+		if err != nil {
+			t.Log(string(out))
+			t.Fatal("verify: ", err)
+		}
+
+		// check if the signature matches the user we expect to do this properly we
+		// might need to have either separate keyrings or export the key from the
+		// keyring before we do the verification. For now we punt and look in the
+		// output.
+		if !bytes.Contains(out, []byte(user)) {
+			t.Fatalf("signature is not from %s", user)
+		}
 	}
 
 	// check signature is an artifact
-	assert.Equal(t, ctx.Artifacts, []string{artifact, signature})
-}
-
-func verifySig(t *testing.T, keyring, file, sig, user string) error {
-	cmd := exec.Command("gpg", "--homedir", keyring, "--verify", sig, file)
-	out, err := cmd.CombinedOutput()
-	if err != nil {
-		t.Log(string(out))
-		return err
-	}
-
-	// check if the signature matches the user we expect to do this properly we
-	// might need to have either separate keyrings or export the key from the
-	// keyring before we do the verification. For now we punt and look in the
-	// output.
-	if !bytes.Contains(out, []byte(user)) {
-		return fmt.Errorf("signature is not from %s", user)
-	}
-
-	return nil
+	assert.Equal(t, ctx.Artifacts, append(artifacts, signatures...))
 }