Skip to content

Commit

Permalink
Add ConversionReviewVersions to CustomResourceDefinitionSpec and defd…
Browse files Browse the repository at this point in the history
…efault it to v1beta1
  • Loading branch information
mbohlool committed Mar 8, 2019
1 parent f7dff47 commit 9b28b91
Show file tree
Hide file tree
Showing 7 changed files with 555 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} {
Strategy: apiextensions.NoneConverter,
}
}
if obj.Conversion.Strategy == apiextensions.WebhookConverter && len(obj.Conversion.ConversionReviewVersions) == 0 {
obj.Conversion.ConversionReviewVersions = []string{"v1beta1"}
}
},
func(obj *apiextensions.CustomResourceDefinition, c fuzz.Continue) {
c.FuzzNoCustom(obj)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@ type CustomResourceConversion struct {

// `webhookClientConfig` is the instructions for how to call the webhook if strategy is `Webhook`.
WebhookClientConfig *WebhookClientConfig

// ConversionReviewVersions is an ordered list of preferred `ConversionReview`
// versions the Webhook expects. API server will try to use first version in
// the list which it supports. If none of the versions specified in this list
// supported by API server, conversion will fail for this object.
// If a persisted Webhook configuration specifies allowed versions and does not
// include any versions known to the API Server, calls to the webhook will fail.
// +optional
ConversionReviewVersions []string
}

// WebhookClientConfig contains the information to make a TLS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ func SetDefaults_CustomResourceDefinitionSpec(obj *CustomResourceDefinitionSpec)
Strategy: NoneConverter,
}
}
if obj.Conversion.Strategy == WebhookConverter && len(obj.Conversion.ConversionReviewVersions) == 0 {
obj.Conversion.ConversionReviewVersions = []string{SchemeGroupVersion.Version}
}
}

// hasPerVersionColumns returns true if a CRD uses per-version columns.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,16 @@ type CustomResourceConversion struct {
// alpha-level and is only honored by servers that enable the CustomResourceWebhookConversion feature.
// +optional
WebhookClientConfig *WebhookClientConfig `json:"webhookClientConfig,omitempty" protobuf:"bytes,2,name=webhookClientConfig"`

// ConversionReviewVersions is an ordered list of preferred `ConversionReview`
// versions the Webhook expects. API server will try to use first version in
// the list which it supports. If none of the versions specified in this list
// supported by API server, conversion will fail for this object.
// If a persisted Webhook configuration specifies allowed versions and does not
// include any versions known to the API Server, calls to the webhook will fail.
// Default to `['v1beta1']`.
// +optional
ConversionReviewVersions []string `json:"conversionReviewVersions,omitempty" protobuf:"bytes,3,rep,name=conversionReviewVersions"`
}

// WebhookClientConfig contains the information to make a TLS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ import (
apiequality "k8s.io/apimachinery/pkg/api/equality"
genericvalidation "k8s.io/apimachinery/pkg/api/validation"
"k8s.io/apimachinery/pkg/util/sets"
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
validationutil "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/apiserver/pkg/util/webhook"

"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
apiservervalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
)
Expand Down Expand Up @@ -113,6 +115,10 @@ func ValidateCustomResourceDefinitionVersion(version *apiextensions.CustomResour

// ValidateCustomResourceDefinitionSpec statically validates
func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, fldPath *field.Path) field.ErrorList {
return validateCustomResourceDefinitionSpec(spec, true, fldPath)
}

func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, requireRecognizedVersion bool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if len(spec.Group) == 0 {
Expand Down Expand Up @@ -205,7 +211,7 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi
}
}

allErrs = append(allErrs, ValidateCustomResourceConversion(spec.Conversion, fldPath.Child("conversion"))...)
allErrs = append(allErrs, validateCustomResourceConversion(spec.Conversion, requireRecognizedVersion, fldPath.Child("conversion"))...)

