Skip to content

Commit 48a4b85

Browse files
committed
Policy check ensures that cert.spec.secretName secret gets labelled
Secret will get labelled anytime when the associated Certificate is reconciled, even if it's outside issuance cycle Signed-off-by: irbekrm <irbekrm@gmail.com>
1 parent f75020b commit 48a4b85

4 files changed

Lines changed: 126 additions & 29 deletions

File tree

internal/controller/certificates/policies/checks.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,29 @@ func SecretTemplateMismatchesSecretManagedFields(fieldManager string) Func {
433433
}
434434
}
435435

436+
func SecretBaseLabelsAreMissing(input Input) (string, string, bool) {
437+
// If certificate has not been issued yet or is in invalid state, do not attempt to update metadata
438+
if len(input.Secret.Data[corev1.TLSCertKey]) > 0 {
439+
var err error
440+
_, err = pki.DecodeX509CertificateBytes(input.Secret.Data[corev1.TLSCertKey])
441+
if err != nil {
442+
// This case should never happen as it should always be caught by the
443+
// secretPublicKeysMatch function beforehand, but handle it just in case.
444+
return InvalidCertificate, fmt.Sprintf("Failed to decode stored certificate: %v", err), true
445+
}
446+
}
447+
448+
// check if Secret has the base labels. Currently there is only one base label
449+
if input.Secret.Labels == nil {
450+
return SecretBaseLabelsMissing, fmt.Sprintf("missing base label %s", cmapi.PartOfCertManagerControllerLabelKey), true
451+
}
452+
if _, ok := input.Secret.Labels[cmapi.PartOfCertManagerControllerLabelKey]; !ok {
453+
return SecretBaseLabelsMissing, fmt.Sprintf("missing base label %s", cmapi.PartOfCertManagerControllerLabelKey), true
454+
}
455+
456+
return "", "", false
457+
}
458+
436459
// SecretAdditionalOutputFormatsDataMismatch validates that the Secret has the
437460
// expected Certificate AdditionalOutputFormats.
438461
// Returns true (violation) if AdditionalOutputFormat(s) are present and any of

internal/controller/certificates/policies/constants.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ const (
4848
// SecretTemplate is not reflected on the target Secret, either by having
4949
// extra, missing, or wrong Annotations or Labels.
5050
SecretTemplateMismatch string = "SecretTemplateMismatch"
51+
52+
// SecretBaseLabelsMissing is a policy violation whereby the Secret is
53+
// missing labels that should have been added by cert-manager
54+
SecretBaseLabelsMissing string = "SecretBaseLabelsMissing"
55+
5156
// AdditionalOutputFormatsMismatch is a policy violation whereby the
5257
// Certificate's AdditionalOutputFormats is not reflected on the target
5358
// Secret, either by having extra, missing, or wrong values.

internal/controller/certificates/policies/policies.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ func NewSecretPostIssuancePolicyChain(ownerRefEnabled bool, fieldManager string)
101101
SecretOwnerReferenceManagedFieldMismatch(ownerRefEnabled, fieldManager),
102102
SecretOwnerReferenceValueMismatch(ownerRefEnabled),
103103
SecretKeystoreFormatMatchesSpec,
104+
SecretBaseLabelsAreMissing,
104105
}
105106
}
106107

pkg/controller/certificates/issuing/secret_manager_test.go

Lines changed: 97 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -257,13 +257,14 @@ func Test_ensureSecretData(t *testing.T) {
257257
},
258258
expectedAction: true,
259259
},
260-
"if Certificate exists in a false Issuing condition, Secret exists and matches the SecretTemplate with the correct managed fields, should do nothing": {
260+
"if Certificate exists in a false Issuing condition, Secret exists and matches the SecretTemplate with the correct managed fields and base labels, should do nothing": {
261261
key: "test-namespace/test-name",
262262
cert: &cmapi.Certificate{
263263
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
264264
Spec: cmapi.CertificateSpec{
265-
SecretName: "test-secret",
266-
SecretTemplate: &cmapi.CertificateSecretTemplate{Annotations: map[string]string{"foo": "bar"}, Labels: map[string]string{"abc": "123"}},
265+
SecretName: "test-secret",
266+
SecretTemplate: &cmapi.CertificateSecretTemplate{Annotations: map[string]string{"foo": "bar"},
267+
Labels: map[string]string{"abc": "123"}},
267268
},
268269
Status: cmapi.CertificateStatus{
269270
Conditions: []cmapi.CertificateCondition{{Type: cmapi.CertificateConditionIssuing, Status: cmmeta.ConditionFalse}},
@@ -272,7 +273,8 @@ func Test_ensureSecretData(t *testing.T) {
272273
secret: &corev1.Secret{
273274
ObjectMeta: metav1.ObjectMeta{
274275
Namespace: "test-namespace", Name: "test-secret",
275-
Annotations: map[string]string{"foo": "bar"}, Labels: map[string]string{"abc": "123"},
276+
Annotations: map[string]string{"foo": "bar"},
277+
Labels: map[string]string{"abc": "123", cmapi.PartOfCertManagerControllerLabelKey: "true"},
276278
ManagedFields: []metav1.ManagedFieldsEntry{{
277279
Manager: fieldManager,
278280
FieldsV1: &metav1.FieldsV1{
@@ -295,6 +297,56 @@ func Test_ensureSecretData(t *testing.T) {
295297
expectedAction: false,
296298
},
297299
"if Certificate exists in a false Issuing condition, Secret exists but does not match SecretTemplate, should apply the Labels and Annotations": {
300+
key: "test-namespace/test-name",
301+
cert: &cmapi.Certificate{
302+
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
303+
Spec: cmapi.CertificateSpec{
304+
SecretName: "test-secret",
305+
SecretTemplate: &cmapi.CertificateSecretTemplate{Annotations: map[string]string{"foo": "bar"}, Labels: map[string]string{"abc": "123"}},
306+
},
307+
Status: cmapi.CertificateStatus{
308+
Conditions: []cmapi.CertificateCondition{
309+
{Type: cmapi.CertificateConditionIssuing, Status: cmmeta.ConditionFalse},
310+
{Type: cmapi.CertificateConditionIssuing, Status: cmmeta.ConditionFalse},
311+
},
312+
},
313+
},
314+
secret: &corev1.Secret{
315+
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret",
316+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}},
317+
Data: map[string][]byte{
318+
"tls.crt": cert,
319+
"tls.key": pk,
320+
},
321+
},
322+
expectedAction: true,
323+
},
324+
"if Certificate exists in a false Issuing condition, Secret exists but is missing the required label, apply the label": {
325+
key: "test-namespace/test-name",
326+
cert: &cmapi.Certificate{
327+
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
328+
Spec: cmapi.CertificateSpec{
329+
SecretName: "test-secret",
330+
SecretTemplate: &cmapi.CertificateSecretTemplate{Annotations: map[string]string{"foo": "bar"}, Labels: map[string]string{"abc": "123"}},
331+
},
332+
Status: cmapi.CertificateStatus{
333+
Conditions: []cmapi.CertificateCondition{
334+
{Type: cmapi.CertificateConditionIssuing, Status: cmmeta.ConditionFalse},
335+
{Type: cmapi.CertificateConditionIssuing, Status: cmmeta.ConditionFalse},
336+
},
337+
},
338+
},
339+
secret: &corev1.Secret{
340+
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret",
341+
Labels: map[string]string{"foo": "bar"}},
342+
Data: map[string][]byte{
343+
"tls.crt": cert,
344+
"tls.key": pk,
345+
},
346+
},
347+
expectedAction: true,
348+
},
349+
"if Certificate exists in a false Issuing condition, Secret exists with some labels, but is missing the required label, apply the label": {
298350
key: "test-namespace/test-name",
299351
cert: &cmapi.Certificate{
300352
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-name"},
@@ -330,7 +382,8 @@ func Test_ensureSecretData(t *testing.T) {
330382
},
331383
},
332384
secret: &corev1.Secret{
333-
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret"},
385+
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret",
386+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}},
334387
Data: map[string][]byte{
335388
"tls.crt": cert,
336389
"tls.key": pk,
@@ -350,7 +403,8 @@ func Test_ensureSecretData(t *testing.T) {
350403
},
351404
},
352405
secret: &corev1.Secret{
353-
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret"},
406+
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret",
407+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}},
354408
Data: map[string][]byte{
355409
"tls.crt": cert,
356410
"tls.key": pk,
@@ -371,7 +425,8 @@ func Test_ensureSecretData(t *testing.T) {
371425
},
372426
},
373427
secret: &corev1.Secret{
374-
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret"},
428+
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret",
429+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}},
375430
Data: map[string][]byte{
376431
"tls.crt": cert,
377432
"tls.key": pk,
@@ -393,6 +448,7 @@ func Test_ensureSecretData(t *testing.T) {
393448
},
394449
secret: &corev1.Secret{
395450
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret",
451+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
396452
ManagedFields: []metav1.ManagedFieldsEntry{{
397453
Manager: fieldManager,
398454
FieldsV1: &metav1.FieldsV1{
@@ -423,6 +479,7 @@ func Test_ensureSecretData(t *testing.T) {
423479
},
424480
secret: &corev1.Secret{
425481
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret",
482+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
426483
ManagedFields: []metav1.ManagedFieldsEntry{{
427484
Manager: fieldManager,
428485
FieldsV1: &metav1.FieldsV1{
@@ -452,12 +509,13 @@ func Test_ensureSecretData(t *testing.T) {
452509
},
453510
secret: &corev1.Secret{
454511
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret",
512+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
455513
ManagedFields: []metav1.ManagedFieldsEntry{
456514
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
457515
Raw: []byte(`
458-
{"f:metadata": {
516+
{"f:metadata": {
459517
"f:ownerReferences": {
460-
"k:{\"uid\":\"uid-123\"}": {}
518+
"k:{\"uid\":\"uid-123\"}": {}
461519
}}}`),
462520
}},
463521
},
@@ -475,15 +533,16 @@ func Test_ensureSecretData(t *testing.T) {
475533
},
476534
secret: &corev1.Secret{
477535
ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "test-secret",
536+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
478537
OwnerReferences: []metav1.OwnerReference{
479538
{APIVersion: "cert-manager.io/v1", Kind: "Certificate", Name: "test-name", UID: types.UID("uid-123"), Controller: pointer.Bool(true), BlockOwnerDeletion: pointer.Bool(true)},
480539
},
481540
ManagedFields: []metav1.ManagedFieldsEntry{
482541
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
483542
Raw: []byte(`
484-
{"f:metadata": {
543+
{"f:metadata": {
485544
"f:ownerReferences": {
486-
"k:{\"uid\":\"uid-123\"}": {}
545+
"k:{\"uid\":\"uid-123\"}": {}
487546
}}}`),
488547
}},
489548
},
@@ -513,15 +572,16 @@ func Test_ensureSecretData(t *testing.T) {
513572
cmapi.IssuerKindAnnotationKey: "IssuerKind",
514573
cmapi.IssuerGroupAnnotationKey: "group.example.com",
515574
},
575+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
516576
OwnerReferences: []metav1.OwnerReference{
517577
{APIVersion: "cert-manager.io/v1", Kind: "Certificate", Name: "test-name", UID: types.UID("uid-234"), Controller: pointer.Bool(true), BlockOwnerDeletion: pointer.Bool(true)},
518578
},
519579
ManagedFields: []metav1.ManagedFieldsEntry{
520580
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
521581
Raw: []byte(`
522-
{"f:metadata": {
582+
{"f:metadata": {
523583
"f:ownerReferences": {
524-
"k:{\"uid\":\"uid-234\"}": {}
584+
"k:{\"uid\":\"uid-234\"}": {}
525585
}}}`),
526586
}},
527587
},
@@ -562,15 +622,16 @@ func Test_ensureSecretData(t *testing.T) {
562622
cmapi.IssuerKindAnnotationKey: "IssuerKind",
563623
cmapi.IssuerGroupAnnotationKey: "group.example.com",
564624
},
625+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
565626
OwnerReferences: []metav1.OwnerReference{
566627
{APIVersion: "cert-manager.io/v1", Kind: "Certificate", Name: "test-name", UID: types.UID("uid-123"), Controller: pointer.Bool(true), BlockOwnerDeletion: pointer.Bool(true)},
567628
},
568629
ManagedFields: []metav1.ManagedFieldsEntry{
569630
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
570631
Raw: []byte(`
571-
{"f:metadata": {
632+
{"f:metadata": {
572633
"f:ownerReferences": {
573-
"k:{\"uid\":\"uid-123\"}": {}
634+
"k:{\"uid\":\"uid-123\"}": {}
574635
}}}`),
575636
}},
576637
},
@@ -610,15 +671,16 @@ func Test_ensureSecretData(t *testing.T) {
610671
cmapi.IssuerKindAnnotationKey: "IssuerKind",
611672
cmapi.IssuerGroupAnnotationKey: "group.example.com",
612673
},
674+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
613675
OwnerReferences: []metav1.OwnerReference{
614676
{APIVersion: "cert-manager.io/v1", Kind: "Certificate", Name: "test-name", UID: types.UID("uid-123"), Controller: pointer.Bool(true), BlockOwnerDeletion: pointer.Bool(true)},
615677
},
616678
ManagedFields: []metav1.ManagedFieldsEntry{
617679
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
618680
Raw: []byte(`
619-
{"f:metadata": {
681+
{"f:metadata": {
620682
"f:ownerReferences": {
621-
"k:{\"uid\":\"uid-123\"}": {}
683+
"k:{\"uid\":\"uid-123\"}": {}
622684
}}}`),
623685
}},
624686
},
@@ -657,15 +719,16 @@ func Test_ensureSecretData(t *testing.T) {
657719
cmapi.IssuerKindAnnotationKey: "IssuerKind",
658720
cmapi.IssuerGroupAnnotationKey: "group.example.com",
659721
},
722+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
660723
OwnerReferences: []metav1.OwnerReference{
661724
{APIVersion: "cert-manager.io/v1", Kind: "Certificate", Name: "test-name", UID: types.UID("uid-123"), Controller: pointer.Bool(true), BlockOwnerDeletion: pointer.Bool(true)},
662725
},
663726
ManagedFields: []metav1.ManagedFieldsEntry{
664727
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
665728
Raw: []byte(`
666-
{"f:metadata": {
729+
{"f:metadata": {
667730
"f:ownerReferences": {
668-
"k:{\"uid\":\"uid-123\"}": {}
731+
"k:{\"uid\":\"uid-123\"}": {}
669732
}}}`),
670733
}},
671734
},
@@ -706,15 +769,16 @@ func Test_ensureSecretData(t *testing.T) {
706769
cmapi.IssuerKindAnnotationKey: "IssuerKind",
707770
cmapi.IssuerGroupAnnotationKey: "group.example.com",
708771
},
772+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
709773
OwnerReferences: []metav1.OwnerReference{
710774
{APIVersion: "cert-manager.io/v1", Kind: "Certificate", Name: "test-name", UID: types.UID("uid-123"), Controller: pointer.Bool(true), BlockOwnerDeletion: pointer.Bool(true)},
711775
},
712776
ManagedFields: []metav1.ManagedFieldsEntry{
713777
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
714778
Raw: []byte(`
715-
{"f:metadata": {
779+
{"f:metadata": {
716780
"f:ownerReferences": {
717-
"k:{\"uid\":\"uid-123\"}": {}
781+
"k:{\"uid\":\"uid-123\"}": {}
718782
}}}`),
719783
}},
720784
},
@@ -754,15 +818,16 @@ func Test_ensureSecretData(t *testing.T) {
754818
cmapi.IssuerKindAnnotationKey: "IssuerKind",
755819
cmapi.IssuerGroupAnnotationKey: "group.example.com",
756820
},
821+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
757822
OwnerReferences: []metav1.OwnerReference{
758823
{APIVersion: "cert-manager.io/v1", Kind: "Certificate", Name: "test-name", UID: types.UID("uid-123"), Controller: pointer.Bool(true), BlockOwnerDeletion: pointer.Bool(true)},
759824
},
760825
ManagedFields: []metav1.ManagedFieldsEntry{
761826
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
762827
Raw: []byte(`
763-
{"f:metadata": {
828+
{"f:metadata": {
764829
"f:ownerReferences": {
765-
"k:{\"uid\":\"uid-123\"}": {}
830+
"k:{\"uid\":\"uid-123\"}": {}
766831
}}}`),
767832
}},
768833
},
@@ -802,15 +867,16 @@ func Test_ensureSecretData(t *testing.T) {
802867
cmapi.IssuerKindAnnotationKey: "IssuerKind",
803868
cmapi.IssuerGroupAnnotationKey: "group.example.com",
804869
},
870+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
805871
OwnerReferences: []metav1.OwnerReference{
806872
{APIVersion: "cert-manager.io/v1", Kind: "Certificate", Name: "test-name", UID: types.UID("uid-123"), Controller: pointer.Bool(true), BlockOwnerDeletion: pointer.Bool(true)},
807873
},
808874
ManagedFields: []metav1.ManagedFieldsEntry{
809875
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
810876
Raw: []byte(`
811-
{"f:metadata": {
877+
{"f:metadata": {
812878
"f:ownerReferences": {
813-
"k:{\"uid\":\"uid-123\"}": {}
879+
"k:{\"uid\":\"uid-123\"}": {}
814880
}}}`),
815881
}},
816882
},
@@ -849,15 +915,16 @@ func Test_ensureSecretData(t *testing.T) {
849915
cmapi.IssuerKindAnnotationKey: "IssuerKind",
850916
cmapi.IssuerGroupAnnotationKey: "group.example.com",
851917
},
918+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
852919
OwnerReferences: []metav1.OwnerReference{
853920
{APIVersion: "cert-manager.io/v1", Kind: "Certificate", Name: "test-name", UID: types.UID("uid-123"), Controller: pointer.Bool(true), BlockOwnerDeletion: pointer.Bool(true)},
854921
},
855922
ManagedFields: []metav1.ManagedFieldsEntry{
856923
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
857924
Raw: []byte(`
858-
{"f:metadata": {
925+
{"f:metadata": {
859926
"f:ownerReferences": {
860-
"k:{\"uid\":\"uid-123\"}": {}
927+
"k:{\"uid\":\"uid-123\"}": {}
861928
}}}`),
862929
}},
863930
},
@@ -898,15 +965,16 @@ func Test_ensureSecretData(t *testing.T) {
898965
cmapi.IssuerKindAnnotationKey: "IssuerKind",
899966
cmapi.IssuerGroupAnnotationKey: "group.example.com",
900967
},
968+
Labels: map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"},
901969
OwnerReferences: []metav1.OwnerReference{
902970
{APIVersion: "cert-manager.io/v1", Kind: "Certificate", Name: "test-name", UID: types.UID("uid-123"), Controller: pointer.Bool(true), BlockOwnerDeletion: pointer.Bool(true)},
903971
},
904972
ManagedFields: []metav1.ManagedFieldsEntry{
905973
{Manager: fieldManager, FieldsV1: &metav1.FieldsV1{
906974
Raw: []byte(`
907-
{"f:metadata": {
975+
{"f:metadata": {
908976
"f:ownerReferences": {
909-
"k:{\"uid\":\"uid-123\"}": {}
977+
"k:{\"uid\":\"uid-123\"}": {}
910978
}}}`),
911979
}},
912980
},

0 commit comments

Comments
 (0)