Properly handle different versioned resources in gwctl (eg. v1beta1 v…

…/s v1 APIs) and related code cleanups (#3001)

* Propagate label selection filter when describing backends

* Allow fetching individual resources through get command

* fix: Display error when failing to construct resourceModel in tests

* Adjust tests to work with NewSimpleDynamicClientWithCustomListKinds().

The dynamic client returned for unit tests may not be smart enough to
automatically guess the APIVersion and Kind (which the real kube-apiserver can
handle automatically). To get over this, we can simply simply include these when
creating the objects.

* Use API discovery to determine the preferred API versions for Gateway API types.

Clusters may not always have the v1 CRDs installed. This means gwctl will have
to use whatever version is available in the cluster. Hence, we will use API
Discovery to determine the "preferred" API version as per the kube-apiserver and
then use a dynamic client to fetch that specific versioned resource.

Once that specific versioned resource is fetched (e.g. v1beta1.Gateway), it will
be converted to a concrete type used within gwctl (eg. v1.Gateway), which will
usually be a more newer/stable version. For most cases, this conversion should
not result in any losses. If in the future, a need arises for such special
cases, we should be able to extend the approach by configuring some conversion
functions (eg. conversion function for v1beta1.Gateway -> v1.Gateway).

* fixup! Use API discovery to determine the preferred API versions for Gateway API types.

gwctl/cmd/describe.go

@@ -78,10 +78,7 @@ func runDescribe(cmd *cobra.Command, args []string, params *utils.CmdParams) {
 		ns = metav1.NamespaceAll
 	}
 
-	discoverer := resourcediscovery.Discoverer{
-		K8sClients:    params.K8sClients,
-		PolicyManager: params.PolicyManager,
-	}
+	discoverer := resourcediscovery.NewDiscoverer(params.K8sClients, params.PolicyManager)
 
 	policiesPrinter := &printer.PoliciesPrinter{Writer: params.Out, Clock: clock.RealClock{}}
 	httpRoutesPrinter := &printer.HTTPRoutesPrinter{Writer: params.Out, Clock: clock.RealClock{}}
@@ -182,8 +179,14 @@ func runDescribe(cmd *cobra.Command, args []string, params *utils.CmdParams) {
 		gwcPrinter.PrintDescribeView(resourceModel)
 
 	case "backend", "backends":
+		selector, err := labels.Parse(labelSelector)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "Unable to find resources that match the label selector \"%s\": %v\n", labelSelector, err)
+			os.Exit(1)
+		}
 		filter := resourcediscovery.Filter{
 			Namespace: ns,
+			Labels:    selector,
 		}
 		if len(args) > 1 {
 			filter.Name = args[1]

gwctl/cmd/get.go

@@ -37,9 +37,9 @@ func NewGetCommand() *cobra.Command {
 	var outputFormat string
 
 	cmd := &cobra.Command{
-		Use:   "get {namespaces|gateways|gatewayclasses|policies|policycrds|httproutes}",
+		Use:   "get {namespaces|gateways|gatewayclasses|policies|policycrds|httproutes} RESOURCE_NAME",
 		Short: "Display one or many resources",
-		Args:  cobra.ExactArgs(1),
+		Args:  cobra.RangeArgs(1, 2),
 		Run: func(cmd *cobra.Command, args []string) {
 			params := getParams(kubeConfigPath)
 			runGet(cmd, args, params)
@@ -87,10 +87,7 @@ func runGet(cmd *cobra.Command, args []string, params *utils.CmdParams) {
 		ns = ""
 	}
 
-	discoverer := resourcediscovery.Discoverer{
-		K8sClients:    params.K8sClients,
-		PolicyManager: params.PolicyManager,
-	}
+	discoverer := resourcediscovery.NewDiscoverer(params.K8sClients, params.PolicyManager)
 	realClock := clock.RealClock{}
 
 	nsPrinter := &printer.NamespacesPrinter{Writer: params.Out, Clock: realClock}

gwctl/pkg/common/clients.go

@@ -23,6 +23,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/dynamic"
 	fakedynamicclient "k8s.io/client-go/dynamic/fake"
@@ -118,9 +119,56 @@ func MustClientsForTest(t *testing.T, initRuntimeObjects ...runtime.Object) *K8s
 		WithIndex(&corev1.Event{}, "involvedObject.namespace", eventNamespaceExtractorFunc).
 		WithIndex(&corev1.Event{}, "involvedObject.uid", eventUIDExtractorFunc).
 		Build()
-	fakeDC := fakedynamicclient.NewSimpleDynamicClient(scheme, initRuntimeObjects...)
 	fakeDiscoveryClient := fakeclientset.NewSimpleClientset().Discovery()
 
+	// Setup a fake DynamicClient, which requires some special handling.
+	//
+	// When objects are injected using `NewSimpleDynamicClient` or
+	// `NewSimpleDynamicClientWithCustomListKinds` to the fake resource tracker,
+	// internally it will use the `meta.UnsafeGuessKindToResource()` function.
+	// This incorrectly guesses the GVR of Gateway with Resource being guessed as
+	// "gatewaies" (instead of "gateways"). As a workaround, we will have to
+	// create Gateway objects separately.
+	//
+	// Also, because of this incorrect guessing, we need to register the
+	// GatewayList type separately.
+	gatewayv1GVR := schema.GroupVersionResource{
+		Group:    gatewayv1.GroupVersion.Group,
+		Version:  gatewayv1.GroupVersion.Version,
+		Resource: "gateways",
+	}
+	gvrToListKind := map[schema.GroupVersionResource]string{
+		gatewayv1GVR: "GatewayList",
+	}
+	for _, obj := range initRuntimeObjects {
+		if crd, ok := obj.(*apiextensionsv1.CustomResourceDefinition); ok {
+			gvr := schema.GroupVersionResource{
+				Group:    crd.Spec.Group,
+				Version:  crd.Spec.Versions[0].Name,
+				Resource: crd.Spec.Names.Plural, // CRD Kinds directly map to the Resource.
+			}
+			gvrToListKind[gvr] = crd.Spec.Names.Kind + "List"
+		}
+	}
+	fakeDC := fakedynamicclient.NewSimpleDynamicClientWithCustomListKinds(scheme, gvrToListKind)
+	for _, obj := range initRuntimeObjects {
+		var err error
+		if gateway, ok := obj.(*gatewayv1.Gateway); ok {
+			// Register Gateway with correct GVR. This needs to be done explicitly for
+			// Gateway since the automatically guess GVR is incorrect.
+			//
+			// Automatic guessing of GVR uses `meta.UnsafeGuessKindToResource()` which
+			// pluralizes "gateway" to "gatewaies" (since the singular ends in a 'y')
+			err = fakeDC.Tracker().Create(gatewayv1GVR, gateway, gateway.Namespace)
+		} else {
+			// Register non-Gateway resources automatically without GVR.
+			err = fakeDC.Tracker().Add(obj)
+		}
+		if err != nil {
+			t.Fatalf("Failed to add object to fake DynamicClient: %v", err)
+		}
+	}
+
 	return &K8sClients{
 		Client:          fakeClient,
 		DC:              fakeDC,

gwctl/pkg/printer/backends_test.go

@@ -208,6 +208,10 @@ func TestBackendsPrinter_Print(t *testing.T) {
 			},
 		},
 		&corev1.Service{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Service",
+				APIVersion: "v1",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "foo-svc-0",
 				Namespace: "default",
@@ -217,6 +221,10 @@ func TestBackendsPrinter_Print(t *testing.T) {
 			},
 		},
 		&corev1.Service{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Service",
+				APIVersion: "v1",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "foo-svc-1",
 				Namespace: "ns1",
@@ -226,6 +234,10 @@ func TestBackendsPrinter_Print(t *testing.T) {
 			},
 		},
 		&corev1.Service{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Service",
+				APIVersion: "v1",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "foo-svc-2",
 				Namespace: "ns2",
@@ -235,6 +247,10 @@ func TestBackendsPrinter_Print(t *testing.T) {
 			},
 		},
 		&corev1.Service{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Service",
+				APIVersion: "v1",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "foo-svc-3",
 				Namespace: "ns3",
@@ -244,6 +260,10 @@ func TestBackendsPrinter_Print(t *testing.T) {
 			},
 		},
 		&corev1.Service{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Service",
+				APIVersion: "v1",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "foo-svc-4",
 				Namespace: "ns3",

gwctl/pkg/printer/gatewayclasses_test.go

@@ -107,7 +107,7 @@ func TestGatewayClassesPrinter_PrintTable(t *testing.T) {
 	}
 	resourceModel, err := discoverer.DiscoverResourcesForGatewayClass(resourcediscovery.Filter{})
 	if err != nil {
-		t.Fatalf("Failed to construct resourceModel: %v", resourceModel)
+		t.Fatalf("Failed to construct resourceModel: %v", err)
 	}
 
 	gcp := &GatewayClassesPrinter{
@@ -236,7 +236,7 @@ Status: {}
 			}
 			resourceModel, err := discoverer.DiscoverResourcesForGatewayClass(resourcediscovery.Filter{})
 			if err != nil {
-				t.Fatalf("Failed to construct resourceModel: %v", resourceModel)
+				t.Fatalf("Failed to construct resourceModel: %v", err)
 			}
 
 			gcp := &GatewayClassesPrinter{

gwctl/pkg/printer/gateways_test.go

@@ -182,7 +182,7 @@ func TestGatewaysPrinter_PrintTable(t *testing.T) {
 	}
 	resourceModel, err := discoverer.DiscoverResourcesForGateway(resourcediscovery.Filter{})
 	if err != nil {
-		t.Fatalf("Failed to construct resourceModel: %v", resourceModel)
+		t.Fatalf("Failed to construct resourceModel: %v", err)
 	}
 
 	gp := &GatewaysPrinter{
@@ -372,7 +372,7 @@ func TestGatewaysPrinter_PrintDescribeView(t *testing.T) {
 	}
 	resourceModel, err := discoverer.DiscoverResourcesForGateway(resourcediscovery.Filter{})
 	if err != nil {
-		t.Fatalf("Failed to construct resourceModel: %v", resourceModel)
+		t.Fatalf("Failed to construct resourceModel: %v", err)
 	}
 
 	gp := &GatewaysPrinter{

gwctl/pkg/printer/httproutes_test.go

@@ -212,7 +212,7 @@ func TestHTTPRoutesPrinter_PrintTable(t *testing.T) {
 	}
 	resourceModel, err := discoverer.DiscoverResourcesForHTTPRoute(resourcediscovery.Filter{})
 	if err != nil {
-		t.Fatalf("Failed to construct resourceModel: %v", resourceModel)
+		t.Fatalf("Failed to construct resourceModel: %v", err)
 	}
 
 	hp := &HTTPRoutesPrinter{
@@ -401,7 +401,7 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) {
 	}
 	resourceModel, err := discoverer.DiscoverResourcesForHTTPRoute(resourcediscovery.Filter{})
 	if err != nil {
-		t.Fatalf("Failed to construct resourceModel: %v", resourceModel)
+		t.Fatalf("Failed to construct resourceModel: %v", err)
 	}
 
 	hp := &HTTPRoutesPrinter{

gwctl/pkg/printer/namespace_test.go

@@ -82,7 +82,7 @@ func TestNamespacePrinter_PrintTable(t *testing.T) {
 	}
 	resourceModel, err := discoverer.DiscoverResourcesForNamespace(resourcediscovery.Filter{})
 	if err != nil {
-		t.Fatalf("Failed to construct resourceModel: %v", resourceModel)
+		t.Fatalf("Failed to construct resourceModel: %v", err)
 	}
 
 	nsp := &NamespacesPrinter{
@@ -222,7 +222,7 @@ func TestNamespacePrinter_PrintDescribeView(t *testing.T) {
 	}
 	resourceModel, err := discoverer.DiscoverResourcesForNamespace(resourcediscovery.Filter{})
 	if err != nil {
-		t.Fatalf("Failed to construct resourceModel: %v", resourceModel)
+		t.Fatalf("Failed to construct resourceModel: %v", err)
 	}
 
 	nsp := &NamespacesPrinter{

gwctl/pkg/printer/policies_test.go

@@ -365,6 +365,10 @@ func TestPoliciesPrinter_PrintCRDs_JsonYaml(t *testing.T) {
 
 	objects := []runtime.Object{
 		&apiextensionsv1.CustomResourceDefinition{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: "apiextensions.k8s.io/v1",
+				Kind:       "CustomResourceDefinition",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "healthcheckpolicies.foo.com",
 				Labels: map[string]string{
@@ -410,6 +414,10 @@ func TestPoliciesPrinter_PrintCRDs_JsonYaml(t *testing.T) {
 		},
 
 		&apiextensionsv1.CustomResourceDefinition{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: "apiextensions.k8s.io/v1",
+				Kind:       "CustomResourceDefinition",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "timeoutpolicies.bar.com",
 				Labels: map[string]string{
@@ -604,6 +612,10 @@ kind: List`, creationTime1.Format(time.RFC3339), creationTime2.Format(time.RFC33
 func TestPolicyCrd_PrintDescribeView(t *testing.T) {
 	objects := []runtime.Object{
 		&apiextensionsv1.CustomResourceDefinition{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: "apiextensions.k8s.io/v1",
+				Kind:       "CustomResourceDefinition",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "healthcheckpolicies.foo.com",
 				Labels: map[string]string{
@@ -647,6 +659,10 @@ func TestPolicyCrd_PrintDescribeView(t *testing.T) {
 		},
 
 		&apiextensionsv1.CustomResourceDefinition{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: "apiextensions.k8s.io/v1",
+				Kind:       "CustomResourceDefinition",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "timeoutpolicies.bar.com",
 				Labels: map[string]string{