Merge pull request #115480 from alexzielenski/kubectl/explain/openapiv3/alias-legacy

kubectl-explain: add --output plaintext-openapiv2 fallback

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

@@ -24,10 +24,10 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
-	"k8s.io/client-go/discovery"
+	openapiclient "k8s.io/client-go/openapi"
 	cmdutil "k8s.io/kubectl/pkg/cmd/util"
 	"k8s.io/kubectl/pkg/explain"
-	explainv2 "k8s.io/kubectl/pkg/explain/v2"
+	openapiv3explain "k8s.io/kubectl/pkg/explain/v2"
 	"k8s.io/kubectl/pkg/util/i18n"
 	"k8s.io/kubectl/pkg/util/openapi"
 	"k8s.io/kubectl/pkg/util/templates"
@@ -51,6 +51,9 @@ var (
 
 		# Get the documentation of a specific field of a resource
 		kubectl explain pods.spec.containers`))
+
+	plaintextTemplateName          = "plaintext"
+	plaintextOpenAPIV2TemplateName = "plaintext-openapiv2"
 )
 
 type ExplainOptions struct {
@@ -74,7 +77,7 @@ type ExplainOptions struct {
 	OutputFormat string
 
 	// Client capable of fetching openapi documents from the user's cluster
-	DiscoveryClient discovery.DiscoveryInterface
+	OpenAPIV3Client openapiclient.Client
 }
 
 func NewExplainOptions(parent string, streams genericclioptions.IOStreams) *ExplainOptions {
@@ -82,7 +85,7 @@ func NewExplainOptions(parent string, streams genericclioptions.IOStreams) *Expl
 		IOStreams:       streams,
 		CmdParent:       parent,
 		EnableOpenAPIV3: cmdutil.ExplainOpenapiV3.IsEnabled(),
-		OutputFormat:    "plaintext",
+		OutputFormat:    plaintextTemplateName,
 	}
 }
 
@@ -107,7 +110,7 @@ func NewCmdExplain(parent string, f cmdutil.Factory, streams genericclioptions.I
 
 	// Only enable --output as a valid flag if the feature is enabled
 	if o.EnableOpenAPIV3 {
-		cmd.Flags().StringVar(&o.OutputFormat, "output", o.OutputFormat, "Format in which to render the schema")
+		cmd.Flags().StringVar(&o.OutputFormat, "output", plaintextTemplateName, "Format in which to render the schema (plaintext, plaintext-openapiv2)")
 	}
 
 	return cmd
@@ -125,13 +128,12 @@ func (o *ExplainOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []
 		return err
 	}
 
-	// Only openapi v3 needs the discovery client.
+	// Only openapi v3 needs the openapiv3client
 	if o.EnableOpenAPIV3 {
-		discoveryClient, err := f.ToDiscoveryClient()
+		o.OpenAPIV3Client, err = f.OpenAPIV3Client()
 		if err != nil {
 			return err
 		}
-		o.DiscoveryClient = discoveryClient
 	}
 
 	o.args = args
@@ -151,13 +153,10 @@ func (o *ExplainOptions) Validate() error {
 
 // Run executes the appropriate steps to print a model's documentation
 func (o *ExplainOptions) Run() error {
-	recursive := o.Recursive
-	apiVersionString := o.APIVersion
-
 	var fullySpecifiedGVR schema.GroupVersionResource
 	var fieldsPath []string
 	var err error
-	if len(apiVersionString) == 0 {
+	if len(o.APIVersion) == 0 {
 		fullySpecifiedGVR, fieldsPath, err = explain.SplitAndParseResourceRequestWithMatchingPrefix(o.args[0], o.Mapper)
 		if err != nil {
 			return err
@@ -172,16 +171,47 @@ func (o *ExplainOptions) Run() error {
 		}
 	}
 
+	// Fallback to openapiv2 implementation using special template name
 	if o.EnableOpenAPIV3 {
-		return explainv2.PrintModelDescription(
-			fieldsPath,
-			o.Out,
-			o.DiscoveryClient.OpenAPIV3(),
-			fullySpecifiedGVR,
-			recursive,
-			o.OutputFormat,
-		)
+		switch o.OutputFormat {
+		case plaintextOpenAPIV2TemplateName:
+			return o.renderOpenAPIV2(fullySpecifiedGVR, fieldsPath)
+		case plaintextTemplateName:
+			// Check whether the server reponds to OpenAPIV3.
+			if _, err := o.OpenAPIV3Client.Paths(); err != nil {
+				// Use v2 renderer if server does not support v3
+				return o.renderOpenAPIV2(fullySpecifiedGVR, fieldsPath)
+			}
+
+			fallthrough
+		default:
+			if len(o.APIVersion) > 0 {
+				apiVersion, err := schema.ParseGroupVersion(o.APIVersion)
+				if err != nil {
+					return err
+				}
+				fullySpecifiedGVR.Group = apiVersion.Group
+				fullySpecifiedGVR.Version = apiVersion.Version
+			}
+
+			return openapiv3explain.PrintModelDescription(
+				fieldsPath,
+				o.Out,
+				o.OpenAPIV3Client,
+				fullySpecifiedGVR,
+				o.Recursive,
+				o.OutputFormat,
+			)
+		}
 	}
+	return o.renderOpenAPIV2(fullySpecifiedGVR, fieldsPath)
+}
+
+func (o *ExplainOptions) renderOpenAPIV2(
+	fullySpecifiedGVR schema.GroupVersionResource,
+	fieldsPath []string,
+) error {
+	var err error
 
 	gvk, _ := o.Mapper.KindFor(fullySpecifiedGVR)
 	if gvk.Empty() {
@@ -191,8 +221,8 @@ func (o *ExplainOptions) Run() error {
 		}
 	}
 
-	if len(apiVersionString) != 0 {
-		apiVersion, err := schema.ParseGroupVersion(apiVersionString)
+	if len(o.APIVersion) != 0 {
+		apiVersion, err := schema.ParseGroupVersion(o.APIVersion)
 		if err != nil {
 			return err
 		}
@@ -204,5 +234,5 @@ func (o *ExplainOptions) Run() error {
 		return fmt.Errorf("couldn't find resource for %q", gvk)
 	}
 
-	return explain.PrintModelDescription(fieldsPath, o.Out, schema, gvk, recursive)
+	return explain.PrintModelDescription(fieldsPath, o.Out, schema, gvk, o.Recursive)
 }

staging/src/k8s.io/kubectl/pkg/cmd/explain/explain_test.go

@@ -14,24 +14,31 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package explain
+package explain_test
 
 import (
+	"errors"
 	"path/filepath"
-	"strings"
+	"regexp"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/api/meta"
 	sptest "k8s.io/apimachinery/pkg/util/strategicpatch/testing"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"k8s.io/client-go/discovery"
+	openapiclient "k8s.io/client-go/openapi"
+	"k8s.io/client-go/rest"
+	clienttestutil "k8s.io/client-go/util/testing"
+	"k8s.io/kubectl/pkg/cmd/explain"
 	cmdtesting "k8s.io/kubectl/pkg/cmd/testing"
 	cmdutil "k8s.io/kubectl/pkg/cmd/util"
 	"k8s.io/kubectl/pkg/util/openapi"
 )
 
 var (
-	fakeSchema        = sptest.Fake{Path: filepath.Join("..", "..", "..", "testdata", "openapi", "swagger.json")}
+	testDataPath      = filepath.Join("..", "..", "..", "testdata")
+	fakeSchema        = sptest.Fake{Path: filepath.Join(testDataPath, "openapi", "swagger.json")}
 	FakeOpenAPISchema = testOpenAPISchema{
 		OpenAPISchemaFn: func() (openapi.Resources, error) {
 			s, err := fakeSchema.OpenAPISchema()
@@ -51,8 +58,8 @@ func TestExplainInvalidArgs(t *testing.T) {
 	tf := cmdtesting.NewTestFactory()
 	defer tf.Cleanup()
 
-	opts := NewExplainOptions("kubectl", genericclioptions.NewTestIOStreamsDiscard())
-	cmd := NewCmdExplain("kubectl", tf, genericclioptions.NewTestIOStreamsDiscard())
+	opts := explain.NewExplainOptions("kubectl", genericclioptions.NewTestIOStreamsDiscard())
+	cmd := explain.NewCmdExplain("kubectl", tf, genericclioptions.NewTestIOStreamsDiscard())
 	err := opts.Complete(tf, cmd, []string{})
 	if err != nil {
 		t.Fatalf("unexpected error %v", err)
@@ -78,8 +85,8 @@ func TestExplainNotExistResource(t *testing.T) {
 	tf := cmdtesting.NewTestFactory()
 	defer tf.Cleanup()
 
-	opts := NewExplainOptions("kubectl", genericclioptions.NewTestIOStreamsDiscard())
-	cmd := NewCmdExplain("kubectl", tf, genericclioptions.NewTestIOStreamsDiscard())
+	opts := explain.NewExplainOptions("kubectl", genericclioptions.NewTestIOStreamsDiscard())
+	cmd := explain.NewCmdExplain("kubectl", tf, genericclioptions.NewTestIOStreamsDiscard())
 	err := opts.Complete(tf, cmd, []string{"foo"})
 	if err != nil {
 		t.Fatalf("unexpected error %v", err)
@@ -96,30 +103,106 @@ func TestExplainNotExistResource(t *testing.T) {
 	}
 }
 
-func TestExplainNotExistVersion(t *testing.T) {
-	tf := cmdtesting.NewTestFactory()
-	defer tf.Cleanup()
+type explainTestCase struct {
+	Name               string
+	Args               []string
+	Flags              map[string]string
+	ExpectPattern      []string
+	ExpectErrorPattern string
 
-	opts := NewExplainOptions("kubectl", genericclioptions.NewTestIOStreamsDiscard())
-	cmd := NewCmdExplain("kubectl", tf, genericclioptions.NewTestIOStreamsDiscard())
-	err := opts.Complete(tf, cmd, []string{"pods"})
-	if err != nil {
-		t.Fatalf("unexpected error %v", err)
-	}
-	opts.APIVersion = "v99"
-
-	err = opts.Validate()
-	if err != nil {
-		t.Fatalf("unexpected error %v", err)
-	}
-
-	err = opts.Run()
-	if err.Error() != "couldn't find resource for \"/v99, Kind=Pod\"" {
-		t.Errorf("unexpected non-error")
-	}
+	// Custom OpenAPI V3 client to use for the test. If nil, a default one will
+	// be provided
+	OpenAPIV3SchemaFn func() (openapiclient.Client, error)
 }
 
-func TestExplain(t *testing.T) {
+var explainV2Cases = []explainTestCase{
+	{
+		Name:          "Basic",
+		Args:          []string{"pods"},
+		ExpectPattern: []string{`\s*KIND:[\t ]*Pod\s*`},
+	},
+	{
+		Name:          "Recursive",
+		Args:          []string{"pods"},
+		Flags:         map[string]string{"recursive": "true"},
+		ExpectPattern: []string{`\s*KIND:[\t ]*Pod\s*`},
+	},
+	{
+		Name:          "DefaultAPIVersion",
+		Args:          []string{"horizontalpodautoscalers"},
+		Flags:         map[string]string{"api-version": "autoscaling/v1"},
+		ExpectPattern: []string{`\s*VERSION:[\t ]*(v1|autoscaling/v1)\s*`},
+	},
+	{
+		Name:               "NonExistingAPIVersion",
+		Args:               []string{"pods"},
+		Flags:              map[string]string{"api-version": "v99"},
+		ExpectErrorPattern: `couldn't find resource for \"/v99, (Kind=Pod|Resource=pods)\"`,
+	},
+	{
+		Name:               "NonExistingResource",
+		Args:               []string{"foo"},
+		ExpectErrorPattern: `the server doesn't have a resource type "foo"`,
+	},
+}
+
+func TestExplainOpenAPIV2(t *testing.T) {
+	runExplainTestCases(t, explainV2Cases)
+}
+
+func TestExplainOpenAPIV3(t *testing.T) {
+
+	fallbackV3SchemaFn := func() (openapiclient.Client, error) {
+		fakeDiscoveryClient := discovery.NewDiscoveryClientForConfigOrDie(&rest.Config{Host: "https://not.a.real.site:65543/"})
+		return fakeDiscoveryClient.OpenAPIV3(), nil
+	}
+	// Returns a client that causes fallback to v2 implementation
+	cases := []explainTestCase{
+		{
+			// No --output, but OpenAPIV3 enabled should fall back to v2 if
+			// v2 is not available. Shows this by making openapiv3 client
+			// point to a bad URL. So the fact the proper data renders is
+			// indication v2 was used instead.
+			Name:              "Fallback",
+			Args:              []string{"pods"},
+			ExpectPattern:     []string{`\s*KIND:[\t ]*Pod\s*`},
+			OpenAPIV3SchemaFn: fallbackV3SchemaFn,
+		},
+		{
+			Name:          "NonDefaultAPIVersion",
+			Args:          []string{"horizontalpodautoscalers"},
+			Flags:         map[string]string{"api-version": "autoscaling/v2"},
+			ExpectPattern: []string{`\s*VERSION:[\t ]*(v2|autoscaling/v2)\s*`},
+		},
+		{
+			// Show that explicitly specifying --output plaintext-openapiv2 causes
+			// old implementation to be used even though OpenAPIV3 is enabled
+			Name:              "OutputPlaintextV2",
+			Args:              []string{"pods"},
+			Flags:             map[string]string{"output": "plaintext-openapiv2"},
+			ExpectPattern:     []string{`\s*KIND:[\t ]*Pod\s*`},
+			OpenAPIV3SchemaFn: fallbackV3SchemaFn,
+		},
+	}
+	cases = append(cases, explainV2Cases...)
+
+	cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{cmdutil.ExplainOpenapiV3}, t, func(t *testing.T) {
+		runExplainTestCases(t, cases)
+	})
+}
+
+func runExplainTestCases(t *testing.T, cases []explainTestCase) {
+	fakeServer, err := clienttestutil.NewFakeOpenAPIV3Server(filepath.Join(testDataPath, "openapi", "v3"))
+	if err != nil {
+		t.Fatalf("error starting fake openapi server: %v", err.Error())
+	}
+	defer fakeServer.HttpServer.Close()
+
+	openapiV3SchemaFn := func() (openapiclient.Client, error) {
+		fakeDiscoveryClient := discovery.NewDiscoveryClientForConfigOrDie(&rest.Config{Host: fakeServer.HttpServer.URL})
+		return fakeDiscoveryClient.OpenAPIV3(), nil
+	}
+
 	tf := cmdtesting.NewTestFactory()
 	defer tf.Cleanup()
 
@@ -127,29 +210,72 @@ func TestExplain(t *testing.T) {
 	tf.ClientConfigVal = cmdtesting.DefaultClientConfig()
 
 	ioStreams, _, buf, _ := genericclioptions.NewTestIOStreams()
-	cmd := NewCmdExplain("kubectl", tf, ioStreams)
-	cmd.Run(cmd, []string{"pods"})
-	if !strings.Contains(buf.String(), "KIND:     Pod") {
-		t.Fatalf("expected output should include pod kind")
-	}
 
-	cmd.Flags().Set("recursive", "true")
-	cmd.Run(cmd, []string{"pods"})
-	if !strings.Contains(buf.String(), "KIND:     Pod") ||
-		!strings.Contains(buf.String(), "annotations\t<map[string]string>") {
-		t.Fatalf("expected output should include pod kind")
-	}
+	type catchFatal error
 
-	cmd.Flags().Set("api-version", "batch/v1")
-	cmd.Run(cmd, []string{"cronjobs"})
-	if !strings.Contains(buf.String(), "VERSION:  batch/v1") {
-		t.Fatalf("expected output should include pod batch/v1")
-	}
+	for _, tcase := range cases {
 
-	cmd.Flags().Set("api-version", "batch/v1beta1")
-	cmd.Run(cmd, []string{"cronjobs"})
-	if !strings.Contains(buf.String(), "VERSION:  batch/v1beta1") {
-		t.Fatalf("expected output should include pod batch/v1beta1")
+		t.Run(tcase.Name, func(t *testing.T) {
+
+			// Catch os.Exit calls for tests which expect them
+			// and replace them with panics that we catch in each test
+			// to check if it is expected.
+			cmdutil.BehaviorOnFatal(func(str string, code int) {
+				panic(catchFatal(errors.New(str)))
+			})
+			defer cmdutil.DefaultBehaviorOnFatal()
+
+			var err error
+
+			func() {
+				defer func() {
+					// Catch panic and check at end of test if it is
+					// expected.
+					if panicErr := recover(); panicErr != nil {
+						if e := panicErr.(catchFatal); e != nil {
+							err = e
+						} else {
+							panic(panicErr)
+						}
+					}
+				}()
+
+				if tcase.OpenAPIV3SchemaFn != nil {
+					tf.OpenAPIV3ClientFunc = tcase.OpenAPIV3SchemaFn
+				} else {
+					tf.OpenAPIV3ClientFunc = openapiV3SchemaFn
+				}
+
+				cmd := explain.NewCmdExplain("kubectl", tf, ioStreams)
+				for k, v := range tcase.Flags {
+					if err := cmd.Flags().Set(k, v); err != nil {
+						t.Fatal(err)
+					}
+				}
+				cmd.Run(cmd, tcase.Args)
+			}()
+
+			for _, rexp := range tcase.ExpectPattern {
+				if matched, err := regexp.MatchString(rexp, buf.String()); err != nil || !matched {
+					if err != nil {
+						t.Error(err)
+					} else {
+						t.Errorf("expected output to match regex:\n\t%s\ninstead got:\n\t%s", rexp, buf.String())
+					}
+				}
+			}
+
+			if err != nil {
+				if matched, regexErr := regexp.MatchString(tcase.ExpectErrorPattern, err.Error()); len(tcase.ExpectErrorPattern) == 0 || regexErr != nil || !matched {
+					t.Fatalf("unexpected error: %s did not match regex %s (%v)", err.Error(),
+						tcase.ExpectErrorPattern, regexErr)
+				}
+			} else if len(tcase.ExpectErrorPattern) > 0 {
+				t.Fatalf("did not trigger expected error: %s in output:\n%s", tcase.ExpectErrorPattern, buf.String())
+			}
+		})
+
+		buf.Reset()
 	}
 }
 
@@ -161,11 +287,11 @@ func TestAlphaEnablement(t *testing.T) {
 		f := cmdtesting.NewTestFactory()
 		defer f.Cleanup()
 
-		cmd := NewCmdExplain("kubectl", f, genericclioptions.NewTestIOStreamsDiscard())
+		cmd := explain.NewCmdExplain("kubectl", f, genericclioptions.NewTestIOStreamsDiscard())
 		require.Nil(t, cmd.Flags().Lookup(flag), "flag %q should not be registered without the %q feature enabled", flag, feature)
 
 		cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{feature}, t, func(t *testing.T) {
-			cmd := NewCmdExplain("kubectl", f, genericclioptions.NewTestIOStreamsDiscard())
+			cmd := explain.NewCmdExplain("kubectl", f, genericclioptions.NewTestIOStreamsDiscard())
 			require.NotNil(t, cmd.Flags().Lookup(flag), "flag %q should be registered with the %q feature enabled", flag, feature)
 		})
 	}

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

@@ -38,6 +38,8 @@ import (
 	"k8s.io/client-go/dynamic"
 	fakedynamic "k8s.io/client-go/dynamic/fake"
 	"k8s.io/client-go/kubernetes"
+	openapiclient "k8s.io/client-go/openapi"
+	"k8s.io/client-go/openapi/openapitest"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/rest/fake"
 	"k8s.io/client-go/restmapper"
@@ -418,6 +420,7 @@ type TestFactory struct {
 
 	UnstructuredClientForMappingFunc resource.FakeClientFunc
 	OpenAPISchemaFunc                func() (openapi.Resources, error)
+	OpenAPIV3ClientFunc              func() (openapiclient.Client, error)
 }
 
 // NewTestFactory returns an initialized TestFactory instance
@@ -533,6 +536,13 @@ func (f *TestFactory) OpenAPISchema() (openapi.Resources, error) {
 	return openapitesting.EmptyResources{}, nil
 }
 
+func (f *TestFactory) OpenAPIV3Client() (openapiclient.Client, error) {
+	if f.OpenAPIV3ClientFunc != nil {
+		return f.OpenAPIV3ClientFunc()
+	}
+	return openapitest.NewFakeClient(), nil
+}
+
 // NewBuilder returns an initialized resource.Builder instance
 func (f *TestFactory) NewBuilder() *resource.Builder {
 	return resource.NewFakeBuilder(

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

@@ -22,6 +22,7 @@ import (
 	"k8s.io/cli-runtime/pkg/resource"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes"
+	openapiclient "k8s.io/client-go/openapi"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/kubectl/pkg/util/openapi"
 	"k8s.io/kubectl/pkg/validation"
@@ -63,4 +64,7 @@ type Factory interface {
 	Validator(validationDirective string) (validation.Schema, error)
 	// OpenAPISchema returns the parsed openapi schema definition
 	OpenAPISchema() (openapi.Resources, error)
+	// OpenAPIV3Schema returns a client for fetching parsed schemas for
+	// any group version
+	OpenAPIV3Client() (openapiclient.Client, error)
 }

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

@@ -30,6 +30,7 @@ import (
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes"
+	openapiclient "k8s.io/client-go/openapi"
 	"k8s.io/client-go/openapi/cached"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
@@ -211,3 +212,12 @@ func (f *factoryImpl) openAPIGetter() discovery.OpenAPISchemaInterface {
 
 	return f.oapi
 }
+
+func (f *factoryImpl) OpenAPIV3Client() (openapiclient.Client, error) {
+	discovery, err := f.clientGetter.ToDiscoveryClient()
+	if err != nil {
+		return nil, err
+	}
+
+	return discovery.OpenAPIV3(), nil
+}

staging/src/k8s.io/kubectl/pkg/explain/v2/explain.go

@@ -72,7 +72,7 @@ func printModelDescriptionWithGenerator(
 	gv, exists := paths[resourcePath]
 
 	if !exists {
-		return fmt.Errorf("could not locate schema for %s", resourcePath)
+		return fmt.Errorf("couldn't find resource for \"%v\"", gvr)
 	}
 
 	openAPISchemaBytes, err := gv.Schema(runtime.ContentTypeJSON)

staging/src/k8s.io/kubectl/pkg/explain/v2/explain_test.go

@@ -45,7 +45,7 @@ func TestExplainErrors(t *testing.T) {
 		Version:  "v1",
 		Resource: "doesntmatter",
 	}, false, "unknown-format")
-	require.ErrorContains(t, err, "could not locate schema")
+	require.ErrorContains(t, err, "couldn't find resource for \"test0.example.com/v1, Resource=doesntmatter\"")
 
 	// Validate error when openapi client returns error.
 	fakeClient.ForcedErr = fmt.Errorf("Always fails")

vendor/modules.txt

@@ -1836,6 +1836,7 @@ k8s.io/client-go/metadata/metadatainformer
 k8s.io/client-go/metadata/metadatalister
 k8s.io/client-go/openapi
 k8s.io/client-go/openapi/cached
+k8s.io/client-go/openapi/openapitest
 k8s.io/client-go/openapi3
 k8s.io/client-go/pkg/apis/clientauthentication
 k8s.io/client-go/pkg/apis/clientauthentication/install