Merge pull request #1257 from thaJeztah/remove_goutils

transport/ca/localca: New(): return error instead of calling os.Exit(1), remove github.com/kisom/goutils/assert

Author: nickysemenza

go.mod

@@ -11,7 +11,6 @@ require (
 	github.com/jmhodges/clock v1.2.0
 	github.com/jmoiron/sqlx v1.3.3
 	github.com/kisielk/sqlstruct v0.0.0-20201105191214-5f3e10d3ab46
-	github.com/kisom/goutils v1.4.3
 	github.com/lib/pq v1.10.1
 	github.com/mattn/go-sqlite3 v1.14.15
 	github.com/prometheus/client_golang v1.13.0

go.sum

@@ -214,8 +214,6 @@ github.com/kataras/sitemap v0.0.5/go.mod h1:KY2eugMKiPwsJgx7+U103YZehfvNGOXURubc
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
 github.com/kisielk/sqlstruct v0.0.0-20201105191214-5f3e10d3ab46 h1:veS9QfglfvqAw2e+eeNT/SbGySq8ajECXJ9e4fPoLhY=
 github.com/kisielk/sqlstruct v0.0.0-20201105191214-5f3e10d3ab46/go.mod h1:yyMNCyc/Ib3bDTKd379tNMpB/7/H5TjM2Y9QJ5THLbE=
-github.com/kisom/goutils v1.4.3 h1:N81mTXtO2LCpoqVtOrKthH5Abm0MknjX54QS8DmpQIk=
-github.com/kisom/goutils v1.4.3/go.mod h1:Lp5qrquG7yhYnWzZCI/68Pa/GpFynw//od6EkGnWpac=
 github.com/klauspost/compress v1.8.2/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A=
 github.com/klauspost/compress v1.9.7/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A=
 github.com/klauspost/cpuid v1.2.1/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek=

transport/ca/localca/signer.go

@@ -7,6 +7,7 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"errors"
+	"fmt"
 	"time"
 
 	"github.com/cloudflare/cfssl/config"
@@ -15,7 +16,6 @@ import (
 	"github.com/cloudflare/cfssl/initca"
 	"github.com/cloudflare/cfssl/signer"
 	"github.com/cloudflare/cfssl/signer/local"
-	"github.com/kisom/goutils/assert"
 )
 
 // CA is a local transport CertificateAuthority that is useful for
@@ -146,13 +146,19 @@ func New(req *csr.CertificateRequest, profiles *config.Signing) (*CA, error) {
 	// CFSSL has become inconsistent, and it can't be trusted.
 
 	priv, err := helpers.ParsePrivateKeyPEM(keyPEM)
-	assert.NoError(err, "CFSSL-generated private key can't be parsed")
+	if err != nil {
+		return nil, fmt.Errorf("CFSSL-generated private key can't be parsed: %w", err)
+	}
 
 	cert, err := helpers.ParseCertificatePEM(certPEM)
-	assert.NoError(err, "CFSSL-generated certificate can't be parsed")
+	if err != nil {
+		return nil, fmt.Errorf("CFSSL-generated private key can't be parsed: %w", err)
+	}
 
 	s, err := local.NewSigner(priv, cert, helpers.SignerAlgo(priv), profiles)
-	assert.NoError(err, "a signer could not be constructed")
+	if err != nil {
+		return nil, fmt.Errorf("a signer could not be constructed: %w", err)
+	}
 
 	return NewFromSigner(s), nil
 }

transport/ca/localca/signer_test.go

@@ -2,76 +2,16 @@ package localca
 
 import (
 	"encoding/pem"
-	"io/ioutil"
+	"errors"
 	"os"
+	"path/filepath"
 	"testing"
 
-	"github.com/cloudflare/cfssl/config"
 	"github.com/cloudflare/cfssl/csr"
 	"github.com/cloudflare/cfssl/helpers"
 	"github.com/cloudflare/cfssl/initca"
-	"github.com/cloudflare/cfssl/selfsign"
-	"github.com/kisom/goutils/assert"
 )
 
-func tempName() (string, error) {
-	tmpf, err := ioutil.TempFile("", "transport_cachedkp_")
-	if err != nil {
-		return "", err
-	}
-
-	name := tmpf.Name()
-	tmpf.Close()
-	return name, nil
-}
-
-func testGenerateKeypair(req *csr.CertificateRequest) (keyFile, certFile string, err error) {
-	fail := func(err error) (string, string, error) {
-		if keyFile != "" {
-			os.Remove(keyFile)
-		}
-		if certFile != "" {
-			os.Remove(certFile)
-		}
-		return "", "", err
-	}
-
-	keyFile, err = tempName()
-	if err != nil {
-		return fail(err)
-	}
-
-	certFile, err = tempName()
-	if err != nil {
-		return fail(err)
-	}
-
-	csrPEM, keyPEM, err := csr.ParseRequest(req)
-	if err != nil {
-		return fail(err)
-	}
-
-	if err = ioutil.WriteFile(keyFile, keyPEM, 0644); err != nil {
-		return fail(err)
-	}
-
-	priv, err := helpers.ParsePrivateKeyPEM(keyPEM)
-	if err != nil {
-		return fail(err)
-	}
-
-	cert, err := selfsign.Sign(priv, csrPEM, config.DefaultConfig())
-	if err != nil {
-		return fail(err)
-	}
-
-	if err = ioutil.WriteFile(certFile, cert, 0644); err != nil {
-		return fail(err)
-	}
-
-	return
-}
-
 func TestEncodePEM(t *testing.T) {
 	p := &pem.Block{
 		Type:  "CERTIFICATE REQUEST",
@@ -83,30 +23,38 @@ func TestEncodePEM(t *testing.T) {
 func TestLoadSigner(t *testing.T) {
 	lca := &CA{}
 	certPEM, csrPEM, keyPEM, err := initca.New(ExampleRequest())
-	assert.NoErrorT(t, err)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	_, err = lca.CACertificate()
-	assert.ErrorEqT(t, errNotSetup, err)
+	if !errors.Is(err, errNotSetup) {
+		t.Fatalf("expected an errNotSetup (%v), got: %v", errNotSetup, err)
+	}
 
 	_, err = lca.SignCSR(csrPEM)
-	assert.ErrorEqT(t, errNotSetup, err)
-
-	lca.KeyFile, err = tempName()
-	assert.NoErrorT(t, err)
-	defer os.Remove(lca.KeyFile)
+	if !errors.Is(err, errNotSetup) {
+		t.Fatalf("expected an errNotSetup (%v), got: %v", errNotSetup, err)
+	}
 
-	lca.CertFile, err = tempName()
-	assert.NoErrorT(t, err)
-	defer os.Remove(lca.CertFile)
+	tmpDir := t.TempDir()
+	lca.KeyFile = filepath.Join(tmpDir, "KeyFile")
+	lca.CertFile = filepath.Join(tmpDir, "CertFile")
 
-	err = ioutil.WriteFile(lca.KeyFile, keyPEM, 0644)
-	assert.NoErrorT(t, err)
+	err = os.WriteFile(lca.KeyFile, keyPEM, 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	err = ioutil.WriteFile(lca.CertFile, certPEM, 0644)
-	assert.NoErrorT(t, err)
+	err = os.WriteFile(lca.CertFile, certPEM, 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	err = Load(lca, ExampleSigningConfig())
-	assert.NoErrorT(t, err)
+	if err != nil {
+		t.Fatal(err)
+	}
 }
 
 var testRequest = &csr.CertificateRequest{
@@ -121,33 +69,50 @@ var testRequest = &csr.CertificateRequest{
 func TestNewSigner(t *testing.T) {
 	req := ExampleRequest()
 	lca, err := New(req, ExampleSigningConfig())
-	assert.NoErrorT(t, err)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	csrPEM, _, err := csr.ParseRequest(testRequest)
-	assert.NoErrorT(t, err)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	certPEM, err := lca.SignCSR(csrPEM)
-	assert.NoErrorT(t, err)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	_, err = helpers.ParseCertificatePEM(certPEM)
-	assert.NoErrorT(t, err)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	certPEM, err = lca.CACertificate()
-	assert.NoErrorT(t, err)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	cert, err := helpers.ParseCertificatePEM(certPEM)
-	assert.NoErrorT(t, err)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	assert.BoolT(t, cert.Subject.CommonName == req.CN,
-		"common names don't match")
+	if cert.Subject.CommonName != req.CN {
+		t.Fatalf("common names don't match: '%s' != '%s'", cert.Subject.CommonName, req.CN)
+	}
 
 	lca.Toggle()
 	_, err = lca.SignCSR(csrPEM)
-	assert.ErrorEqT(t, errDisabled, err)
+	if !errors.Is(err, errDisabled) {
+		t.Fatalf("expected an errDisabled (%v), got: %v", errDisabled, err)
+	}
 	lca.Toggle()
 
 	_, err = lca.SignCSR(certPEM)
-	assert.ErrorT(t, err, "shouldn't be able to sign non-CSRs")
+	if err == nil {
+		t.Fatal("shouldn't be able to sign non-CSRs")
+	}
 
 	p := &pem.Block{
 		Type:  "CERTIFICATE REQUEST",
@@ -156,6 +121,8 @@ func TestNewSigner(t *testing.T) {
 	junkCSR := pem.EncodeToMemory(p)
 
 	_, err = lca.SignCSR(junkCSR)
-	assert.ErrorT(t, err, "signing a junk CSR should fail")
+	if err == nil {
+		t.Fatal("signing a junk CSR should fail")
+	}
 	t.Logf("error: %s", err)
 }