weiguoqiang/cert-manager
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

improve label and annotation checks

Browse Source 
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
This commit is contained in:
Tim Ramlot 2023-06-23 10:22:46 +02:00
parent 4d1486bbfc
commit a9339849e5
No known key found for this signature in database
GPG Key ID: 47428728E0C2878D
5 changed files with 546 additions and 222 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

310
internal/controller/certificates/policies/checks.go
View File

@ -355,6 +355,145 @@ func SecretTemplateMismatchesSecret(input Input) (string, string, bool) {
return "", "", false
}
func certificateDataAnnotationsForSecret(secret *corev1.Secret) (annotations map[string]string, err error) {
var certificate *x509.Certificate
if len(secret.Data[corev1.TLSCertKey]) > 0 {
certificate, err = pki.DecodeX509CertificateBytes(secret.Data[corev1.TLSCertKey])
if err != nil {
return nil, err
}
}
certificateAnnotations, err := internalcertificates.AnnotationsForCertificate(certificate)
if err != nil {
return nil, err
}
return certificateAnnotations, nil
}
func secretLabelsAndAnnotationsManagedFields(secret *corev1.Secret, fieldManager string) (labels, annotations sets.Set[string], err error) {
managedLabels, managedAnnotations := sets.New[string](), sets.New[string]()
for _, managedField := range secret.ManagedFields {
// If the managed field isn't owned by the cert-manager controller, ignore.
if managedField.Manager != fieldManager || managedField.FieldsV1 == nil {
continue
}
// Decode the managed field.
var fieldset fieldpath.Set
if err := fieldset.FromJSON(bytes.NewReader(managedField.FieldsV1.Raw)); err != nil {
return nil, nil, err
}
// Extract the labels and annotations of the managed fields.
metadata := fieldset.Children.Descend(fieldpath.PathElement{
FieldName: pointer.String("metadata"),
})
labels := metadata.Children.Descend(fieldpath.PathElement{
FieldName: pointer.String("labels"),
})
annotations := metadata.Children.Descend(fieldpath.PathElement{
FieldName: pointer.String("annotations"),
})
// Gather the annotations and labels on the managed fields. Remove the '.'
// prefix which appears on managed field keys.
labels.Iterate(func(path fieldpath.Path) {
managedLabels.Insert(strings.TrimPrefix(path.String(), "."))
})
annotations.Iterate(func(path fieldpath.Path) {
managedAnnotations.Insert(strings.TrimPrefix(path.String(), "."))
})
}
return managedLabels, managedAnnotations, nil
}
// SecretManagedLabelsAndAnnotationsManagedFieldsMismatch will inspect the given Secret's
// managed fields for its Annotations and Labels, and compare this against the
// Labels and Annotations that are managed by cert-manager. Returns false if Annotations and
// Labels match on both the Certificate's SecretTemplate and the Secret's
// managed fields, true otherwise.
// Also returns true if the managed fields or signed certificate were not able
// to be decoded.
func SecretManagedLabelsAndAnnotationsManagedFieldsMismatch(fieldManager string) Func {
return func(input Input) (string, string, bool) {
managedLabels, managedAnnotations, err := secretLabelsAndAnnotationsManagedFields(input.Secret, fieldManager)
if err != nil {
return ManagedFieldsParseError, fmt.Sprintf("failed to decode managed fields on Secret: %s", err), true
}
// Remove the non cert-manager annotations from the managed Annotations so we can compare
// 1 to 1 all the cert-manager annotations.
for k := range managedAnnotations {
if strings.HasPrefix(k, "cert-manager.io/") ||
strings.HasPrefix(k, "controller.cert-manager.io/") {
continue
}
delete(managedAnnotations, k)
}
// Ignore the CertificateName and IssuerRef annotations as these cannot be set by the postIssuance controller.
for _, k := range []string{
cmapi.CertificateNameKey, // SecretCertificateNameAnnotationMismatch checks the value
cmapi.IssuerNameAnnotationKey, // SecretIssuerAnnotationsMismatch checks the value
cmapi.IssuerKindAnnotationKey, // SecretIssuerAnnotationsMismatch checks the value
cmapi.IssuerGroupAnnotationKey, // SecretIssuerAnnotationsMismatch checks the value
} {
delete(managedAnnotations, k)
}
// Remove the non cert-manager labels from the managed Annotations so we can compare
// 1 to 1 all the cert-manager labels.
for k := range managedLabels {
if strings.HasPrefix(k, "cert-manager.io/") ||
strings.HasPrefix(k, "controller.cert-manager.io/") {
continue
}
delete(managedLabels, k)
}
expCertificateDataAnnotations, err := certificateDataAnnotationsForSecret(input.Secret)
if err != nil {
return InvalidCertificate, fmt.Sprintf("Failed getting secret annotations: %v", err), true
}
expLabels := sets.New[string](
cmapi.PartOfCertManagerControllerLabelKey, // SecretBaseLabelsMismatch checks the value
)
expAnnotations := sets.New[string]()
for k := range expCertificateDataAnnotations { // SecretCertificateDetailsAnnotationsMismatch checks the value
expAnnotations.Insert(k)
}
if !managedLabels.Equal(expLabels) {
missingLabels := expLabels.Difference(managedLabels)
if len(missingLabels) > 0 {
return SecretManagedMetadataMismatch, fmt.Sprintf("Secret is missing these Managed Labels: %v", missingLabels.UnsortedList()), true
}
extraLabels := managedLabels.Difference(expLabels)
return SecretManagedMetadataMismatch, fmt.Sprintf("Secret has these extra Labels: %v", extraLabels.UnsortedList()), true
}
if !managedAnnotations.Equal(expAnnotations) {
missingAnnotations := expAnnotations.Difference(managedAnnotations)
if len(missingAnnotations) > 0 {
return SecretManagedMetadataMismatch, fmt.Sprintf("Secret is missing these Managed Annotations: %v", missingAnnotations.UnsortedList()), true
}
extraAnnotations := managedAnnotations.Difference(expAnnotations)
return SecretManagedMetadataMismatch, fmt.Sprintf("Secret has these extra Annotations: %v", extraAnnotations.UnsortedList()), true
}
return "", "", false
}
}
// SecretTemplateMismatchesSecretManagedFields will inspect the given Secret's
// managed fields for its Annotations and Labels, and compare this against the
// SecretTemplate on the given Certificate. Returns false if Annotations and
@ -364,134 +503,101 @@ func SecretTemplateMismatchesSecret(input Input) (string, string, bool) {
// to be decoded.
func SecretTemplateMismatchesSecretManagedFields(fieldManager string) Func {
return func(input Input) (string, string, bool) {
// Only attempt to decode the signed certificate, if one is available.
var x509cert *x509.Certificate
if len(input.Secret.Data[corev1.TLSCertKey]) > 0 {
var err error
x509cert, err = pki.DecodeX509CertificateBytes(input.Secret.Data[corev1.TLSCertKey])
if err != nil {
// This case should never happen as it should always be caught by the
// secretPublicKeysMatch function beforehand, but handle it just in case.
return InvalidCertificate, fmt.Sprintf("Failed to decode stored certificate: %v", err), true
}
}
baseAnnotations, err := internalcertificates.AnnotationsForCertificate(x509cert)
managedLabels, managedAnnotations, err := secretLabelsAndAnnotationsManagedFields(input.Secret, fieldManager)
if err != nil {
return InvalidCertificate, fmt.Sprintf("Failed getting secret annotations: %v", err), true
return ManagedFieldsParseError, fmt.Sprintf("failed to decode managed fields on Secret: %s", err), true
}
// We don't use the values of these annotations, but we need to make sure
// that the keys are present in the map so that we can compare the sets.
baseAnnotations[cmapi.CertificateNameKey] = "<certificate-value>"
baseAnnotations[cmapi.IssuerNameAnnotationKey] = "<issuer-name>"
baseAnnotations[cmapi.IssuerKindAnnotationKey] = "<issuer-kind>"
baseAnnotations[cmapi.IssuerGroupAnnotationKey] = "<issuer-group>"
managedLabels, managedAnnotations := sets.NewString(), sets.NewString()
for _, managedField := range input.Secret.ManagedFields {
// If the managed field isn't owned by the cert-manager controller, ignore.
if managedField.Manager != fieldManager || managedField.FieldsV1 == nil {
// Remove the cert-manager annotations from the managed Annotations so we can compare
// 1 to 1 against the SecretTemplate.
for k := range managedAnnotations {
if !strings.HasPrefix(k, "cert-manager.io/") &&
!strings.HasPrefix(k, "controller.cert-manager.io/") {
continue
}
// Decode the managed field.
var fieldset fieldpath.Set
if err := fieldset.FromJSON(bytes.NewReader(managedField.FieldsV1.Raw)); err != nil {
return ManagedFieldsParseError, fmt.Sprintf("failed to decode managed fields on Secret: %s", err), true
}
// Extract the labels and annotations of the managed fields.
metadata := fieldset.Children.Descend(fieldpath.PathElement{
FieldName: pointer.String("metadata"),
})
labels := metadata.Children.Descend(fieldpath.PathElement{
FieldName: pointer.String("labels"),
})
annotations := metadata.Children.Descend(fieldpath.PathElement{
FieldName: pointer.String("annotations"),
})
// Gather the annotations and labels on the managed fields. Remove the '.'
// prefix which appears on managed field keys.
labels.Iterate(func(path fieldpath.Path) {
managedLabels.Insert(strings.TrimPrefix(path.String(), "."))
})
annotations.Iterate(func(path fieldpath.Path) {
managedAnnotations.Insert(strings.TrimPrefix(path.String(), "."))
})
delete(managedAnnotations, k)
}
// Remove the base Annotations from the managed Annotations so we can compare
// 1 to 1 against the SecretTemplate.
for k := range baseAnnotations {
managedAnnotations = managedAnnotations.Delete(k)
}
// Remove the base label from the managed Labels so we can
// Remove the cert-manager labels from the managed Labels so we can
// compare 1 to 1 against the SecretTemplate
managedLabels.Delete(cmapi.PartOfCertManagerControllerLabelKey)
// Check early for Secret Template being nil, and whether managed
// labels/annotations are not.
if input.Certificate.Spec.SecretTemplate == nil {
if len(managedLabels) > 0 || len(managedAnnotations) > 0 {
return SecretTemplateMismatch, "SecretTemplate is nil, but Secret contains extra managed entries", true
for k := range managedLabels {
if !strings.HasPrefix(k, "cert-manager.io/") &&
!strings.HasPrefix(k, "controller.cert-manager.io/") {
continue
}
// SecretTemplate is nil. Managed annotations and labels are also empty.
// Return false.
return "", "", false
delete(managedLabels, k)
}
// SecretTemplate is not nil. Do length checks.
if len(input.Certificate.Spec.SecretTemplate.Labels) != len(managedLabels) ||
len(input.Certificate.Spec.SecretTemplate.Annotations) != len(managedAnnotations) {
return SecretTemplateMismatch, "Certificate's SecretTemplate doesn't match Secret", true
expLabels := sets.New[string]()
expAnnotations := sets.New[string]()
if input.Certificate.Spec.SecretTemplate != nil {
for k := range input.Certificate.Spec.SecretTemplate.Labels {
expLabels.Insert(k)
}
for k := range input.Certificate.Spec.SecretTemplate.Annotations {
expAnnotations.Insert(k)
}
}
// Check equal unsorted for SecretTemplate keys, and the managed fields
// equivalents.
for _, smap := range []struct {
specMap map[string]string
managedSet sets.String
}{
{specMap: input.Certificate.Spec.SecretTemplate.Labels, managedSet: managedLabels},
{specMap: input.Certificate.Spec.SecretTemplate.Annotations, managedSet: managedAnnotations},
} {
specSet := sets.NewString()
for kSpec := range smap.specMap {
specSet.Insert(kSpec)
if !managedLabels.Equal(expLabels) {
missingLabels := expLabels.Difference(managedLabels)
if len(missingLabels) > 0 {
return SecretTemplateMismatch, fmt.Sprintf("Secret is missing these Template Labels: %v", missingLabels.UnsortedList()), true
}
if !specSet.Equal(smap.managedSet) {
return SecretTemplateMismatch, "Certificate's SecretTemplate doesn't match Secret", true
extraLabels := managedLabels.Difference(expLabels)
return SecretTemplateMismatch, fmt.Sprintf("Secret has these extra Labels: %v", extraLabels.UnsortedList()), true
}
if !managedAnnotations.Equal(expAnnotations) {
missingAnnotations := expAnnotations.Difference(managedAnnotations)
if len(missingAnnotations) > 0 {
return SecretTemplateMismatch, fmt.Sprintf("Secret is missing these Template Annotations: %v", missingAnnotations.UnsortedList()), true
}
extraAnnotations := managedAnnotations.Difference(expAnnotations)
return SecretTemplateMismatch, fmt.Sprintf("Secret has these extra Annotations: %v", extraAnnotations.UnsortedList()), true
}
return "", "", false
}
}
func SecretBaseLabelsAreMissing(input Input) (string, string, bool) {
// If certificate has not been issued yet or is in invalid state, do not attempt to update metadata
if len(input.Secret.Data[corev1.TLSCertKey]) > 0 {
var err error
_, err = pki.DecodeX509CertificateBytes(input.Secret.Data[corev1.TLSCertKey])
if err != nil {
// This case should never happen as it should always be caught by the
// secretPublicKeysMatch function beforehand, but handle it just in case.
return InvalidCertificate, fmt.Sprintf("Failed to decode stored certificate: %v", err), true
}
}
// NOTE: The presence of the controller.cert-manager.io/fao label is checked
// by the SecretManagedLabelsAndAnnotationsManagedFieldsMismatch function.
func SecretBaseLabelsMismatch(input Input) (string, string, bool) {
// check if Secret has the base labels. Currently there is only one base label
if input.Secret.Labels == nil {
return SecretBaseLabelsMissing, fmt.Sprintf("missing base label %s", cmapi.PartOfCertManagerControllerLabelKey), true
return "", "", false
}
if _, ok := input.Secret.Labels[cmapi.PartOfCertManagerControllerLabelKey]; !ok {
return SecretBaseLabelsMissing, fmt.Sprintf("missing base label %s", cmapi.PartOfCertManagerControllerLabelKey), true
value, ok := input.Secret.Labels[cmapi.PartOfCertManagerControllerLabelKey]
if !ok || value == "true" {
return "", "", false
}
return SecretManagedMetadataMismatch, fmt.Sprintf("wrong base label %s value %q, expected \"true\"", cmapi.PartOfCertManagerControllerLabelKey, value), true
}
// SecretCertificateDetailsAnnotationsMismatch - When the certificate details annotations are
// not matching, the secret is updated.
// NOTE: The presence of the certificate details annotations is checked
// by the SecretManagedLabelsAndAnnotationsManagedFieldsMismatch function.
func SecretCertificateDetailsAnnotationsMismatch(input Input) (string, string, bool) {
dataAnnotations, err := certificateDataAnnotationsForSecret(input.Secret)
if err != nil {
return InvalidCertificate, fmt.Sprintf("Failed getting secret annotations: %v", err), true
}
for k, v := range dataAnnotations {
existing, ok := input.Secret.Annotations[k]
if !ok || existing == v {
continue
}
return SecretManagedMetadataMismatch, fmt.Sprintf("Secret metadata %s does not match certificate metadata %s", input.Secret.Annotations[k], v), true
}
return "", "", false

208
internal/controller/certificates/policies/checks_test.go
View File

@ -537,7 +537,126 @@ func Test_NewTriggerPolicyChain(t *testing.T) {
}
}
func Test_SecretTemplateMismatchesSecret(t *testing.T) {
func Test_SecretManagedLabelsAndAnnotationsManagedFieldsMismatch(t *testing.T) {
const fieldManager = "cert-manager-unit-test"
var (
fixedClockStart = time.Now()
fixedClock = fakeclock.NewFakeClock(fixedClockStart)
baseCertBundle = testcrypto.MustCreateCryptoBundle(t,
gen.Certificate("test-certificate", gen.SetCertificateCommonName("cert-manager")), fixedClock)
)
tests := map[string]struct {
secretManagedFields []metav1.ManagedFieldsEntry
secretData map[string][]byte
expReason string
expMessage string
expViolation bool
}{
"if there are no cert-manager annotations and the certificate data is nil, should return false": {
secretManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
}
}}`),
}},
},
expReason: "",
expMessage: "",
expViolation: false,
},
"if optional cert-manager annotations are present with no certificate data, should return false": {
secretManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:foo1": {},
"f:foo2": {},
"f:cert-manager.io/certificate-name": {},
"f:cert-manager.io/issuer-name": {},
"f:cert-manager.io/issuer-kind": {},
"f:cert-manager.io/issuer-group": {}
}
}}`),
}},
},
expReason: "",
expMessage: "",
expViolation: false,
},
"if cert-manager annotations are present with certificate data, should return false": {
secretManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:foo1": {},
"f:foo2": {},
"f:cert-manager.io/certificate-name": {},
"f:cert-manager.io/issuer-name": {},
"f:cert-manager.io/issuer-kind": {},
"f:cert-manager.io/issuer-group": {},
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
}
}}`),
}},
},
secretData: map[string][]byte{corev1.TLSCertKey: baseCertBundle.CertBytes},
expReason: "",
expMessage: "",
expViolation: false,
},
"if required and optional cert-manager annotations are present with certificate data but certificate data is nil, should return true": {
secretManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:foo1": {},
"f:foo2": {},
"f:cert-manager.io/certificate-name": {},
"f:cert-manager.io/issuer-name": {},
"f:cert-manager.io/issuer-kind": {},
"f:cert-manager.io/issuer-group": {},
"f:cert-manager.io/uri-sans": {}
}
}}`),
}},
},
expReason: SecretManagedMetadataMismatch,
expMessage: "Secret has these extra Annotations: [cert-manager.io/uri-sans]",
expViolation: true,
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
gotReason, gotMessage, gotViolation := SecretManagedLabelsAndAnnotationsManagedFieldsMismatch(fieldManager)(Input{
Secret: &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ManagedFields: test.secretManagedFields}, Data: test.secretData},
})
assert.Equal(t, test.expReason, gotReason, "unexpected reason")
assert.Equal(t, test.expMessage, gotMessage, "unexpected message")
assert.Equal(t, test.expViolation, gotViolation, "unexpected violation")
})
}
}
func Test_SecretSecretTemplateMismatch(t *testing.T) {
tests := map[string]struct {
tmpl *cmapi.CertificateSecretTemplate
secret *corev1.Secret
@ -683,20 +802,12 @@ func Test_SecretTemplateMismatchesSecret(t *testing.T) {
}
}
func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
func Test_SecretSecretTemplateManagedFieldsMismatch(t *testing.T) {
const fieldManager = "cert-manager-unit-test"
var (
fixedClockStart = time.Now()
fixedClock = fakeclock.NewFakeClock(fixedClockStart)
baseCertBundle = testcrypto.MustCreateCryptoBundle(t,
gen.Certificate("test-certificate", gen.SetCertificateCommonName("cert-manager")), fixedClock)
)
tests := map[string]struct {
tmpl *cmapi.CertificateSecretTemplate
secretManagedFields []metav1.ManagedFieldsEntry
secretData map[string][]byte
expReason string
expMessage string
@ -741,7 +852,7 @@ func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
},
secretManagedFields: nil,
expReason: SecretTemplateMismatch,
expMessage: "Certificate's SecretTemplate doesn't match Secret",
expMessage: "Secret is missing these Template Labels: [abc]",
expViolation: true,
},
"if template is nil but managed fields is not nil, should return true": {
@ -759,7 +870,7 @@ func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
}},
},
expReason: SecretTemplateMismatch,
expMessage: "SecretTemplate is nil, but Secret contains extra managed entries",
expMessage: "Secret has these extra Labels: [abc]",
expViolation: true,
},
"if template annotations do not match managed fields, should return true": {
@ -782,7 +893,7 @@ func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
}},
},
expReason: SecretTemplateMismatch,
expMessage: "Certificate's SecretTemplate doesn't match Secret",
expMessage: "Secret is missing these Template Annotations: [foo2]",
expViolation: true,
},
"if template labels do not match managed fields, should return true": {
@ -805,7 +916,7 @@ func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
}},
},
expReason: SecretTemplateMismatch,
expMessage: "Certificate's SecretTemplate doesn't match Secret",
expMessage: "Secret is missing these Template Labels: [def]",
expViolation: true,
},
"if template annotations and labels match managed fields, should return false": {
@ -852,7 +963,7 @@ func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
}},
},
expReason: SecretTemplateMismatch,
expMessage: "Certificate's SecretTemplate doesn't match Secret",
expMessage: "Secret has these extra Annotations: [foo3]",
expViolation: true,
},
"if template labels is a subset of managed fields, return true": {
@ -876,7 +987,7 @@ func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
}},
},
expReason: SecretTemplateMismatch,
expMessage: "Certificate's SecretTemplate doesn't match Secret",
expMessage: "Secret has these extra Labels: [ghi]",
expViolation: true,
},
"if managed fields annotations is a subset of template, return true": {
@ -899,7 +1010,7 @@ func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
}},
},
expReason: SecretTemplateMismatch,
expMessage: "Certificate's SecretTemplate doesn't match Secret",
expMessage: "Secret is missing these Template Annotations: [foo3]",
expViolation: true,
},
"if managed fields labels is a subset of template, return true": {
@ -922,7 +1033,7 @@ func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
}},
},
expReason: SecretTemplateMismatch,
expMessage: "Certificate's SecretTemplate doesn't match Secret",
expMessage: "Secret is missing these Template Labels: [ghi]",
expViolation: true,
},
"if managed fields matches template but is split across multiple managed fields, should return false": {
@ -968,7 +1079,7 @@ func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
expMessage: "",
expViolation: false,
},
"if managed fields matches template and base cert-manager annotations are present with no certificate data, should return false": {
"if managed fields matches template and cert-manager annotations are present, should return false": {
tmpl: &cmapi.CertificateSecretTemplate{
Annotations: map[string]string{"foo1": "bar1", "foo2": "bar2"},
},
@ -978,10 +1089,8 @@ func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
"f:annotations": {
"f:foo1": {},
"f:foo2": {},
"f:cert-manager.io/certificate-name": {},
"f:cert-manager.io/issuer-name": {},
"f:cert-manager.io/issuer-kind": {},
"f:cert-manager.io/issuer-group": {}
"f:cert-manager.io/foo1": {},
"f:cert-manager.io/foo2": {}
}
}}`),
}},
@ -990,64 +1099,13 @@ func Test_SecretTemplateMismatchesSecretManagedFields(t *testing.T) {
expMessage: "",
expViolation: false,
},
"if managed fields matches template and base cert-manager annotations are present with certificate data, should return false": {
tmpl: &cmapi.CertificateSecretTemplate{
Annotations: map[string]string{"foo1": "bar1", "foo2": "bar2"},
},
secretManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:metadata": {
"f:annotations": {
"f:foo1": {},
"f:foo2": {},
"f:cert-manager.io/certificate-name": {},
"f:cert-manager.io/issuer-name": {},
"f:cert-manager.io/issuer-kind": {},
"f:cert-manager.io/issuer-group": {},
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
}
}}`),
}},
},
secretData: map[string][]byte{corev1.TLSCertKey: baseCertBundle.CertBytes},
expViolation: false,
},
"if managed fields matches template and base cert-manager annotations are present with certificate data but certificate data is nil, should return true": {
tmpl: &cmapi.CertificateSecretTemplate{
Annotations: map[string]string{"foo1": "bar1", "foo2": "bar2"},
},
secretManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:metadata": {
"f:annotations": {
"f:foo1": {},
"f:foo2": {},
"f:cert-manager.io/certificate-name": {},
"f:cert-manager.io/issuer-name": {},
"f:cert-manager.io/issuer-kind": {},
"f:cert-manager.io/issuer-group": {},
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
}
}}`),
}},
},
expReason: SecretTemplateMismatch,
expMessage: "Certificate's SecretTemplate doesn't match Secret",
expViolation: true,
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
gotReason, gotMessage, gotViolation := SecretTemplateMismatchesSecretManagedFields(fieldManager)(Input{
Certificate: &cmapi.Certificate{Spec: cmapi.CertificateSpec{SecretTemplate: test.tmpl}},
Secret: &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ManagedFields: test.secretManagedFields}, Data: test.secretData},
Secret: &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ManagedFields: test.secretManagedFields}, Data: map[string][]byte{}},
})
assert.Equal(t, test.expReason, gotReason, "unexpected reason")

5
internal/controller/certificates/policies/constants.go
View File

@ -52,10 +52,9 @@ const (
// SecretTemplate is not reflected on the target Secret, either by having
// extra, missing, or wrong Annotations or Labels.
SecretTemplateMismatch string = "SecretTemplateMismatch"
// SecretBaseLabelsMissing is a policy violation whereby the Secret is
// SecretManagedMetadataMismatch is a policy violation whereby the Secret is
// missing labels that should have been added by cert-manager
SecretBaseLabelsMissing string = "SecretBaseLabelsMissing"
SecretManagedMetadataMismatch string = "SecretManagedMetadataMismatch"
// AdditionalOutputFormatsMismatch is a policy violation whereby the
// Certificate's AdditionalOutputFormats is not reflected on the target

4
internal/controller/certificates/policies/policies.go
View File

@ -96,6 +96,9 @@ func NewReadinessPolicyChain(c clock.Clock) Chain {
// correctness of metadata and output formats of Certificate's Secrets.
func NewSecretPostIssuancePolicyChain(ownerRefEnabled bool, fieldManager string) Chain {
return Chain{
SecretBaseLabelsMismatch,
SecretCertificateDetailsAnnotationsMismatch,
SecretManagedLabelsAndAnnotationsManagedFieldsMismatch(fieldManager),
SecretTemplateMismatchesSecret,
SecretTemplateMismatchesSecretManagedFields(fieldManager),
SecretAdditionalOutputFormatsDataMismatch,
@ -103,7 +106,6 @@ func NewSecretPostIssuancePolicyChain(ownerRefEnabled bool, fieldManager string)
SecretOwnerReferenceManagedFieldMismatch(ownerRefEnabled, fieldManager),
SecretOwnerReferenceValueMismatch(ownerRefEnabled),
SecretKeystoreFormatMatchesSpec,
SecretBaseLabelsAreMissing,
}
}

241
pkg/controller/certificates/issuing/secret_manager_test.go
View File

@ -202,10 +202,15 @@ func Test_ensureSecretData(t *testing.T) {
FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:metadata": {
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {},
"f:foo": {},
"f:another-annotation": {}
},
"f:labels": {
"f:controller.cert-manager.io/fao": {},
"f:abc": {},
"f:another-label": {}
}
@ -241,9 +246,14 @@ func Test_ensureSecretData(t *testing.T) {
FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:metadata": {
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {},
"f:foo": {}
},
"f:labels": {
"f:controller.cert-manager.io/fao": {},
"f:abc": {}
}
}}`),
@ -280,9 +290,14 @@ func Test_ensureSecretData(t *testing.T) {
FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:metadata": {
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {},
"f:foo": {}
},
"f:labels": {
"f:controller.cert-manager.io/fao": {},
"f:abc": {}
}
}}`),
@ -452,10 +467,27 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{{
Manager: fieldManager,
FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:data": {
"f:tls-combined.pem": {},
"f:key.der": {}
}}`),
Raw: []byte(`
{
"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}
},
"f:data": {
"f:tls-combined.pem": {},
"f:key.der": {}
}
}`),
},
}},
},
@ -483,10 +515,27 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{{
Manager: fieldManager,
FieldsV1: &metav1.FieldsV1{
Raw: []byte(`{"f:data": {
"f:tls-combined.pem": {},
"f:key.der": {}
}}`),
Raw: []byte(`
{
"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}
},
"f:data": {
"f:tls-combined.pem": {},
"f:key.der": {}
}
}`),
},
}},
},
@ -513,10 +562,20 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`
{"f:metadata": {
{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}}}`),
"k:{\"uid\":\"uid-123\"}": {}
}
}}`),
}},
},
},
@ -540,10 +599,20 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`
{"f:metadata": {
{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}}}`),
"k:{\"uid\":\"uid-123\"}": {}
}
}}`),
}},
},
},
@ -579,10 +648,20 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`
{"f:metadata": {
{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-234\"}": {}
}}}`),
"k:{\"uid\":\"uid-123\"}": {}
}
}}`),
}},
},
},
@ -629,10 +708,20 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`
{"f:metadata": {
{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}}}`),
"k:{\"uid\":\"uid-123\"}": {}
}
}}`),
}},
},
},
@ -678,10 +767,20 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`
{"f:metadata": {
{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}}}`),
"k:{\"uid\":\"uid-123\"}": {}
}
}}`),
}},
},
},
@ -726,10 +825,20 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`
{"f:metadata": {
{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}}}`),
"k:{\"uid\":\"uid-123\"}": {}
}
}}`),
}},
},
},
@ -776,10 +885,20 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`
{"f:metadata": {
{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}}}`),
"k:{\"uid\":\"uid-123\"}": {}
}
}}`),
}},
},
},
@ -825,10 +944,20 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`
{"f:metadata": {
{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}}}`),
"k:{\"uid\":\"uid-123\"}": {}
}
}}`),
}},
},
},
@ -874,10 +1003,20 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`
{"f:metadata": {
{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}}}`),
"k:{\"uid\":\"uid-123\"}": {}
}
}}`),
}},
},
},
@ -922,10 +1061,20 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`
{"f:metadata": {
{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}}}`),
"k:{\"uid\":\"uid-123\"}": {}
}
}}`),
}},
},
},
@ -972,10 +1121,20 @@ func Test_ensureSecretData(t *testing.T) {
ManagedFields: []metav1.ManagedFieldsEntry{
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
Raw: []byte(`
{"f:metadata": {
{"f:metadata": {
"f:labels": {
"f:controller.cert-manager.io/fao": {}
},
"f:annotations": {
"f:cert-manager.io/common-name": {},
"f:cert-manager.io/alt-names": {},
"f:cert-manager.io/ip-sans": {},
"f:cert-manager.io/uri-sans": {}
},
"f:ownerReferences": {
"k:{\"uid\":\"uid-123\"}": {}
}}}`),
"k:{\"uid\":\"uid-123\"}": {}
}
}}`),
}},
},
},