Skip to content

Commit

Permalink
Merge pull request cert-manager#5703 from jetstack-bot/cherry-pick-56…
Browse files Browse the repository at this point in the history 
…60-to-release-1.11

[release-1.11] Ensures that certificate.spec.secretName and temporary private key Secrets are labelled
  • Loading branch information
@jetstack-bot
jetstack-bot authored Jan 9, 2023
2 parents 16e0f1a + d3fbe40 commit 6a848de
Show file tree
Hide file tree
Showing 9 changed files with 223 additions and 55 deletions.
27 changes: 27 additions & 0 deletions internal/controller/certificates/policies/checks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,10 @@ func SecretTemplateMismatchesSecretManagedFields(fieldManager string) Func {
managedAnnotations = managedAnnotations.Delete(k)
}

// Remove the base label 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 {
Expand Down Expand Up @@ -429,6 +433,29 @@ func SecretTemplateMismatchesSecretManagedFields(fieldManager string) Func {
}
}

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
}
}

// 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
}
if _, ok := input.Secret.Labels[cmapi.PartOfCertManagerControllerLabelKey]; !ok {
return SecretBaseLabelsMissing, fmt.Sprintf("missing base label %s", cmapi.PartOfCertManagerControllerLabelKey), true
}

return "", "", false
}

// SecretAdditionalOutputFormatsDataMismatch validates that the Secret has the
// expected Certificate AdditionalOutputFormats.
// Returns true (violation) if AdditionalOutputFormat(s) are present and any of
Expand Down
5 changes: 5 additions & 0 deletions internal/controller/certificates/policies/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ 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
// missing labels that should have been added by cert-manager
SecretBaseLabelsMissing string = "SecretBaseLabelsMissing"

// AdditionalOutputFormatsMismatch is a policy violation whereby the
// Certificate's AdditionalOutputFormats is not reflected on the target
// Secret, either by having extra, missing, or wrong values.
Expand Down
1 change: 1 addition & 0 deletions internal/controller/certificates/policies/policies.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ func NewSecretPostIssuancePolicyChain(ownerRefEnabled bool, fieldManager string)
SecretOwnerReferenceManagedFieldMismatch(ownerRefEnabled, fieldManager),
SecretOwnerReferenceValueMismatch(ownerRefEnabled),
SecretKeystoreFormatMatchesSpec,
SecretBaseLabelsAreMissing,
}
}

Expand Down
9 changes: 8 additions & 1 deletion pkg/apis/certmanager/v1/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,15 @@ limitations under the License.

package v1

