Add FieldRef support

config/core/configmaps/features.yaml

@@ -20,7 +20,7 @@ metadata:
   labels:
     serving.knative.dev/release: devel
   annotations:
-    knative.dev/example-checksum: 84c3c6c3
+    knative.dev/example-checksum: f9125b41
 data:
   _example: |
     ################################
@@ -40,3 +40,6 @@ data:
 
     # Indicates whether multi container support is enabled
     multi-container: "disabled"
+
+    # Indicates whether Kubernetes FieldRef support is enabled
+    kubernetes/field-ref: "disabled"

pkg/apis/config/features.go

@@ -36,15 +36,18 @@ const (
 
 func defaultFeaturesConfig() *Features {
 	return &Features{
-		MultiContainer: Disabled,
+		MultiContainer:     Disabled,
+		KubernetesFieldRef: Disabled,
 	}
 }
 
 // NewFeaturesConfigFromMap creates a Features from the supplied Map
 func NewFeaturesConfigFromMap(data map[string]string) (*Features, error) {
 	nc := defaultFeaturesConfig()
 
-	if err := cm.Parse(data, asFlag("multi-container", &nc.MultiContainer)); err != nil {
+	if err := cm.Parse(data,
+		asFlag("multi-container", &nc.MultiContainer),
+		asFlag("kubernetes/field-ref", &nc.KubernetesFieldRef)); err != nil {
 		return nil, err
 	}
 	return nc, nil
@@ -57,7 +60,8 @@ func NewFeaturesConfigFromConfigMap(config *corev1.ConfigMap) (*Features, error)
 
 // Features specifies which features are allowed by the webhook.
 type Features struct {
-	MultiContainer Flag
+	MultiContainer     Flag
+	KubernetesFieldRef Flag
 }
 
 // asFlag parses the value at key as a Flag into the target, if it exists.

pkg/apis/config/features_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package config
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -57,21 +58,57 @@ func TestFeaturesConfiguration(t *testing.T) {
 	}, {
 		name:    "multi-container Allowed",
 		wantErr: false,
-		wantFeatures: &Features{
+		wantFeatures: defaultWith(&Features{
 			MultiContainer: Allowed,
-		},
+		}),
 		data: map[string]string{
 			"multi-container": "Allowed",
 		},
 	}, {
 		name:    "multi-container Enabled",
 		wantErr: false,
-		wantFeatures: &Features{
+		wantFeatures: defaultWith(&Features{
 			MultiContainer: Enabled,
-		},
+		}),
 		data: map[string]string{
 			"multi-container": "Enabled",
 		},
+	}, {
+		name:    "multi-container Disabled",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			MultiContainer: Disabled,
+		}),
+		data: map[string]string{
+			"multi-container": "Disabled",
+		},
+	}, {
+		name:    "kubernetes/field-ref Allowed",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			KubernetesFieldRef: Allowed,
+		}),
+		data: map[string]string{
+			"kubernetes/field-ref": "Allowed",
+		},
+	}, {
+		name:    "kubernetes/field-ref Enabled",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			KubernetesFieldRef: Enabled,
+		}),
+		data: map[string]string{
+			"kubernetes/field-ref": "Enabled",
+		},
+	}, {
+		name:    "kubernetes/field-ref Disabled",
+		wantErr: false,
+		wantFeatures: defaultWith(&Features{
+			KubernetesFieldRef: Disabled,
+		}),
+		data: map[string]string{
+			"kubernetes/field-ref": "Disabled",
+		},
 	}}
 
 	for _, tt := range configTests {
@@ -91,3 +128,16 @@ func TestFeaturesConfiguration(t *testing.T) {
 		})
 	}
 }
+
+// defaultWith returns the default *Feature patched with the provided *Features.
+func defaultWith(p *Features) *Features {
+	f := defaultFeaturesConfig()
+	pType := reflect.ValueOf(p).Elem()
+	fType := reflect.ValueOf(f).Elem()
+	for i := 0; i < pType.NumField(); i++ {
+		if pType.Field(i).Interface().(Flag) != "" {
+			fType.Field(i).Set(pType.Field(i))
+		}
+	}
+	return f
+}

pkg/apis/serving/fieldmask.go

