Adds more tests for deprecated secret annotations and update secret

annotations if deprecated ones exist

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
This commit is contained in:
JoshVanL
2019-11-27 15:26:07 +00:00
parent f7fe88ade6
commit 8d2ed09896
4 changed files with 179 additions and 20 deletions
+3 -3
View File
@@ -28,10 +28,10 @@ const (
CertificateNameKey = "cert-manager.io/certificate-name"
)
// Legacy annotation names for Secrets
// Deprecated annotation names for Secrets
const (
LegacyIssuerNameAnnotationKey = "certmanager.k8s.io/issuer-name"
LegacyIssuerKindAnnotationKey = "certmanager.k8s.io/issuer-kind"
DeprecatedIssuerNameAnnotationKey = "certmanager.k8s.io/issuer-name"
DeprecatedIssuerKindAnnotationKey = "certmanager.k8s.io/issuer-kind"
)
const (
+11
View File
@@ -487,6 +487,7 @@ func (c *certificateRequestManager) processCertificate(ctx context.Context, crt
// Otherwise, the existing resource will be updated.
// The first return argument will be true if the resource was updated/created
// without error.
// updateSecretData will also update deprecated annotations if they exist.
func (c *certificateRequestManager) updateSecretData(ctx context.Context, crt *cmapi.Certificate, existingSecret *corev1.Secret, data secretData) (bool, error) {
s := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
@@ -774,6 +775,7 @@ type secretData struct {
// setSecretValues will NOT actually update the resource in the apiserver.
// If updating an existing Secret resource returned by an api client 'lister',
// make sure to DeepCopy the object first to avoid modifying data in-cache.
// It will also update depreciated issuer name and kind annotations if they exist.
func setSecretValues(ctx context.Context, crt *cmapi.Certificate, s *corev1.Secret, data secretData) error {
// initialize the `Data` field if it is nil
if s.Data == nil {
@@ -792,6 +794,15 @@ func setSecretValues(ctx context.Context, crt *cmapi.Certificate, s *corev1.Secr
s.Annotations[cmapi.IssuerNameAnnotationKey] = crt.Spec.IssuerRef.Name
s.Annotations[cmapi.IssuerKindAnnotationKey] = apiutil.IssuerKind(crt.Spec.IssuerRef)
// If deprecated annotations exist with any value, then they too shall be
// updated
if _, ok := s.Annotations[cmapi.DeprecatedIssuerNameAnnotationKey]; ok {
s.Annotations[cmapi.DeprecatedIssuerNameAnnotationKey] = crt.Spec.IssuerRef.Name
}
if _, ok := s.Annotations[cmapi.DeprecatedIssuerKindAnnotationKey]; ok {
s.Annotations[cmapi.DeprecatedIssuerKindAnnotationKey] = apiutil.IssuerKind(crt.Spec.IssuerRef)
}
// if the certificate data is empty, clear the subject related annotations
if len(data.cert) == 0 {
delete(s.Annotations, cmapi.CommonNameAnnotationKey)
+22 -8
View File
@@ -121,16 +121,30 @@ func certificateMatchesSpec(crt *v1alpha2.Certificate, key crypto.Signer, cert *
secret.Annotations = make(map[string]string)
}
// validate that the issuer is correct
if crt.Spec.IssuerRef.Name != secret.Annotations[v1alpha2.IssuerNameAnnotationKey] &&
crt.Spec.IssuerRef.Name != secret.Annotations[v1alpha2.LegacyIssuerNameAnnotationKey] {
errs = append(errs, fmt.Sprintf("Issuer of the certificate is not up to date: %q", secret.Annotations[v1alpha2.IssuerNameAnnotationKey]))
// Validate that the issuer name and kind is correct
// If the new annotation exists and doesn't match then error
// If the new annotation doesn't exist and the old annotation doesn't match then error
annotationError := func(k, v string) {
errs = append(errs, fmt.Sprintf("Issuer %q of the certificate is not up to date: %q", k, v))
}
// validate that the issuer kind is correct
if apiutil.IssuerKind(crt.Spec.IssuerRef) != secret.Annotations[v1alpha2.IssuerKindAnnotationKey] &&
apiutil.IssuerKind(crt.Spec.IssuerRef) != secret.Annotations[v1alpha2.LegacyIssuerKindAnnotationKey] {
errs = append(errs, fmt.Sprintf("Issuer kind of the certificate is not up to date: %q", secret.Annotations[v1alpha2.IssuerKindAnnotationKey]))
name, ok := secret.Annotations[v1alpha2.IssuerNameAnnotationKey]
if !ok {
if secret.Annotations[v1alpha2.DeprecatedIssuerNameAnnotationKey] != crt.Spec.IssuerRef.Name {
annotationError(v1alpha2.DeprecatedIssuerNameAnnotationKey, secret.Annotations[v1alpha2.DeprecatedIssuerNameAnnotationKey])
}
} else if name != crt.Spec.IssuerRef.Name {
annotationError(v1alpha2.IssuerNameAnnotationKey, secret.Annotations[v1alpha2.IssuerNameAnnotationKey])
}
kind, ok := secret.Annotations[v1alpha2.IssuerKindAnnotationKey]
if !ok {
if secret.Annotations[v1alpha2.DeprecatedIssuerKindAnnotationKey] != apiutil.IssuerKind(crt.Spec.IssuerRef) {
annotationError(v1alpha2.DeprecatedIssuerKindAnnotationKey, secret.Annotations[v1alpha2.DeprecatedIssuerKindAnnotationKey])
}
} else if kind != apiutil.IssuerKind(crt.Spec.IssuerRef) {
annotationError(v1alpha2.IssuerKindAnnotationKey, secret.Annotations[v1alpha2.IssuerKindAnnotationKey])
}
return len(errs) == 0, errs
+143 -9
View File
@@ -116,21 +116,155 @@ func TestCertificateMatchesSpec(t *testing.T) {
},
},
"if the issuer name and kind uses the lagacy annotation then it should still match the spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate,
gen.SetCertificateCommonName(""),
)),
certificate: gen.CertificateFrom(exampleBundle.certificate,
gen.SetCertificateCommonName(""),
),
"if the issuer name and kind uses v1alpha2 annotation then it should still match the spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
secret: gen.SecretFrom(secret,
gen.SetSecretAnnotations(map[string]string{
cmapi.LegacyIssuerNameAnnotationKey: "ca-issuer",
cmapi.LegacyIssuerKindAnnotationKey: "Issuer",
cmapi.IssuerNameAnnotationKey: "ca-issuer",
cmapi.IssuerKindAnnotationKey: "Issuer",
})),
expMatch: true,
expErrors: nil,
},
"if the issuer name uses v1alpha2 annotation but kind uses depreicated then it should still match the spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
secret: gen.SecretFrom(secret,
gen.SetSecretAnnotations(map[string]string{
cmapi.IssuerNameAnnotationKey: "ca-issuer",
cmapi.DeprecatedIssuerKindAnnotationKey: "Issuer",
})),
expMatch: true,
expErrors: nil,
},
"if the issuer name uses deprecated annotation but kind uses v1alpha2 then it should still match the spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
secret: gen.SecretFrom(secret,
gen.SetSecretAnnotations(map[string]string{
cmapi.DeprecatedIssuerNameAnnotationKey: "ca-issuer",
cmapi.IssuerKindAnnotationKey: "Issuer",
})),
expMatch: true,
expErrors: nil,
},
"if the issuer name and kind uses the deprecated annotation then it should still match the spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
secret: gen.SecretFrom(secret,
gen.SetSecretAnnotations(map[string]string{
cmapi.DeprecatedIssuerNameAnnotationKey: "ca-issuer",
cmapi.DeprecatedIssuerKindAnnotationKey: "Issuer",
})),
expMatch: true,
expErrors: nil,
},
"if the issuer name uses v1alpha2 and kind uses both the deprecated and v1alpha2 annotation then it should still match the spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
secret: gen.SecretFrom(secret,
gen.SetSecretAnnotations(map[string]string{
cmapi.DeprecatedIssuerKindAnnotationKey: "Issuer",
cmapi.IssuerNameAnnotationKey: "ca-issuer",
cmapi.IssuerKindAnnotationKey: "Issuer",
})),
expMatch: true,
expErrors: nil,
},
"if the issuer name both the deprecated and v1alpha2 annotation and kind uses deprecated then it should still match the spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
secret: gen.SecretFrom(secret,
gen.SetSecretAnnotations(map[string]string{
cmapi.DeprecatedIssuerNameAnnotationKey: "Issuer",
cmapi.IssuerNameAnnotationKey: "ca-issuer",
cmapi.IssuerKindAnnotationKey: "Issuer",
})),
expMatch: true,
expErrors: nil,
},
"if the issuer name and kind uses both the deprecated and v1alpha2 annotation then it should still match the spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
secret: gen.SecretFrom(secret,
gen.SetSecretAnnotations(map[string]string{
cmapi.DeprecatedIssuerNameAnnotationKey: "ca-issuer",
cmapi.DeprecatedIssuerKindAnnotationKey: "Issuer",
cmapi.IssuerNameAnnotationKey: "ca-issuer",
cmapi.IssuerKindAnnotationKey: "Issuer",
})),
expMatch: true,
expErrors: nil,
},
"if the issuer name and kind uses both the deprecated and v1alpha2 annotation but no values in deprecated annotations then should match spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
secret: gen.SecretFrom(secret,
gen.SetSecretAnnotations(map[string]string{
cmapi.DeprecatedIssuerNameAnnotationKey: "foo",
cmapi.DeprecatedIssuerKindAnnotationKey: "bar",
cmapi.IssuerNameAnnotationKey: "ca-issuer",
cmapi.IssuerKindAnnotationKey: "Issuer",
})),
expMatch: true,
expErrors: nil,
},
"if the issuer name and kind deprecated annotations are correct but v1alpha2 values are wrong then should not match spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
secret: gen.SecretFrom(secret,
gen.SetSecretAnnotations(map[string]string{
cmapi.DeprecatedIssuerNameAnnotationKey: "ca-issuer",
cmapi.DeprecatedIssuerKindAnnotationKey: "Issuer",
cmapi.IssuerNameAnnotationKey: "foo",
cmapi.IssuerKindAnnotationKey: "bar",
})),
expMatch: false,
expErrors: []string{
`Issuer "cert-manager.io/issuer-name" of the certificate is not up to date: "foo"`,
`Issuer "cert-manager.io/issuer-kind" of the certificate is not up to date: "bar"`,
},
},
"if the issuer name and kind deprecated annotations are correct but v1alpha2 values are empty but exist then should not match spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
secret: gen.SecretFrom(secret,
gen.SetSecretAnnotations(map[string]string{
cmapi.DeprecatedIssuerNameAnnotationKey: "ca-issuer",
cmapi.DeprecatedIssuerKindAnnotationKey: "Issuer",
cmapi.IssuerNameAnnotationKey: "",
cmapi.IssuerKindAnnotationKey: "",
})),
expMatch: false,
expErrors: []string{
`Issuer "cert-manager.io/issuer-name" of the certificate is not up to date: ""`,
`Issuer "cert-manager.io/issuer-kind" of the certificate is not up to date: ""`,
},
},
"if the issuer name and kind deprecated annotations are wrong and no v1alpha2 values then should not match spec": {
cb: mustCreateCryptoBundle(t, gen.CertificateFrom(exampleBundle.certificate)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
secret: gen.SecretFrom(secret,
gen.SetSecretAnnotations(map[string]string{
cmapi.DeprecatedIssuerNameAnnotationKey: "foo",
cmapi.DeprecatedIssuerKindAnnotationKey: "bar",
})),
expMatch: false,
expErrors: []string{
`Issuer "certmanager.k8s.io/issuer-name" of the certificate is not up to date: "foo"`,
`Issuer "certmanager.k8s.io/issuer-kind" of the certificate is not up to date: "bar"`,
},
},
} {
t.Run(name, func(t *testing.T) {
match, errs := certificateMatchesSpec(