// Common annotation keys added to resources.
const (

// Common label keys added to resources

// Label key that indicates that a resource is of interest to cert-manager controller
PartOfCertManagerControllerLabelKey = "controller.cert-manager.io/fao"

// Common annotation keys added to resources

// Annotation key for DNS subjectAltNames.
AltNamesAnnotationKey = "cert-manager.io/alt-names"

Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/certificates/issuing/internal/secret.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ func (s *SecretsManager) setValues(crt *cmapi.Certificate, secret *corev1.Secret
secret.Labels = make(map[string]string)
}

secret.Labels[cmapi.PartOfCertManagerControllerLabelKey] = "true"

if crt.Spec.SecretTemplate != nil {
for k, v := range crt.Spec.SecretTemplate.Labels {
secret.Labels[k] = v
Expand Down
94 changes: 75 additions & 19 deletions pkg/controller/certificates/issuing/internal/secret_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(make(map[string]string)).
WithLabels(map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: []byte("test-key"),
Expand Down Expand Up @@ -172,7 +172,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.AltNamesAnnotationKey: strings.Join(baseCertBundle.Cert.DNSNames, ","), cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(make(map[string]string)).
WithLabels(map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{corev1.TLSCertKey: baseCertBundle.CertBytes, corev1.TLSPrivateKeyKey: []byte("test-key"), cmmeta.TLSCAKey: []byte("test-ca")}).
WithType(corev1.SecretTypeTLS).
WithOwnerReferences(&applymetav1.OwnerReferenceApplyConfiguration{
Expand All @@ -191,15 +191,15 @@ func Test_SecretsManager(t *testing.T) {
expectedErr: false,
},

"if secret does exist, update existing Secret and leave custom annotations, with owner disabled": {
"if secret does exist, update existing Secret and leave custom annotations and labels, with owner disabled": {
certificateOptions: controllerpkg.CertificateOptions{EnableOwnerRef: false},
certificate: baseCertBundle.Certificate,
existingSecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: gen.DefaultTestNamespace,
Name: "output",
Annotations: map[string]string{"my-custom": "annotation"},
Labels: map[string]string{},
Labels: map[string]string{"my-custom": "label"},
},
Data: map[string][]byte{corev1.TLSCertKey: []byte("foo"), corev1.TLSPrivateKeyKey: []byte("foo"), cmmeta.TLSCAKey: []byte("foo")},
Type: corev1.SecretTypeTLS,
Expand All @@ -218,7 +218,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(make(map[string]string)).
WithLabels(map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: []byte("test-key"),
Expand All @@ -235,15 +235,15 @@ func Test_SecretsManager(t *testing.T) {
},
expectedErr: false,
},
"if secret does exist, update existing Secret and leave custom annotations, with owner enabled": {
"if secret does exist, update existing Secret and leave custom annotations and labels, with owner enabled": {
certificateOptions: controllerpkg.CertificateOptions{EnableOwnerRef: true},
certificate: baseCertBundle.Certificate,
existingSecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: gen.DefaultTestNamespace,
Name: "output",
Annotations: map[string]string{"my-custom": "annotation"},
Labels: map[string]string{},
Labels: map[string]string{"my-custom": "label"},
},
Data: map[string][]byte{corev1.TLSCertKey: []byte("foo"), corev1.TLSPrivateKeyKey: []byte("foo"), cmmeta.TLSCAKey: []byte("foo")},
Type: corev1.SecretTypeTLS,
Expand All @@ -263,7 +263,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(make(map[string]string)).
WithLabels(map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: []byte("test-key"),
Expand All @@ -286,15 +286,15 @@ func Test_SecretsManager(t *testing.T) {
expectedErr: false,
},

"if secret does not exist, create new Secret using the secret template": {
"if secret does exist, update existing Secret and add annotations set in secretTemplate": {
certificateOptions: controllerpkg.CertificateOptions{EnableOwnerRef: false},
certificate: baseCertWithSecretTemplate,
existingSecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: gen.DefaultTestNamespace,
Name: "output",
Annotations: map[string]string{"my-custom": "annotation"},
Labels: map[string]string{},
Labels: map[string]string{"my-custom": "label"},
},
Data: map[string][]byte{corev1.TLSCertKey: []byte("foo"), corev1.TLSPrivateKeyKey: []byte("foo"), cmmeta.TLSCAKey: []byte("foo")},
Type: corev1.SecretTypeTLS,
Expand All @@ -315,7 +315,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(map[string]string{"template": "label"}).
WithLabels(map[string]string{"template": "label", cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: []byte("test-key"),
Expand All @@ -333,7 +333,54 @@ func Test_SecretsManager(t *testing.T) {
expectedErr: false,
},

"if secret does exist, update existing Secret and add annotations set in secretTemplate": {
"if secret does exist, ensure that any missing base labels and annotations are added": {
certificateOptions: controllerpkg.CertificateOptions{EnableOwnerRef: false},
certificate: baseCertWithSecretTemplate,
existingSecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: gen.DefaultTestNamespace,
Name: "output",
Annotations: map[string]string{"my-custom": "annotation"},
Labels: map[string]string{"my-custom": "label"},
},
Data: map[string][]byte{corev1.TLSCertKey: []byte("foo"), corev1.TLSPrivateKeyKey: []byte("foo"), cmmeta.TLSCAKey: []byte("foo")},
Type: corev1.SecretTypeTLS,
},
secretData: SecretData{Certificate: baseCertBundle.CertBytes, CA: []byte("test-ca"), PrivateKey: []byte("test-key")},
applyFn: func(t *testing.T) testcoreclients.ApplyFn {
return func(_ context.Context, gotCnf *applycorev1.SecretApplyConfiguration, gotOpts metav1.ApplyOptions) (*corev1.Secret, error) {
expCnf := applycorev1.Secret("output", gen.DefaultTestNamespace).
WithAnnotations(
map[string]string{
"template": "annotation",
"my-custom": "annotation-from-secret",
cmapi.CertificateNameKey: "test", cmapi.IssuerGroupAnnotationKey: "foo.io",
cmapi.IssuerKindAnnotationKey: "Issuer", cmapi.IssuerNameAnnotationKey: "ca-issuer",

cmapi.CommonNameAnnotationKey: baseCertBundle.Cert.Subject.CommonName,
cmapi.AltNamesAnnotationKey: strings.Join(baseCertBundle.Cert.DNSNames, ","),
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(map[string]string{"template": "label", cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: []byte("test-key"),
cmmeta.TLSCAKey: []byte("test-ca"),
}).
WithType(corev1.SecretTypeTLS)
assert.Equal(t, expCnf, gotCnf)

expOpts := metav1.ApplyOptions{FieldManager: "cert-manager-test", Force: true}
assert.Equal(t, expOpts, gotOpts)

return nil, nil
}
},
expectedErr: false,
},

"if secret does not exist, create new Secret using the secret template": {
certificateOptions: controllerpkg.CertificateOptions{EnableOwnerRef: true},
certificate: baseCertWithSecretTemplate,
existingSecret: nil,
Expand All @@ -354,7 +401,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(map[string]string{"template": "label"}).
WithLabels(map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true", "template": "label"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: []byte("test-key"),
Expand Down Expand Up @@ -395,7 +442,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(make(map[string]string)).
WithLabels(map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: baseCertBundle.PrivateKeyBytes,
Expand Down Expand Up @@ -432,7 +479,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(make(map[string]string)).
WithLabels(map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: baseCertBundle.PrivateKeyBytes,
Expand Down Expand Up @@ -469,7 +516,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(make(map[string]string)).
WithLabels(map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: baseCertBundle.PrivateKeyBytes,
Expand Down Expand Up @@ -500,6 +547,9 @@ func Test_SecretsManager(t *testing.T) {
Annotations: map[string]string{
"my-custom": "annotation",
},
Labels: map[string]string{
"my-custom": "label",
},
},
Data: map[string][]byte{
corev1.TLSCertKey: []byte("foo"),
Expand All @@ -524,7 +574,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(make(map[string]string)).
WithLabels(map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: baseCertBundle.PrivateKeyBytes,
Expand Down Expand Up @@ -553,6 +603,9 @@ func Test_SecretsManager(t *testing.T) {
Annotations: map[string]string{
"my-custom": "annotation",
},
Labels: map[string]string{
"my-custom": "label",
},
},
Data: map[string][]byte{
corev1.TLSCertKey: []byte("foo"),
Expand All @@ -577,7 +630,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(make(map[string]string)).
WithLabels(map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: baseCertBundle.PrivateKeyBytes,
Expand Down Expand Up @@ -607,6 +660,9 @@ func Test_SecretsManager(t *testing.T) {
Annotations: map[string]string{
"my-custom": "annotation",
},
Labels: map[string]string{
"my-custom": "label",
},
},
Data: map[string][]byte{
corev1.TLSCertKey: []byte("foo"),
Expand All @@ -631,7 +687,7 @@ func Test_SecretsManager(t *testing.T) {
cmapi.IPSANAnnotationKey: strings.Join(utilpki.IPAddressesToString(baseCertBundle.Cert.IPAddresses), ","),
cmapi.URISANAnnotationKey: strings.Join(utilpki.URLsToString(baseCertBundle.Cert.URIs), ","),
}).
WithLabels(make(map[string]string)).
WithLabels(map[string]string{cmapi.PartOfCertManagerControllerLabelKey: "true"}).
WithData(map[string][]byte{
corev1.TLSCertKey: baseCertBundle.CertBytes,
corev1.TLSPrivateKeyKey: baseCertBundle.PrivateKeyBytes,
Expand Down
Loading

0 comments on commit 6a848de

Please sign in to comment.