diff --git a/pkg/api/util/names.go b/pkg/api/util/names.go index 2f3e2a0e7..e74cdacf4 100644 --- a/pkg/api/util/names.go +++ b/pkg/api/util/names.go @@ -22,30 +22,35 @@ import ( "hash/fnv" "regexp" - - cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" ) -func ComputeCertificateRequestName(crt *cmapi.Certificate) (string, error) { - specBytes, err := json.Marshal(crt.Spec) +func ComputeName(prefix string, obj interface{}) (string, error) { + objectBytes, err := json.Marshal(obj) if err != nil { return "", err } hashF := fnv.New32() - _, err = hashF.Write(specBytes) + _, err = hashF.Write(objectBytes) if err != nil { return "", err } - crtName := crt.Name - if len(crtName) >= 52 { + // we're shortening to stay under 64 as we use this in services + // and pods down the road for ACME resources. + prefix = DNSSafeShortenTo52Characters(prefix) + + return fmt.Sprintf("%s-%d", prefix, hashF.Sum32()), nil +} + +func DNSSafeShortenTo52Characters(in string) string { + if len(in) >= 52 { // shorten the cert name to 52 chars to ensure the total length of the name // also shorten the 52 char string to the last non-symbol character // is less than or equal to 64 characters - validCharIndexes := regexp.MustCompile(`[a-zA-Z\d]`).FindAllStringIndex(fmt.Sprintf("%.52s", crtName), -1) - crtName = crtName[:validCharIndexes[len(validCharIndexes)-1][1]] + validCharIndexes := regexp.MustCompile(`[a-zA-Z\d]`).FindAllStringIndex(fmt.Sprintf("%.52s", in), -1) + in = in[:validCharIndexes[len(validCharIndexes)-1][1]] } - return fmt.Sprintf("%s-%d", crtName, hashF.Sum32()), nil + return in } diff --git a/pkg/api/util/names_test.go b/pkg/api/util/names_test.go index 556698ed8..1e00d4cf6 100644 --- a/pkg/api/util/names_test.go +++ b/pkg/api/util/names_test.go @@ -24,7 +24,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation" ) -func TestComputeCertificateRequestName(t *testing.T) { +func TestComputeName(t *testing.T) { type args struct { crt *cmapi.Certificate } @@ -97,16 +97,16 @@ func TestComputeCertificateRequestName(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ComputeCertificateRequestName(tt.args.crt) + got, err := ComputeName(tt.args.crt.Name, tt.args.crt.Spec) if (err != nil) != tt.wantErr { - t.Errorf("ComputeCertificateRequestName() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("ComputeName() error = %v, wantErr %v", err, tt.wantErr) return } if got != tt.want { - t.Errorf("ComputeCertificateRequestName() = %v, want %v", got, tt.want) + t.Errorf("ComputeName() = %v, want %v", got, tt.want) } if len(validation.IsQualifiedName(got)) != 0 { - t.Errorf("ComputeCertificateRequestName() = %v is not DNS-1123 valid", got) + t.Errorf("ComputeName() = %v is not DNS-1123 valid", got) } }) } diff --git a/pkg/controller/acmeorders/BUILD.bazel b/pkg/controller/acmeorders/BUILD.bazel index 6d345dee3..4382434f3 100644 --- a/pkg/controller/acmeorders/BUILD.bazel +++ b/pkg/controller/acmeorders/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "//pkg/acme:go_default_library", "//pkg/acme/accounts:go_default_library", "//pkg/acme/client:go_default_library", + "//pkg/api/util:go_default_library", "//pkg/apis/acme/v1:go_default_library", "//pkg/apis/certmanager/v1:go_default_library", "//pkg/client/clientset/versioned:go_default_library", diff --git a/pkg/controller/acmeorders/util.go b/pkg/controller/acmeorders/util.go index 8b4697ef1..f30e16935 100644 --- a/pkg/controller/acmeorders/util.go +++ b/pkg/controller/acmeorders/util.go @@ -20,12 +20,13 @@ import ( "context" "encoding/json" "fmt" - "github.com/jetstack/cert-manager/pkg/acme" "hash/fnv" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/jetstack/cert-manager/pkg/acme" acmecl "github.com/jetstack/cert-manager/pkg/acme/client" + "github.com/jetstack/cert-manager/pkg/api/util" cmacme "github.com/jetstack/cert-manager/pkg/apis/acme/v1" cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" "github.com/jetstack/cert-manager/pkg/controller/acmeorders/selectors" @@ -64,7 +65,7 @@ func buildChallenge(ctx context.Context, cl acmecl.Interface, issuer cmapi.Gener return nil, err } - chName, err := buildChallengeName(o.Name, *chSpec) + chName, err := util.ComputeName(o.Name, chSpec) if err != nil { return nil, err } @@ -80,15 +81,6 @@ func buildChallenge(ctx context.Context, cl acmecl.Interface, issuer cmapi.Gener }, nil } -func buildChallengeName(orderName string, chSpec cmacme.ChallengeSpec) (string, error) { - hash, err := hashChallenge(chSpec) - if err != nil { - return "", err - } - - return fmt.Sprintf("%s-%d", orderName, hash), nil -} - func hashChallenge(spec cmacme.ChallengeSpec) (uint32, error) { specBytes, err := json.Marshal(spec) if err != nil { diff --git a/pkg/controller/certificaterequests/acme/acme.go b/pkg/controller/certificaterequests/acme/acme.go index 234df746c..29bfee2a6 100644 --- a/pkg/controller/certificaterequests/acme/acme.go +++ b/pkg/controller/certificaterequests/acme/acme.go @@ -19,9 +19,7 @@ package acme import ( "context" "crypto/x509" - "encoding/json" "fmt" - "hash/fnv" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -208,7 +206,11 @@ func buildOrder(cr *v1.CertificateRequest, csr *x509.CertificateRequest) (*cmacm CommonName: csr.Subject.CommonName, DNSNames: csr.DNSNames, } - hash, err := hashOrder(spec) + + computeNameSpec := spec.DeepCopy() + // create a deep copy of the OrderSpec so we can overwrite the Request field + computeNameSpec.Request = nil + name, err := apiutil.ComputeName(cr.Name, computeNameSpec) if err != nil { return nil, err } @@ -218,7 +220,7 @@ func buildOrder(cr *v1.CertificateRequest, csr *x509.CertificateRequest) (*cmacm // the hyphen. return &cmacme.Order{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%.52s-%d", cr.Name, hash), + Name: name, Namespace: cr.Namespace, Labels: cr.Labels, Annotations: cr.Annotations, @@ -229,21 +231,3 @@ func buildOrder(cr *v1.CertificateRequest, csr *x509.CertificateRequest) (*cmacm Spec: spec, }, nil } - -func hashOrder(orderSpec cmacme.OrderSpec) (uint32, error) { - // create a shallow copy of the OrderSpec so we can overwrite the Request field - orderSpec.Request = nil - - orderSpecBytes, err := json.Marshal(orderSpec) - if err != nil { - return 0, err - } - - hashF := fnv.New32() - _, err = hashF.Write(orderSpecBytes) - if err != nil { - return 0, err - } - - return hashF.Sum32(), nil -} diff --git a/pkg/controller/certificates/internal/test/test.go b/pkg/controller/certificates/internal/test/test.go index 3561ec703..65435c9f7 100644 --- a/pkg/controller/certificates/internal/test/test.go +++ b/pkg/controller/certificates/internal/test/test.go @@ -84,7 +84,7 @@ func createCryptoBundle(originalCert *cmapi.Certificate, fixedClock *fakeclock.F if crt.Spec.PrivateKey == nil { crt.Spec.PrivateKey = &cmapi.CertificatePrivateKey{} } - reqName, err := apiutil.ComputeCertificateRequestName(crt) + reqName, err := apiutil.ComputeName(crt.Name, crt.Spec) if err != nil { return nil, err } diff --git a/pkg/controller/certificates/requestmanager/requestmanager_controller.go b/pkg/controller/certificates/requestmanager/requestmanager_controller.go index d1d334526..3334d21b2 100644 --- a/pkg/controller/certificates/requestmanager/requestmanager_controller.go +++ b/pkg/controller/certificates/requestmanager/requestmanager_controller.go @@ -322,7 +322,7 @@ func (c *controller) createNewCertificateRequest(ctx context.Context, crt *cmapi cr := &cmapi.CertificateRequest{ ObjectMeta: metav1.ObjectMeta{ Namespace: crt.Namespace, - GenerateName: crt.Name + "-", + GenerateName: apiutil.DNSSafeShortenTo52Characters(crt.Name) + "-", Annotations: annotations, Labels: crt.Labels, OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(crt, certificateGvk)}, diff --git a/pkg/controller/certificates/requestmanager/util_test.go b/pkg/controller/certificates/requestmanager/util_test.go index 7b7e7a1ed..58310d2c9 100644 --- a/pkg/controller/certificates/requestmanager/util_test.go +++ b/pkg/controller/certificates/requestmanager/util_test.go @@ -73,7 +73,7 @@ func createCryptoBundle(originalCert *cmapi.Certificate) (*cryptoBundle, error) if crt.Spec.PrivateKey == nil { crt.Spec.PrivateKey = &cmapi.CertificatePrivateKey{} } - reqName, err := apiutil.ComputeCertificateRequestName(crt) + reqName, err := apiutil.ComputeName(crt.Name, crt.Spec) if err != nil { return nil, err } diff --git a/test/e2e/framework/helper/certificates.go b/test/e2e/framework/helper/certificates.go index f9ba53b69..549138bb7 100644 --- a/test/e2e/framework/helper/certificates.go +++ b/test/e2e/framework/helper/certificates.go @@ -364,7 +364,7 @@ func (h *Helper) describeCertificateRequestFromCertificate(ns string, certificat return } - crName, err := apiutil.ComputeCertificateRequestName(certificate) + crName, err := apiutil.ComputeName(certificate.Name, certificate.Spec) if err != nil { log.Logf("Failed to compute CertificateRequest name from certificate: %s", err) return