add feature flags for affinity/nodeSelector/tolerations

update formatting

improvements from review feedback

spell out functions to enable features

put context first

config/core/configmaps/features.yaml

@@ -20,7 +20,7 @@ metadata:
   labels:
     serving.knative.dev/release: devel
   annotations:
-    knative.dev/example-checksum: "52231144"
+    knative.dev/example-checksum: "38e90f86"
 data:
   _example: |
     ################################
@@ -41,6 +41,15 @@ data:
     # Indicates whether multi container support is enabled
     multi-container: "disabled"
 
+    # Indicates whether Kubernetes affinity support is enabled
+    kubernetes.podspec-affinity: "disabled"
+
+    # Indicates whether Kubernetes nodeSelector support is enabled
+    kubernetes.podspec-nodeselector: "disabled"
+
+    # Indicates whether Kubernetes tolerations support is enabled
+    kubernetes.podspec-tolerations: "disabled"
+
     # Indicates whether Kubernetes FieldRef support is enabled
     kubernetes.podspec-fieldref: "disabled"
 

pkg/apis/config/features.go

@@ -41,8 +41,11 @@ const (
 func defaultFeaturesConfig() *Features {
 	return &Features{
 		MultiContainer:       Disabled,
+		PodSpecAffinity:      Disabled,
 		PodSpecFieldRef:      Disabled,
 		PodSpecDryRun:        Enabled,
+		PodSpecNodeSelector:  Disabled,
+		PodSpecTolerations:   Disabled,
 		ResponsiveRevisionGC: Disabled,
 	}
 }
@@ -53,8 +56,11 @@ func NewFeaturesConfigFromMap(data map[string]string) (*Features, error) {
 
 	if err := cm.Parse(data,
 		asFlag("multi-container", &nc.MultiContainer),
+		asFlag("kubernetes.podspec-affinity", &nc.PodSpecAffinity),
 		asFlag("kubernetes.podspec-fieldref", &nc.PodSpecFieldRef),
 		asFlag("kubernetes.podspec-dryrun", &nc.PodSpecDryRun),
+		asFlag("kubernetes.podspec-nodeselector", &nc.PodSpecNodeSelector),
+		asFlag("kubernetes.podspec-tolerations", &nc.PodSpecTolerations),
 		asFlag("responsive-revision-gc", &nc.ResponsiveRevisionGC)); err != nil {
 		return nil, err
 	}
@@ -69,8 +75,11 @@ func NewFeaturesConfigFromConfigMap(config *corev1.ConfigMap) (*Features, error)
 // Features specifies which features are allowed by the webhook.
 type Features struct {
 	MultiContainer       Flag
+	PodSpecAffinity      Flag
 	PodSpecFieldRef      Flag
 	PodSpecDryRun        Flag
+	PodSpecNodeSelector  Flag
+	PodSpecTolerations   Flag
 	ResponsiveRevisionGC Flag
 }
 

pkg/apis/config/features_test.go

@@ -60,13 +60,19 @@ func TestFeaturesConfiguration(t *testing.T) {
 		wantErr: false,
 		wantFeatures: defaultWith(&Features{
 			MultiContainer:       Enabled,
+			PodSpecAffinity:      Enabled,
 			PodSpecDryRun:        Enabled,
+			PodSpecNodeSelector:  Enabled,
+			PodSpecTolerations:   Enabled,
 			ResponsiveRevisionGC: Enabled,
 		}),
 		data: map[string]string{
-			"multi-container":           "Enabled",
-			"kubernetes.podspec-dryrun": "Enabled",
-			"responsive-revision-gc":    "Enabled",
+			"multi-container":                 "Enabled",
+			"kubernetes.podspec-affinity":     "Enabled",
+			"kubernetes.podspec-dryrun":       "Enabled",
+			"kubernetes.podspec-nodeselector": "Enabled",
+			"kubernetes.podspec-tolerations":  "Enabled",
+			"responsive-revision-gc":          "Enabled",
 		},
 	}, {
 		name:    "multi-container Allowed",
@@ -86,6 +92,33 @@ func TestFeaturesConfiguration(t *testing.T) {
 		data: map[string]string{
 			"multi-container": "Disabled",
 		},
+	}, {
+		name:    "kubernetes.podspec-affinity Allowed",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			PodSpecAffinity: Allowed,
+		}),
+		data: map[string]string{
+			"kubernetes.podspec-affinity": "Allowed",
+		},
+	}, {
+		name:    "kubernetes.podspec-affinity Enabled",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			PodSpecAffinity: Enabled,
+		}),
+		data: map[string]string{
+			"kubernetes.podspec-affinity": "Enabled",
+		},
+	}, {
+		name:    "kubernetes.podspec-affinity Disabled",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			PodSpecAffinity: Disabled,
+		}),
+		data: map[string]string{
+			"kubernetes.podspec-affinity": "Disabled",
+		},
 	}, {
 		name:    "kubernetes.podspec-fieldref Allowed",
 		wantErr: false,
@@ -122,6 +155,60 @@ func TestFeaturesConfiguration(t *testing.T) {
 		data: map[string]string{
 			"kubernetes.podspec-dryrun": "Disabled",
 		},
+	}, {
+		name:    "kubernetes.podspec-nodeselector Allowed",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			PodSpecNodeSelector: Allowed,
+		}),
+		data: map[string]string{
+			"kubernetes.podspec-nodeselector": "Allowed",
+		},
+	}, {
+		name:    "kubernetes.podspec-nodeselector Enabled",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			PodSpecNodeSelector: Enabled,
+		}),
+		data: map[string]string{
+			"kubernetes.podspec-nodeselector": "Enabled",
+		},
+	}, {
+		name:    "kubernetes.podspec-nodeselector Disabled",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			PodSpecNodeSelector: Disabled,
+		}),
+		data: map[string]string{
+			"kubernetes.podspec-nodeselector": "Disabled",
+		},
+	}, {
+		name:    "kubernetes.podspec-tolerations Allowed",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			PodSpecTolerations: Allowed,
+		}),
+		data: map[string]string{
+			"kubernetes.podspec-tolerations": "Allowed",
+		},
+	}, {
+		name:    "kubernetes.podspec-tolerations Enabled",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			PodSpecTolerations: Enabled,
+		}),
+		data: map[string]string{
+			"kubernetes.podspec-tolerations": "Enabled",
+		},
+	}, {
+		name:    "kubernetes.podspec-tolerations Disabled",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			PodSpecTolerations: Disabled,
+		}),
+		data: map[string]string{
+			"kubernetes.podspec-tolerations": "Disabled",
+		},
 	}, {
 		name:    "responsive-revision-gc Allowed",
 		wantErr: false,

pkg/apis/config/store.go

@@ -45,17 +45,23 @@ func FromContext(ctx context.Context) *Config {
 // FromContextOrDefaults is like FromContext, but when no Config is attached it
 // returns a Config populated with the defaults for each of the Config fields.
 func FromContextOrDefaults(ctx context.Context) *Config {
-	if cfg := FromContext(ctx); cfg != nil {
-		return cfg
+	cfg := FromContext(ctx)
+	if cfg == nil {
+		cfg = &Config{}
 	}
-	defaults, _ := NewDefaultsConfigFromMap(map[string]string{})
-	features, _ := NewFeaturesConfigFromMap(map[string]string{})
-	autoscaler, _ := autoscalerconfig.NewConfigFromMap(map[string]string{})
-	return &Config{
-		Defaults:   defaults,
-		Features:   features,
-		Autoscaler: autoscaler,
+
+	if cfg.Defaults == nil {
+		cfg.Defaults, _ = NewDefaultsConfigFromMap(map[string]string{})
+	}
+
+	if cfg.Features == nil {
+		cfg.Features, _ = NewFeaturesConfigFromMap(map[string]string{})
+	}
+
+	if cfg.Autoscaler == nil {
+		cfg.Autoscaler, _ = autoscalerconfig.NewConfigFromMap(map[string]string{})
 	}
+	return cfg
 }
 
 // ToContext attaches the provided Config to the provided context, returning the

pkg/apis/serving/fieldmask.go

@@ -17,7 +17,10 @@ limitations under the License.
 package serving
 
 import (
+	"context"
+
 	corev1 "k8s.io/api/core/v1"
+	"knative.dev/serving/pkg/apis/config"
 )
 
 // VolumeMask performs a _shallow_ copy of the Kubernetes Volume object to a new
@@ -136,11 +139,12 @@ func KeyToPathMask(in *corev1.KeyToPath) *corev1.KeyToPath {
 // PodSpecMask performs a _shallow_ copy of the Kubernetes PodSpec object to a new
 // Kubernetes PodSpec object bringing over only the fields allowed in the Knative API. This
 // does not validate the contents or the bounds of the provided fields.
-func PodSpecMask(in *corev1.PodSpec) *corev1.PodSpec {
+func PodSpecMask(ctx context.Context, in *corev1.PodSpec) *corev1.PodSpec {
 	if in == nil {
 		return nil
 	}
 
+	cfg := config.FromContextOrDefaults(ctx)
 	out := new(corev1.PodSpec)
 
 	// Allowed fields
@@ -150,14 +154,24 @@ func PodSpecMask(in *corev1.PodSpec) *corev1.PodSpec {
 	out.ImagePullSecrets = in.ImagePullSecrets
 	out.EnableServiceLinks = in.EnableServiceLinks
 
+	// Feature fields
+	if cfg.Features.PodSpecAffinity != config.Disabled {
+		out.Affinity = in.Affinity
+	}
+	if cfg.Features.PodSpecNodeSelector != config.Disabled {
+		out.NodeSelector = in.NodeSelector
+	}
+	if cfg.Features.PodSpecTolerations != config.Disabled {
+		out.Tolerations = in.Tolerations
+	}
+
 	// Disallowed fields
 	// This list is unnecessary, but added here for clarity
 	out.InitContainers = nil
 	out.RestartPolicy = ""
 	out.TerminationGracePeriodSeconds = nil
 	out.ActiveDeadlineSeconds = nil
 	out.DNSPolicy = ""
-	out.NodeSelector = nil
 	out.AutomountServiceAccountToken = nil
 	out.NodeName = ""
 	out.HostNetwork = false
@@ -167,9 +181,7 @@ func PodSpecMask(in *corev1.PodSpec) *corev1.PodSpec {
 	out.SecurityContext = nil
 	out.Hostname = ""
 	out.Subdomain = ""
-	out.Affinity = nil
 	out.SchedulerName = ""
-	out.Tolerations = nil
 	out.HostAliases = nil
 	out.PriorityClassName = ""
 	out.Priority = nil

pkg/apis/serving/fieldmask_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package serving
 
 import (
+	"context"
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
@@ -117,7 +118,8 @@ func TestPodSpecMask(t *testing.T) {
 		}},
 	}
 
-	got := PodSpecMask(in)
+	ctx := context.Background()
+	got := PodSpecMask(ctx, in)
 
 	if &want == &got {
 		t.Error("Input and output share addresses. Want different addresses")
@@ -129,7 +131,7 @@ func TestPodSpecMask(t *testing.T) {
 		t.Errorf("PodSpecMask (-want, +got): %s", diff)
 	}
 
-	if got = PodSpecMask(nil); got != nil {
+	if got = PodSpecMask(ctx, nil); got != nil {
 		t.Errorf("PodSpecMask(nil) = %v, want: nil", got)
 	}
 }

pkg/apis/serving/k8s_validation.go

@@ -263,7 +263,7 @@ func ValidatePodSpec(ctx context.Context, ps corev1.PodSpec) *apis.FieldError {
 	// 	return apis.ErrMissingField(apis.CurrentField)
 	// }
 
-	errs := apis.CheckDisallowedFields(ps, *PodSpecMask(&ps))
+	errs := apis.CheckDisallowedFields(ps, *PodSpecMask(ctx, &ps))
 
 	volumes, err := ValidateVolumes(ps.Volumes, AllMountedVolumes(ps.Containers))
 	if err != nil {