return allErrs
}
Expand All @@ -225,8 +231,66 @@ func validateEnumStrings(fldPath *field.Path, value string, accepted []string, r
return field.ErrorList{field.NotSupported(fldPath, value, accepted)}
}

var acceptedConversionReviewVersion = []string{v1beta1.SchemeGroupVersion.Version}

func isAcceptedConversionReviewVersion(v string) bool {
for _, version := range acceptedConversionReviewVersion {
if v == version {
return true
}
}
return false
}

func validateConversionReviewVersions(versions []string, requireRecognizedVersion bool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if len(versions) < 1 {
allErrs = append(allErrs, field.Required(fldPath, ""))
} else {
seen := map[string]bool{}
hasAcceptedVersion := false
for i, v := range versions {
if seen[v] {
allErrs = append(allErrs, field.Invalid(fldPath.Index(i), v, "duplicate version"))
continue
}
seen[v] = true
for _, errString := range utilvalidation.IsDNS1035Label(v) {
allErrs = append(allErrs, field.Invalid(fldPath.Index(i), v, errString))
}
if isAcceptedConversionReviewVersion(v) {
hasAcceptedVersion = true
}
}
if requireRecognizedVersion && !hasAcceptedVersion {
allErrs = append(allErrs, field.Invalid(
fldPath, versions,
fmt.Sprintf("none of the versions accepted by this server. accepted version(s) are %v",
strings.Join(acceptedConversionReviewVersion, ", "))))
}
}
return allErrs
}

// hasValidConversionReviewVersion return true if there is a valid version or if the list is empty.
func hasValidConversionReviewVersionOrEmpty(versions []string) bool {
if len(versions) < 1 {
return true
}
for _, v := range versions {
if isAcceptedConversionReviewVersion(v) {
return true
}
}
return false
}

// ValidateCustomResourceConversion statically validates
func ValidateCustomResourceConversion(conversion *apiextensions.CustomResourceConversion, fldPath *field.Path) field.ErrorList {
return validateCustomResourceConversion(conversion, true, fldPath)
}

func validateCustomResourceConversion(conversion *apiextensions.CustomResourceConversion, requireRecognizedVersion bool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if conversion == nil {
return allErrs
Expand All @@ -250,15 +314,22 @@ func ValidateCustomResourceConversion(conversion *apiextensions.CustomResourceCo
allErrs = append(allErrs, webhook.ValidateWebhookService(fldPath.Child("webhookClientConfig").Child("service"), cc.Service.Name, cc.Service.Namespace, cc.Service.Path)...)
}
}
} else if conversion.WebhookClientConfig != nil {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("webhookClientConfig"), "should not be set when strategy is not set to Webhook"))
allErrs = append(allErrs, validateConversionReviewVersions(conversion.ConversionReviewVersions, requireRecognizedVersion, fldPath.Child("conversionReviewVersions"))...)
} else {
if conversion.WebhookClientConfig != nil {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("webhookClientConfig"), "should not be set when strategy is not set to Webhook"))
}
if len(conversion.ConversionReviewVersions) > 0 {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("conversionReviewVersions"), "should not be set when strategy is not set to Webhook"))
}
}
return allErrs
}

// ValidateCustomResourceDefinitionSpecUpdate statically validates
func ValidateCustomResourceDefinitionSpecUpdate(spec, oldSpec *apiextensions.CustomResourceDefinitionSpec, established bool, fldPath *field.Path) field.ErrorList {
allErrs := ValidateCustomResourceDefinitionSpec(spec, fldPath)
requireRecognizedVersion := oldSpec.Conversion == nil || hasValidConversionReviewVersionOrEmpty(oldSpec.Conversion.ConversionReviewVersions)
allErrs := validateCustomResourceDefinitionSpec(spec, requireRecognizedVersion, fldPath)

if established {
// these effect the storage and cannot be changed therefore
Expand Down
Loading

0 comments on commit 9b28b91

Please sign in to comment.