@@ -372,7 +372,7 @@ func EnvVarMask(in *corev1.EnvVar) *corev1.EnvVar {
 // EnvVarSourceMask performs a _shallow_ copy of the Kubernetes EnvVarSource object to a new
 // Kubernetes EnvVarSource 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 EnvVarSourceMask(in *corev1.EnvVarSource) *corev1.EnvVarSource {
+func EnvVarSourceMask(in *corev1.EnvVarSource, fieldRef bool) *corev1.EnvVarSource {
 	if in == nil {
 		return nil
 	}
@@ -383,10 +383,10 @@ func EnvVarSourceMask(in *corev1.EnvVarSource) *corev1.EnvVarSource {
 	out.ConfigMapKeyRef = in.ConfigMapKeyRef
 	out.SecretKeyRef = in.SecretKeyRef
 
-	// Disallowed
-	// This list is unnecessary, but added here for clarity
-	out.FieldRef = nil
-	out.ResourceFieldRef = nil
+	if fieldRef {
+		out.FieldRef = in.FieldRef
+		out.ResourceFieldRef = in.ResourceFieldRef
+	}
 
 	return out
 }

pkg/apis/serving/fieldmask_test.go

@@ -415,31 +415,59 @@ func TestEnvVarMask(t *testing.T) {
 }
 
 func TestEnvVarSourceMask(t *testing.T) {
-	want := &corev1.EnvVarSource{
-		ConfigMapKeyRef: &corev1.ConfigMapKeySelector{},
-		SecretKeyRef:    &corev1.SecretKeySelector{},
-	}
-	in := &corev1.EnvVarSource{
-		ConfigMapKeyRef:  &corev1.ConfigMapKeySelector{},
-		SecretKeyRef:     &corev1.SecretKeySelector{},
-		FieldRef:         &corev1.ObjectFieldSelector{},
-		ResourceFieldRef: &corev1.ResourceFieldSelector{},
-	}
-
-	got := EnvVarSourceMask(in)
-
-	if &want == &got {
-		t.Error("Input and output share addresses. Want different addresses")
-	}
-
-	if diff, err := kmp.SafeDiff(want, got); err != nil {
-		t.Errorf("Got error comparing output, err = %v", err)
-	} else if diff != "" {
-		t.Errorf("EnvVarSourceMask (-want, +got): %s", diff)
-	}
-
-	if got = EnvVarSourceMask(nil); got != nil {
-		t.Errorf("EnvVarSourceMask(nil) = %v, want: nil", got)
+	tests := []struct {
+		name     string
+		fieldRef bool
+		in       *corev1.EnvVarSource
+		want     *corev1.EnvVarSource
+	}{{
+		name:     "FieldRef false",
+		fieldRef: false,
+		in: &corev1.EnvVarSource{
+			ConfigMapKeyRef:  &corev1.ConfigMapKeySelector{},
+			SecretKeyRef:     &corev1.SecretKeySelector{},
+			FieldRef:         &corev1.ObjectFieldSelector{},
+			ResourceFieldRef: &corev1.ResourceFieldSelector{},
+		},
+		want: &corev1.EnvVarSource{
+			ConfigMapKeyRef: &corev1.ConfigMapKeySelector{},
+			SecretKeyRef:    &corev1.SecretKeySelector{},
+		},
+	}, {
+		name:     "FieldRef true",
+		fieldRef: true,
+		in: &corev1.EnvVarSource{
+			ConfigMapKeyRef:  &corev1.ConfigMapKeySelector{},
+			SecretKeyRef:     &corev1.SecretKeySelector{},
+			FieldRef:         &corev1.ObjectFieldSelector{},
+			ResourceFieldRef: &corev1.ResourceFieldSelector{},
+		},
+		want: &corev1.EnvVarSource{
+			ConfigMapKeyRef:  &corev1.ConfigMapKeySelector{},
+			SecretKeyRef:     &corev1.SecretKeySelector{},
+			FieldRef:         &corev1.ObjectFieldSelector{},
+			ResourceFieldRef: &corev1.ResourceFieldSelector{},
+		},
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := EnvVarSourceMask(test.in, test.fieldRef)
+
+			if &test.want == &got {
+				t.Error("Input and output share addresses. Want different addresses")
+			}
+
+			if diff, err := kmp.SafeDiff(test.want, got); err != nil {
+				t.Errorf("Got error comparing output, err = %v", err)
+			} else if diff != "" {
+				t.Errorf("EnvVarSourceMask (-want, +got): %s", diff)
+			}
+
+			if got = EnvVarSourceMask(nil, test.fieldRef); got != nil {
+				t.Errorf("EnvVarSourceMask(nil) = %v, want: nil", got)
+			}
+		})
 	}
 }
 

pkg/apis/serving/k8s_validation.go

@@ -181,14 +181,15 @@ func validateKeyToPath(k2p corev1.KeyToPath) *apis.FieldError {
 	return errs
 }
 
-func validateEnvValueFrom(source *corev1.EnvVarSource) *apis.FieldError {
+func validateEnvValueFrom(ctx context.Context, source *corev1.EnvVarSource) *apis.FieldError {
 	if source == nil {
 		return nil
 	}
-	return apis.CheckDisallowedFields(*source, *EnvVarSourceMask(source))
+	features := config.FromContextOrDefaults(ctx).Features
+	return apis.CheckDisallowedFields(*source, *EnvVarSourceMask(source, features.KubernetesFieldRef != config.Disabled))
 }
 
-func validateEnvVar(env corev1.EnvVar) *apis.FieldError {
+func validateEnvVar(ctx context.Context, env corev1.EnvVar) *apis.FieldError {
 	errs := apis.CheckDisallowedFields(env, *EnvVarMask(&env))
 
 	if env.Name == "" {
@@ -200,13 +201,13 @@ func validateEnvVar(env corev1.EnvVar) *apis.FieldError {
 		})
 	}
 
-	return errs.Also(validateEnvValueFrom(env.ValueFrom).ViaField("valueFrom"))
+	return errs.Also(validateEnvValueFrom(ctx, env.ValueFrom).ViaField("valueFrom"))
 }
 
-func validateEnv(envVars []corev1.EnvVar) *apis.FieldError {
+func validateEnv(ctx context.Context, envVars []corev1.EnvVar) *apis.FieldError {
 	var errs *apis.FieldError
 	for i, env := range envVars {
-		errs = errs.Also(validateEnvVar(env).ViaIndex(i))
+		errs = errs.Also(validateEnvVar(ctx, env).ViaIndex(i))
 	}
 	return errs
 }
@@ -257,7 +258,7 @@ func ValidatePodSpec(ctx context.Context, ps corev1.PodSpec) *apis.FieldError {
 	case 0:
 		errs = errs.Also(apis.ErrMissingField("containers"))
 	case 1:
-		errs = errs.Also(ValidateContainer(ps.Containers[0], volumes).
+		errs = errs.Also(ValidateContainer(ctx, ps.Containers[0], volumes).
 			ViaFieldIndex("containers", 0))
 	default:
 		errs = errs.Also(validateContainers(ctx, ps.Containers, volumes))
@@ -282,9 +283,9 @@ func validateContainers(ctx context.Context, containers []corev1.Container, volu
 			// Probes are not allowed on other than serving container,
 			// ref: http://bit.ly/probes-condition
 			if len(containers[i].Ports) == 0 {
-				errs = errs.Also(validateSidecarContainer(containers[i], volumes).ViaFieldIndex("containers", i))
+				errs = errs.Also(validateSidecarContainer(ctx, containers[i], volumes).ViaFieldIndex("containers", i))
 			} else {
-				errs = errs.Also(ValidateContainer(containers[i], volumes).ViaFieldIndex("containers", i))
+				errs = errs.Also(ValidateContainer(ctx, containers[i], volumes).ViaFieldIndex("containers", i))
 			}
 		}
 	}
@@ -309,7 +310,7 @@ func validateContainersPorts(containers []corev1.Container) *apis.FieldError {
 }
 
 // validateSidecarContainer validate fields for non serving containers
-func validateSidecarContainer(container corev1.Container, volumes sets.String) *apis.FieldError {
+func validateSidecarContainer(ctx context.Context, container corev1.Container, volumes sets.String) *apis.FieldError {
 	var errs *apis.FieldError
 	if container.LivenessProbe != nil {
 		errs = errs.Also(apis.CheckDisallowedFields(*container.LivenessProbe,
@@ -319,19 +320,19 @@ func validateSidecarContainer(container corev1.Container, volumes sets.String) *
 		errs = errs.Also(apis.CheckDisallowedFields(*container.ReadinessProbe,
 			*ProbeMask(&corev1.Probe{})).ViaField("readinessProbe"))
 	}
-	return errs.Also(validate(container, volumes))
+	return errs.Also(validate(ctx, container, volumes))
 }
 
 // ValidateContainer validate fields for serving containers
-func ValidateContainer(container corev1.Container, volumes sets.String) *apis.FieldError {
+func ValidateContainer(ctx context.Context, container corev1.Container, volumes sets.String) *apis.FieldError {
 	var errs *apis.FieldError
 	// Single container cannot have multiple ports
 	errs = errs.Also(portValidation(container.Ports).ViaField("ports"))
 	// Liveness Probes
 	errs = errs.Also(validateProbe(container.LivenessProbe).ViaField("livenessProbe"))
 	// Readiness Probes
 	errs = errs.Also(validateReadinessProbe(container.ReadinessProbe).ViaField("readinessProbe"))
-	return errs.Also(validate(container, volumes))
+	return errs.Also(validate(ctx, container, volumes))
 }
 
 func portValidation(containerPorts []corev1.ContainerPort) *apis.FieldError {
@@ -345,7 +346,7 @@ func portValidation(containerPorts []corev1.ContainerPort) *apis.FieldError {
 	return nil
 }
 
-func validate(container corev1.Container, volumes sets.String) *apis.FieldError {
+func validate(ctx context.Context, container corev1.Container, volumes sets.String) *apis.FieldError {
 	if equality.Semantic.DeepEqual(container, corev1.Container{}) {
 		return apis.ErrMissingField(apis.CurrentField)
 	}
@@ -360,7 +361,7 @@ func validate(container corev1.Container, volumes sets.String) *apis.FieldError
 	}
 
 	// Env
-	errs = errs.Also(validateEnv(container.Env).ViaField("env"))
+	errs = errs.Also(validateEnv(ctx, container.Env).ViaField("env"))
 	// EnvFrom
 	errs = errs.Also(validateEnvFrom(container.EnvFrom).ViaField("envFrom"))
 	// Image