[gwctl] Expand the output of gwctl describe httproute (#3181)

* gwctl: Add InheritedPolicies in resource discovery

* gwctl: Use new Describer and expand output for HTTPRoute

* gwctl: Update expected value of HTTPRoutePrinter describe view test case

* Add tests and related cleanups

---------

Co-authored-by: Tarun Duhan <itarunduhan@gmail.com>

gwctl/cmd/subcommand.go

@@ -314,7 +314,7 @@ func runGetOrDescribeHTTPRoutes(f cmdutils.Factory, o *getOrDescribeOptions) {
 	handleErrOrExitWithMsg(err, "failed to discover HTTPRoute resources")
 
 	realClock := clock.RealClock{}
-	httpRoutesPrinter := &printer.HTTPRoutesPrinter{Writer: o.out, Clock: realClock}
+	httpRoutesPrinter := &printer.HTTPRoutesPrinter{Writer: o.out, Clock: realClock, EventFetcher: discoverer}
 	if o.cmdName == commandNameGet {
 		printer.Print(httpRoutesPrinter, resourceModel, o.outputFormat)
 	} else {

gwctl/pkg/printer/backends.go

@@ -134,8 +134,8 @@ func (bp *BackendsPrinter) PrintDescribeView(resourceModel *resourcediscovery.Re
 		pairs = append(pairs, &DescriberKV{Key: "ReferencedByRoutes", Value: routes})
 
 		// DirectlyAttachedPolicies
-		policyRefs := resourcediscovery.ConvertPoliciesMapToPolicyRefs(backendNode.Policies)
-		pairs = append(pairs, &DescriberKV{Key: "DirectlyAttachedPolicies", Value: convertPolicyRefsToTable(policyRefs)})
+		policies := SortByString(maps.Values(backendNode.Policies))
+		pairs = append(pairs, &DescriberKV{Key: "DirectlyAttachedPolicies", Value: convertPoliciesToRefsTable(policies, false)})
 
 		// EffectivePolicies
 		if len(backendNode.EffectivePolicies) != 0 {

gwctl/pkg/printer/common.go

@@ -31,7 +31,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/yaml"
 
-	"sigs.k8s.io/gateway-api/gwctl/pkg/common"
+	"sigs.k8s.io/gateway-api/gwctl/pkg/resourcediscovery"
 )
 
 // DescriberKV stores key-value pairs that are used with Describing a resource.
@@ -154,20 +154,42 @@ func convertEventsSliceToTable(events []corev1.Event, clock clock.Clock) *Table
 	return table
 }
 
-func convertPolicyRefsToTable(policyRefs []common.ObjRef) *Table {
+func convertPoliciesToRefsTable(policies []*resourcediscovery.PolicyNode, includeTarget bool) *Table {
 	table := &Table{
 		ColumnNames:  []string{"Type", "Name"},
 		UseSeparator: true,
 	}
-	for _, policyRef := range policyRefs {
-		name := policyRef.Name
-		if policyRef.Namespace != "" {
-			name = fmt.Sprintf("%v/%v", policyRef.Namespace, name)
+	if includeTarget {
+		table.ColumnNames = append(table.ColumnNames, "Target Kind", "Target Name")
+	}
+
+	for _, policyNode := range policies {
+		policyType := fmt.Sprintf("%v.%v", policyNode.Policy.Unstructured().GroupVersionKind().Kind, policyNode.Policy.Unstructured().GroupVersionKind().Group)
+
+		policyName := policyNode.Policy.Unstructured().GetName()
+		if ns := policyNode.Policy.Unstructured().GetNamespace(); ns != "" {
+			policyName = fmt.Sprintf("%v/%v", ns, policyName)
+		}
+
+		targetKind := policyNode.Policy.TargetRef().Kind
+
+		targetName := policyNode.Policy.TargetRef().Name
+		if ns := policyNode.Policy.TargetRef().Namespace; ns != "" {
+			targetName = fmt.Sprintf("%v/%v", ns, targetName)
 		}
+
 		row := []string{
-			fmt.Sprintf("%v.%v", policyRef.Kind, policyRef.Group), // Type
-			name, // Name
+			policyType, // Type
+			policyName, // Name
 		}
+
+		if includeTarget {
+			row = append(row,
+				targetKind, // Target Kind
+				targetName, // Target Name
+			)
+		}
+
 		table.Rows = append(table.Rows, row)
 	}
 	return table

gwctl/pkg/printer/gatewayclasses.go

@@ -104,8 +104,8 @@ func (gcp *GatewayClassesPrinter) PrintDescribeView(resourceModel *resourcedisco
 		}
 
 		// DirectlyAttachedPolicies
-		policyRefs := resourcediscovery.ConvertPoliciesMapToPolicyRefs(gatewayClassNode.Policies)
-		pairs = append(pairs, &DescriberKV{Key: "DirectlyAttachedPolicies", Value: convertPolicyRefsToTable(policyRefs)})
+		policies := SortByString(maps.Values(gatewayClassNode.Policies))
+		pairs = append(pairs, &DescriberKV{Key: "DirectlyAttachedPolicies", Value: convertPoliciesToRefsTable(policies, false)})
 
 		// Events
 		eventList := gcp.EventFetcher.FetchEventsFor(context.Background(), gatewayClassNode.GatewayClass)

gwctl/pkg/printer/gateways.go

@@ -140,8 +140,8 @@ func (gp *GatewaysPrinter) PrintDescribeView(resourceModel *resourcediscovery.Re
 		pairs = append(pairs, &DescriberKV{Key: "AttachedRoutes", Value: attachedRoutes})
 
 		// DirectlyAttachedPolicies
-		policyRefs := resourcediscovery.ConvertPoliciesMapToPolicyRefs(gatewayNode.Policies)
-		pairs = append(pairs, &DescriberKV{Key: "DirectlyAttachedPolicies", Value: convertPolicyRefsToTable(policyRefs)})
+		policies := SortByString(maps.Values(gatewayNode.Policies))
+		pairs = append(pairs, &DescriberKV{Key: "DirectlyAttachedPolicies", Value: convertPoliciesToRefsTable(policies, false)})
 
 		// EffectivePolicies
 		if len(gatewayNode.EffectivePolicies) != 0 {

gwctl/pkg/printer/httproutes.go

@@ -17,26 +17,24 @@ limitations under the License.
 package printer
 
 import (
+	"context"
 	"fmt"
 	"io"
-	"os"
 	"strings"
 
 	"golang.org/x/exp/maps"
 	"k8s.io/apimachinery/pkg/util/duration"
 	"k8s.io/utils/clock"
-	"sigs.k8s.io/yaml"
 
-	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
-	"sigs.k8s.io/gateway-api/gwctl/pkg/common"
 	"sigs.k8s.io/gateway-api/gwctl/pkg/resourcediscovery"
 )
 
 var _ Printer = (*HTTPRoutesPrinter)(nil)
 
 type HTTPRoutesPrinter struct {
 	io.Writer
-	Clock clock.Clock
+	Clock        clock.Clock
+	EventFetcher eventFetcher
 }
 
 func (hp *HTTPRoutesPrinter) GetPrintableNodes(resourceModel *resourcediscovery.ResourceModel) []NodeResource {
@@ -91,50 +89,55 @@ func (hp *HTTPRoutesPrinter) PrintTable(resourceModel *resourcediscovery.Resourc
 	table.Write(hp, 0)
 }
 
-type httpRouteDescribeView struct {
-	Name                     string                      `json:",omitempty"`
-	Namespace                string                      `json:",omitempty"`
-	Hostnames                []gatewayv1.Hostname        `json:",omitempty"`
-	ParentRefs               []gatewayv1.ParentReference `json:",omitempty"`
-	DirectlyAttachedPolicies []common.ObjRef             `json:",omitempty"`
-	EffectivePolicies        any                         `json:",omitempty"`
-}
-
 func (hp *HTTPRoutesPrinter) PrintDescribeView(resourceModel *resourcediscovery.ResourceModel) {
 	index := 0
+
 	for _, httpRouteNode := range resourceModel.HTTPRoutes {
 		index++
 
-		views := []httpRouteDescribeView{
-			{
-				Name:      httpRouteNode.HTTPRoute.GetName(),
-				Namespace: httpRouteNode.HTTPRoute.GetNamespace(),
-			},
-			{
-				Hostnames:  httpRouteNode.HTTPRoute.Spec.Hostnames,
-				ParentRefs: httpRouteNode.HTTPRoute.Spec.ParentRefs,
-			},
-		}
-		if policyRefs := resourcediscovery.ConvertPoliciesMapToPolicyRefs(httpRouteNode.Policies); len(policyRefs) != 0 {
-			views = append(views, httpRouteDescribeView{
-				DirectlyAttachedPolicies: policyRefs,
-			})
+		metadata := httpRouteNode.HTTPRoute.ObjectMeta.DeepCopy()
+		metadata.Labels = nil
+		metadata.Annotations = nil
+		metadata.Name = ""
+		metadata.Namespace = ""
+		metadata.ManagedFields = nil
+
+		pairs := []*DescriberKV{
+			{"Name", httpRouteNode.HTTPRoute.GetName()},
+			{"Namespace", httpRouteNode.HTTPRoute.Namespace},
+			{"Label", httpRouteNode.HTTPRoute.Labels},
+			{"Annotations", httpRouteNode.HTTPRoute.Annotations},
+			{"APIVersion", httpRouteNode.HTTPRoute.APIVersion},
+			{"Kind", httpRouteNode.HTTPRoute.Kind},
+			{"Metadata", metadata},
+			{"Spec", httpRouteNode.HTTPRoute.Spec},
+			{"Status", httpRouteNode.HTTPRoute.Status},
 		}
+
+		// DirectlyAttachedPolicies
+		policies := SortByString(maps.Values(httpRouteNode.Policies))
+		pairs = append(pairs, &DescriberKV{Key: "DirectlyAttachedPolicies", Value: convertPoliciesToRefsTable(policies, false)})
+
+		// InheritedPolicies
+		inheritedPolicies := SortByString(maps.Values(httpRouteNode.InheritedPolicies))
+		pairs = append(pairs, &DescriberKV{Key: "InheritedPolicies", Value: convertPoliciesToRefsTable(inheritedPolicies, true)})
+
+		// EffectivePolices
 		if len(httpRouteNode.EffectivePolicies) != 0 {
-			views = append(views, httpRouteDescribeView{
-				EffectivePolicies: httpRouteNode.EffectivePolicies,
-			})
+			pairs = append(pairs, &DescriberKV{Key: "EffectivePolicies", Value: httpRouteNode.EffectivePolicies})
 		}
 
-		for _, view := range views {
-			b, err := yaml.Marshal(view)
-			if err != nil {
-				fmt.Fprintf(os.Stderr, "failed to marshal to yaml: %v\n", err)
-				os.Exit(1)
-			}
-			fmt.Fprint(hp, string(b))
+		// Analysis
+		if len(httpRouteNode.Errors) != 0 {
+			pairs = append(pairs, &DescriberKV{Key: "Analysis", Value: convertErrorsToString(httpRouteNode.Errors)})
 		}
 
+		// Events
+		eventList := hp.EventFetcher.FetchEventsFor(context.Background(), httpRouteNode.HTTPRoute)
+		pairs = append(pairs, &DescriberKV{Key: "Events", Value: convertEventsSliceToTable(eventList.Items, hp.Clock)})
+
+		Describe(hp, pairs)
+
 		if index+1 <= len(resourceModel.HTTPRoutes) {
 			fmt.Fprintf(hp, "\n\n")
 		}

gwctl/pkg/printer/httproutes_test.go

@@ -295,6 +295,10 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) {
 	fakeClock := testingclock.NewFakeClock(time.Now())
 	objects := []runtime.Object{
 		&gatewayv1.GatewayClass{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: gatewayv1.GroupVersion.String(),
+				Kind:       "GatewayClass",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "foo-gatewayclass",
 			},
@@ -330,6 +334,10 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) {
 		},
 
 		&gatewayv1.Gateway{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: gatewayv1.GroupVersion.String(),
+				Kind:       "Gateway",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "foo-gateway",
 				Namespace: "default",
@@ -343,7 +351,8 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) {
 				"apiVersion": "foo.com/v1",
 				"kind":       "HealthCheckPolicy",
 				"metadata": map[string]interface{}{
-					"name": "health-check-gateway",
+					"name":      "health-check-gateway",
+					"namespace": "default",
 				},
 				"spec": map[string]interface{}{
 					"override": map[string]interface{}{
@@ -364,8 +373,13 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) {
 		},
 
 		&gatewayv1.HTTPRoute{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: gatewayv1.GroupVersion.String(),
+				Kind:       "HTTPRoute",
+			},
 			ObjectMeta: metav1.ObjectMeta{
-				Name: "foo-httproute",
+				Name:      "foo-httproute",
+				Namespace: "default",
 			},
 			Spec: gatewayv1.HTTPRouteSpec{
 				CommonRouteSpec: gatewayv1.CommonRouteSpec{
@@ -382,7 +396,8 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) {
 				"apiVersion": "bar.com/v1",
 				"kind":       "TimeoutPolicy",
 				"metadata": map[string]interface{}{
-					"name": "timeout-policy-httproute",
+					"name":      "timeout-policy-httproute",
+					"namespace": "default",
 				},
 				"spec": map[string]interface{}{
 					"condition": "path=/def",
@@ -463,22 +478,39 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) {
 	}
 
 	hp := &HTTPRoutesPrinter{
-		Writer: buff,
-		Clock:  fakeClock,
+		Writer:       buff,
+		Clock:        fakeClock,
+		EventFetcher: discoverer,
 	}
 	hp.PrintDescribeView(resourceModel)
 
 	got := buff.String()
 	want := `
 Name: foo-httproute
-ParentRefs:
-- group: gateway.networking.k8s.io
-  kind: Gateway
-  name: foo-gateway
+Namespace: default
+Label: null
+Annotations: null
+APIVersion: gateway.networking.k8s.io/v1
+Kind: HTTPRoute
+Metadata:
+  creationTimestamp: null
+  resourceVersion: "999"
+Spec:
+  parentRefs:
+  - group: gateway.networking.k8s.io
+    kind: Gateway
+    name: foo-gateway
+Status:
+  parents: null
 DirectlyAttachedPolicies:
-- Group: bar.com
-  Kind: TimeoutPolicy
-  Name: timeout-policy-httproute
+  Type                   Name
+  ----                   ----
+  TimeoutPolicy.bar.com  default/timeout-policy-httproute
+InheritedPolicies:
+  Type                       Name                          Target Kind   Target Name
+  ----                       ----                          -----------   -----------
+  HealthCheckPolicy.foo.com  health-check-gatewayclass     GatewayClass  foo-gatewayclass
+  HealthCheckPolicy.foo.com  default/health-check-gateway  Gateway       default/foo-gateway
 EffectivePolicies:
   default/foo-gateway:
     HealthCheckPolicy.foo.com:
@@ -490,6 +522,7 @@ EffectivePolicies:
     TimeoutPolicy.bar.com:
       condition: path=/def
       seconds: 60
+Events: <none>
 `
 	if diff := cmp.Diff(common.YamlString(want), common.YamlString(got), common.YamlStringTransformer); diff != "" {
 		t.Errorf("Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got, want, diff)

gwctl/pkg/printer/namespace.go

@@ -92,8 +92,8 @@ func (nsp *NamespacesPrinter) PrintDescribeView(resourceModel *resourcediscovery
 		}
 
 		// DirectlyAttachedPolicies
-		policyRefs := resourcediscovery.ConvertPoliciesMapToPolicyRefs(namespaceNode.Policies)
-		pairs = append(pairs, &DescriberKV{Key: "DirectlyAttachedPolicies", Value: convertPolicyRefsToTable(policyRefs)})
+		policies := SortByString(maps.Values(namespaceNode.Policies))
+		pairs = append(pairs, &DescriberKV{Key: "DirectlyAttachedPolicies", Value: convertPoliciesToRefsTable(policies, false)})
 
 		// Events
 		eventList := nsp.EventFetcher.FetchEventsFor(context.Background(), namespaceNode.Namespace)

gwctl/pkg/resourcediscovery/discoverer.go

@@ -158,6 +158,10 @@ func (d Discoverer) DiscoverResourcesForGateway(filter Filter) (*ResourceModel,
 	d.discoverNamespaces(ctx, resourceModel)
 	d.discoverPolicies(resourceModel)
 
+	if err := resourceModel.calculateInheritedPolicies(); err != nil {
+		return resourceModel, err
+	}
+
 	if err := resourceModel.calculateEffectivePolicies(); err != nil {
 		return resourceModel, err
 	}
@@ -182,6 +186,10 @@ func (d Discoverer) DiscoverResourcesForHTTPRoute(filter Filter) (*ResourceModel
 	d.discoverNamespaces(ctx, resourceModel)
 	d.discoverPolicies(resourceModel)
 
+	if err := resourceModel.calculateInheritedPolicies(); err != nil {
+		return resourceModel, err
+	}
+
 	if err := resourceModel.calculateEffectivePolicies(); err != nil {
 		return resourceModel, err
 	}
@@ -207,6 +215,10 @@ func (d Discoverer) DiscoverResourcesForBackend(filter Filter) (*ResourceModel,
 	d.discoverNamespaces(ctx, resourceModel)
 	d.discoverPolicies(resourceModel)
 
+	if err := resourceModel.calculateInheritedPolicies(); err != nil {
+		return resourceModel, err
+	}
+
 	if err := resourceModel.calculateEffectivePolicies(); err != nil {
 		return resourceModel, err
 	}