From 82e2b4e078eec62ce8a392a2cf8a73361da41c88 Mon Sep 17 00:00:00 2001 From: joshvanl Date: Mon, 14 Jun 2021 11:18:37 +0100 Subject: [PATCH] Refactor all validations into validations package Signed-off-by: joshvanl --- test/e2e/framework/helper/validate.go | 85 ++++-------- .../framework/helper/validation/BUILD.bazel | 31 +++++ .../certificates}/BUILD.bazel | 2 +- .../certificates}/certificates.go | 5 +- .../helper/validation/featureset/BUILD.bazel | 22 --- .../validation/featureset/featureset.go | 131 ------------------ .../framework/helper/validation/validation.go | 97 +++++++++++++ 7 files changed, 159 insertions(+), 214 deletions(-) create mode 100644 test/e2e/framework/helper/validation/BUILD.bazel rename test/e2e/framework/helper/{validations => validation/certificates}/BUILD.bazel (95%) rename test/e2e/framework/helper/{validations => validation/certificates}/certificates.go (98%) delete mode 100644 test/e2e/framework/helper/validation/featureset/BUILD.bazel delete mode 100644 test/e2e/framework/helper/validation/featureset/featureset.go create mode 100644 test/e2e/framework/helper/validation/validation.go diff --git a/test/e2e/framework/helper/validate.go b/test/e2e/framework/helper/validate.go index 12dbc6ee5..5202608e7 100644 --- a/test/e2e/framework/helper/validate.go +++ b/test/e2e/framework/helper/validate.go @@ -18,72 +18,19 @@ package helper import ( "context" + "crypto" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" - "github.com/jetstack/cert-manager/test/e2e/framework/helper/featureset" - "github.com/jetstack/cert-manager/test/e2e/framework/helper/validations" + "github.com/jetstack/cert-manager/test/e2e/framework/helper/validation" + "github.com/jetstack/cert-manager/test/e2e/framework/helper/validation/certificates" + "github.com/jetstack/cert-manager/test/e2e/framework/helper/validation/certificatesigningrequests" ) -// ValidationFunc describes a certificate validation helper function -type ValidationFunc func(certificate *cmapi.Certificate, secret *v1.Secret) error - -func (h *Helper) DefaultValidationSet() []ValidationFunc { - return []ValidationFunc{ - validations.Expect2Or3KeysInSecret, - validations.ExpectCertificateDNSNamesToMatch, - validations.ExpectCertificateOrganizationToMatch, - validations.ExpectCertificateURIsToMatch, - validations.ExpectCorrectTrustChain, - validations.ExpectCARootCertificate, - validations.ExpectEmailsToMatch, - validations.ExpectValidAnnotations, - validations.ExpectValidCertificate, - validations.ExpectValidCommonName, - validations.ExpectValidNotAfterDate, - validations.ExpectValidPrivateKeyData, - validations.ExpectConditionReadyObservedGeneration, - } -} - -func (h *Helper) ValidationSetForUnsupportedFeatureSet(fs featureset.FeatureSet) []ValidationFunc { - // basics - out := []ValidationFunc{ - validations.Expect2Or3KeysInSecret, - validations.ExpectCertificateDNSNamesToMatch, - validations.ExpectCertificateOrganizationToMatch, - validations.ExpectValidAnnotations, - validations.ExpectValidCertificate, - validations.ExpectValidCommonName, - validations.ExpectValidNotAfterDate, - validations.ExpectValidPrivateKeyData, - validations.ExpectConditionReadyObservedGeneration, - } - - if !fs.Contains(featureset.URISANsFeature) { - out = append(out, validations.ExpectCertificateURIsToMatch) - } - - if !fs.Contains(featureset.EmailSANsFeature) { - out = append(out, validations.ExpectEmailsToMatch) - } - - if !fs.Contains(featureset.SaveCAToSecret) { - out = append(out, validations.ExpectCorrectTrustChain) - if !fs.Contains(featureset.SaveRootCAToSecret) { - out = append(out, validations.ExpectCARootCertificate) - } - } - - return out -} - // ValidateCertificate retrieves the issued certificate and runs all validation functions -func (h *Helper) ValidateCertificate(ns, name string, validations ...ValidationFunc) error { +func (h *Helper) ValidateCertificate(ns, name string, validations ...certificates.ValidationFunc) error { if len(validations) == 0 { - validations = h.DefaultValidationSet() + validations = validation.DefaultCertificateSet() } certificate, err := h.CMClient.CertmanagerV1().Certificates(ns).Get(context.TODO(), name, metav1.GetOptions{}) if err != nil { @@ -103,3 +50,23 @@ func (h *Helper) ValidateCertificate(ns, name string, validations ...ValidationF return nil } + +// ValidateCertificateSigningRequest retrieves the issued certificate and runs all validation functions +func (h *Helper) ValidateCertificateSigningRequest(name string, key crypto.Signer, validations ...certificatesigningrequests.ValidationFunc) error { + if len(validations) == 0 { + validations = validation.DefaultCertificateSigningRequestSet() + } + csr, err := h.KubeClient.CertificatesV1().CertificateSigningRequests().Get(context.TODO(), name, metav1.GetOptions{}) + if err != nil { + return err + } + + for _, fn := range validations { + err := fn(csr, key) + if err != nil { + return err + } + } + + return nil +} diff --git a/test/e2e/framework/helper/validation/BUILD.bazel b/test/e2e/framework/helper/validation/BUILD.bazel new file mode 100644 index 000000000..3c625ee1b --- /dev/null +++ b/test/e2e/framework/helper/validation/BUILD.bazel @@ -0,0 +1,31 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["validation.go"], + importpath = "github.com/jetstack/cert-manager/test/e2e/framework/helper/validation", + visibility = ["//visibility:public"], + deps = [ + "//test/e2e/framework/helper/featureset:go_default_library", + "//test/e2e/framework/helper/validation/certificates:go_default_library", + "//test/e2e/framework/helper/validation/certificatesigningrequests:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [ + ":package-srcs", + "//test/e2e/framework/helper/validation/certificates:all-srcs", + "//test/e2e/framework/helper/validation/certificatesigningrequests:all-srcs", + ], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/test/e2e/framework/helper/validations/BUILD.bazel b/test/e2e/framework/helper/validation/certificates/BUILD.bazel similarity index 95% rename from test/e2e/framework/helper/validations/BUILD.bazel rename to test/e2e/framework/helper/validation/certificates/BUILD.bazel index a1bd89eb4..0d34efab0 100644 --- a/test/e2e/framework/helper/validations/BUILD.bazel +++ b/test/e2e/framework/helper/validation/certificates/BUILD.bazel @@ -3,7 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", srcs = ["certificates.go"], - importpath = "github.com/jetstack/cert-manager/test/e2e/framework/helper/validations", + importpath = "github.com/jetstack/cert-manager/test/e2e/framework/helper/validation/certificates", visibility = ["//visibility:public"], deps = [ "//pkg/api/util:go_default_library", diff --git a/test/e2e/framework/helper/validations/certificates.go b/test/e2e/framework/helper/validation/certificates/certificates.go similarity index 98% rename from test/e2e/framework/helper/validations/certificates.go rename to test/e2e/framework/helper/validation/certificates/certificates.go index ffbe138b0..1c0324d72 100644 --- a/test/e2e/framework/helper/validations/certificates.go +++ b/test/e2e/framework/helper/validation/certificates/certificates.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package validations +package certificates import ( "bytes" @@ -34,6 +34,9 @@ import ( "github.com/jetstack/cert-manager/pkg/util/pki" ) +// ValidationFunc describes a Certificate validation helper function +type ValidationFunc func(certificate *cmapi.Certificate, secret *corev1.Secret) error + // Expect2Or3KeysInSecret checks if the secret resource has the correct amount of fields in the secret data func Expect2Or3KeysInSecret(_ *cmapi.Certificate, secret *corev1.Secret) error { if !(len(secret.Data) == 2 || len(secret.Data) == 3) { diff --git a/test/e2e/framework/helper/validation/featureset/BUILD.bazel b/test/e2e/framework/helper/validation/featureset/BUILD.bazel deleted file mode 100644 index 0ab63b6ce..000000000 --- a/test/e2e/framework/helper/validation/featureset/BUILD.bazel +++ /dev/null @@ -1,22 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") - -go_library( - name = "go_default_library", - srcs = ["featureset.go"], - importpath = "github.com/jetstack/cert-manager/test/e2e/framework/helper/validation/featureset", - visibility = ["//visibility:public"], -) - -filegroup( - name = "package-srcs", - srcs = glob(["**"]), - tags = ["automanaged"], - visibility = ["//visibility:private"], -) - -filegroup( - name = "all-srcs", - srcs = [":package-srcs"], - tags = ["automanaged"], - visibility = ["//visibility:public"], -) diff --git a/test/e2e/framework/helper/validation/featureset/featureset.go b/test/e2e/framework/helper/validation/featureset/featureset.go deleted file mode 100644 index 3be3fb454..000000000 --- a/test/e2e/framework/helper/validation/featureset/featureset.go +++ /dev/null @@ -1,131 +0,0 @@ -/* -Copyright 2020 The cert-manager Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package featureset - -import "strings" - -// NewFeatureSet constructs a new feature set with the given features. -func NewFeatureSet(feats ...Feature) FeatureSet { - fs := make(FeatureSet) - for _, f := range feats { - fs.Add(f) - } - return fs -} - -// FeatureSet represents a set of features. -// This type does not indicate whether or not features are enabled, rather it -// just defines a grouping of features (i.e. a 'set'). -type FeatureSet map[Feature]struct{} - -// Add adds a feature to the set -func (fs FeatureSet) Add(f Feature) { - fs[f] = struct{}{} -} - -// Delete removes a feature from the set -func (fs FeatureSet) Delete(f Feature) { - _, ok := fs[f] - if ok { - delete(fs, f) - } -} - -// Contains returns true if the FeatureSet contains the given feature -func (fs FeatureSet) Contains(f Feature) bool { - _, ok := fs[f] - return ok -} - -// String returns this FeatureSet as a comma separated string -func (fs FeatureSet) String() string { - featsSlice := make([]string, len(fs)) - - i := 0 - for f := range fs { - featsSlice[i] = string(f) - i++ - } - - return strings.Join(featsSlice, ", ") -} - -type Feature string - -// String returns the Feature name as a string -func (f Feature) String() string { - return string(f) -} - -const ( - // IPAddressFeature denotes tests that set the IPAddresses field. - // Some issuer's are never going to allow issuing certificates with IP SANs - // set as they are considered bad-practice. - IPAddressFeature Feature = "IPAddresses" - - // DurationFeature denotes tests that set the 'duration' field to some - // custom value. - // Some issuers enforce a particular certificate duration, meaning they - // will never pass tests that validate the duration is as expected. - DurationFeature Feature = "Duration" - - // WildcardsFeature denotes tests that request certificates for wildcard - // domains. Some issuer's disable wildcard certificate issuance, so this - // feature allows runs of the suite to exclude those tests that utilise - // wildcards. - WildcardsFeature Feature = "Wildcards" - - // ECDSAFeature denotes whether the target issuer is able to sign - // certificates with an elliptic curve private key. This is useful for some - // issuers that have trouble being configured to support this feature. - ECDSAFeature Feature = "ECDSA" - - // ReusePrivateKey denotes whether the target issuer is able to sign multiple - // certificates for the same private key. This is useful for some issuers - // that have trouble being configured to support this feature. - ReusePrivateKeyFeature Feature = "ReusePrivateKey" - - // URISANs denotes whether to the target issuer is able to sign a certificate - // that includes a URISANs. ACME providers do not support this. - URISANsFeature Feature = "URISANs" - - // EmailSANs denotes whether to the target issuer is able to sign a certificate - // that includes a EmailSANs. - EmailSANsFeature Feature = "EmailSANs" - - // CommonName denotes whether the target issuer is able to sign certificates - // with a distinct CommonName. This is useful for issuers such as ACME - // providers that ignore, or otherwise have special requirements for the - // CommonName such as needing to be present in the DNS Name list. - CommonNameFeature = "CommonName" - - // KeyUsages denotes whether the target issuer is able to sign certificates - // with arbitrary key usages. - KeyUsagesFeature = "KeyUsages" - - // OnlySAN denotes whether the target issuer is able to sign certificates - // with only SANs set - OnlySAN = "OnlySAN" - - // SaveCAToSecret denotes whether the target issuer returns a CA - // certificate which can be stored in the ca.crt field of the Secret. - SaveCAToSecret = "SaveCAToSecret" - - // SaveRootCAToSecret denotes whether the CA certificate is expected to - // represent a root CA (sub-feature of SaveCAToSecret) - SaveRootCAToSecret = "SaveRootCAToSecret" -) diff --git a/test/e2e/framework/helper/validation/validation.go b/test/e2e/framework/helper/validation/validation.go new file mode 100644 index 000000000..abddc921b --- /dev/null +++ b/test/e2e/framework/helper/validation/validation.go @@ -0,0 +1,97 @@ +/* +Copyright 2021 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "github.com/jetstack/cert-manager/test/e2e/framework/helper/featureset" + "github.com/jetstack/cert-manager/test/e2e/framework/helper/validation/certificates" + "github.com/jetstack/cert-manager/test/e2e/framework/helper/validation/certificatesigningrequests" +) + +func DefaultCertificateSet() []certificates.ValidationFunc { + return []certificates.ValidationFunc{ + certificates.Expect2Or3KeysInSecret, + certificates.ExpectCertificateDNSNamesToMatch, + certificates.ExpectCertificateOrganizationToMatch, + certificates.ExpectCertificateURIsToMatch, + certificates.ExpectCorrectTrustChain, + certificates.ExpectCARootCertificate, + certificates.ExpectEmailsToMatch, + certificates.ExpectValidAnnotations, + certificates.ExpectValidCertificate, + certificates.ExpectValidCommonName, + certificates.ExpectValidNotAfterDate, + certificates.ExpectValidPrivateKeyData, + certificates.ExpectConditionReadyObservedGeneration, + } +} + +func DefaultCertificateSigningRequestSet() []certificatesigningrequests.ValidationFunc { + return []certificatesigningrequests.ValidationFunc{ + certificatesigningrequests.ExpectValidCertificate, + certificatesigningrequests.ExpectCertificateOrganizationToMatch, + certificatesigningrequests.ExpectValidPrivateKeyData, + certificatesigningrequests.ExpectCertificateDNSNamesToMatch, + certificatesigningrequests.ExpectCertificateURIsToMatch, + certificatesigningrequests.ExpectCertificateIPsToMatch, + certificatesigningrequests.ExpectValidCommonName, + certificatesigningrequests.ExpectValidDuration, + certificatesigningrequests.ExpectEmailsToMatch, + certificatesigningrequests.ExpectCorrectTrustChain, + certificatesigningrequests.ExpectCARootCertificate, + certificatesigningrequests.ExpectIsCA, + certificatesigningrequests.ExpectConditionApproved, + certificatesigningrequests.ExpectConditiotNotDenied, + } +} + +func CertificateSetForUnsupportedFeatureSet(fs featureset.FeatureSet) []certificates.ValidationFunc { + // basics + out := []certificates.ValidationFunc{ + certificates.Expect2Or3KeysInSecret, + certificates.ExpectCertificateDNSNamesToMatch, + certificates.ExpectCertificateOrganizationToMatch, + certificates.ExpectValidAnnotations, + certificates.ExpectValidCertificate, + certificates.ExpectValidCommonName, + certificates.ExpectValidNotAfterDate, + certificates.ExpectValidPrivateKeyData, + certificates.ExpectConditionReadyObservedGeneration, + } + + if !fs.Contains(featureset.URISANsFeature) { + out = append(out, certificates.ExpectCertificateURIsToMatch) + } + + if !fs.Contains(featureset.EmailSANsFeature) { + out = append(out, certificates.ExpectEmailsToMatch) + } + + if !fs.Contains(featureset.SaveCAToSecret) { + out = append(out, certificates.ExpectCorrectTrustChain) + if !fs.Contains(featureset.SaveRootCAToSecret) { + out = append(out, certificates.ExpectCARootCertificate) + } + } + + return out +} + +func CertificateSigningRequestSetForUnsupportedFeatureSet(fs featureset.FeatureSet) []certificatesigningrequests.ValidationFunc { + // Add exclusions if and when needed + return DefaultCertificateSigningRequestSet() +}