Add tests to secret manager for additional output formats

Signed-off-by: joshvanl <vleeuwenjoshua@gmail.com>
This commit is contained in:
joshvanl 2022-02-01 15:33:47 +00:00
parent fdf7743f21
commit 9ca869c2cf
3 changed files with 228 additions and 18 deletions

View File

@ -17,10 +17,8 @@ limitations under the License.
package internal
import (
"bytes"
"context"
"crypto/x509"
"encoding/pem"
"fmt"
corev1 "k8s.io/api/core/v1"
@ -287,11 +285,10 @@ func setAdditionalOutputFormats(crt *cmapi.Certificate, secret *corev1.Secret, d
switch format.Type {
case cmapi.CertificateOutputFormatDER:
// Store binary format of the private key
block, _ := pem.Decode(data.PrivateKey)
secret.Data[cmapi.CertificateOutputFormatDERKey] = block.Bytes
secret.Data[cmapi.CertificateOutputFormatDERKey] = certificates.OutputFormatDER(data.PrivateKey)
case cmapi.CertificateOutputFormatCombinedPEM:
// Combine tls.key and tls.crt
secret.Data[cmapi.CertificateOutputFormatCombinedPEMKey] = bytes.Join([][]byte{data.PrivateKey, data.Certificate}, []byte("\n"))
secret.Data[cmapi.CertificateOutputFormatCombinedPEMKey] = certificates.OutputFormatCombinedPEM(data.PrivateKey, data.Certificate)
default:
return fmt.Errorf("unknown additional output format %s", format.Type)
}

View File

@ -32,18 +32,17 @@ import (
)
// ensureSecretData ensures that the Certificate's Secret is up to date with
// non-issuing condition related data. Currently only reconciles on Annotations
// and Labels from the Certificate's SecretTemplate.
// non-issuing condition related data.
// Reconciles over the Certificate's SecretTemplate, and
// AdditionalOutputFormats.
func (c *controller) ensureSecretData(ctx context.Context, log logr.Logger, crt *cmapi.Certificate) error {
dbg := log.V(logf.DebugLevel)
// Retrieve the Secret which is associated with this Certificate.
secret, err := c.secretLister.Secrets(crt.Namespace).Get(crt.Spec.SecretName)
// Secret doesn't exist so we can't do anything. The Certificate will be
// marked for a re-issuance and the resulting Secret will be evaluated again.
if apierrors.IsNotFound(err) {
dbg.Info("secret not found", "error", err.Error())
log.V(logf.DebugLevel).Info("secret not found", "error", err.Error())
return nil
}
@ -55,13 +54,21 @@ func (c *controller) ensureSecretData(ctx context.Context, log logr.Logger, crt
log = log.WithValues("secret", secret.Name)
var data internal.SecretData
if secret.Data != nil {
data = internal.SecretData{
PrivateKey: secret.Data[corev1.TLSPrivateKeyKey],
Certificate: secret.Data[corev1.TLSCertKey],
CA: secret.Data[cmmeta.TLSCAKey],
}
// If there is no certificate or private key data available at the target
// Secret then exit early. The absense of these keys should cause an issuance
// of the Certificate, so there is no need to run post issuance checks.
if secret.Data == nil ||
len(secret.Data[corev1.TLSCertKey]) == 0 ||
len(secret.Data[corev1.TLSPrivateKeyKey]) == 0 {
log.V(logf.DebugLevel).Info("secret doesn't contain both certificate and private key data",
"cert_data_len", len(secret.Data[corev1.TLSCertKey]), "key_data_len", len(secret.Data[corev1.TLSPrivateKeyKey]))
return nil
}
data := internal.SecretData{
PrivateKey: secret.Data[corev1.TLSPrivateKeyKey],
Certificate: secret.Data[corev1.TLSCertKey],
CA: secret.Data[cmmeta.TLSCAKey],
}
// Check whether the Certificate's Secret has correct output format and

View File

@ -18,6 +18,7 @@ package issuing
import (
"context"
"encoding/pem"
"testing"
"github.com/stretchr/testify/assert"
@ -29,11 +30,18 @@ import (
cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
"github.com/cert-manager/cert-manager/pkg/controller/certificates/issuing/internal"
testpkg "github.com/cert-manager/cert-manager/pkg/controller/test"
testcrypto "github.com/cert-manager/cert-manager/test/unit/crypto"
)
func Test_ensureSecretData(t *testing.T) {
const fieldManager = "cert-manager-unit-tests"
pk := testcrypto.MustCreatePEMPrivateKey(t)
cert := testcrypto.MustCreateCert(t, pk, &cmapi.Certificate{Spec: cmapi.CertificateSpec{CommonName: "test"}})
block, _ := pem.Decode(pk)
pkDER := block.Bytes
combinedPEM := append(append(pk, '\n'), cert...)
tests := map[string]struct {
// key that should be passed to ProcessItem.
// if not set, the 'namespace/name' of the 'Certificate' field will be used.
@ -61,7 +69,57 @@ func Test_ensureSecretData(t *testing.T) {
key: "random-namespace/random-certificate",
expectedAction: false,
},
"if Certificate and Secret exists, but the Certificate has a True Issuing condition, do nothing": {key: "test-namespace/test-name",
"if Certificate and Secret exists, but the Secret contains no certificate or private key data, do nothing": {
key: "test-namespace/test-name",
cert: &cmapi.Certificate{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
Spec: cmapi.CertificateSpec{
SecretName: "test-secret",
SecretTemplate: &cmapi.CertificateSecretTemplate{Annotations: map[string]string{"foo": "bar"}, Labels: map[string]string{"abc": "123"}},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret"},
Data: map[string][]byte{},
},
expectedAction: false,
},
"if Certificate and Secret exists, but the Secret contains no certificate data, do nothing": {
key: "test-namespace/test-name",
cert: &cmapi.Certificate{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
Spec: cmapi.CertificateSpec{
SecretName: "test-secret",
SecretTemplate: &cmapi.CertificateSecretTemplate{Annotations: map[string]string{"foo": "bar"}, Labels: map[string]string{"abc": "123"}},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret"},
Data: map[string][]byte{
"tls.key": pk,
},
},
expectedAction: false,
},
"if Certificate and Secret exists, but the Secret contains no private key data, do nothing": {
key: "test-namespace/test-name",
cert: &cmapi.Certificate{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
Spec: cmapi.CertificateSpec{
SecretName: "test-secret",
SecretTemplate: &cmapi.CertificateSecretTemplate{Annotations: map[string]string{"foo": "bar"}, Labels: map[string]string{"abc": "123"}},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret"},
Data: map[string][]byte{
"tls.cert": cert,
},
},
expectedAction: false,
},
"if Certificate and Secret exists, but the Certificate has a True Issuing condition, do nothing": {
key: "test-namespace/test-name",
cert: &cmapi.Certificate{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
Spec: cmapi.CertificateSpec{
@ -74,6 +132,10 @@ func Test_ensureSecretData(t *testing.T) {
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret"},
Data: map[string][]byte{
"tls.crt": cert,
"tls.key": pk,
},
},
expectedAction: false,
},
@ -107,6 +169,10 @@ func Test_ensureSecretData(t *testing.T) {
Namespace: "test-namespace", Name: "test-secret",
Annotations: map[string]string{"foo": "bar"}, Labels: map[string]string{"abc": "123"},
},
Data: map[string][]byte{
"tls.crt": cert,
"tls.key": pk,
},
},
expectedAction: true,
},
@ -142,6 +208,10 @@ func Test_ensureSecretData(t *testing.T) {
},
}},
},
Data: map[string][]byte{
"tls.crt": cert,
"tls.key": pk,
},
},
expectedAction: true,
},
@ -175,6 +245,10 @@ func Test_ensureSecretData(t *testing.T) {
}},
},
},
Data: map[string][]byte{
"tls.crt": cert,
"tls.key": pk,
},
},
expectedAction: true,
},
@ -208,6 +282,10 @@ func Test_ensureSecretData(t *testing.T) {
}},
},
},
Data: map[string][]byte{
"tls.crt": cert,
"tls.key": pk,
},
},
expectedAction: false,
},
@ -228,6 +306,134 @@ func Test_ensureSecretData(t *testing.T) {
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret"},
Data: map[string][]byte{
"tls.crt": cert,
"tls.key": pk,
},
},
expectedAction: true,
},
"if Certificate with combined pem and Secret exists, but the Secret doesn't have combined pem, should apply the combined pem": {
key: "test-namespace/test-name",
cert: &cmapi.Certificate{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
Spec: cmapi.CertificateSpec{
SecretName: "test-secret",
AdditionalOutputFormats: []cmapi.CertificateAdditionalOutputFormat{
{Type: "CombinedPEM"},
},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret"},
Data: map[string][]byte{
"tls.crt": cert,
"tls.key": pk,
},
},
expectedAction: true,
},
"if Certificate with der and Secret exists, but the Secret doesn't have der, should apply the der": {
key: "test-namespace/test-name",
cert: &cmapi.Certificate{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
Spec: cmapi.CertificateSpec{
SecretName: "test-secret",
AdditionalOutputFormats: []cmapi.CertificateAdditionalOutputFormat{
{Type: "DER"},
},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret"},
Data: map[string][]byte{
"tls.crt": cert,
"tls.key": pk,
},
},
expectedAction: true,
},
"if Certificate with combined pem and der, and Secret exists, but the Secret doesn't have combined pem or der, should apply the combined pem and der": {
key: "test-namespace/test-name",
cert: &cmapi.Certificate{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
Spec: cmapi.CertificateSpec{
SecretName: "test-secret",
AdditionalOutputFormats: []cmapi.CertificateAdditionalOutputFormat{
{Type: "CombinedPEM"},
{Type: "DER"},
},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret"},
Data: map[string][]byte{
"tls.crt": cert,
"tls.key": pk,
},
},
expectedAction: true,
},
"if Certificate with combined pem and der, and Secret exists with combined pem and der with managed fields, should do nothing": {
key: "test-namespace/test-name",
cert: &cmapi.Certificate{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
Spec: cmapi.CertificateSpec{
SecretName: "test-secret",
AdditionalOutputFormats: []cmapi.CertificateAdditionalOutputFormat{
{Type: "CombinedPEM"},
{Type: "DER"},
},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret",
ManagedFields: []metav1.ManagedFieldsEntry{{
Manager: fieldManager,
FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:data": {
"f:tls-combined.pem": {},
"f:key.der": {}
}}`),
},
}},
},
Data: map[string][]byte{
"tls.crt": cert,
"tls.key": pk,
"tls-combined.pem": combinedPEM,
"key.der": pkDER,
},
},
expectedAction: false,
},
"if Certificate with no combined pem or der, and Secret exists with combined pem and der managed by field manager, should apply to remove them": {
key: "test-namespace/test-name",
cert: &cmapi.Certificate{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
Spec: cmapi.CertificateSpec{
SecretName: "test-secret",
AdditionalOutputFormats: []cmapi.CertificateAdditionalOutputFormat{},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret",
ManagedFields: []metav1.ManagedFieldsEntry{{
Manager: fieldManager,
FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:data": {
"f:tls-combined.pem": {},
"f:key.der": {}
}}`),
},
}},
},
Data: map[string][]byte{
"tls.crt": cert,
"tls.key": pk,
"tls-combined.pem": combinedPEM,
"key.der": pkDER,
},
},
expectedAction: true,
},