Repository: skupperproject/skupper (go)

Issue:
Poor Error handling in package internal/certs
**Describe the bug**
The `certs.GenerateSecret` function takes arguments for the properties of a TLS Certificate to generate, including a CA Secret, and returns just a resuling Secret containing the new TLS Certificate. It is not reasonable to expect this operation to always succeed, and we should return an error and handle it appropriately at the caller.

In many cases this function will `log.Fatal` - which will cause the controller to exit.

**Example**
* create a secret in a namesapce with a running skupper Site containing anything but a valid TLS credentials `kubectl create secret generic invalid-ca --from-literal=tls.crt=not --from-literal=tls.key=valid`
* create a v2alpha1.Certificate resource referencing that secret as a ca.
```
cat << EOF | kubectl apply -f -
apiVersion: skupper.io/v2alpha1
kind: Certificate
metadata:
  name: my-site-server
spec:
  ca: invalid-ca
  hosts:
  - skupper-router
  - skupper-router.default
  - 172.18.255.113
  server: true
  subject: skupper-router
EOF
```
* See the skupper controller go into a crash loop.

Pull request description:
Fixes #2118: Modified GenerateSecret() in certs.go to return an error…
… instead of log.Fatal() messages. This would ensure that the controller does not exit

Patch:
diff --git a/internal/certs/certs.go b/internal/certs/certs.go
index 26140212..6ab82947 100644
--- a/internal/certs/certs.go
+++ b/internal/certs/certs.go
@@ -21,7 +21,6 @@ import (
 	"crypto/x509/pkix"
 	"encoding/pem"
 	"fmt"
-	"log"
 	"math/big"
 	"net"
 	"strings"
@@ -57,31 +56,43 @@ type CertificateAuthority struct {
 
 type CertificateData map[string][]byte
 
-func decodeDataElement(in []byte, name string) []byte {
+func decodeDataElement(in []byte, name string) ([]byte, error) {
 	block, _ := pem.Decode(in)
 	if block == nil {
-		log.Fatal("failed to decode PEM block of type " + name)
+		return nil, fmt.Errorf("failed to read PEM encoded data from %q", name)
 	}
-	return block.Bytes
+	return block.Bytes, nil
 }
 
-func getCAFromSecret(secret *corev1.Secret) *CertificateAuthority {
+func getCAFromSecret(secret *corev1.Secret) (*CertificateAuthority, error) {
 	if secret == nil || secret.Data == nil {
-		return nil
+		return nil, nil
+	}
+
+	certBytes, err := decodeDataElement(secret.Data["tls.crt"], "tls.crt")
+	if err != nil {
+		return nil, err
+	}
+
+	cert, err := x509.ParseCertificate(certBytes)
+	if err != nil {
+		return nil, err
 	}
-	cert, err := x509.ParseCertificate(decodeDataElement(secret.Data["tls.crt"], "certificate"))
+
+	privateKeyBytes, err := decodeDataElement(secret.Data["tls.key"], "tls.key")
 	if err != nil {
-		log.Fatal("failed to get CA certificate from secret")
+		return nil, err
 	}
-	key, err := x509.ParsePKCS1PrivateKey(decodeDataElement(secret.Data["tls.key"], "private key"))
+	key, err := x509.ParsePKCS1PrivateKey(privateKeyBytes)
 	if err != nil {
-		log.Fatal("failed to get CA private key from secret", err)
+		return nil, fmt.Errorf("failed to get CA private key from secret %s", err)
 	}
+
 	return &CertificateAuthority{
 		Certificate: cert,
 		Key:         key,
 		CrtData:     secret.Data["tls.crt"],
-	}
+	}, nil
 }
 
 // GenerateSecret generates a kubernetes secret.
@@ -90,12 +101,17 @@ func getCAFromSecret(secret *corev1.Secret) *CertificateAuthority {
 // hosts are the host names in the x509 certificate. Comma separated if more than one hostname
 // expiration is when the secret expires, if zero is passed in, the expiration is set to 5 years from now
 // ca is the certificate authority, if nil a ca cert will be created.
-func GenerateSecret(name string, subject string, hosts string, expiration time.Duration, ca *corev1.Secret) corev1.Secret {
-	caCert := getCAFromSecret(ca)
+func GenerateSecret(name string, subject string, hosts string, expiration time.Duration, ca *corev1.Secret) (*corev1.Secret, error) {
+	caCert, err := getCAFromSecret(ca)
+
+	if err != nil {
+		return nil, fmt.Errorf("error reading CA Certificate from Secret %q: %s", ca.Name, err)
+	}
 
 	priv, err := rsa.GenerateKey(rand.Reader, 2048)
 	if err != nil {
-		log.Fatalf("failed to generate private key: %s", err)
+		return nil, fmt.Errorf("failed to generate private key: %v", err)
+
 	}
 
 	notBefore := time.Now()
@@ -107,7 +123,7 @@ func GenerateSecret(name string, subject string, hosts string, expiration time.D
 	serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)
 	serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
 	if err != nil {
-		log.Fatalf("failed to generate serial number: %s", err)
+		return nil, err
 	}
 
 	template := x509.Certificate{
@@ -147,7 +163,7 @@ func GenerateSecret(name string, subject string, hosts string, expiration time.D
 
 	derBytes, err := x509.CreateCertificate(rand.Reader, &template, parent, publicKey(priv), cakey)
 	if err != nil {
-		log.Fatalf("Failed to create certificate: %s", err)
+		return nil, err
 	}
 
 	secret := corev1.Secret{
@@ -173,7 +189,7 @@ func GenerateSecret(name string, subject string, hosts string, expiration time.D
 		secret.Data["ca.crt"] = secret.Data["tls.crt"] //self.signed
 	}
 
-	return secret
+	return &secret, nil
 }
 
 func DecodeCertificate(data []byte) (*x509.Certificate, error) {
diff --git a/internal/kube/certificates/mgr.go b/internal/kube/certificates/mgr.go
index 503ebe43..9d0d046b 100644
--- a/internal/kube/certificates/mgr.go
+++ b/internal/kube/certificates/mgr.go
@@ -312,9 +312,13 @@ func (m *CertificateManagerImpl) updateSecret(key string, certificate *skupperv2
 }
 
 func (m *CertificateManagerImpl) generateSecret(certificate *skupperv2alpha1.Certificate) (*corev1.Secret, error) {
-	var secret corev1.Secret
+	var secret *corev1.Secret
+	var err error
 	if certificate.Spec.Signing {
-		secret = certs.GenerateSecret(certificate.Name, certificate.Spec.Subject, "", 0, nil)
+		secret, err = certs.GenerateSecret(certificate.Name, certificate.Spec.Subject, "", 0, nil)
+		if err != nil {
+			return secret, err
+		}
 	} else {
 		expiration := time.Hour * 24 * 365 * 5 // TODO: make this configurable (through controller setting or field on certificate?)
 		caKey := fmt.Sprintf("%s/%s", certificate.Namespace, certificate.Spec.Ca)
@@ -324,10 +328,13 @@ func (m *CertificateManagerImpl) generateSecret(certificate *skupperv2alpha1.Cer
 			return nil, fmt.Errorf("CA %q not found", caKey)
 		}
 		// TODO: handle server and client roles properly
-		secret = certs.GenerateSecret(certificate.Name, certificate.Spec.Subject, strings.Join(certificate.Spec.Hosts, ","), expiration, ca)
+		secret, err = certs.GenerateSecret(certificate.Name, certificate.Spec.Subject, strings.Join(certificate.Spec.Hosts, ","), expiration, ca)
+		if err != nil {
+			return nil, err
+		}
 	}
 	secret.ObjectMeta.OwnerReferences = ownerReferences(certificate)
-	return &secret, nil
+	return secret, nil
 }
 
 func (m *CertificateManagerImpl) createSecret(key string, certificate *skupperv2alpha1.Certificate) error {
diff --git a/internal/kube/controller/controller.go b/internal/kube/controller/controller.go
index 6c560601..ce06283b 100644
--- a/internal/kube/controller/controller.go
+++ b/internal/kube/controller/controller.go
@@ -414,7 +414,10 @@ func (c *Controller) generateLinkConfig(namespace string, name string, subject s
 	if err != nil {
 		return err
 	}
-	token := generator.NewCertToken(name, subject)
+	token, err := generator.NewCertToken(name, subject)
+	if err != nil {
+		return err
+	}
 	return token.Write(writer)
 }
 
diff --git a/internal/kube/grants/tokens.go b/internal/kube/grants/tokens.go
index f003616a..c668b9a5 100644
--- a/internal/kube/grants/tokens.go
+++ b/internal/kube/grants/tokens.go
@@ -88,10 +88,13 @@ func (g *TokenGenerator) setValidHostsFromSite(site *skupperv2alpha1.Site) bool
 	return true
 }
 
-func (g *TokenGenerator) NewCertToken(name string, subject string) Token {
-	cert := certs.GenerateSecret(name, subject, strings.Join(g.hosts, ","), 0, g.ca)
+func (g *TokenGenerator) NewCertToken(name string, subject string) (Token, error) {
+	cert, err := certs.GenerateSecret(name, subject, strings.Join(g.hosts, ","), 0, g.ca)
+	if err != nil {
+		return nil, err
+	}
 	token := &CertToken{
-		tlsCredentials: &cert,
+		tlsCredentials: cert,
 	}
 	for i, endpoints := range g.endpoints {
 		linkName := name
@@ -113,7 +116,7 @@ func (g *TokenGenerator) NewCertToken(name string, subject string) Token {
 		}
 		token.links = append(token.links, link)
 	}
-	return token
+	return token, nil
 }
 
 func (t *CertToken) Write(writer io.Writer) error {
diff --git a/internal/nonkube/common/fs_config_renderer.go b/internal/nonkube/common/fs_config_renderer.go
index afaf0448..0d3a425e 100644
--- a/internal/nonkube/common/fs_config_renderer.go
+++ b/internal/nonkube/common/fs_config_renderer.go
@@ -282,7 +282,7 @@ func (c *FileSystemConfigurationRenderer) createTokens(siteState *api.SiteState)
 		if err != nil {
 			return fmt.Errorf("unable to load client secret %s: %v", secretName, err)
 		}
-		routerTokens := api.CreateTokens(*linkAccess, *serverSecret, *secret)
+		routerTokens := api.CreateTokens(*linkAccess, serverSecret, *secret)
 		// routerAccess is valid (inter-router and edge endpoints defined)
 		if len(routerTokens) > 0 {
 			tokens = append(tokens, routerTokens...)
@@ -371,18 +371,21 @@ func (c *FileSystemConfigurationRenderer) createTlsCertificates(siteState *api.S
 		if certificate.Spec.Signing == false {
 			continue
 		}
-		secret := certs.GenerateSecret(name, certificate.Spec.Subject, "", 0, nil)
+		secret, err := certs.GenerateSecret(name, certificate.Spec.Subject, "", 0, nil)
+		if err != nil {
+			return err
+		}
 
 		ignoreExisting := true
 		userCaSecret, err := c.loadUserCertAsSecret(siteState, "ca", name)
 		if userCaSecret != nil && err == nil {
 			// override with user provided CA
 			ignoreExisting = false
-			secret = *userCaSecret
+			secret = userCaSecret
 			fmt.Printf("-> User provided CA found: %s\n", name)
 		}
 		caPath := path.Join(outputPath, string(api.IssuersPath), name)
-		err = writeSecretFilesIgnore(caPath, &secret, ignoreExisting)
+		err = writeSecretFilesIgnore(caPath, secret, ignoreExisting)
 		if err != nil {
 			return err
 		}
@@ -390,7 +393,7 @@ func (c *FileSystemConfigurationRenderer) createTlsCertificates(siteState *api.S
 	// generate all other certificates now
 	for name, certificate := range siteState.Certificates {
 		var purpose string
-		var secret corev1.Secret
+		var secret *corev1.Secret
 		var caSecret *corev1.Secret
 		if certificate.Spec.Ca != "" {
 			caSecret, err = c.loadCASecret(siteState, certificate.Spec.Ca)
@@ -400,25 +403,31 @@ func (c *FileSystemConfigurationRenderer) createTlsCertificates(siteState *api.S
 		}
 		if certificate.Spec.Client {
 			purpose = "client"
-			secret = certs.GenerateSecret(name, certificate.Spec.Subject, strings.Join(certificate.Spec.Hosts, ","), 0, caSecret)
+			secret, err = certs.GenerateSecret(name, certificate.Spec.Subject, strings.Join(certificate.Spec.Hosts, ","), 0, caSecret)
+			if err != nil {
+				return err
+			}
 			// TODO Not sure if connect.json is needed (probably need to get rid of it)
 			if connectJson := c.connectJson(siteState); connectJson != nil {
 				secret.Data["connect.json"] = []byte(*connectJson)
 			}
 		} else if certificate.Spec.Server {
 			purpose = "server"
-			secret = certs.GenerateSecret(name, certificate.Spec.Subject, strings.Join(certificate.Spec.Hosts, ","), 0, caSecret)
+			secret, err = certs.GenerateSecret(name, certificate.Spec.Subject, strings.Join(certificate.Spec.Hosts, ","), 0, caSecret)
+			if err != nil {
+				return err
+			}
 		} else {
 			continue
 		}
 		userSecret, err := c.loadUserCertAsSecret(siteState, purpose, name)
 		if userSecret != nil && err == nil {
 			// override with user provided secret
-			secret = *userSecret
+			secret = userSecret
 			fmt.Printf("-> User provided %s certificate found: %s\n", purpose, name)
 		}
 		certPath := path.Join(outputPath, string(api.CertificatesPath), name)
-		err = writeSecretFiles(certPath, &secret)
+		err = writeSecretFiles(certPath, secret)
 		if err != nil {
 			return err
 		}
diff --git a/pkg/nonkube/api/token.go b/pkg/nonkube/api/token.go
index f6791152..36dd206f 100644
--- a/pkg/nonkube/api/token.go
+++ b/pkg/nonkube/api/token.go
@@ -11,7 +11,7 @@ import (
 
 	"github.com/skupperproject/skupper/internal/utils"
 	"github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/serializer/json"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -44,7 +44,7 @@ func (t *Token) Marshal() ([]byte, error) {
 	return buffer.Bytes(), nil
 }
 
-func CreateTokens(routerAccess v2alpha1.RouterAccess, serverSecret v1.Secret, clientSecret v1.Secret) []*Token {
+func CreateTokens(routerAccess v2alpha1.RouterAccess, serverSecret *v1.Secret, clientSecret v1.Secret) []*Token {
 	var tokens []*Token
 	interRouter := 0
 	edge := 0
@@ -106,19 +106,22 @@ func CreateTokens(routerAccess v2alpha1.RouterAccess, serverSecret v1.Secret, cl
 	var hosts []string
 	hosts = append(hosts, utils.DefaultStr(routerAccess.Spec.BindHost, "127.0.0.1"))
 	// reading SANs from server certificate
-	serverCertificateData := serverSecret.Data["tls.crt"]
-	serverCertificateBlk, _ := pem.Decode(serverCertificateData)
-	if serverCertificateBlk != nil {
-		serverCertificate, err := x509.ParseCertificate(serverCertificateBlk.Bytes)
-		if err == nil {
-			for _, ipAddr := range serverCertificate.IPAddresses {
-				if ipAddr.String() != "" && !slices.Contains(hosts, ipAddr.String()) {
-					hosts = append(hosts, ipAddr.String())
+
+	if serverSecret != nil {
+		serverCertificateData := serverSecret.Data["tls.crt"]
+		serverCertificateBlk, _ := pem.Decode(serverCertificateData)
+		if serverCertificateBlk != nil {
+			serverCertificate, err := x509.ParseCertificate(serverCertificateBlk.Bytes)
+			if err == nil {
+				for _, ipAddr := range serverCertificate.IPAddresses {
+					if ipAddr.String() != "" && !slices.Contains(hosts, ipAddr.String()) {
+						hosts = append(hosts, ipAddr.String())
+					}
 				}
-			}
-			for _, dnsName := range serverCertificate.DNSNames {
-				if dnsName != "" && !slices.Contains(hosts, dnsName) {
-					hosts = append(hosts, dnsName)
+				for _, dnsName := range serverCertificate.DNSNames {
+					if dnsName != "" && !slices.Contains(hosts, dnsName) {
+						hosts = append(hosts, dnsName)
+					}
 				}
 			}
 		}


Test patch:
diff --git a/internal/certs/certs_test.go b/internal/certs/certs_test.go
index b51eab22..c6925f20 100644
--- a/internal/certs/certs_test.go
+++ b/internal/certs/certs_test.go
@@ -18,6 +18,8 @@ import (
 	"testing"
 
 	"gotest.tools/v3/assert"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestGenerateCASecret(t *testing.T) {
@@ -26,7 +28,10 @@ func TestGenerateCASecret(t *testing.T) {
 	name := "ca-secret"
 	host := host1 + ", " + host2
 	cn := "www.example.com"
-	ca_secret := GenerateSecret(name, cn, host, 0, nil)
+	ca_secret, err := GenerateSecret(name, cn, host, 0, nil)
+	if err != nil {
+		t.Error(err)
+	}
 	data, ok := ca_secret.Data["tls.crt"]
 	if !ok {
 		t.Error("Invalid secret, tls.crt is missing")
@@ -46,10 +51,16 @@ func TestGenerateCASecret(t *testing.T) {
 
 func TestGenerateSecret(t *testing.T) {
 	ca_cn := "www.example.com"
-	ca_secret := GenerateSecret("test-secret", ca_cn, "134.565.56.77", 0, nil)
+	ca_secret, err := GenerateSecret("test-secret", ca_cn, "134.565.56.77", 0, nil)
+	if err != nil {
+		t.Error(err)
+	}
 	my_secret_cn := "www.my.example.com"
 	my_secret_host := "172.565.56.77"
-	my_secret := GenerateSecret("my_secret", my_secret_cn, my_secret_host, 86400000000000 /*duration of 1 day*/, &ca_secret)
+	my_secret, err := GenerateSecret("my_secret", my_secret_cn, my_secret_host, 86400000000000 /*duration of 1 day*/, ca_secret)
+	if err != nil {
+		t.Error(err)
+	}
 	data, ok := my_secret.Data["tls.crt"]
 	if !ok {
 		t.Error("Invalid secret, tls.crt is missing")
@@ -61,4 +72,20 @@ func TestGenerateSecret(t *testing.T) {
 
 	assert.Equal(t, my_secret_cn, my_cert.Subject.CommonName)
 	assert.Equal(t, ca_cn, my_cert.Issuer.CommonName)
+
+	// Instantiate the Secret object.
+	caSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "emptyCASecret",
+			Namespace: "testNamespace",
+		},
+		//  To create a 0-byte data field, initialize the map as empty.
+		//  The type is map[string][]byte.
+		Data: map[string][]byte{},
+		Type: corev1.SecretTypeOpaque,
+	}
+
+	_, err = GenerateSecret("test-secret", ca_cn, "134.565.56.77", 0, caSecret)
+	errorText := err.Error()
+	assert.Equal(t, errorText, "error reading CA Certificate from Secret \"emptyCASecret\": failed to read PEM encoded data from \"tls.crt\"")
 }
diff --git a/internal/kube/certificates/mgr_test.go b/internal/kube/certificates/mgr_test.go
index c588cbd7..e13417cf 100644
--- a/internal/kube/certificates/mgr_test.go
+++ b/internal/kube/certificates/mgr_test.go
@@ -520,7 +520,10 @@ func (c *Call) eventcount(events int) *Call {
 
 func fixtureCASecret(t *testing.T, name, namespace string) *corev1.Secret {
 	t.Helper()
-	secret := certs.GenerateSecret(name, "skupper test CA", "", time.Hour*8, nil)
+	secret, err := certs.GenerateSecret(name, "skupper test CA", "", time.Hour*8, nil)
+	if err != nil {
+		t.Error(err)
+	}
 	secret.Namespace = namespace
-	return &secret
+	return secret
 }
diff --git a/internal/kube/grants/common_test.go b/internal/kube/grants/common_test.go
index 5941350e..b635d9d6 100644
--- a/internal/kube/grants/common_test.go
+++ b/internal/kube/grants/common_test.go
@@ -132,10 +132,13 @@ func (*factory) grant(name string, namespace string, uid string) *v2alpha1.Acces
 
 }
 
-func (*factory) secret(name string, namespace string, subject string, hosts []string) *corev1.Secret {
-	secret := certs.GenerateSecret(name, subject, strings.Join(hosts, ","), 0, nil)
+func (*factory) secret(name string, namespace string, subject string, hosts []string) (*corev1.Secret, error) {
+	secret, err := certs.GenerateSecret(name, subject, strings.Join(hosts, ","), 0, nil)
+	if err != nil {
+		return nil, err
+	}
 	secret.ObjectMeta.Namespace = namespace
-	return &secret
+	return secret, nil
 }
 
 func (*factory) genericSecret(name string, namespace string) *corev1.Secret {
diff --git a/internal/kube/grants/enabled_test.go b/internal/kube/grants/enabled_test.go
index 85a950d7..a8f15458 100644
--- a/internal/kube/grants/enabled_test.go
+++ b/internal/kube/grants/enabled_test.go
@@ -7,6 +7,11 @@ import (
 )
 
 func Test_tlsCredentialsUpdated(t *testing.T) {
+	secret, err := tf.secret("simple", "test", "My Subject", []string{"foo.com", "bar.org"})
+	if err != nil {
+		t.Error(err)
+	}
+
 	var tests = []struct {
 		name   string
 		key    string
@@ -15,7 +20,7 @@ func Test_tlsCredentialsUpdated(t *testing.T) {
 		{
 			name:   "simple",
 			key:    "test/simple",
-			secret: tf.secret("simple", "test", "My Subject", []string{"foo.com", "bar.org"}),
+			secret: secret,
 		},
 		{
 			name:   "non tls secret",
diff --git a/internal/kube/grants/init_test.go b/internal/kube/grants/init_test.go
index b899a969..27c45e61 100644
--- a/internal/kube/grants/init_test.go
+++ b/internal/kube/grants/init_test.go
@@ -15,6 +15,14 @@ import (
 
 func Test_Initialise(t *testing.T) {
 	grantUid := "0bde3bc8-a4a2-404a-bfbe-44fdf7bf3231"
+	myCredsSecret, err := tf.secret("my-creds", "test", "grant server", nil)
+	if err != nil {
+		t.Error(err)
+	}
+	grantSecret, err := tf.secret("skupper-grant-server", "test", "grant server", nil)
+	if err != nil {
+		t.Error(err)
+	}
 	var tests = []struct {
 		name           string
 		config         GrantConfig
@@ -35,7 +43,7 @@ func Test_Initialise(t *testing.T) {
 				BaseUrl:              "foo:5432",
 				TlsCredentialsSecret: "my-creds",
 			},
-			k8sObjects:     []runtime.Object{tf.secret("my-creds", "test", "grant server", nil)},
+			k8sObjects:     []runtime.Object{myCredsSecret},
 			expectedStatus: "OK",
 			expectedUrl:    "https://foo:5432/" + grantUid,
 		},
@@ -48,7 +56,7 @@ func Test_Initialise(t *testing.T) {
 				TlsCredentialsSecret: "skupper-grant-server",
 			},
 			//add pregenerated secret as the certificate controller is not in action in this test
-			k8sObjects: []runtime.Object{tf.pod("my-pod", "test", map[string]string{"foo": "bar"}, ref1), tf.secret("skupper-grant-server", "test", "grant server", nil)},
+			k8sObjects: []runtime.Object{tf.pod("my-pod", "test", map[string]string{"foo": "bar"}, ref1), grantSecret},
 			endpoint: &v2alpha1.Endpoint{
 				Host: "my-host",
 				Port: "1234",
@@ -65,7 +73,7 @@ func Test_Initialise(t *testing.T) {
 				TlsCredentialsSecret: "skupper-grant-server",
 			},
 			//add pregenerated secret as the certificate controller is not in action in this test
-			k8sObjects:     []runtime.Object{tf.secret("skupper-grant-server", "test", "grant server", nil)},
+			k8sObjects:     []runtime.Object{grantSecret},
 			expectedStatus: "Pending",
 		},
 	}
diff --git a/internal/kube/grants/redeem_test.go b/internal/kube/grants/redeem_test.go
index e0a9d88a..f83d921e 100644
--- a/internal/kube/grants/redeem_test.go
+++ b/internal/kube/grants/redeem_test.go
@@ -100,6 +100,14 @@ func (t *TestTripper) RoundTrip(request *http.Request) (*http.Response, error) {
 }
 
 func Test_handleTokenResponse(t *testing.T) {
+	myToken, err := tf.secret("my-token", "", "My Subject", nil)
+	if err != nil {
+		t.Error(err)
+	}
+	myToken1, err := tf.secret("my-token", "test", "Another Subject", nil)
+	if err != nil {
+		t.Error(err)
+	}
 	var tests = []struct {
 		name                 string
 		token                *v2alpha1.AccessToken
@@ -120,7 +128,7 @@ func Test_handleTokenResponse(t *testing.T) {
 			token: tf.token("my-token", "test", "http://foo/xyz", "mycode", ""),
 			site:  tf.site("my-site", "test"),
 			body: &CertToken{
-				tlsCredentials: tf.secret("my-token", "", "My Subject", nil),
+				tlsCredentials: myToken,
 				links: []*v2alpha1.Link{
 					tf.link("my-token", "", []v2alpha1.Endpoint{
 						{
@@ -145,7 +153,7 @@ func Test_handleTokenResponse(t *testing.T) {
 			token: tf.token("my-token", "test", "http://foo/xyz", "mycode", ""),
 			site:  tf.site("my-site", "test"),
 			body: &CertToken{
-				tlsCredentials: tf.secret("my-token", "", "My Subject", nil),
+				tlsCredentials: myToken,
 				links: []*v2alpha1.Link{
 					tf.link("my-token", "", []v2alpha1.Endpoint{
 						{
@@ -163,7 +171,7 @@ func Test_handleTokenResponse(t *testing.T) {
 			token: tf.token("my-token", "test", "http://foo/xyz", "mycode", ""),
 			site:  tf.site("my-site", "test"),
 			body: &CertToken{
-				tlsCredentials: tf.secret("my-token", "", "My Subject", nil),
+				tlsCredentials: myToken,
 				links: []*v2alpha1.Link{
 					tf.link("my-token", "", []v2alpha1.Endpoint{
 						{
@@ -174,14 +182,14 @@ func Test_handleTokenResponse(t *testing.T) {
 				},
 			},
 			expectedStatus:  "Controller could not create received secret: secrets \"my-token\" already exists",
-			extraK8sObjects: []runtime.Object{tf.secret("my-token", "test", "Another Subject", nil)},
+			extraK8sObjects: []runtime.Object{myToken1},
 		},
 		{
 			name:  "link collision",
 			token: tf.token("my-token", "test", "http://foo/xyz", "mycode", ""),
 			site:  tf.site("my-site", "test"),
 			body: &CertToken{
-				tlsCredentials: tf.secret("my-token", "", "My Subject", nil),
+				tlsCredentials: myToken,
 				links: []*v2alpha1.Link{
 					tf.link("my-token", "", []v2alpha1.Endpoint{
 						{
@@ -199,7 +207,7 @@ func Test_handleTokenResponse(t *testing.T) {
 			token: tf.addLinkCost(tf.token("my-token", "test", "http://foo/xyz", "mycode", ""), 10),
 			site:  tf.site("my-site", "test"),
 			body: &CertToken{
-				tlsCredentials: tf.secret("my-token", "", "My Subject", nil),
+				tlsCredentials: myToken,
 				links: []*v2alpha1.Link{
 					tf.link("my-token", "", []v2alpha1.Endpoint{
 						{
@@ -384,7 +392,11 @@ func Test_RedeemAccessToken(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			client, err := fake.NewFakeClient("test", []runtime.Object{tf.secret("skupper-site-ca", "test", "Test Site CA", nil)}, []runtime.Object{tf.site("my-site", "test"), tf.grant("my-grant", "test", "")}, "")
+			siteCa, err := tf.secret("skupper-site-ca", "test", "Test Site CA", nil)
+			if err != nil {
+				t.Error(err)
+			}
+			client, err := fake.NewFakeClient("test", []runtime.Object{siteCa}, []runtime.Object{tf.site("my-site", "test"), tf.grant("my-grant", "test", "")}, "")
 			site, err := client.GetSkupperClient().SkupperV2alpha1().Sites("test").Get(context.TODO(), "my-site", metav1.GetOptions{})
 			if err != nil {
 				t.Error(err)
@@ -404,10 +416,14 @@ func Test_RedeemAccessToken(t *testing.T) {
 			server := newServer(":0", tt.scheme == "https", grants)
 			server.listen()
 			grants.setUrl(fmt.Sprintf("localhost:%d", server.port()))
+			myCreds, err := tf.secret("my-creds", "test", "grant server", []string{"localhost"})
+			if err != nil {
+				t.Error(err)
+			}
 			go server.serve()
 			defer server.stop()
 			if tt.scheme == "https" {
-				secret := tf.secret("my-creds", "test", "grant server", []string{"localhost"})
+				secret := myCreds
 				err = server.setCertificateFromSecret(secret)
 				if err != nil {
 					t.Error(err)
@@ -478,7 +494,10 @@ func (g *TestTokenGenerator) generate(namespace string, name string, subject str
 	if err != nil {
 		return err
 	}
-	token := generator.NewCertToken(name, subject)
+	token, err := generator.NewCertToken(name, subject)
+	if err != nil {
+		return err
+	}
 	return token.Write(writer)
 }
 
diff --git a/internal/kube/grants/server_test.go b/internal/kube/grants/server_test.go
index f09d9a39..337d2d5f 100644
--- a/internal/kube/grants/server_test.go
+++ b/internal/kube/grants/server_test.go
@@ -11,6 +11,10 @@ import (
 )
 
 func Test_setCertificateFromSecret(t *testing.T) {
+	simpleSecret, err := tf.secret("simple", "", "My Subject", []string{"foo.com", "bar.org"})
+	if err != nil {
+		t.Error(err)
+	}
 	var tests = []struct {
 		name             string
 		secret           *corev1.Secret
@@ -20,7 +24,7 @@ func Test_setCertificateFromSecret(t *testing.T) {
 	}{
 		{
 			name:             "simple",
-			secret:           tf.secret("simple", "", "My Subject", []string{"foo.com", "bar.org"}),
+			secret:           simpleSecret,
 			expectedSubject:  "My Subject",
 			expectedDNSNames: []string{"foo.com", "bar.org"},
 		},
diff --git a/internal/kube/grants/tokens_test.go b/internal/kube/grants/tokens_test.go
index 31fd2399..cf8fc8a1 100644
--- a/internal/kube/grants/tokens_test.go
+++ b/internal/kube/grants/tokens_test.go
@@ -11,6 +11,10 @@ import (
 )
 
 func Test_CertTokenWrite(t *testing.T) {
+	mySecret, err := tf.secret("my-token", "", "My Subject", nil)
+	if err != nil {
+		t.Error(err)
+	}
 	var tests = []struct {
 		name          string
 		cert          *CertToken
@@ -20,7 +24,7 @@ func Test_CertTokenWrite(t *testing.T) {
 		{
 			name: "good",
 			cert: &CertToken{
-				tlsCredentials: tf.secret("my-token", "", "My Subject", nil),
+				tlsCredentials: mySecret,
 				links: []*v2alpha1.Link{
 					tf.link("my-token", "", []v2alpha1.Endpoint{
 						{
@@ -34,7 +38,7 @@ func Test_CertTokenWrite(t *testing.T) {
 		{
 			name: "bad",
 			cert: &CertToken{
-				tlsCredentials: tf.secret("my-token", "", "My Subject", nil),
+				tlsCredentials: mySecret,
 				links: []*v2alpha1.Link{
 					tf.link("my-token", "", []v2alpha1.Endpoint{
 						{
@@ -50,7 +54,7 @@ func Test_CertTokenWrite(t *testing.T) {
 		{
 			name: "later bad",
 			cert: &CertToken{
-				tlsCredentials: tf.secret("my-token", "", "My Subject", nil),
+				tlsCredentials: mySecret,
 				links: []*v2alpha1.Link{
 					tf.link("my-token", "", []v2alpha1.Endpoint{
 						{
diff --git a/internal/nonkube/client/runtime/certs_test.go b/internal/nonkube/client/runtime/certs_test.go
index 82b2564b..38322a02 100644
--- a/internal/nonkube/client/runtime/certs_test.go
+++ b/internal/nonkube/client/runtime/certs_test.go
@@ -38,7 +38,10 @@ func TestTlsConfigRetriever(t *testing.T) {
 	t.Run("tls-config-retriever-valid-certs", func(t *testing.T) {
 		validity, err := time.ParseDuration("1h")
 		assert.Assert(t, err)
-		ca := certs.GenerateSecret("my-ca", "my-ca", "my-ca.host", validity, nil)
+		ca, err := certs.GenerateSecret("my-ca", "my-ca", "my-ca.host", validity, nil)
+		if err != nil {
+			t.Error(err)
+		}
 
 		certPath := path.Join(baseCertsPath, "valid-certificate")
 		assert.Assert(t, os.MkdirAll(certPath, 0755))
diff --git a/internal/nonkube/common/fs_config_renderer_test.go b/internal/nonkube/common/fs_config_renderer_test.go
index 32cd0df6..d1962b11 100644
--- a/internal/nonkube/common/fs_config_renderer_test.go
+++ b/internal/nonkube/common/fs_config_renderer_test.go
@@ -123,15 +123,24 @@ func compareCertificates(t *testing.T, customOutputPath string) {
 func createInputCertificates(t *testing.T, customOutputPath string) {
 	// preparing certificates
 	fakeHosts := "10.0.0.1,10.0.0.2,fake.domain"
-	ca := certs.GenerateSecret("fake-ca", "fake-ca", "", 0, nil)
-	server := certs.GenerateSecret("fake-server-cert", "fake-server-cert", fakeHosts, 0, &ca)
-	client := certs.GenerateSecret("fake-client-cert", "fake-client-cert", "", 0, &ca)
+	ca, err := certs.GenerateSecret("fake-ca", "fake-ca", "", 0, nil)
+	if err != nil {
+		t.Error(err)
+	}
+	server, err := certs.GenerateSecret("fake-server-cert", "fake-server-cert", fakeHosts, 0, ca)
+	if err != nil {
+		t.Error(err)
+	}
+	client, err := certs.GenerateSecret("fake-client-cert", "fake-client-cert", "", 0, ca)
+	if err != nil {
+		t.Error(err)
+	}
 
 	// paths for each provided certificate
 	caPath := path.Join(customOutputPath, "namespaces/default", string(api.InputIssuersPath), "skupper-site-ca")
 	serverPath := path.Join(customOutputPath, "namespaces/default", string(api.InputCertificatesPath), "link-access-one")
 	clientPath := path.Join(customOutputPath, "namespaces/default", string(api.InputCertificatesPath), "client-link-access-one")
-	certsMap := map[string]corev1.Secret{
+	certsMap := map[string]*corev1.Secret{
 		caPath:     ca,
 		serverPath: server,
 		clientPath: client,
diff --git a/pkg/nonkube/api/token_test.go b/pkg/nonkube/api/token_test.go
index 92cedfc0..e931777a 100644
--- a/pkg/nonkube/api/token_test.go
+++ b/pkg/nonkube/api/token_test.go
@@ -42,7 +42,7 @@ func TestCreateTokens(t *testing.T) {
 	tests := []struct {
 		name          string
 		ra            v2alpha1.RouterAccess
-		serverSecret  v1.Secret
+		serverSecret  *v1.Secret
 		expectedHosts []string
 	}{
 		{
@@ -67,7 +67,7 @@ func TestCreateTokens(t *testing.T) {
 		{
 			name:         "invalid-server-cert",
 			ra:           fakeRouterAccessBothRoles(),
-			serverSecret: fakeServerSecretBad(),
+			serverSecret: fakeServerSecretBad(t),
 			expectedHosts: []string{
 				"127.0.0.1",
 			},
@@ -75,7 +75,7 @@ func TestCreateTokens(t *testing.T) {
 		{
 			name:         "server-cert-no-hosts",
 			ra:           fakeRouterAccessBothRoles(),
-			serverSecret: fakeServerSecret([]string{}),
+			serverSecret: fakeServerSecret([]string{}, t),
 			expectedHosts: []string{
 				"127.0.0.1",
 			},
@@ -92,7 +92,7 @@ func TestCreateTokens(t *testing.T) {
 		{
 			name:         "sans-provided-empty-server-cert",
 			ra:           fakeRouterAccessWithSANs("fake.host.one", "fake.host.two"),
-			serverSecret: fakeServerSecret([]string{}),
+			serverSecret: fakeServerSecret([]string{}, t),
 			expectedHosts: []string{
 				"127.0.0.1",
 				"fake.host.one",
@@ -102,7 +102,7 @@ func TestCreateTokens(t *testing.T) {
 		{
 			name:         "sans-provided-server-cert-with-hosts",
 			ra:           fakeRouterAccessWithSANs("fake.host.one", "fake.host.two"),
-			serverSecret: fakeServerSecret([]string{"server.host.one", "server.host.two"}),
+			serverSecret: fakeServerSecret([]string{"server.host.one", "server.host.two"}, t),
 			expectedHosts: []string{
 				"127.0.0.1",
 				"server.host.one",
@@ -112,7 +112,7 @@ func TestCreateTokens(t *testing.T) {
 		{
 			name:         "sans-provided-server-cert-with-hosts-and-ips",
 			ra:           fakeRouterAccessWithSANs("fake.host.one", "fake.host.two"),
-			serverSecret: fakeServerSecret([]string{"server.host.one", "server.host.two", "10.0.0.1", "10.0.0.2"}),
+			serverSecret: fakeServerSecret([]string{"server.host.one", "server.host.two", "10.0.0.1", "10.0.0.2"}, t),
 			expectedHosts: []string{
 				"127.0.0.1",
 				"10.0.0.1",
@@ -205,16 +205,29 @@ func fakeRouterAccessWithSANs(sans ...string) v2alpha1.RouterAccess {
 	return ra
 }
 
-func fakeServerSecretBad() v1.Secret {
-	ca := certs.GenerateSecret("fake-ca", "fake-ca", "", 0, nil)
-	server := certs.GenerateSecret("fake-server-cert", "fake-server-cert", "", 0, &ca)
-	delete(server.Data, "tls.crt")
-	return server
+func fakeServerSecretBad(t *testing.T) *v1.Secret {
+	ca, err := certs.GenerateSecret("fake-ca", "fake-ca", "", 0, nil)
+	if err != nil {
+		t.Error(err)
+	}
+	serverCert, err := certs.GenerateSecret("fake-server-cert", "fake-server-cert", "", 0, ca)
+	if err != nil {
+		t.Error(err)
+	}
+
+	delete(serverCert.Data, "tls.crt")
+	return serverCert
 }
 
-func fakeServerSecret(hosts []string) v1.Secret {
+func fakeServerSecret(hosts []string, t *testing.T) *v1.Secret {
 	hostsCsv := strings.Join(hosts, ",")
-	ca := certs.GenerateSecret("fake-ca", "fake-ca", "", 0, nil)
-	server := certs.GenerateSecret("fake-server-cert", "fake-server-cert", hostsCsv, 0, &ca)
-	return server
+	ca, err := certs.GenerateSecret("fake-ca", "fake-ca", "", 0, nil)
+	if err != nil {
+		t.Error(err)
+	}
+	serverCert, err := certs.GenerateSecret("fake-server-cert", "fake-server-cert", hostsCsv, 0, ca)
+	if err != nil {
+		t.Error(err)
+	}
+	return serverCert
 }