Skip to content

K
k3s
Commit 998e22dd authored by Mehdy Bohlool's avatar Mehdy Bohlool
Browse files
Options

FeatureGate and API Validation for CRD Webhook Conversion

parent 1587d189
Hide whitespace changes
Inline Side-by-side
Showing with 307 additions and 17 deletions
pkg/features/kube_features.go
View file @ 998e22dd
...@@ -456,8 +456,9 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS ...@@ -456,8 +456,9 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
// inherited features from apiextensions-apiserver, relisted here to get a conflict if it is changed // inherited features from apiextensions-apiserver, relisted here to get a conflict if it is changed
// unintentionally on either side: // unintentionally on either side:
apiextensionsfeatures.CustomResourceValidation: {Default: true, PreRelease: utilfeature.Beta}, apiextensionsfeatures.CustomResourceValidation: {Default: true, PreRelease: utilfeature.Beta},
apiextensionsfeatures.CustomResourceSubresources: {Default: true, PreRelease: utilfeature.Beta}, apiextensionsfeatures.CustomResourceSubresources: {Default: true, PreRelease: utilfeature.Beta},
apiextensionsfeatures.CustomResourceWebhookConversion: {Default: false, PreRelease: utilfeature.Alpha},
// features that enable backwards compatibility but are scheduled to be removed // features that enable backwards compatibility but are scheduled to be removed
// ... // ...
......
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go
View file @ 998e22dd
...@@ -61,6 +61,11 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} { ...@@ -61,6 +61,11 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} {
{Name: "Age", Type: "date", Description: swaggerMetadataDescriptions["creationTimestamp"], JSONPath: ".metadata.creationTimestamp"}, {Name: "Age", Type: "date", Description: swaggerMetadataDescriptions["creationTimestamp"], JSONPath: ".metadata.creationTimestamp"},
} }
} }
if obj.Conversion == nil {
obj.Conversion = &apiextensions.CustomResourceConversion{
Strategy: apiextensions.NoneConverter,
}
}
}, },
func(obj *apiextensions.CustomResourceDefinition, c fuzz.Continue) { func(obj *apiextensions.CustomResourceDefinition, c fuzz.Continue) {
c.FuzzNoCustom(obj) c.FuzzNoCustom(obj)
......
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go
View file @ 998e22dd
...@@ -71,4 +71,9 @@ func SetDefaults_CustomResourceDefinitionSpec(obj *CustomResourceDefinitionSpec) ...@@ -71,4 +71,9 @@ func SetDefaults_CustomResourceDefinitionSpec(obj *CustomResourceDefinitionSpec)
{Name: "Age", Type: "date", Description: swaggerMetadataDescriptions["creationTimestamp"], JSONPath: ".metadata.creationTimestamp"}, {Name: "Age", Type: "date", Description: swaggerMetadataDescriptions["creationTimestamp"], JSONPath: ".metadata.creationTimestamp"},
} }
} }
if obj.Conversion == nil {
obj.Conversion = &CustomResourceConversion{
Strategy: NoneConverter,
}
}
} }
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go
View file @ 998e22dd
...@@ -18,6 +18,7 @@ package validation ...@@ -18,6 +18,7 @@ package validation
import ( import (
"fmt" "fmt"
"k8s.io/apiserver/pkg/util/webhook"
"reflect" "reflect"
"strings" "strings"
...@@ -110,13 +111,7 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi ...@@ -110,13 +111,7 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi
allErrs = append(allErrs, field.Invalid(fldPath.Child("group"), spec.Group, "should be a domain with at least one dot")) allErrs = append(allErrs, field.Invalid(fldPath.Child("group"), spec.Group, "should be a domain with at least one dot"))
} }
switch spec.Scope { allErrs = append(allErrs, validateEnumStrings(fldPath.Child("scope"), string(spec.Scope), []string{string(apiextensions.ClusterScoped), string(apiextensions.NamespaceScoped)}, true)...)
case "":
allErrs = append(allErrs, field.Required(fldPath.Child("scope"), ""))
case apiextensions.ClusterScoped, apiextensions.NamespaceScoped:
default:
allErrs = append(allErrs, field.NotSupported(fldPath.Child("scope"), spec.Scope, []string{string(apiextensions.ClusterScoped), string(apiextensions.NamespaceScoped)}))
}
storageFlagCount := 0 storageFlagCount := 0
versionsMap := map[string]bool{} versionsMap := map[string]bool{}
...@@ -187,6 +182,54 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi ...@@ -187,6 +182,54 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi
} }
} }
allErrs = append(allErrs, ValidateCustomResourceConversion(spec.Conversion, fldPath.Child("conversion"))...)
return allErrs
}
func validateEnumStrings(fldPath *field.Path, value string, accepted []string, required bool) field.ErrorList {
if value == "" {
if required {
return field.ErrorList{field.Required(fldPath, "")}
}
return field.ErrorList{}
}
for _, a := range accepted {
if a == value {
return field.ErrorList{}
}
}
return field.ErrorList{field.NotSupported(fldPath, value, accepted)}
}
// ValidateCustomResourceConversion statically validates
func ValidateCustomResourceConversion(conversion *apiextensions.CustomResourceConversion, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if conversion == nil {
return allErrs
}
allErrs = append(allErrs, validateEnumStrings(fldPath.Child("strategy"), string(conversion.Strategy), []string{string(apiextensions.NoneConverter), string(apiextensions.WebhookConverter)}, true)...)
if conversion.Strategy == apiextensions.WebhookConverter {
if conversion.WebhookClientConfig == nil {
if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceWebhookConversion) {
allErrs = append(allErrs, field.Required(fldPath.Child("webhookClientConfig"), "required when strategy is set to Webhook"))
} else {
allErrs = append(allErrs, field.Required(fldPath.Child("webhookClientConfig"), "required when strategy is set to Webhook, but not allowed because the CustomResourceWebhookConversion feature is disabled"))
}
} else {
cc := conversion.WebhookClientConfig
switch {
case (cc.URL == nil) == (cc.Service == nil):
allErrs = append(allErrs, field.Required(fldPath.Child("webhookClientConfig"), "exactly one of url or service is required"))
case cc.URL != nil:
allErrs = append(allErrs, webhook.ValidateWebhookURL(fldPath.Child("webhookClientConfig").Child("url"), *cc.URL, true)...)
case cc.Service != nil:
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"))
}
return allErrs return allErrs
} }
......
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go
View file @ 998e22dd
...@@ -49,6 +49,8 @@ func (v validationMatch) matches(err *field.Error) bool { ...@@ -49,6 +49,8 @@ func (v validationMatch) matches(err *field.Error) bool {
return err.Type == v.errorType && err.Field == v.path.String() return err.Type == v.errorType && err.Field == v.path.String()
} }
func strPtr(s string) *string { return &s }
func TestValidateCustomResourceDefinition(t *testing.T) { func TestValidateCustomResourceDefinition(t *testing.T) {
singleVersionList := []apiextensions.CustomResourceDefinitionVersion{ singleVersionList := []apiextensions.CustomResourceDefinitionVersion{
{ {
...@@ -63,6 +65,205 @@ func TestValidateCustomResourceDefinition(t *testing.T) { ...@@ -63,6 +65,205 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
errors []validationMatch errors []validationMatch
}{ }{
{ {
name: "webhookconfig: blank URL",
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Scope: apiextensions.ResourceScope("Cluster"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "Plural",
ListKind: "PluralList",
},
Versions: []apiextensions.CustomResourceDefinitionVersion{
{
Name: "version",
Served: true,
Storage: true,
},
{
Name: "version2",
Served: true,
Storage: false,
},
},
Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("Webhook"),
WebhookClientConfig: &apiextensions.WebhookClientConfig{
URL: strPtr("https://example.com/webhook"),
Service: &apiextensions.ServiceReference{
Name: "n",
Namespace: "ns",
},
},
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
StoredVersions: []string{"version"},
},
},
errors: []validationMatch{
required("spec", "conversion", "webhookClientConfig"),
},
},
{
name: "webhookconfig: both service and URL provided",
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Scope: apiextensions.ResourceScope("Cluster"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "Plural",
ListKind: "PluralList",
},
Versions: []apiextensions.CustomResourceDefinitionVersion{
{
Name: "version",
Served: true,
Storage: true,
},
{
Name: "version2",
Served: true,
Storage: false,
},
},
Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("Webhook"),
WebhookClientConfig: &apiextensions.WebhookClientConfig{
URL: strPtr(""),
},
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
StoredVersions: []string{"version"},
},
},
errors: []validationMatch{
invalid("spec", "conversion", "webhookClientConfig", "url"),
invalid("spec", "conversion", "webhookClientConfig", "url"),
},
},
{
name: "webhookconfig_should_not_be_set",
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Scope: apiextensions.ResourceScope("Cluster"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "Plural",
ListKind: "PluralList",
},
Versions: []apiextensions.CustomResourceDefinitionVersion{
{
Name: "version",
Served: true,
Storage: true,
},
{
Name: "version2",
Served: true,
Storage: false,
},
},
Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("None"),
WebhookClientConfig: &apiextensions.WebhookClientConfig{
URL: strPtr("https://example.com/webhook"),
},
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
StoredVersions: []string{"version"},
},
},
errors: []validationMatch{
forbidden("spec", "conversion", "webhookClientConfig"),
},
},
{
name: "missing_webhookconfig",
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Scope: apiextensions.ResourceScope("Cluster"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "Plural",
ListKind: "PluralList",
},
Versions: []apiextensions.CustomResourceDefinitionVersion{
{
Name: "version",
Served: true,
Storage: true,
},
{
Name: "version2",
Served: true,
Storage: false,
},
},
Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("Webhook"),
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
StoredVersions: []string{"version"},
},
},
errors: []validationMatch{
required("spec", "conversion", "webhookClientConfig"),
},
},
{
name: "invalid_conversion_strategy",
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Scope: apiextensions.ResourceScope("Cluster"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "Plural",
ListKind: "PluralList",
},
Versions: []apiextensions.CustomResourceDefinitionVersion{
{
Name: "version",
Served: true,
Storage: true,
},
{
Name: "version2",
Served: true,
Storage: false,
},
},
Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("non_existing_conversion"),
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
StoredVersions: []string{"version"},
},
},
errors: []validationMatch{
unsupported("spec", "conversion", "strategy"),
},
},
{
name: "no_storage_version", name: "no_storage_version",
resource: &apiextensions.CustomResourceDefinition{ resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
...@@ -87,6 +288,9 @@ func TestValidateCustomResourceDefinition(t *testing.T) { ...@@ -87,6 +288,9 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
Storage: false, Storage: false,
}, },
}, },
Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("None"),
},
}, },
Status: apiextensions.CustomResourceDefinitionStatus{ Status: apiextensions.CustomResourceDefinitionStatus{
StoredVersions: []string{"version"}, StoredVersions: []string{"version"},
...@@ -121,6 +325,9 @@ func TestValidateCustomResourceDefinition(t *testing.T) { ...@@ -121,6 +325,9 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
Storage: true, Storage: true,
}, },
}, },
Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("None"),
},
}, },
Status: apiextensions.CustomResourceDefinitionStatus{ Status: apiextensions.CustomResourceDefinitionStatus{
StoredVersions: []string{"version"}, StoredVersions: []string{"version"},
...@@ -156,6 +363,9 @@ func TestValidateCustomResourceDefinition(t *testing.T) { ...@@ -156,6 +363,9 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
Storage: true, Storage: true,
}, },
}, },
Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("None"),
},
}, },
Status: apiextensions.CustomResourceDefinitionStatus{ Status: apiextensions.CustomResourceDefinitionStatus{
StoredVersions: []string{"version"}, StoredVersions: []string{"version"},
...@@ -185,6 +395,9 @@ func TestValidateCustomResourceDefinition(t *testing.T) { ...@@ -185,6 +395,9 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
Storage: true, Storage: true,
}, },
}, },
Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("None"),
},
}, },
Status: apiextensions.CustomResourceDefinitionStatus{ Status: apiextensions.CustomResourceDefinitionStatus{
StoredVersions: []string{}, StoredVersions: []string{},
...@@ -283,6 +496,9 @@ func TestValidateCustomResourceDefinition(t *testing.T) { ...@@ -283,6 +496,9 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
Group: "group.c(*&om", Group: "group.c(*&om",
Version: "version", Version: "version",
Versions: singleVersionList, Versions: singleVersionList,
Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("None"),
},
Names: apiextensions.CustomResourceDefinitionNames{ Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural", Plural: "plural",
Singular: "singular", Singular: "singular",
...@@ -316,7 +532,10 @@ func TestValidateCustomResourceDefinition(t *testing.T) { ...@@ -316,7 +532,10 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
Group: "group.com", Group: "group.com",
Version: "version", Version: "version",
Versions: singleVersionList, Versions: singleVersionList,
Scope: apiextensions.NamespaceScoped, Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("None"),
},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{ Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural", Plural: "plural",
Singular: "singular", Singular: "singular",
...@@ -348,7 +567,10 @@ func TestValidateCustomResourceDefinition(t *testing.T) { ...@@ -348,7 +567,10 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
Group: "group.com", Group: "group.com",
Version: "version", Version: "version",
Versions: singleVersionList, Versions: singleVersionList,
Scope: apiextensions.NamespaceScoped, Conversion: &apiextensions.CustomResourceConversion{
Strategy: apiextensions.ConversionStrategyType("None"),
},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{ Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural", Plural: "plural",
Singular: "singular", Singular: "singular",
......
staging/src/k8s.io/apiextensions-apiserver/pkg/features/kube_features.go
View file @ 998e22dd
...@@ -40,6 +40,12 @@ const ( ...@@ -40,6 +40,12 @@ const (
// //
// CustomResourceSubresources defines the subresources for CustomResources // CustomResourceSubresources defines the subresources for CustomResources
CustomResourceSubresources utilfeature.Feature = "CustomResourceSubresources" CustomResourceSubresources utilfeature.Feature = "CustomResourceSubresources"
// owner: @mbohlool
// alpha: v1.13
//
// CustomResourceWebhookConversion defines the webhook conversion for Custom Resources.
CustomResourceWebhookConversion utilfeature.Feature = "CustomResourceWebhookConversion"
) )
func init() { func init() {
...@@ -50,6 +56,7 @@ func init() { ...@@ -50,6 +56,7 @@ func init() {
// To add a new feature, define a key for it above and add it here. The features will be // To add a new feature, define a key for it above and add it here. The features will be
// available throughout Kubernetes binaries. // available throughout Kubernetes binaries.
var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureSpec{ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureSpec{
CustomResourceValidation: {Default: true, PreRelease: utilfeature.Beta}, CustomResourceValidation: {Default: true, PreRelease: utilfeature.Beta},
CustomResourceSubresources: {Default: true, PreRelease: utilfeature.Beta}, CustomResourceSubresources: {Default: true, PreRelease: utilfeature.Beta},
CustomResourceWebhookConversion: {Default: false, PreRelease: utilfeature.Alpha},
} }
staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go
View file @ 998e22dd
...@@ -20,6 +20,9 @@ import ( ...@@ -20,6 +20,9 @@ import (
"context" "context"
"fmt" "fmt"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation"
apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
apiequality "k8s.io/apimachinery/pkg/api/equality" apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/labels"
...@@ -29,10 +32,6 @@ import ( ...@@ -29,10 +32,6 @@ import (
"k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names" "k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature" utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation"
apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
) )
// strategy implements behavior for CustomResources. // strategy implements behavior for CustomResources.
...@@ -62,6 +61,9 @@ func (strategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { ...@@ -62,6 +61,9 @@ func (strategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceSubresources) { if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceSubresources) {
crd.Spec.Subresources = nil crd.Spec.Subresources = nil
} }
if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceWebhookConversion) && crd.Spec.Conversion != nil {
crd.Spec.Conversion.WebhookClientConfig = nil
}
for _, v := range crd.Spec.Versions { for _, v := range crd.Spec.Versions {
if v.Storage { if v.Storage {
...@@ -99,6 +101,11 @@ func (strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { ...@@ -99,6 +101,11 @@ func (strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
newCRD.Spec.Subresources = nil newCRD.Spec.Subresources = nil
oldCRD.Spec.Subresources = nil oldCRD.Spec.Subresources = nil
} }
if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceWebhookConversion) && newCRD.Spec.Conversion != nil {
if oldCRD.Spec.Conversion == nil || newCRD.Spec.Conversion.WebhookClientConfig == nil {
newCRD.Spec.Conversion.WebhookClientConfig = nil
}
}
for _, v := range newCRD.Spec.Versions { for _, v := range newCRD.Spec.Versions {
if v.Storage { if v.Storage {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment