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

Merge pull request #3232 from meyskens/52characme

Browse Source 
Fix invalid DNS-1123 on ACME computed names
This commit is contained in:
jetstack-bot 2020-09-01 14:52:18 +01:00 committed by GitHub
commit b33ce8ddb2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 34 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
pkg/api/util/names.go
View File

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

10
pkg/api/util/names_test.go
View File

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

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

@ -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",

14
pkg/controller/acmeorders/util.go
View File

@ -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 {

28
pkg/controller/certificaterequests/acme/acme.go
View File

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

2
pkg/controller/certificates/internal/test/test.go
View File

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

2
pkg/controller/certificates/requestmanager/requestmanager_controller.go
View File

@ -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)},

2
pkg/controller/certificates/requestmanager/util_test.go
View File

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

2
test/e2e/framework/helper/certificates.go
View File

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