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

Fix AdditationOutputFormat validation, and adds unit tests. Use correct

Browse Source 
feature set

Signed-off-by: joshvanl <vleeuwenjoshua@gmail.com>
This commit is contained in:
joshvanl 2022-02-01 17:03:37 +00:00
parent 1cf06889bf
commit 8b219a45b2
3 changed files with 146 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
internal/apis/certmanager/validation/BUILD.bazel
View File

@ -17,7 +17,7 @@ go_library(
"//internal/apis/certmanager:go_default_library",
"//internal/apis/certmanager/validation/util:go_default_library",
"//internal/apis/meta:go_default_library",
"//internal/controller/feature:go_default_library",
"//internal/webhook/feature:go_default_library",
"//pkg/api/util:go_default_library",
"//pkg/apis/acme:go_default_library",
"//pkg/apis/certmanager:go_default_library",
@ -31,6 +31,7 @@ go_library(
"@io_k8s_apimachinery//pkg/api/validation:go_default_library",
"@io_k8s_apimachinery//pkg/apis/meta/v1/validation:go_default_library",
"@io_k8s_apimachinery//pkg/runtime:go_default_library",
"@io_k8s_apimachinery//pkg/util/sets:go_default_library",
"@io_k8s_apimachinery//pkg/util/validation/field:go_default_library",
],
)
@ -49,7 +50,9 @@ go_test(
"//internal/apis/acme:go_default_library",
"//internal/apis/certmanager:go_default_library",
"//internal/apis/meta:go_default_library",
"//internal/webhook/feature:go_default_library",
"//pkg/apis/certmanager/v1:go_default_library",
"//pkg/util/feature:go_default_library",
"//pkg/util/pki:go_default_library",
"//test/unit/gen:go_default_library",
"@com_github_stretchr_testify//assert:go_default_library",
@ -57,6 +60,7 @@ go_test(
"@io_k8s_api//core/v1:go_default_library",
"@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
"@io_k8s_apimachinery//pkg/util/validation/field:go_default_library",
"@io_k8s_component_base//featuregate/testing:go_default_library",
],
)

41
internal/apis/certmanager/validation/certificate.go
View File

@ -26,11 +26,12 @@ import (
apivalidation "k8s.io/apimachinery/pkg/api/validation"
metavalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field"
internalcmapi "github.com/jetstack/cert-manager/internal/apis/certmanager"
cmmeta "github.com/jetstack/cert-manager/internal/apis/meta"
"github.com/jetstack/cert-manager/internal/controller/feature"
"github.com/jetstack/cert-manager/internal/webhook/feature"
"github.com/jetstack/cert-manager/pkg/api/util"
cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
utilfeature "github.com/jetstack/cert-manager/pkg/util/feature"
@ -78,19 +79,6 @@ func ValidateCertificateSpec(crt *internalcmapi.CertificateSpec, fldPath *field.
default:
el = append(el, field.Invalid(fldPath.Child("privateKey", "algorithm"), crt.PrivateKey.Algorithm, "must be either empty or one of rsa or ecdsa"))
}
if crt.AdditionalOutputFormats != nil {
if !utilfeature.DefaultFeatureGate.Enabled(feature.AdditionalCertificateOutputFormats) {
el = append(el, field.Forbidden(fldPath.Child("AdditionalOutputFormat"), "Feature gate AdditionalCertificateOutputFormats must be enabled"))
}
check := make(map[string]bool)
for _, val := range crt.AdditionalOutputFormats {
if _, exists := check[string(val.Type)]; !exists {
check[string(val.Type)] = true
} else {
el = append(el, field.Invalid(fldPath.Child("AdditionalOutputFormats"), crt.AdditionalOutputFormats, "Duplicate Type in additionalOutputFormats"))
}
}
}
}
if crt.Duration != nil || crt.RenewBefore != nil {
@ -112,6 +100,8 @@ func ValidateCertificateSpec(crt *internalcmapi.CertificateSpec, fldPath *field.
}
}
el = append(el, validateAdditionalOutputFormats(crt, fldPath)...)
return el
}
@ -224,3 +214,26 @@ func ValidateDuration(crt *internalcmapi.CertificateSpec, fldPath *field.Path) f
}
return el
}
func validateAdditionalOutputFormats(crt *internalcmapi.CertificateSpec, fldPath *field.Path) field.ErrorList {
var el field.ErrorList
if !utilfeature.DefaultFeatureGate.Enabled(feature.AdditionalCertificateOutputFormats) {
if len(crt.AdditionalOutputFormats) > 0 {
el = append(el, field.Forbidden(fldPath.Child("additionalOutputFormats"), "feature gate AdditionalCertificateOutputFormats must be enabled"))
}
return el
}
// Ensure the set of output formats is unique, keyed on "Type".
aofSet := sets.NewString()
for _, val := range crt.AdditionalOutputFormats {
if aofSet.Has(string(val.Type)) {
el = append(el, field.Duplicate(fldPath.Child("additionalOutputFormats").Key("type"), string(val.Type)))
continue
}
aofSet.Insert(string(val.Type))
}
return el
}

114
internal/apis/certmanager/validation/certificate_test.go
View File

