Merge pull request #72714 from mourya007/features_gate

Move TokenRequestProjection feature gate out of validation

pkg/api/pod/util.go

@@ -285,6 +285,18 @@ func dropDisabledFields(
 		podSpec = &api.PodSpec{}
 	}
 
+	if !utilfeature.DefaultFeatureGate.Enabled(features.TokenRequestProjection) &&
+		!tokenRequestProjectionInUse(oldPodSpec) {
+		for i := range podSpec.Volumes {
+			if podSpec.Volumes[i].Projected != nil {
+				for j := range podSpec.Volumes[i].Projected.Sources {
+					podSpec.Volumes[i].Projected.Sources[j].ServiceAccountToken = nil
+				}
+			}
+
+		}
+	}
+
 	if !utilfeature.DefaultFeatureGate.Enabled(features.AppArmor) && !appArmorInUse(oldPodAnnotations) {
 		for k := range podAnnotations {
 			if strings.HasPrefix(k, apparmor.ContainerAnnotationKeyPrefix) {
@@ -480,6 +492,23 @@ func shareProcessNamespaceInUse(podSpec *api.PodSpec) bool {
 	return false
 }
 
+func tokenRequestProjectionInUse(podSpec *api.PodSpec) bool {
+	if podSpec == nil {
+		return false
+	}
+	for _, v := range podSpec.Volumes {
+		if v.Projected == nil {
+			continue
+		}
+		for _, s := range v.Projected.Sources {
+			if s.ServiceAccountToken != nil {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // podPriorityInUse returns true if the pod spec is non-nil and has Priority or PriorityClassName set.
 func podPriorityInUse(podSpec *api.PodSpec) bool {
 	if podSpec == nil {

pkg/api/pod/util_test.go

@@ -1177,7 +1177,124 @@ func TestDropReadinessGates(t *testing.T) {
 						}
 						// new pod should not have ReadinessGates
 						if !reflect.DeepEqual(newPod, podWithoutReadinessGates()) {
-							t.Errorf("new pod had ReadinessGates: %v", diff.ObjectReflectDiff(newPod, podWithoutReadinessGates()))
+							t.Errorf("new pod had ReadinessGates: %v",
+								diff.ObjectReflectDiff(newPod, podWithoutReadinessGates()))
+						}
+					default:
+						// new pod should not need to be changed
+						if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
+							t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod()))
+						}
+					}
+				})
+			}
+		}
+	}
+}
+
+func TestDropTokenRequestProjection(t *testing.T) {
+	podWithoutTRProjection := func() *api.Pod {
+		return &api.Pod{
+			Spec: api.PodSpec{
+				Volumes: []api.Volume{{
+					VolumeSource: api.VolumeSource{
+						Projected: &api.ProjectedVolumeSource{
+							Sources: []api.VolumeProjection{{
+								ServiceAccountToken: nil,
+							}},
+						}}},
+				},
+			},
+		}
+	}
+	podWithoutProjectedVolumeSource := func() *api.Pod {
+		return &api.Pod{
+			Spec: api.PodSpec{
+				Volumes: []api.Volume{
+					{VolumeSource: api.VolumeSource{
+						ConfigMap: &api.ConfigMapVolumeSource{},
+					}},
+				},
+			},
+		}
+	}
+	podWithTRProjection := func() *api.Pod {
+		return &api.Pod{
+			Spec: api.PodSpec{
+				Volumes: []api.Volume{{
+					VolumeSource: api.VolumeSource{
+						Projected: &api.ProjectedVolumeSource{
+							Sources: []api.VolumeProjection{{
+								ServiceAccountToken: &api.ServiceAccountTokenProjection{
+									Audience:          "api",
+									ExpirationSeconds: 3600,
+									Path:              "token",
+								}},
+							}},
+					},
+				},
+				},
+			}}
+	}
+	podInfo := []struct {
+		description     string
+		hasTRProjection bool
+		pod             func() *api.Pod
+	}{
+		{
+			description:     "has TokenRequestProjection",
+			hasTRProjection: true,
+			pod:             podWithTRProjection,
+		},
+		{
+			description:     "does not have TokenRequestProjection",
+			hasTRProjection: false,
+			pod:             podWithoutTRProjection,
+		},
+		{
+			description:     "does not have ProjectedVolumeSource",
+			hasTRProjection: false,
+			pod:             podWithoutProjectedVolumeSource,
+		},
+		{
+			description:     "is nil",
+			hasTRProjection: false,
+			pod:             func() *api.Pod { return nil },
+		},
+	}
+	for _, enabled := range []bool{true, false} {
+		for _, oldPodInfo := range podInfo {
+			for _, newPodInfo := range podInfo {
+				oldPodhasTRProjection, oldPod := oldPodInfo.hasTRProjection, oldPodInfo.pod()
+				newPodhasTRProjection, newPod := newPodInfo.hasTRProjection, newPodInfo.pod()
+				if newPod == nil {
+					continue
+				}
+				t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) {
+					defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TokenRequestProjection, enabled)()
+					var oldPodSpec *api.PodSpec
+					if oldPod != nil {
+						oldPodSpec = &oldPod.Spec
+					}
+					dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil)
+					// old pod should never be changed
+					if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) {
+						t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod()))
+					}
+					switch {
+					case enabled || oldPodhasTRProjection:
+						if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
+							t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod()))
+						}
+					case newPodhasTRProjection:
+						// new pod should be changed
+						if reflect.DeepEqual(newPod, newPodInfo.pod()) {
+							t.Errorf("%v", oldPod)
+							t.Errorf("%v", newPod)
+							t.Errorf("new pod was not changed")
+						}
+						if !reflect.DeepEqual(newPod, podWithoutTRProjection()) {
+							t.Errorf("new pod had Tokenrequestprojection: %v", diff.ObjectReflectDiff(newPod, podWithoutTRProjection()))
 						}
 					default:
 						// new pod should not need to be changed

pkg/apis/core/validation/validation.go

@@ -1039,9 +1039,6 @@ func validateProjectionSources(projection *core.ProjectedVolumeSource, projectio
 		}
 		if projPath := srcPath.Child("serviceAccountToken"); source.ServiceAccountToken != nil {
 			numSources++
-			if !utilfeature.DefaultFeatureGate.Enabled(features.TokenRequestProjection) {
-				allErrs = append(allErrs, field.Forbidden(projPath, "TokenRequestProjection feature is not enabled"))
-			}
 			if source.ServiceAccountToken.ExpirationSeconds < 10*60 {
 				allErrs = append(allErrs, field.Invalid(projPath.Child("expirationSeconds"), source.ServiceAccountToken.ExpirationSeconds, "may not specify a duration less than 10 minutes"))
 			}