From 6dcc40874125d4fa3a72578ac6f33cb276d74243 Mon Sep 17 00:00:00 2001 From: Laurent Rolaz Date: Fri, 30 Nov 2018 19:15:54 +0100 Subject: [PATCH 1/7] Add IP Address in CSR Signed-off-by: Laurent Rolaz --- .gitignore | 2 ++ pkg/apis/certmanager/v1alpha1/types.go | 1 + .../certmanager/v1alpha1/types_certificate.go | 3 +++ .../v1alpha1/zz_generated.deepcopy.go | 5 ++++ .../certmanager/validation/certificate.go | 18 +++++++++++++ .../validation/certificate_test.go | 23 ++++++++++++++++ pkg/controller/certificates/sync.go | 7 +++++ pkg/issuer/acme/issue_test.go | 2 ++ pkg/issuer/vault/issue.go | 17 +++++++++--- pkg/util/pki/csr.go | 27 ++++++++++++++++--- pkg/util/util.go | 9 +++++++ 11 files changed, 108 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index 90ac3d2f0..a264827f4 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,5 @@ .vscode .venv bazel-* +/.settings/ +/.project diff --git a/pkg/apis/certmanager/v1alpha1/types.go b/pkg/apis/certmanager/v1alpha1/types.go index 8d80ced45..c450513e5 100644 --- a/pkg/apis/certmanager/v1alpha1/types.go +++ b/pkg/apis/certmanager/v1alpha1/types.go @@ -18,6 +18,7 @@ package v1alpha1 const ( AltNamesAnnotationKey = "certmanager.k8s.io/alt-names" + IpSansAnnotationKey = "certmanager.k8s.io/ip-sans" CommonNameAnnotationKey = "certmanager.k8s.io/common-name" IssuerNameAnnotationKey = "certmanager.k8s.io/issuer-name" IssuerKindAnnotationKey = "certmanager.k8s.io/issuer-kind" diff --git a/pkg/apis/certmanager/v1alpha1/types_certificate.go b/pkg/apis/certmanager/v1alpha1/types_certificate.go index 368bfecb1..0c6bf3542 100644 --- a/pkg/apis/certmanager/v1alpha1/types_certificate.go +++ b/pkg/apis/certmanager/v1alpha1/types_certificate.go @@ -66,6 +66,9 @@ type CertificateSpec struct { // DNSNames is a list of subject alt names to be used on the Certificate DNSNames []string `json:"dnsNames,omitempty"` + // IPAddresses is a list of IP addresses to be used on the Certificate + IPAddresses []string `json:"ipAddresses,omitempty"` + // SecretName is the name of the secret resource to store this secret in SecretName string `json:"secretName"` diff --git a/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go index d5c3e7cf5..923ff20f7 100644 --- a/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go @@ -491,6 +491,11 @@ func (in *CertificateSpec) DeepCopyInto(out *CertificateSpec) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.IPAddresses != nil { + in, out := &in.IPAddresses, &out.IPAddresses + *out = make([]string, len(*in)) + copy(*out, *in) + } out.IssuerRef = in.IssuerRef if in.ACME != nil { in, out := &in.ACME, &out.ACME diff --git a/pkg/apis/certmanager/validation/certificate.go b/pkg/apis/certmanager/validation/certificate.go index eeb638d33..3f8a3286f 100644 --- a/pkg/apis/certmanager/validation/certificate.go +++ b/pkg/apis/certmanager/validation/certificate.go @@ -18,6 +18,7 @@ package validation import ( "fmt" + "net" "k8s.io/apimachinery/pkg/util/validation/field" @@ -49,6 +50,9 @@ func ValidateCertificateSpec(crt *v1alpha1.CertificateSpec, fldPath *field.Path) if len(crt.CommonName) == 0 && len(crt.DNSNames) == 0 { el = append(el, field.Required(fldPath.Child("dnsNames"), "at least one dnsName is required if commonName is not set")) } + if len(crt.IPAddresses) > 0 { + el = append(el, validateIPAddresses(crt, fldPath)...) + } if crt.ACME != nil { el = append(el, validateACMEConfigForAllDNSNames(crt, fldPath)...) el = append(el, ValidateACMECertificateConfig(crt.ACME, fldPath.Child("acme"))...) @@ -104,6 +108,20 @@ func validateACMEConfigForAllDNSNames(a *v1alpha1.CertificateSpec, fldPath *fiel return el } +func validateIPAddresses(a *v1alpha1.CertificateSpec, fldPath *field.Path) field.ErrorList { + if len(a.IPAddresses) <= 0 { + return nil + } + el := field.ErrorList{} + for i, d := range a.IPAddresses { + ip := net.ParseIP(d) + if ip == nil { + el = append(el, field.Invalid(fldPath.Child("ipAddresses").Index(i), d, "Invalid IP Address")) + } + } + return el +} + func ValidateACMECertificateConfig(a *v1alpha1.ACMECertificateConfig, fldPath *field.Path) field.ErrorList { el := field.ErrorList{} for i, cfg := range a.Config { diff --git a/pkg/apis/certmanager/validation/certificate_test.go b/pkg/apis/certmanager/validation/certificate_test.go index a033439b1..3fb5c1b2b 100644 --- a/pkg/apis/certmanager/validation/certificate_test.go +++ b/pkg/apis/certmanager/validation/certificate_test.go @@ -372,6 +372,29 @@ func TestValidateCertificate(t *testing.T) { field.Invalid(fldPath.Child("keyAlgorithm"), v1alpha1.KeyAlgorithm("blah"), "must be either empty or one of rsa or ecdsa"), }, }, + "valid certificate with ipAddresses": { + cfg: &v1alpha1.Certificate{ + Spec: v1alpha1.CertificateSpec{ + CommonName: "testcn", + IPAddresses: []string{"127.0.0.1"}, + SecretName: "abc", + IssuerRef: validIssuerRef, + }, + }, + }, + "certificate with invalid ipAddresses": { + cfg: &v1alpha1.Certificate{ + Spec: v1alpha1.CertificateSpec{ + CommonName: "testcn", + IPAddresses: []string{"blah"}, + SecretName: "abc", + IssuerRef: validIssuerRef, + }, + }, + errs: []*field.Error{ + field.Invalid(fldPath.Child("ipAddresses").Index(0), "blah", "Invalid IP Address"), + }, + }, } for n, s := range scenarios { t.Run(n, func(t *testing.T) { diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index 1792b3eb9..773e3b71b 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -216,6 +216,12 @@ func (c *Controller) certificateMatchesSpec(crt *v1alpha1.Certificate, key crypt errs = append(errs, fmt.Sprintf("DNS names on TLS certificate not up to date: %q", cert.DNSNames)) } + // validate the ip addresses are correct + expectedIPAddresses := pki.IPAddressesNameForCertificate(crt) + if !util.EqualUnsorted(util.IPAddressesToString(cert.IPAddresses), expectedIPAddresses) { + errs = append(errs, fmt.Sprintf("IP Addresses on TLS certificate not up to date: %q", util.IPAddressesToString(cert.IPAddresses))) + } + return len(errs) == 0, errs } @@ -314,6 +320,7 @@ func (c *Controller) updateSecret(crt *v1alpha1.Certificate, namespace string, c secret.Annotations[v1alpha1.IssuerKindAnnotationKey] = issuerKind(crt) secret.Annotations[v1alpha1.CommonNameAnnotationKey] = x509Cert.Subject.CommonName secret.Annotations[v1alpha1.AltNamesAnnotationKey] = strings.Join(x509Cert.DNSNames, ",") + secret.Annotations[v1alpha1.IpSansAnnotationKey] = strings.Join(util.IPAddressesToString(x509Cert.IPAddresses), ",") } // Always set the certificate name label on the target secret diff --git a/pkg/issuer/acme/issue_test.go b/pkg/issuer/acme/issue_test.go index 6c6d4f457..72d62b4bd 100644 --- a/pkg/issuer/acme/issue_test.go +++ b/pkg/issuer/acme/issue_test.go @@ -57,6 +57,7 @@ var serialNumberLimit = new(big.Int).Lsh(big.NewInt(1), 128) func generateSelfSignedCert(t *testing.T, crt *v1alpha1.Certificate, key crypto.Signer, duration time.Duration) (derBytes, pemBytes []byte) { commonName := pki.CommonNameForCertificate(crt) dnsNames := pki.DNSNamesForCertificate(crt) + ipAddresses := pki.IPAddressesForCertificate(crt) serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) if err != nil { @@ -76,6 +77,7 @@ func generateSelfSignedCert(t *testing.T, crt *v1alpha1.Certificate, key crypto. // see http://golang.org/pkg/crypto/x509/#KeyUsage KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, DNSNames: dnsNames, + IPAddresses: ipAddresses, } derBytes, err = x509.CreateCertificate(rand.Reader, template, template, key.Public(), key) diff --git a/pkg/issuer/vault/issue.go b/pkg/issuer/vault/issue.go index 2ef4a2aee..df703b21e 100644 --- a/pkg/issuer/vault/issue.go +++ b/pkg/issuer/vault/issue.go @@ -26,6 +26,7 @@ import ( "path" "strings" "time" + "net" "github.com/golang/glog" vault "github.com/hashicorp/vault/api" @@ -93,7 +94,7 @@ func (v *Vault) Issue(ctx context.Context, crt *v1alpha1.Certificate) (*issuer.I certDuration = crt.Spec.Duration.Duration } - certPem, caPem, err := v.requestVaultCert(template.Subject.CommonName, certDuration, template.DNSNames, pemRequestBuf.Bytes()) + certPem, caPem, err := v.requestVaultCert(template.Subject.CommonName, certDuration, template.DNSNames, ipAddressesToString(template.IPAddresses), pemRequestBuf.Bytes()) if err != nil { v.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Failed to request certificate: %v", err) return nil, err @@ -214,17 +215,19 @@ func (v *Vault) requestTokenWithAppRoleRef(client *vault.Client, appRole *v1alph return token, nil } -func (v *Vault) requestVaultCert(commonName string, certDuration time.Duration, altNames []string, csr []byte) ([]byte, []byte, error) { +func (v *Vault) requestVaultCert(commonName string, certDuration time.Duration, altNames []string, ipSans []string, csr []byte) ([]byte, []byte, error) { + client, err := v.initVaultClient() if err != nil { return nil, nil, err } - glog.V(4).Infof("Vault certificate request for commonName %s altNames: %q", commonName, altNames) + glog.V(4).Infof("Vault certificate request for commonName %s altNames: %q ipSans: %q", commonName, altNames, ipSans) parameters := map[string]string{ "common_name": commonName, "alt_names": strings.Join(altNames, ","), + "ip_sans": strings.Join(ipSans, ","), "ttl": certDuration.String(), "csr": string(csr), "exclude_cn_from_sans": "true", @@ -314,3 +317,11 @@ func (v *Vault) vaultTokenRef(name, key string) (string, error) { return token, nil } + +func ipAddressesToString(ipAddresses []net.IP) []string { + var ipNames []string + for _, ip := range ipAddresses { + ipNames = append(ipNames, ip.String()) + } + return ipNames +} diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index a2f2bec7c..ea9b7f419 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -25,6 +25,7 @@ import ( "encoding/pem" "fmt" "math/big" + "net" "time" "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" @@ -57,6 +58,22 @@ func DNSNamesForCertificate(crt *v1alpha1.Certificate) []string { return crt.Spec.DNSNames } +func IPAddressesForCertificate(crt *v1alpha1.Certificate) []net.IP { + var ipAddresses []net.IP + for _, ip := range IPAddressesNameForCertificate(crt) { + ipAddresses = append(ipAddresses, net.ParseIP(ip)) + } + return ipAddresses +} + +func IPAddressesNameForCertificate(crt *v1alpha1.Certificate) []string { + var ipAddressNames []string + for _, ip := range crt.Spec.IPAddresses { + ipAddressNames = append(ipAddressNames, ip) + } + return removeDuplicates(ipAddressNames) +} + func removeDuplicates(in []string) []string { var found []string Outer: @@ -93,6 +110,7 @@ var serialNumberLimit = new(big.Int).Lsh(big.NewInt(1), 128) func GenerateCSR(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) (*x509.CertificateRequest, error) { commonName := CommonNameForCertificate(crt) dnsNames := DNSNamesForCertificate(crt) + iPAddresses := IPAddressesForCertificate(crt) organization := OrganizationForCertificate(crt) if len(commonName) == 0 && len(dnsNames) == 0 { @@ -112,7 +130,8 @@ func GenerateCSR(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) (*x50 Organization: organization, CommonName: commonName, }, - DNSNames: dnsNames, + DNSNames: dnsNames, + IPAddresses: iPAddresses, // TODO: work out how best to handle extensions/key usages here ExtraExtensions: []pkix.Extension{}, }, nil @@ -125,6 +144,7 @@ func GenerateCSR(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) (*x50 func GenerateTemplate(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) (*x509.Certificate, error) { commonName := CommonNameForCertificate(crt) dnsNames := DNSNamesForCertificate(crt) + iPAddresses := IPAddressesForCertificate(crt) organization := OrganizationForCertificate(crt) if len(commonName) == 0 && len(dnsNames) == 0 { @@ -164,8 +184,9 @@ func GenerateTemplate(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) NotBefore: time.Now(), NotAfter: time.Now().Add(certDuration), // see http://golang.org/pkg/crypto/x509/#KeyUsage - KeyUsage: keyUsages, - DNSNames: dnsNames, + KeyUsage: keyUsages, + DNSNames: dnsNames, + IPAddresses: iPAddresses, }, nil } diff --git a/pkg/util/util.go b/pkg/util/util.go index 7463a8a7a..1317b2c50 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -20,6 +20,7 @@ import ( "math/rand" "sort" "time" + "net" ) func OnlyOneNotNil(items ...interface{}) (any bool, one bool) { @@ -75,3 +76,11 @@ func Contains(ss []string, s string) bool { } return false } + +func IPAddressesToString(ipAddresses []net.IP) []string { + var ipNames []string + for _, ip := range ipAddresses { + ipNames = append(ipNames, ip.String()) + } + return ipNames +} From 531c26061cfa1fabd0f79df00b8b66b43b7526d5 Mon Sep 17 00:00:00 2001 From: Laurent Rolaz Date: Mon, 3 Dec 2018 15:30:46 +0100 Subject: [PATCH 2/7] GO Format Signed-off-by: Laurent Rolaz Signed-off-by: Laurent Rolaz --- pkg/controller/certificates/sync.go | 2 +- pkg/issuer/acme/issue_test.go | 4 ++-- pkg/issuer/vault/issue.go | 2 +- pkg/util/util.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index 773e3b71b..6fafccdff 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -320,7 +320,7 @@ func (c *Controller) updateSecret(crt *v1alpha1.Certificate, namespace string, c secret.Annotations[v1alpha1.IssuerKindAnnotationKey] = issuerKind(crt) secret.Annotations[v1alpha1.CommonNameAnnotationKey] = x509Cert.Subject.CommonName secret.Annotations[v1alpha1.AltNamesAnnotationKey] = strings.Join(x509Cert.DNSNames, ",") - secret.Annotations[v1alpha1.IpSansAnnotationKey] = strings.Join(util.IPAddressesToString(x509Cert.IPAddresses), ",") + secret.Annotations[v1alpha1.IpSansAnnotationKey] = strings.Join(util.IPAddressesToString(x509Cert.IPAddresses), ",") } // Always set the certificate name label on the target secret diff --git a/pkg/issuer/acme/issue_test.go b/pkg/issuer/acme/issue_test.go index 72d62b4bd..fd0fab536 100644 --- a/pkg/issuer/acme/issue_test.go +++ b/pkg/issuer/acme/issue_test.go @@ -75,8 +75,8 @@ func generateSelfSignedCert(t *testing.T, crt *v1alpha1.Certificate, key crypto. NotBefore: time.Now(), NotAfter: time.Now().Add(duration), // see http://golang.org/pkg/crypto/x509/#KeyUsage - KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, - DNSNames: dnsNames, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, + DNSNames: dnsNames, IPAddresses: ipAddresses, } diff --git a/pkg/issuer/vault/issue.go b/pkg/issuer/vault/issue.go index df703b21e..f582307ad 100644 --- a/pkg/issuer/vault/issue.go +++ b/pkg/issuer/vault/issue.go @@ -22,11 +22,11 @@ import ( "crypto/x509" "encoding/pem" "fmt" + "net" "net/http" "path" "strings" "time" - "net" "github.com/golang/glog" vault "github.com/hashicorp/vault/api" diff --git a/pkg/util/util.go b/pkg/util/util.go index 1317b2c50..b7717b573 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -18,9 +18,9 @@ package util import ( "math/rand" + "net" "sort" "time" - "net" ) func OnlyOneNotNil(items ...interface{}) (any bool, one bool) { From 55cafeae331bd7011baf5d5252bd370c0c8de0ff Mon Sep 17 00:00:00 2001 From: Laurent Rolaz Date: Mon, 3 Dec 2018 15:53:10 +0100 Subject: [PATCH 3/7] Generate doc Signed-off-by: Laurent Rolaz --- docs/generated/reference/output/reference/api-docs/index.html | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/generated/reference/output/reference/api-docs/index.html b/docs/generated/reference/output/reference/api-docs/index.html index 760ea28d7..9a1e85d64 100755 --- a/docs/generated/reference/output/reference/api-docs/index.html +++ b/docs/generated/reference/output/reference/api-docs/index.html @@ -101,6 +101,10 @@ Appears In: Certificate default Duration +ipAddresses
string array +IPAddresses is a list of IP addresses to be used on the Certificate + + isCA
boolean IsCA will mark this Certificate as valid for signing. This implies that the 'signing' usage is set From c5fa2022392971b160461ccd9f6da844013de93c Mon Sep 17 00:00:00 2001 From: Laurent Rolaz Date: Tue, 15 Jan 2019 16:23:42 +0100 Subject: [PATCH 4/7] Fix some GO Style Signed-off-by: Laurent Rolaz (+2 squashed commits) Squashed commits: [ce6cc2eb] Fix some GO Style Signed-off-by: Laurent Rolaz [563b7275] Fix some GO Style Signed-off-by: Laurent Rolaz --- pkg/apis/certmanager/v1alpha1/types.go | 2 +- .../validation/certificate_for_issuer.go | 4 +++ .../validation/certificate_for_issuer_test.go | 25 +++++++++++++++++++ pkg/controller/certificates/sync.go | 2 +- pkg/util/pki/csr.go | 12 ++++++--- 5 files changed, 39 insertions(+), 6 deletions(-) diff --git a/pkg/apis/certmanager/v1alpha1/types.go b/pkg/apis/certmanager/v1alpha1/types.go index c450513e5..41cb788eb 100644 --- a/pkg/apis/certmanager/v1alpha1/types.go +++ b/pkg/apis/certmanager/v1alpha1/types.go @@ -18,7 +18,7 @@ package v1alpha1 const ( AltNamesAnnotationKey = "certmanager.k8s.io/alt-names" - IpSansAnnotationKey = "certmanager.k8s.io/ip-sans" + IPSANAnnotationKey = "certmanager.k8s.io/ip-sans" CommonNameAnnotationKey = "certmanager.k8s.io/common-name" IssuerNameAnnotationKey = "certmanager.k8s.io/issuer-name" IssuerKindAnnotationKey = "certmanager.k8s.io/issuer-kind" diff --git a/pkg/apis/certmanager/validation/certificate_for_issuer.go b/pkg/apis/certmanager/validation/certificate_for_issuer.go index 098a3e806..c531a77b4 100644 --- a/pkg/apis/certmanager/validation/certificate_for_issuer.go +++ b/pkg/apis/certmanager/validation/certificate_for_issuer.go @@ -63,6 +63,10 @@ func ValidateCertificateForACMEIssuer(crt *v1alpha1.CertificateSpec, issuer *v1a el = append(el, field.Invalid(specPath.Child("duration"), crt.Duration, "ACME does not support certificate durations")) } + if len(crt.IPAddresses) != 0 { + el = append(el, field.Invalid(specPath.Child("ipAddresses"), crt.IPAddresses, "ACME does not support certificate ip addresses")) + } + return el } diff --git a/pkg/apis/certmanager/validation/certificate_for_issuer_test.go b/pkg/apis/certmanager/validation/certificate_for_issuer_test.go index e7cd036ac..11c0b346c 100644 --- a/pkg/apis/certmanager/validation/certificate_for_issuer_test.go +++ b/pkg/apis/certmanager/validation/certificate_for_issuer_test.go @@ -158,6 +158,31 @@ func TestValidateCertificateForIssuer(t *testing.T) { field.Invalid(fldPath.Child("duration"), &metav1.Duration{Duration: time.Minute * 60}, "ACME does not support certificate durations"), }, }, + "acme certificate with ipAddresses set": { + crt: &v1alpha1.Certificate{ + Spec: v1alpha1.CertificateSpec{ + IPAddresses: []string{"127.0.0.1"}, + IssuerRef: validIssuerRef, + ACME: &v1alpha1.ACMECertificateConfig{ + Config: []v1alpha1.DomainSolverConfig{ + { + Domains: []string{"example.com"}, + SolverConfig: v1alpha1.SolverConfig{ + HTTP01: &v1alpha1.HTTP01SolverConfig{}, + }, + }, + }, + }, + }, + }, + issuer: generate.Issuer(generate.IssuerConfig{ + Name: defaultTestIssuerName, + Namespace: defaultTestNamespace, + }), + errs: []*field.Error{ + field.Invalid(fldPath.Child("ipAddresses"), []string{"127.0.0.1"}, "ACME does not support certificate ip addresses"), + }, + }, "acme certificate with renewBefore set": { crt: &v1alpha1.Certificate{ Spec: v1alpha1.CertificateSpec{ diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index 6fafccdff..9dfc01e88 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -320,7 +320,7 @@ func (c *Controller) updateSecret(crt *v1alpha1.Certificate, namespace string, c secret.Annotations[v1alpha1.IssuerKindAnnotationKey] = issuerKind(crt) secret.Annotations[v1alpha1.CommonNameAnnotationKey] = x509Cert.Subject.CommonName secret.Annotations[v1alpha1.AltNamesAnnotationKey] = strings.Join(x509Cert.DNSNames, ",") - secret.Annotations[v1alpha1.IpSansAnnotationKey] = strings.Join(util.IPAddressesToString(x509Cert.IPAddresses), ",") + secret.Annotations[v1alpha1.IPSANAnnotationKey] = strings.Join(util.IPAddressesToString(x509Cert.IPAddresses), ",") } // Always set the certificate name label on the target secret diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index ea9b7f419..a5718f9e1 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -60,8 +60,12 @@ func DNSNamesForCertificate(crt *v1alpha1.Certificate) []string { func IPAddressesForCertificate(crt *v1alpha1.Certificate) []net.IP { var ipAddresses []net.IP - for _, ip := range IPAddressesNameForCertificate(crt) { - ipAddresses = append(ipAddresses, net.ParseIP(ip)) + var ip net.IP + for _, ipName := range IPAddressesNameForCertificate(crt) { + ip = net.ParseIP(ipName) + if ip != nil { + ipAddresses = append(ipAddresses, ip) + } } return ipAddresses } @@ -144,7 +148,7 @@ func GenerateCSR(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) (*x50 func GenerateTemplate(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) (*x509.Certificate, error) { commonName := CommonNameForCertificate(crt) dnsNames := DNSNamesForCertificate(crt) - iPAddresses := IPAddressesForCertificate(crt) + ipAddresses := IPAddressesForCertificate(crt) organization := OrganizationForCertificate(crt) if len(commonName) == 0 && len(dnsNames) == 0 { @@ -186,7 +190,7 @@ func GenerateTemplate(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) // see http://golang.org/pkg/crypto/x509/#KeyUsage KeyUsage: keyUsages, DNSNames: dnsNames, - IPAddresses: iPAddresses, + IPAddresses: ipAddresses, }, nil } From 18daea16ae4b8b6cb25362ff4e249050f2f2e0fa Mon Sep 17 00:00:00 2001 From: Laurent Rolaz Date: Wed, 16 Jan 2019 09:43:14 +0100 Subject: [PATCH 5/7] Remove duplicate IPAddressesToString Signed-off-by: Laurent Rolaz --- pkg/controller/certificates/sync.go | 6 +++--- pkg/issuer/vault/issue.go | 10 +--------- pkg/util/pki/csr.go | 8 ++++++++ pkg/util/util.go | 8 -------- 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index 9dfc01e88..8430c4386 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -218,8 +218,8 @@ func (c *Controller) certificateMatchesSpec(crt *v1alpha1.Certificate, key crypt // validate the ip addresses are correct expectedIPAddresses := pki.IPAddressesNameForCertificate(crt) - if !util.EqualUnsorted(util.IPAddressesToString(cert.IPAddresses), expectedIPAddresses) { - errs = append(errs, fmt.Sprintf("IP Addresses on TLS certificate not up to date: %q", util.IPAddressesToString(cert.IPAddresses))) + if !util.EqualUnsorted(pki.IPAddressesToString(cert.IPAddresses), expectedIPAddresses) { + errs = append(errs, fmt.Sprintf("IP Addresses on TLS certificate not up to date: %q", pki.IPAddressesToString(cert.IPAddresses))) } return len(errs) == 0, errs @@ -320,7 +320,7 @@ func (c *Controller) updateSecret(crt *v1alpha1.Certificate, namespace string, c secret.Annotations[v1alpha1.IssuerKindAnnotationKey] = issuerKind(crt) secret.Annotations[v1alpha1.CommonNameAnnotationKey] = x509Cert.Subject.CommonName secret.Annotations[v1alpha1.AltNamesAnnotationKey] = strings.Join(x509Cert.DNSNames, ",") - secret.Annotations[v1alpha1.IPSANAnnotationKey] = strings.Join(util.IPAddressesToString(x509Cert.IPAddresses), ",") + secret.Annotations[v1alpha1.IPSANAnnotationKey] = strings.Join(pki.IPAddressesToString(x509Cert.IPAddresses), ",") } // Always set the certificate name label on the target secret diff --git a/pkg/issuer/vault/issue.go b/pkg/issuer/vault/issue.go index f582307ad..76f32ea1a 100644 --- a/pkg/issuer/vault/issue.go +++ b/pkg/issuer/vault/issue.go @@ -22,7 +22,6 @@ import ( "crypto/x509" "encoding/pem" "fmt" - "net" "net/http" "path" "strings" @@ -94,7 +93,7 @@ func (v *Vault) Issue(ctx context.Context, crt *v1alpha1.Certificate) (*issuer.I certDuration = crt.Spec.Duration.Duration } - certPem, caPem, err := v.requestVaultCert(template.Subject.CommonName, certDuration, template.DNSNames, ipAddressesToString(template.IPAddresses), pemRequestBuf.Bytes()) + certPem, caPem, err := v.requestVaultCert(template.Subject.CommonName, certDuration, template.DNSNames, pki.IPAddressesToString(template.IPAddresses), pemRequestBuf.Bytes()) if err != nil { v.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Failed to request certificate: %v", err) return nil, err @@ -318,10 +317,3 @@ func (v *Vault) vaultTokenRef(name, key string) (string, error) { return token, nil } -func ipAddressesToString(ipAddresses []net.IP) []string { - var ipNames []string - for _, ip := range ipAddresses { - ipNames = append(ipNames, ip.String()) - } - return ipNames -} diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index a5718f9e1..1bc7a8fe4 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -78,6 +78,14 @@ func IPAddressesNameForCertificate(crt *v1alpha1.Certificate) []string { return removeDuplicates(ipAddressNames) } +func IPAddressesToString(ipAddresses []net.IP) []string { + var ipNames []string + for _, ip := range ipAddresses { + ipNames = append(ipNames, ip.String()) + } + return ipNames +} + func removeDuplicates(in []string) []string { var found []string Outer: diff --git a/pkg/util/util.go b/pkg/util/util.go index b7717b573..cfbeb678d 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -18,7 +18,6 @@ package util import ( "math/rand" - "net" "sort" "time" ) @@ -77,10 +76,3 @@ func Contains(ss []string, s string) bool { return false } -func IPAddressesToString(ipAddresses []net.IP) []string { - var ipNames []string - for _, ip := range ipAddresses { - ipNames = append(ipNames, ip.String()) - } - return ipNames -} From 597cda40afc974d09ab0cb9dc810711b91b2ed66 Mon Sep 17 00:00:00 2001 From: Laurent Rolaz Date: Wed, 16 Jan 2019 09:57:54 +0100 Subject: [PATCH 6/7] Fix some GO Style Signed-off-by: Laurent Rolaz --- pkg/issuer/vault/issue.go | 1 - pkg/util/util.go | 1 - 2 files changed, 2 deletions(-) diff --git a/pkg/issuer/vault/issue.go b/pkg/issuer/vault/issue.go index 76f32ea1a..ac2e8d322 100644 --- a/pkg/issuer/vault/issue.go +++ b/pkg/issuer/vault/issue.go @@ -316,4 +316,3 @@ func (v *Vault) vaultTokenRef(name, key string) (string, error) { return token, nil } - diff --git a/pkg/util/util.go b/pkg/util/util.go index cfbeb678d..7463a8a7a 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -75,4 +75,3 @@ func Contains(ss []string, s string) bool { } return false } - From ed82465df5494ad681980081d08879b3f3c5ecfe Mon Sep 17 00:00:00 2001 From: Laurent ROLAZ Date: Wed, 30 Jan 2019 12:58:12 +0100 Subject: [PATCH 7/7] Refactoring Signed-off-by: Laurent Rolaz --- pkg/apis/certmanager/validation/certificate.go | 2 +- pkg/apis/certmanager/validation/certificate_test.go | 2 +- pkg/controller/certificates/sync.go | 5 ++--- pkg/util/pki/csr.go | 10 +--------- 4 files changed, 5 insertions(+), 14 deletions(-) diff --git a/pkg/apis/certmanager/validation/certificate.go b/pkg/apis/certmanager/validation/certificate.go index 3f8a3286f..2e1a3b95f 100644 --- a/pkg/apis/certmanager/validation/certificate.go +++ b/pkg/apis/certmanager/validation/certificate.go @@ -116,7 +116,7 @@ func validateIPAddresses(a *v1alpha1.CertificateSpec, fldPath *field.Path) field for i, d := range a.IPAddresses { ip := net.ParseIP(d) if ip == nil { - el = append(el, field.Invalid(fldPath.Child("ipAddresses").Index(i), d, "Invalid IP Address")) + el = append(el, field.Invalid(fldPath.Child("ipAddresses").Index(i), d, "invalid IP address")) } } return el diff --git a/pkg/apis/certmanager/validation/certificate_test.go b/pkg/apis/certmanager/validation/certificate_test.go index 3fb5c1b2b..c46297f63 100644 --- a/pkg/apis/certmanager/validation/certificate_test.go +++ b/pkg/apis/certmanager/validation/certificate_test.go @@ -392,7 +392,7 @@ func TestValidateCertificate(t *testing.T) { }, }, errs: []*field.Error{ - field.Invalid(fldPath.Child("ipAddresses").Index(0), "blah", "Invalid IP Address"), + field.Invalid(fldPath.Child("ipAddresses").Index(0), "blah", "invalid IP address"), }, }, } diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index 8430c4386..da8eac0cf 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -217,9 +217,8 @@ func (c *Controller) certificateMatchesSpec(crt *v1alpha1.Certificate, key crypt } // validate the ip addresses are correct - expectedIPAddresses := pki.IPAddressesNameForCertificate(crt) - if !util.EqualUnsorted(pki.IPAddressesToString(cert.IPAddresses), expectedIPAddresses) { - errs = append(errs, fmt.Sprintf("IP Addresses on TLS certificate not up to date: %q", pki.IPAddressesToString(cert.IPAddresses))) + if !util.EqualUnsorted(pki.IPAddressesToString(cert.IPAddresses), crt.Spec.IPAddresses) { + errs = append(errs, fmt.Sprintf("IP addresses on TLS certificate not up to date: %q", pki.IPAddressesToString(cert.IPAddresses))) } return len(errs) == 0, errs diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index 1bc7a8fe4..669742400 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -61,7 +61,7 @@ func DNSNamesForCertificate(crt *v1alpha1.Certificate) []string { func IPAddressesForCertificate(crt *v1alpha1.Certificate) []net.IP { var ipAddresses []net.IP var ip net.IP - for _, ipName := range IPAddressesNameForCertificate(crt) { + for _, ipName := range crt.Spec.IPAddresses { ip = net.ParseIP(ipName) if ip != nil { ipAddresses = append(ipAddresses, ip) @@ -70,14 +70,6 @@ func IPAddressesForCertificate(crt *v1alpha1.Certificate) []net.IP { return ipAddresses } -func IPAddressesNameForCertificate(crt *v1alpha1.Certificate) []string { - var ipAddressNames []string - for _, ip := range crt.Spec.IPAddresses { - ipAddressNames = append(ipAddressNames, ip) - } - return removeDuplicates(ipAddressNames) -} - func IPAddressesToString(ipAddresses []net.IP) []string { var ipNames []string for _, ip := range ipAddresses {