Add feature toggle for OpenAPI V3 apply in kubectl

staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go

@@ -288,7 +288,7 @@ func (flags *ApplyFlags) ToOptions(f cmdutil.Factory, cmd *cobra.Command, baseNa
 	openAPISchema, _ := f.OpenAPISchema()
 	var openAPIV3Root openapi3.Root
 	openAPIV3Client, err := f.OpenAPIV3Client()
-	if err == nil {
+	if err == nil && !cmdutil.OpenAPIV3Apply.IsDisabled() {
 		cachedOpenAPIV3Client := cachedopenapi.NewClient(openAPIV3Client)
 		openAPIV3Root = openapi3.NewRoot(cachedOpenAPIV3Client)
 	}

staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_test.go

@@ -68,7 +68,9 @@ var (
 	fakeSchema          = sptest.Fake{Path: filepath.Join("..", "..", "..", "testdata", "openapi", "swagger.json")}
 	fakeOpenAPIV3Legacy = sptest.OpenAPIV3Getter{Path: filepath.Join("..", "..", "..", "testdata", "openapi", "v3", "api", "v1.json")}
 	fakeOpenAPIV3AppsV1 = sptest.OpenAPIV3Getter{Path: filepath.Join("..", "..", "..", "testdata", "openapi", "v3", "apis", "apps", "v1.json")}
+	// testingOpenAPISchemas = []testOpenAPISchema{FakeOpenAPISchema}
 	testingOpenAPISchemas = []testOpenAPISchema{AlwaysErrorsOpenAPISchema, FakeOpenAPISchema}
+
 	AlwaysErrorsOpenAPISchema = testOpenAPISchema{
 		OpenAPISchemaFn: func() (openapi.Resources, error) {
 			return nil, errors.New("cannot get openapi spec")
@@ -92,9 +94,37 @@ var (
 			return c, nil
 		},
 	}
+	OpenAPIV3PanicSchema = testOpenAPISchema{
+		OpenAPISchemaFn: func() (openapi.Resources, error) {
+			s, err := fakeSchema.OpenAPISchema()
+			if err != nil {
+				return nil, err
+			}
+			return openapi.NewOpenAPIData(s)
+		},
+		OpenAPIV3ClientFunc: func() (openapiclient.Client, error) {
+			return &OpenAPIV3ClientAlwaysPanic{}, nil
+		},
+	}
+
 	codec = scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...)
 )
 
+type OpenAPIV3ClientAlwaysPanic struct{}
+
+func (o *OpenAPIV3ClientAlwaysPanic) Paths() (map[string]openapiclient.GroupVersion, error) {
+	panic("Cannot get paths")
+}
+
+func noopOpenAPIV3Apply(t *testing.T, f func(t *testing.T)) {
+	f(t)
+}
+func disableOpenAPIV3Apply(t *testing.T, f func(t *testing.T)) {
+	cmdtesting.WithAlphaEnvsDisabled([]cmdutil.FeatureGate{cmdutil.OpenAPIV3Apply}, t, f)
+}
+
+var applyFeatureToggles = []func(*testing.T, func(t *testing.T)){noopOpenAPIV3Apply, disableOpenAPIV3Apply}
+
 type testOpenAPISchema struct {
 	OpenAPISchemaFn     func() (openapi.Resources, error)
 	OpenAPIV3ClientFunc func() (openapiclient.Client, error)
@@ -717,14 +747,71 @@ func TestApplyObjectWithoutAnnotation(t *testing.T) {
 	}
 }
 
+func TestOpenAPIV3ApplyFeatureFlag(t *testing.T) {
+	// OpenAPIV3 smp apply is on by default.
+	// Test that users can disable it to use OpenAPI V2 smp
+	// An OpenAPI V3 root that always panics is used to ensure
+	// the v3 code path is never exercised when the feature is disabled
+	cmdtesting.InitTestErrorHandler(t)
+	nameRC, currentRC := readAndAnnotateReplicationController(t, filenameRC)
+	pathRC := "/namespaces/test/replicationcontrollers/" + nameRC
+
+	t.Run("test apply when a local object is specified - openapi v2 smp", func(t *testing.T) {
+		disableOpenAPIV3Apply(t, func(t *testing.T) {
+			tf := cmdtesting.NewTestFactory().WithNamespace("test")
+			defer tf.Cleanup()
+
+			tf.UnstructuredClient = &fake.RESTClient{
+				NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer,
+				Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+					switch p, m := req.URL.Path, req.Method; {
+					case p == pathRC && m == "GET":
+						bodyRC := io.NopCloser(bytes.NewReader(currentRC))
+						return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: bodyRC}, nil
+					case p == pathRC && m == "PATCH":
+						validatePatchApplication(t, req, types.StrategicMergePatchType)
+						bodyRC := io.NopCloser(bytes.NewReader(currentRC))
+						return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: bodyRC}, nil
+					default:
+						t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
+						return nil, nil
+					}
+				}),
+			}
+			tf.OpenAPISchemaFunc = OpenAPIV3PanicSchema.OpenAPISchemaFn
+			tf.OpenAPIV3ClientFunc = OpenAPIV3PanicSchema.OpenAPIV3ClientFunc
+			tf.ClientConfigVal = cmdtesting.DefaultClientConfig()
+
+			ioStreams, _, buf, errBuf := genericiooptions.NewTestIOStreams()
+			cmd := NewCmdApply("kubectl", tf, ioStreams)
+			cmd.Flags().Set("filename", filenameRC)
+			cmd.Flags().Set("output", "name")
+			cmd.Run(cmd, []string{})
+
+			// uses the name from the file, not the response
+			expectRC := "replicationcontroller/" + nameRC + "\n"
+			if buf.String() != expectRC {
+				t.Fatalf("unexpected output: %s\nexpected: %s", buf.String(), expectRC)
+			}
+			if errBuf.String() != "" {
+				t.Fatalf("unexpected error output: %s", errBuf.String())
+			}
+		})
+	})
+
+}
+
 func TestApplyObject(t *testing.T) {
 	cmdtesting.InitTestErrorHandler(t)
 	nameRC, currentRC := readAndAnnotateReplicationController(t, filenameRC)
 	pathRC := "/namespaces/test/replicationcontrollers/" + nameRC
 
 	for _, testingOpenAPISchema := range testingOpenAPISchemas {
-		t.Run("test apply when a local object is specified", func(t *testing.T) {
+		for _, openAPIFeatureToggle := range applyFeatureToggles {
+			t.Run("test apply when a local object is specified - openapi v3 smp", func(t *testing.T) {
+				openAPIFeatureToggle(t, func(t *testing.T) {
 					tf := cmdtesting.NewTestFactory().WithNamespace("test")
+
 					defer tf.Cleanup()
 
 					tf.UnstructuredClient = &fake.RESTClient{
@@ -763,6 +850,8 @@ func TestApplyObject(t *testing.T) {
 						t.Fatalf("unexpected error output: %s", errBuf.String())
 					}
 				})
+			})
+		}
 	}
 }
 
@@ -772,7 +861,10 @@ func TestApplyPruneObjects(t *testing.T) {
 	pathRC := "/namespaces/test/replicationcontrollers/" + nameRC
 
 	for _, testingOpenAPISchema := range testingOpenAPISchemas {
+		for _, openAPIFeatureToggle := range applyFeatureToggles {
+
 			t.Run("test apply returns correct output", func(t *testing.T) {
+				openAPIFeatureToggle(t, func(t *testing.T) {
 					tf := cmdtesting.NewTestFactory().WithNamespace("test")
 					defer tf.Cleanup()
 
@@ -813,6 +905,8 @@ func TestApplyPruneObjects(t *testing.T) {
 						t.Fatalf("unexpected error output: %s", errBuf.String())
 					}
 				})
+			})
+		}
 	}
 }
 
@@ -1111,6 +1205,8 @@ func TestApplyCSAMigration(t *testing.T) {
 	nameRC, rcWithManagedFields := readAndAnnotateReplicationController(t, filenameRCManagedFieldsLA)
 	pathRC := "/namespaces/test/replicationcontrollers/" + nameRC
 
+	for _, openAPIFeatureToggle := range applyFeatureToggles {
+		openAPIFeatureToggle(t, func(t *testing.T) {
 			tf := cmdtesting.NewTestFactory().WithNamespace("test")
 			defer tf.Cleanup()
 
@@ -1253,6 +1349,8 @@ func TestApplyCSAMigration(t *testing.T) {
 			require.Empty(t, errBuf)
 			require.Equal(t, 4, applies, "only a single call to server-side apply should have been performed")
 			require.Equal(t, targetPatches, patches, "no more json patches should have been needed")
+		})
+	}
 }
 
 func TestApplyObjectOutput(t *testing.T) {
@@ -1277,7 +1375,9 @@ func TestApplyObjectOutput(t *testing.T) {
 	}
 
 	for _, testingOpenAPISchema := range testingOpenAPISchemas {
+		for _, openAPIFeatureToggle := range applyFeatureToggles {
 			t.Run("test apply returns correct output", func(t *testing.T) {
+				openAPIFeatureToggle(t, func(t *testing.T) {
 					tf := cmdtesting.NewTestFactory().WithNamespace("test")
 					defer tf.Cleanup()
 
@@ -1318,6 +1418,8 @@ func TestApplyObjectOutput(t *testing.T) {
 						t.Fatalf("unexpected error output: %s", errBuf.String())
 					}
 				})
+			})
+		}
 	}
 }
 
@@ -1327,7 +1429,10 @@ func TestApplyRetry(t *testing.T) {
 	pathRC := "/namespaces/test/replicationcontrollers/" + nameRC
 
 	for _, testingOpenAPISchema := range testingOpenAPISchemas {
+		for _, openAPIFeatureToggle := range applyFeatureToggles {
+
 			t.Run("test apply retries on conflict error", func(t *testing.T) {
+				openAPIFeatureToggle(t, func(t *testing.T) {
 					firstPatch := true
 					retry := false
 					getCount := 0
@@ -1383,6 +1488,8 @@ func TestApplyRetry(t *testing.T) {
 						t.Fatalf("unexpected error output: %s", errBuf.String())
 					}
 				})
+			})
+		}
 	}
 }
 
@@ -1590,7 +1697,10 @@ func TestApplyNULLPreservation(t *testing.T) {
 	deploymentBytes := readDeploymentFromFile(t, filenameDeployObjServerside)
 
 	for _, testingOpenAPISchema := range testingOpenAPISchemas {
+		for _, openAPIFeatureToggle := range applyFeatureToggles {
+
 			t.Run("test apply preserves NULL fields", func(t *testing.T) {
+				openAPIFeatureToggle(t, func(t *testing.T) {
 					tf := cmdtesting.NewTestFactory().WithNamespace("test")
 					defer tf.Cleanup()
 
@@ -1654,6 +1764,8 @@ func TestApplyNULLPreservation(t *testing.T) {
 						t.Fatal("No server-side patch call detected")
 					}
 				})
+			})
+		}
 	}
 }
 
@@ -1666,7 +1778,10 @@ func TestUnstructuredApply(t *testing.T) {
 	verifiedPatch := false
 
 	for _, testingOpenAPISchema := range testingOpenAPISchemas {
+		for _, openAPIFeatureToggle := range applyFeatureToggles {
+
 			t.Run("test apply works correctly with unstructured objects", func(t *testing.T) {
+				openAPIFeatureToggle(t, func(t *testing.T) {
 					tf := cmdtesting.NewTestFactory().WithNamespace("test")
 					defer tf.Cleanup()
 
@@ -1716,6 +1831,8 @@ func TestUnstructuredApply(t *testing.T) {
 						t.Fatal("No server-side patch call detected")
 					}
 				})
+			})
+		}
 	}
 }
 
@@ -1731,7 +1848,11 @@ func TestUnstructuredIdempotentApply(t *testing.T) {
 	path := "/namespaces/test/widgets/widget"
 
 	for _, testingOpenAPISchema := range testingOpenAPISchemas {
+		for _, openAPIFeatureToggle := range applyFeatureToggles {
+
 			t.Run("test repeated apply operations on an unstructured object", func(t *testing.T) {
+				openAPIFeatureToggle(t, func(t *testing.T) {
+
 					tf := cmdtesting.NewTestFactory().WithNamespace("test")
 					defer tf.Cleanup()
 
@@ -1778,6 +1899,8 @@ func TestUnstructuredIdempotentApply(t *testing.T) {
 						t.Fatalf("unexpected error output: %s", errBuf.String())
 					}
 				})
+			})
+		}
 	}
 }
 
@@ -1922,7 +2045,10 @@ func TestForceApply(t *testing.T) {
 	}
 
 	for _, testingOpenAPISchema := range testingOpenAPISchemas {
+		for _, openAPIFeatureToggle := range applyFeatureToggles {
+
 			t.Run("test apply with --force", func(t *testing.T) {
+				openAPIFeatureToggle(t, func(t *testing.T) {
 					deleted := false
 					isScaledDownToZero := false
 					counts := map[string]int{}
@@ -2028,6 +2154,8 @@ func TestForceApply(t *testing.T) {
 						t.Fatalf("unexpected error output: %s", errBuf.String())
 					}
 				})
+			})
+		}
 	}
 }
 

staging/src/k8s.io/kubectl/pkg/cmd/testing/util.go

@@ -195,3 +195,18 @@ func WithAlphaEnvs(features []cmdutil.FeatureGate, t *testing.T, f func(*testing
 	}
 	f(t)
 }
+
+// WithAlphaEnvs calls func f with the given env-var-based feature gates disabled,
+// and then restores the original values of those variables.
+func WithAlphaEnvsDisabled(features []cmdutil.FeatureGate, t *testing.T, f func(*testing.T)) {
+	for _, feature := range features {
+		key := string(feature)
+		if key != "" {
+			oldValue := os.Getenv(key)
+			err := os.Setenv(key, "false")
+			require.NoError(t, err, "unexpected error setting alpha env")
+			defer os.Setenv(key, oldValue)
+		}
+	}
+	f(t)
+}

staging/src/k8s.io/kubectl/pkg/cmd/util/helpers.go

@@ -428,6 +428,7 @@ const (
 	ApplySet                FeatureGate = "KUBECTL_APPLYSET"
 	CmdPluginAsSubcommand   FeatureGate = "KUBECTL_ENABLE_CMD_SHADOW"
 	InteractiveDelete       FeatureGate = "KUBECTL_INTERACTIVE_DELETE"
+	OpenAPIV3Apply          FeatureGate = "KUBECTL_OPENAPIV3_APPLY"
 	RemoteCommandWebsockets FeatureGate = "KUBECTL_REMOTE_COMMAND_WEBSOCKETS"
 )