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

Remove getting secret from lister in matches spec func

Browse Source 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
This commit is contained in:
JoshVanL 2019-10-03 09:47:41 +01:00
parent dc7cc388e1
commit 7d615ff8e4
4 changed files with 21 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pkg/controller/certificates/BUILD.bazel
View File

@ -55,7 +55,6 @@ go_test(
"//pkg/util:go_default_library",
"//pkg/util/pki:go_default_library",
"//test/unit/gen:go_default_library",
"//test/unit/listers:go_default_library",
"@io_k8s_api//core/v1:go_default_library",
"@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
"@io_k8s_apimachinery//pkg/runtime:go_default_library",

15
pkg/controller/certificates/sync.go
View File

@ -102,7 +102,12 @@ func (c *certificateRequestManager) updateCertificateStatus(ctx context.Context,
var matches bool
var matchErrs []string
if key != nil && cert != nil {
matches, matchErrs = certificateMatchesSpec(crt, key, cert, c.secretLister)
secret, err := c.secretLister.Secrets(crt.Namespace).Get(crt.Spec.SecretName)
if err != nil {
return err
}
matches, matchErrs = certificateMatchesSpec(crt, key, cert, secret)
}
isTempCert := isTemporaryCertificate(cert)
@ -255,7 +260,7 @@ func (c *certificateRequestManager) processCertificate(ctx context.Context, crt
// a valid partner to the stored certificate.
var matchErrs []string
dbg.Info("checking if existing certificate stored in Secret resource is not expiring soon and matches certificate spec")
needsIssue, matchErrs, err = c.certificateRequiresIssuance(ctx, crt, existingKey, existingCert)
needsIssue, matchErrs, err = c.certificateRequiresIssuance(ctx, crt, existingKey, existingCert, existingSecret)
if err != nil && !errors.IsInvalidData(err) {
return err
}
@ -330,7 +335,7 @@ func (c *certificateRequestManager) processCertificate(ctx context.Context, crt
}
// We don't issue a temporary certificate if the existing stored
// certificate already 'matches', even if it isn't a temporary certificate.
matches, _ := certificateMatchesSpec(crt, privateKey, existingX509Cert, c.secretLister)
matches, _ := certificateMatchesSpec(crt, privateKey, existingX509Cert, existingSecret)
if !matches {
log.Info("existing certificate fields do not match certificate spec, issuing temporary certificate")
return c.issueTemporaryCertificate(ctx, existingSecret, crt, existingKey)
@ -556,7 +561,7 @@ func (c *certificateRequestManager) issueTemporaryCertificate(ctx context.Contex
return nil
}
func (c *certificateRequestManager) certificateRequiresIssuance(ctx context.Context, crt *cmapi.Certificate, keyBytes, certBytes []byte) (bool, []string, error) {
func (c *certificateRequestManager) certificateRequiresIssuance(ctx context.Context, crt *cmapi.Certificate, keyBytes, certBytes []byte, secret *corev1.Secret) (bool, []string, error) {
key, err := pki.DecodePrivateKeyBytes(keyBytes)
if err != nil {
return false, nil, err
@ -568,7 +573,7 @@ func (c *certificateRequestManager) certificateRequiresIssuance(ctx context.Cont
if isTemporaryCertificate(cert) {
return true, nil, nil
}
matches, matchErrs := certificateMatchesSpec(crt, key, cert, c.secretLister)
matches, matchErrs := certificateMatchesSpec(crt, key, cert, secret)
if !matches {
return true, matchErrs, nil
}

8
pkg/controller/certificates/util.go
View File

@ -27,6 +27,7 @@ import (
"time"
"github.com/kr/pretty"
corev1 "k8s.io/api/core/v1"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
corelisters "k8s.io/client-go/listers/core/v1"
"k8s.io/client-go/tools/cache"
@ -75,7 +76,7 @@ func certificateGetter(lister cmlisters.CertificateLister) func(namespace, name
var keyFunc = controllerpkg.KeyFunc
func certificateMatchesSpec(crt *v1alpha2.Certificate, key crypto.Signer, cert *x509.Certificate, secretLister corelisters.SecretLister) (bool, []string) {
func certificateMatchesSpec(crt *v1alpha2.Certificate, key crypto.Signer, cert *x509.Certificate, secret *corev1.Secret) (bool, []string) {
var errs []string
// TODO: add checks for KeySize, KeyAlgorithm fields
@ -116,11 +117,6 @@ func certificateMatchesSpec(crt *v1alpha2.Certificate, key crypto.Signer, cert *
errs = append(errs, fmt.Sprintf("IP addresses on TLS certificate not up to date: %q", pki.IPAddressesToString(cert.IPAddresses)))
}
// get a copy of the current secret resource
// Note that we already know that it exists, no need to check for errors
// TODO: Refactor so that the secret is passed as argument?
secret, err := secretLister.Secrets(crt.Namespace).Get(crt.Spec.SecretName)
if secret.Annotations == nil {
secret.Annotations = make(map[string]string)
}

34
pkg/controller/certificates/util_test.go
View File

@ -27,7 +27,6 @@ import (
cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1"
"github.com/jetstack/cert-manager/pkg/util"
"github.com/jetstack/cert-manager/test/unit/gen"
"github.com/jetstack/cert-manager/test/unit/listers"
)
func TestCertificateMatchesSpec(t *testing.T) {
@ -55,7 +54,7 @@ func TestCertificateMatchesSpec(t *testing.T) {
type testT struct {
cb cryptoBundle
certificate *cmapi.Certificate
fakeLister *listers.FakeSecretLister
secret *corev1.Secret
expMatch bool
expErrors []string
}
@ -64,11 +63,8 @@ func TestCertificateMatchesSpec(t *testing.T) {
"if all match then return matched": {
cb: exampleBundle,
certificate: exampleBundle.certificate,
fakeLister: listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
listers.SetFakeSecretNamespaceListerGet(secret, nil),
),
expMatch: true,
expErrors: nil,
expMatch: true,
expErrors: nil,
},
"if no common name but DNS and all match then return matched": {
@ -78,9 +74,6 @@ func TestCertificateMatchesSpec(t *testing.T) {
certificate: gen.CertificateFrom(exampleBundle.certificate,
gen.SetCertificateCommonName(""),
),
fakeLister: listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
listers.SetFakeSecretNamespaceListerGet(secret, nil),
),
expMatch: true,
expErrors: nil,
},
@ -91,11 +84,8 @@ func TestCertificateMatchesSpec(t *testing.T) {
gen.SetCertificateCommonName(""),
)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
fakeLister: listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
listers.SetFakeSecretNamespaceListerGet(secret, nil),
),
expMatch: true,
expErrors: nil,
expMatch: true,
expErrors: nil,
},
"if common name random string but requested common name in DNS names then match": {
@ -104,11 +94,8 @@ func TestCertificateMatchesSpec(t *testing.T) {
gen.SetCertificateCommonName("foobar"),
)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
fakeLister: listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
listers.SetFakeSecretNamespaceListerGet(secret, nil),
),
expMatch: true,
expErrors: nil,
expMatch: true,
expErrors: nil,
},
"if common name random string and no request DNS names but request common name then error missing common name": {
@ -117,10 +104,7 @@ func TestCertificateMatchesSpec(t *testing.T) {
gen.SetCertificateCommonName("foobar"),
)),
certificate: gen.CertificateFrom(exampleBundle.certificate),
fakeLister: listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
listers.SetFakeSecretNamespaceListerGet(secret, nil),
),
expMatch: false,
expMatch: false,
expErrors: []string{
`Common Name on TLS certificate not up to date ("common.name.example.com"): [foobar]`,
"DNS names on TLS certificate not up to date: []",
@ -129,7 +113,7 @@ func TestCertificateMatchesSpec(t *testing.T) {
} {
t.Run(name, func(t *testing.T) {
match, errs := certificateMatchesSpec(
test.certificate, test.cb.privateKey, test.cb.cert, test.fakeLister)
test.certificate, test.cb.privateKey, test.cb.cert, secret)
if match != test.expMatch {
t.Errorf("got unexpected match bool, exp=%t got=%t",