@ -25,10 +25,13 @@ import (
admissionv1 "k8s.io/api/admission/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
featuregatetesting "k8s.io/component-base/featuregate/testing"
internalcmapi "github.com/jetstack/cert-manager/internal/apis/certmanager"
cmmeta "github.com/jetstack/cert-manager/internal/apis/meta"
"github.com/jetstack/cert-manager/internal/webhook/feature"
cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
utilfeature "github.com/jetstack/cert-manager/pkg/util/feature"
"github.com/stretchr/testify/assert"
)
@ -784,3 +787,114 @@ func TestValidateDuration(t *testing.T) {
})
}
}
func Test_validateAdditionalOutputFormats(t *testing.T) {
tests := map[string]struct {
featureEnabled bool
spec *internalcmapi.CertificateSpec
expErr field.ErrorList
}{
"if feature disabled and no formats defined, expect no error": {
featureEnabled: false,
spec: &internalcmapi.CertificateSpec{
AdditionalOutputFormats: []internalcmapi.CertificateAdditionalOutputFormat{},
},
expErr: nil,
},
"if feature disabled and 1 format defined, expect error": {
featureEnabled: false,
spec: &internalcmapi.CertificateSpec{
AdditionalOutputFormats: []internalcmapi.CertificateAdditionalOutputFormat{
{Type: internalcmapi.CertificateOutputFormatType("foo")},
},
},
expErr: field.ErrorList{
field.Forbidden(field.NewPath("spec", "additionalOutputFormats"), "feature gate AdditionalCertificateOutputFormats must be enabled"),
},
},
"if feature disabled and multiple formats defined, expect error": {
featureEnabled: false,
spec: &internalcmapi.CertificateSpec{
AdditionalOutputFormats: []internalcmapi.CertificateAdditionalOutputFormat{
{Type: internalcmapi.CertificateOutputFormatType("foo")},
{Type: internalcmapi.CertificateOutputFormatType("bar")},
{Type: internalcmapi.CertificateOutputFormatType("random")},
},
},
expErr: field.ErrorList{
field.Forbidden(field.NewPath("spec", "additionalOutputFormats"), "feature gate AdditionalCertificateOutputFormats must be enabled"),
},
},
"if feature enabled and no formats defined, expect no error": {
featureEnabled: true,
spec: &internalcmapi.CertificateSpec{
AdditionalOutputFormats: []internalcmapi.CertificateAdditionalOutputFormat{},
},
expErr: nil,
},
"if feature enabled and single format defined, expect no error": {
featureEnabled: true,
spec: &internalcmapi.CertificateSpec{
AdditionalOutputFormats: []internalcmapi.CertificateAdditionalOutputFormat{
{Type: internalcmapi.CertificateOutputFormatType("foo")},
},
},
expErr: nil,
},
"if feature enabled and multiple unique formats defined, expect no error": {
featureEnabled: true,
spec: &internalcmapi.CertificateSpec{
AdditionalOutputFormats: []internalcmapi.CertificateAdditionalOutputFormat{
{Type: internalcmapi.CertificateOutputFormatType("foo")},
{Type: internalcmapi.CertificateOutputFormatType("bar")},
{Type: internalcmapi.CertificateOutputFormatType("random")},
},
},
expErr: nil,
},
"if feature enabled and multiple formats defined but 2 non-unique, expect error": {
featureEnabled: true,
spec: &internalcmapi.CertificateSpec{
AdditionalOutputFormats: []internalcmapi.CertificateAdditionalOutputFormat{
{Type: internalcmapi.CertificateOutputFormatType("foo")},
{Type: internalcmapi.CertificateOutputFormatType("bar")},
{Type: internalcmapi.CertificateOutputFormatType("random")},
{Type: internalcmapi.CertificateOutputFormatType("foo")},
},
},
expErr: field.ErrorList{
field.Duplicate(field.NewPath("spec", "additionalOutputFormats").Key("type"), "foo"),
},
},
"if feature enabled and multiple formats defined but multiple non-unique, expect error": {
featureEnabled: true,
spec: &internalcmapi.CertificateSpec{
AdditionalOutputFormats: []internalcmapi.CertificateAdditionalOutputFormat{
{Type: internalcmapi.CertificateOutputFormatType("foo")},
{Type: internalcmapi.CertificateOutputFormatType("bar")},
{Type: internalcmapi.CertificateOutputFormatType("random")},
{Type: internalcmapi.CertificateOutputFormatType("random")},
{Type: internalcmapi.CertificateOutputFormatType("foo")},
{Type: internalcmapi.CertificateOutputFormatType("bar")},
{Type: internalcmapi.CertificateOutputFormatType("bar")},
{Type: internalcmapi.CertificateOutputFormatType("123")},
{Type: internalcmapi.CertificateOutputFormatType("456")},
},
},
expErr: field.ErrorList{
field.Duplicate(field.NewPath("spec", "additionalOutputFormats").Key("type"), "random"),
field.Duplicate(field.NewPath("spec", "additionalOutputFormats").Key("type"), "foo"),
field.Duplicate(field.NewPath("spec", "additionalOutputFormats").Key("type"), "bar"),
field.Duplicate(field.NewPath("spec", "additionalOutputFormats").Key("type"), "bar"),
},
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultMutableFeatureGate, feature.AdditionalCertificateOutputFormats, test.featureEnabled)()
gotErr := validateAdditionalOutputFormats(test.spec, field.NewPath("spec"))
assert.Equal(t, test.expErr, gotErr)
})
}
}