From 667e73bd82d71bfe12a2131484100ce0d487551e Mon Sep 17 00:00:00 2001 From: Dennis Zhou Date: Thu, 25 Jul 2024 18:12:40 +0800 Subject: [PATCH 1/4] Make gwctl work with Policies having multiple targetRefs --- gwctl/pkg/policymanager/manager.go | 75 ++-- gwctl/pkg/policymanager/merger.go | 27 +- gwctl/pkg/policymanager/merger_test.go | 24 +- gwctl/pkg/printer/backends_test.go | 20 +- gwctl/pkg/printer/common.go | 78 +++- gwctl/pkg/printer/gatewayclasses_test.go | 10 +- gwctl/pkg/printer/gateways_test.go | 54 ++- gwctl/pkg/printer/httproutes_test.go | 59 +-- gwctl/pkg/printer/namespace_test.go | 24 +- gwctl/pkg/printer/policies.go | 18 +- gwctl/pkg/printer/policies_test.go | 381 +++++++++++++++--- gwctl/pkg/resourcediscovery/resourcemodel.go | 94 ++--- .../resourcediscovery/resourcemodel_test.go | 28 +- 13 files changed, 650 insertions(+), 242 deletions(-) diff --git a/gwctl/pkg/policymanager/manager.go b/gwctl/pkg/policymanager/manager.go index b427f04712..024b7e1601 100644 --- a/gwctl/pkg/policymanager/manager.go +++ b/gwctl/pkg/policymanager/manager.go @@ -22,14 +22,13 @@ import ( "fmt" "strings" - "sigs.k8s.io/controller-runtime/pkg/client" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/dynamic" + "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/gwctl/pkg/common" @@ -216,10 +215,10 @@ func (p PolicyCRD) IsClusterScoped() bool { type Policy struct { u unstructured.Unstructured - // targetRef references the target object this policy is attached to. This + // targetRefs references the target object this policy is attached to. This // only makes sense in case of a directly-attached-policy, or an // unmerged-inherited-policy. - targetRef common.ObjRef + targetRefs []common.ObjRef // Indicates whether the policy is supposed to be "inherited" (as opposed to // "direct"). inherited bool @@ -230,29 +229,32 @@ func (p Policy) ClientObject() client.Object { return p.Unstructured() } func PolicyFromUnstructured(u unstructured.Unstructured, policyCRDs map[PolicyCrdID]PolicyCRD) (Policy, error) { result := Policy{u: u} - // Identify targetRef of Policy. + // Identify targetRefs of Policy. type genericPolicy struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` Spec struct { - TargetRef gatewayv1alpha2.NamespacedPolicyTargetReference + TargetRefs []gatewayv1alpha2.NamespacedPolicyTargetReference `json:"targetRefs"` } } structuredPolicy := &genericPolicy{} if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), structuredPolicy); err != nil { return Policy{}, fmt.Errorf("failed to convert unstructured policy resource to structured: %v", err) } - result.targetRef = common.ObjRef{ - Group: string(structuredPolicy.Spec.TargetRef.Group), - Kind: string(structuredPolicy.Spec.TargetRef.Kind), - Name: string(structuredPolicy.Spec.TargetRef.Name), - Namespace: structuredPolicy.GetNamespace(), - } - if result.targetRef.Namespace == "" { - result.targetRef.Namespace = result.u.GetNamespace() - } - if structuredPolicy.Spec.TargetRef.Namespace != nil { - result.targetRef.Namespace = string(*structuredPolicy.Spec.TargetRef.Namespace) + for _, targetRef := range structuredPolicy.Spec.TargetRefs { + objRef := common.ObjRef{ + Group: string(targetRef.Group), + Kind: string(targetRef.Kind), + Name: string(targetRef.Name), + Namespace: u.GetNamespace(), + } + if objRef.Namespace == "" { + objRef.Namespace = u.GetNamespace() + } + if targetRef.Namespace != nil { + objRef.Namespace = string(*targetRef.Namespace) + } + result.targetRefs = append(result.targetRefs, objRef) } // Get the CRD corresponding to this policy object. @@ -274,8 +276,8 @@ func (p Policy) PolicyCrdID() PolicyCrdID { return PolicyCrdID(p.u.GetObjectKind().GroupVersionKind().Kind + "." + p.u.GetObjectKind().GroupVersionKind().Group) } -func (p Policy) TargetRef() common.ObjRef { - return p.targetRef +func (p Policy) TargetRefs() []common.ObjRef { + return p.targetRefs } func (p Policy) IsInherited() bool { @@ -287,19 +289,24 @@ func (p Policy) IsDirect() bool { } func (p Policy) IsAttachedTo(objRef common.ObjRef) bool { - if p.targetRef.Kind == "Namespace" && p.targetRef.Name == "" { - p.targetRef.Name = "default" - } - if objRef.Kind == "Namespace" && objRef.Name == "" { - objRef.Name = "default" - } - if p.targetRef.Kind != "Namespace" && p.targetRef.Namespace == "" { - p.targetRef.Namespace = "default" - } - if objRef.Kind != "Namespace" && objRef.Namespace == "" { - objRef.Namespace = "default" + for _, targetRef := range p.targetRefs { + if targetRef.Kind == "Namespace" && targetRef.Name == "" { + targetRef.Name = "default" + } + if objRef.Kind == "Namespace" && objRef.Name == "" { + objRef.Name = "default" + } + if targetRef.Kind != "Namespace" && targetRef.Namespace == "" { + targetRef.Namespace = "default" + } + if objRef.Kind != "Namespace" && objRef.Namespace == "" { + objRef.Namespace = "default" + } + if targetRef == objRef { + return true + } } - return p.targetRef == objRef + return false } func (p Policy) Unstructured() *unstructured.Unstructured { @@ -308,9 +315,9 @@ func (p Policy) Unstructured() *unstructured.Unstructured { func (p Policy) DeepCopy() Policy { clone := Policy{ - u: *p.u.DeepCopy(), - targetRef: p.targetRef, - inherited: p.inherited, + u: *p.u.DeepCopy(), + targetRefs: p.targetRefs, + inherited: p.inherited, } return clone } diff --git a/gwctl/pkg/policymanager/merger.go b/gwctl/pkg/policymanager/merger.go index a53a1686ad..944a397a15 100644 --- a/gwctl/pkg/policymanager/merger.go +++ b/gwctl/pkg/policymanager/merger.go @@ -58,7 +58,14 @@ func MergePoliciesOfSimilarKind(policies []Policy) (map[PolicyCrdID]Policy, erro return nil, err } - result[policyCrdID] = mergedPolicies[policyCrdID] + mergedPolicy := mergedPolicies[policyCrdID] + + // Check and set targetRefs to nil if it's an empty slice + if len(mergedPolicy.targetRefs) == 0 { + mergedPolicy.targetRefs = nil + } + + result[policyCrdID] = mergedPolicy } return result, nil } @@ -149,10 +156,26 @@ func mergePolicy(parent, child Policy) (Policy, error) { result.u.SetUnstructuredContent(resultUnstructured) // Merging two policies means the targetRef no longer makes any sense since // since they can be conflicting. So we unset the targetRef. - result.targetRef = common.ObjRef{} + result.targetRefs = mergeTargetRefs(parent.targetRefs, child.targetRefs) return result, nil } +// Helper function to merge targetRefs from two policies +func mergeTargetRefs(refs1, refs2 []common.ObjRef) []common.ObjRef { + refMap := make(map[common.ObjRef]struct{}) + for _, ref := range refs1 { + refMap[ref] = struct{}{} + } + for _, ref := range refs2 { + refMap[ref] = struct{}{} + } + result := make([]common.ObjRef, 0, len(refMap)) + for ref := range refMap { + result = append(result, ref) + } + return result +} + func mergeUnstructured(parent, patch map[string]interface{}) (map[string]interface{}, error) { currentJSON, err := json.Marshal(parent) if err != nil { diff --git a/gwctl/pkg/policymanager/merger_test.go b/gwctl/pkg/policymanager/merger_test.go index 03b9b006be..a377eed962 100644 --- a/gwctl/pkg/policymanager/merger_test.go +++ b/gwctl/pkg/policymanager/merger_test.go @@ -87,9 +87,11 @@ func TestMergePoliciesOfSimilarKind(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/def", "seconds": float64(30), - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "default", + }, }, }, }, @@ -106,9 +108,11 @@ func TestMergePoliciesOfSimilarKind(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/abc", "seconds": float64(60), - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "default", + }, }, }, }, @@ -152,9 +156,11 @@ func TestMergePoliciesOfSimilarKind(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/def", "seconds": float64(30), - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "default", + }, }, }, }, diff --git a/gwctl/pkg/printer/backends_test.go b/gwctl/pkg/printer/backends_test.go index 1ebcfbd039..a672188951 100644 --- a/gwctl/pkg/printer/backends_test.go +++ b/gwctl/pkg/printer/backends_test.go @@ -164,11 +164,19 @@ func TestBackendsPrinter_Print(t *testing.T) { "default": map[string]interface{}{ "key2": "value-parent-2", }, - "targetRef": map[string]interface{}{ - "group": "", - "kind": "Service", - "name": "foo-svc-1", - "namespace": "ns1", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "", + "kind": "Service", + "name": "foo-svc-1", + "namespace": "ns1", + }, + map[string]interface{}{ + "group": "", + "kind": "Service", + "name": "foo-svc-2", + "namespace": "ns1", + }, }, }, }, @@ -219,7 +227,7 @@ ns1 foo-svc-2 Service 2d want2 := ` NAMESPACE NAME TYPE AGE REFERRED BY ROUTES POLICIES ns1 foo-svc-1 Service 3d ns1/foo-httproute-1 1 -ns1 foo-svc-2 Service 2d ns1/foo-httproute-2, ns1/foo-httproute-3 + 2 more 0 +ns1 foo-svc-2 Service 2d ns1/foo-httproute-2, ns1/foo-httproute-3 + 2 more 1 ` if diff := cmp.Diff(common.YamlString(want2), common.YamlString(got2), common.YamlStringTransformer); diff != "" { t.Errorf("Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got2, want2, diff) diff --git a/gwctl/pkg/printer/common.go b/gwctl/pkg/printer/common.go index 4e9ac26749..50ba243e69 100644 --- a/gwctl/pkg/printer/common.go +++ b/gwctl/pkg/printer/common.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "os" + "sigs.k8s.io/gateway-api/gwctl/pkg/resourcediscovery" "sort" "strings" "text/tabwriter" @@ -30,8 +31,6 @@ import ( "k8s.io/utils/clock" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" - - "sigs.k8s.io/gateway-api/gwctl/pkg/resourcediscovery" ) // DescriberKV stores key-value pairs that are used with Describing a resource. @@ -154,6 +153,46 @@ func convertEventsSliceToTable(events []corev1.Event, clock clock.Clock) *Table return table } +// func convertPoliciesToRefsTable(policies []*resourcediscovery.PolicyNode, includeTarget bool) *Table { +// table := &Table{ +// ColumnNames: []string{"Type", "Name"}, +// UseSeparator: true, +// } +// 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{ +// policyType, // Type +// policyName, // Name +// } +// +// if includeTarget { +// row = append(row, +// targetKind, // Target Kind +// targetName, // Target Name +// ) +// } +// +// table.Rows = append(table.Rows, row) +// } +// return table +// } func convertPoliciesToRefsTable(policies []*resourcediscovery.PolicyNode, includeTarget bool) *Table { table := &Table{ ColumnNames: []string{"Type", "Name"}, @@ -171,26 +210,27 @@ func convertPoliciesToRefsTable(policies []*resourcediscovery.PolicyNode, includ 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) - } + // Iterate over each TargetRef + for _, targetRef := range policyNode.Policy.TargetRefs() { + targetKind := targetRef.Kind + targetName := targetRef.Name + if ns := targetRef.Namespace; ns != "" { + targetName = fmt.Sprintf("%v/%v", ns, targetName) + } - row := []string{ - policyType, // Type - policyName, // Name - } + row := []string{ + policyType, // Type + policyName, // Name + } - if includeTarget { - row = append(row, - targetKind, // Target Kind - targetName, // Target Name - ) + if includeTarget { + row = append(row, + targetKind, // Target Kind + targetName, // Target Name + ) + } + table.Rows = append(table.Rows, row) } - - table.Rows = append(table.Rows, row) } return table } diff --git a/gwctl/pkg/printer/gatewayclasses_test.go b/gwctl/pkg/printer/gatewayclasses_test.go index 0a76050422..a9b9641e93 100644 --- a/gwctl/pkg/printer/gatewayclasses_test.go +++ b/gwctl/pkg/printer/gatewayclasses_test.go @@ -228,10 +228,12 @@ func TestGatewayClassesPrinter_PrintDescribeView(t *testing.T) { "name": "policy-name", }, "spec": map[string]interface{}{ - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "GatewayClass", - "name": "foo-gatewayclass", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "GatewayClass", + "name": "foo-gatewayclass", + }, }, }, }, diff --git a/gwctl/pkg/printer/gateways_test.go b/gwctl/pkg/printer/gateways_test.go index 0306738a9f..fb00f0de32 100644 --- a/gwctl/pkg/printer/gateways_test.go +++ b/gwctl/pkg/printer/gateways_test.go @@ -228,10 +228,12 @@ func TestGatewaysPrinter_PrintTable(t *testing.T) { "key2": "value-parent-2", "key4": "value-parent-4", }, - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "GatewayClass", - "name": "regional-internal-class", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "GatewayClass", + "name": "regional-internal-class", + }, }, }, }, @@ -251,11 +253,13 @@ func TestGatewaysPrinter_PrintTable(t *testing.T) { "key2": "value-child-2", "key5": "value-child-5", }, - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "Gateway", - "name": "random-gateway", - "namespace": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "random-gateway", + "namespace": "default", + }, }, }, }, @@ -386,10 +390,12 @@ func TestGatewaysPrinter_PrintDescribeView(t *testing.T) { "key2": "value-parent-2", "key4": "value-parent-4", }, - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "GatewayClass", - "name": "foo-gatewayclass", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "GatewayClass", + "name": "foo-gatewayclass", + }, }, }, }, @@ -409,11 +415,12 @@ func TestGatewaysPrinter_PrintDescribeView(t *testing.T) { "key2": "value-child-2", "key5": "value-child-5", }, - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "Gateway", - "name": "foo-gateway", - "namespace": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "foo-gateway", + }, }, }, }, @@ -446,9 +453,11 @@ func TestGatewaysPrinter_PrintDescribeView(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/abc", "seconds": int64(30), - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "default", + }, }, }, }, @@ -525,6 +534,9 @@ EffectivePolicies: TimeoutPolicy.bar.com: condition: path=/abc seconds: 30 + targetRefs: + - kind: Namespace + name: default Events: Type Reason Age From Message ---- ------ --- ---- ------- diff --git a/gwctl/pkg/printer/httproutes_test.go b/gwctl/pkg/printer/httproutes_test.go index 1d54aaadae..57c7b06fe2 100644 --- a/gwctl/pkg/printer/httproutes_test.go +++ b/gwctl/pkg/printer/httproutes_test.go @@ -230,10 +230,12 @@ func TestHTTPRoutesPrinter_PrintTable(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/def", "seconds": int64(60), - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "HTTPRoute", - "name": "foo-httproute-1", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "HTTPRoute", + "name": "foo-httproute-1", + }, }, }, }, @@ -324,20 +326,18 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) { "key2": "value-parent-2", "key4": "value-parent-4", }, - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "GatewayClass", - "name": "foo-gatewayclass", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "GatewayClass", + "name": "foo-gatewayclass", + }, }, }, }, }, &gatewayv1.Gateway{ - TypeMeta: metav1.TypeMeta{ - APIVersion: gatewayv1.GroupVersion.String(), - Kind: "Gateway", - }, ObjectMeta: metav1.ObjectMeta{ Name: "foo-gateway", Namespace: "default", @@ -362,11 +362,13 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) { "key2": "value-child-2", "key5": "value-child-5", }, - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "Gateway", - "name": "foo-gateway", - "namespace": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "foo-gateway", + "namespace": "default", + }, }, }, }, @@ -402,10 +404,12 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/def", "seconds": int64(60), - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "HTTPRoute", - "name": "foo-httproute", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "HTTPRoute", + "name": "foo-httproute", + }, }, }, }, @@ -456,9 +460,11 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/abc", "seconds": int64(30), - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "default", + }, }, }, }, @@ -522,7 +528,12 @@ EffectivePolicies: TimeoutPolicy.bar.com: condition: path=/def seconds: 60 + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: foo-httproute Events: + ` 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) diff --git a/gwctl/pkg/printer/namespace_test.go b/gwctl/pkg/printer/namespace_test.go index 68fc8bc773..04d9d4abcf 100644 --- a/gwctl/pkg/printer/namespace_test.go +++ b/gwctl/pkg/printer/namespace_test.go @@ -101,9 +101,11 @@ func TestNamespacePrinter_PrintTable(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/abc", "seconds": int64(30), - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "ns1", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "ns1", + }, }, }, }, @@ -212,9 +214,11 @@ func TestNamespacePrinter_PrintDescribeView(t *testing.T) { "key2": "value-parent-2", "key4": "value-parent-4", }, - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "development", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "development", + }, }, }, }, @@ -260,9 +264,11 @@ func TestNamespacePrinter_PrintDescribeView(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/abc", "seconds": int64(30), - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "production", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "production", + }, }, }, }, diff --git a/gwctl/pkg/printer/policies.go b/gwctl/pkg/printer/policies.go index 7d4493fd83..b06a5534f4 100644 --- a/gwctl/pkg/printer/policies.go +++ b/gwctl/pkg/printer/policies.go @@ -21,6 +21,7 @@ import ( "io" "os" "sort" + "strings" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -54,7 +55,7 @@ func (pp *PoliciesPrinter) printClientObjects(objects []client.Object, format ut func (pp *PoliciesPrinter) printPoliciesTable(sortedPoliciesList []policymanager.Policy) { table := &Table{ - ColumnNames: []string{"NAME", "KIND", "TARGET NAME", "TARGET KIND", "POLICY TYPE", "AGE"}, + ColumnNames: []string{"NAME", "KIND", "TARGET REFS", "POLICY TYPE", "AGE"}, UseSeparator: false, } @@ -65,14 +66,23 @@ func (pp *PoliciesPrinter) printPoliciesTable(sortedPoliciesList []policymanager } kind := fmt.Sprintf("%v.%v", policy.Unstructured().GroupVersionKind().Kind, policy.Unstructured().GroupVersionKind().Group) - age := duration.HumanDuration(pp.Clock.Since(policy.Unstructured().GetCreationTimestamp().Time)) + targetRefs := policy.TargetRefs() + var displayedRefs []string + for i, targetRef := range targetRefs { + if i >= 2 { + displayedRefs = append(displayedRefs, fmt.Sprintf("+%d more", len(targetRefs)-2)) + break + } + displayedRefs = append(displayedRefs, fmt.Sprintf("%s (%s)", targetRef.Name, targetRef.Kind)) + } + targetRefsDisplay := strings.Join(displayedRefs, ", ") + row := []string{ policy.Unstructured().GetName(), kind, - policy.TargetRef().Name, - policy.TargetRef().Kind, + targetRefsDisplay, policyType, age, } diff --git a/gwctl/pkg/printer/policies_test.go b/gwctl/pkg/printer/policies_test.go index cc1420a482..951b075d02 100644 --- a/gwctl/pkg/printer/policies_test.go +++ b/gwctl/pkg/printer/policies_test.go @@ -73,10 +73,12 @@ func TestPoliciesPrinter_Print_And_PrintDescribeView(t *testing.T) { "key2": "value-parent-2", "key4": "value-parent-4", }, - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "GatewayClass", - "name": "foo-gatewayclass", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "GatewayClass", + "name": "foo-gatewayclass", + }, }, }, }, @@ -97,11 +99,13 @@ func TestPoliciesPrinter_Print_And_PrintDescribeView(t *testing.T) { "key2": "value-child-2", "key5": "value-child-5", }, - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "Gateway", - "name": "foo-gateway", - "namespace": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "foo-gateway", + "namespace": "default", + }, }, }, }, @@ -135,9 +139,11 @@ func TestPoliciesPrinter_Print_And_PrintDescribeView(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/abc", "seconds": int64(30), - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "default", + }, }, }, }, @@ -153,10 +159,12 @@ func TestPoliciesPrinter_Print_And_PrintDescribeView(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/def", "seconds": int64(60), - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "HTTPRoute", - "name": "foo-httproute", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "HTTPRoute", + "name": "foo-httproute", + }, }, }, }, @@ -174,11 +182,11 @@ func TestPoliciesPrinter_Print_And_PrintDescribeView(t *testing.T) { pp.PrintPolicies(policyManager.GetPolicies(), utils.OutputFormatTable) got := pp.Writer.(*bytes.Buffer).String() want := ` -NAME KIND TARGET NAME TARGET KIND POLICY TYPE AGE -health-check-gateway HealthCheckPolicy.foo.com foo-gateway Gateway Inherited 20d -health-check-gatewayclass HealthCheckPolicy.foo.com foo-gatewayclass GatewayClass Inherited 6d -timeout-policy-httproute TimeoutPolicy.bar.com foo-httproute HTTPRoute Direct 13m -timeout-policy-namespace TimeoutPolicy.bar.com default Namespace Direct 5m +NAME KIND TARGET REFS POLICY TYPE AGE +health-check-gateway HealthCheckPolicy.foo.com foo-gateway (Gateway) Inherited 20d +health-check-gatewayclass HealthCheckPolicy.foo.com foo-gatewayclass (GatewayClass) Inherited 6d +timeout-policy-httproute TimeoutPolicy.bar.com foo-httproute (HTTPRoute) Direct 13m +timeout-policy-namespace TimeoutPolicy.bar.com default (Namespace) Direct 5m ` if diff := cmp.Diff(common.YamlString(want), common.YamlString(got), common.YamlStringTransformer); diff != "" { t.Errorf("Print: Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got, want, diff) @@ -198,8 +206,8 @@ Spec: key5: value-child-5 override: key1: value-child-1 - targetRef: - group: gateway.networking.k8s.io + targetRefs: + - group: gateway.networking.k8s.io kind: Gateway name: foo-gateway namespace: default @@ -217,8 +225,8 @@ Spec: key1: value-parent-1 key3: value-parent-3 key5: value-parent-5 - targetRef: - group: gateway.networking.k8s.io + targetRefs: + - group: gateway.networking.k8s.io kind: GatewayClass name: foo-gatewayclass @@ -230,8 +238,8 @@ Inherited: "false" Spec: condition: path=/def seconds: 60 - targetRef: - group: gateway.networking.k8s.io + targetRefs: + - group: gateway.networking.k8s.io kind: HTTPRoute name: foo-httproute @@ -243,8 +251,263 @@ Inherited: "false" Spec: condition: path=/abc seconds: 30 - targetRef: - kind: Namespace + targetRefs: + - kind: Namespace + name: default +` + if diff := cmp.Diff(common.YamlString(want), common.YamlString(got), common.YamlStringTransformer); diff != "" { + t.Errorf("PrintDescribeView: Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got, want, diff) + } +} + +func TestPoliciesPrinter_Print_And_PrintDescribeView2(t *testing.T) { + fakeClock := testingclock.NewFakeClock(time.Now()) + objects := []runtime.Object{ + &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "healthcheckpolicies.foo.com", + Labels: map[string]string{ + gatewayv1alpha2.PolicyLabelKey: "inherited", + }, + }, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Scope: apiextensionsv1.ClusterScoped, + Group: "foo.com", + Versions: []apiextensionsv1.CustomResourceDefinitionVersion{{Name: "v1"}}, + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "healthcheckpolicies", + Kind: "HealthCheckPolicy", + }, + }, + }, + &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "foo.com/v1", + "kind": "HealthCheckPolicy", + "metadata": map[string]interface{}{ + "name": "health-check-gatewayclass", + "creationTimestamp": fakeClock.Now().Add(-6 * 24 * time.Hour).Format(time.RFC3339), + }, + "spec": map[string]interface{}{ + "override": map[string]interface{}{ + "key1": "value-parent-1", + "key3": "value-parent-3", + "key5": "value-parent-5", + }, + "default": map[string]interface{}{ + "key2": "value-parent-2", + "key4": "value-parent-4", + }, + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "GatewayClass", + "name": "foo-gatewayclass", + }, + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "foo-gateway", + "namespace": "default", + }, + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "bar-gateway", + "namespace": "default", + }, + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "foo-gateway2", + "namespace": "default", + }, + }, + }, + }, + }, + &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "foo.com/v1", + "kind": "HealthCheckPolicy", + "metadata": map[string]interface{}{ + "name": "health-check-gateway", + "creationTimestamp": fakeClock.Now().Add(-20 * 24 * time.Hour).Format(time.RFC3339), + }, + "spec": map[string]interface{}{ + "override": map[string]interface{}{ + "key1": "value-child-1", + }, + "default": map[string]interface{}{ + "key2": "value-child-2", + "key5": "value-child-5", + }, + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "foo-gateway", + "namespace": "default", + }, + }, + }, + }, + }, + + &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "timeoutpolicies.bar.com", + Labels: map[string]string{ + gatewayv1alpha2.PolicyLabelKey: "direct", + }, + }, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Scope: apiextensionsv1.ClusterScoped, + Group: "bar.com", + Versions: []apiextensionsv1.CustomResourceDefinitionVersion{{Name: "v1"}}, + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "timeoutpolicies", + Kind: "TimeoutPolicy", + }, + }, + }, + &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "bar.com/v1", + "kind": "TimeoutPolicy", + "metadata": map[string]interface{}{ + "name": "timeout-policy-namespace", + "creationTimestamp": fakeClock.Now().Add(-5 * time.Minute).Format(time.RFC3339), + }, + "spec": map[string]interface{}{ + "condition": "path=/abc", + "seconds": int64(30), + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "default", + }, + }, + }, + }, + }, + &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "bar.com/v1", + "kind": "TimeoutPolicy", + "metadata": map[string]interface{}{ + "name": "timeout-policy-httproute", + "creationTimestamp": fakeClock.Now().Add(-13 * time.Minute).Format(time.RFC3339), + }, + "spec": map[string]interface{}{ + "condition": "path=/def", + "seconds": int64(60), + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "HTTPRoute", + "name": "foo-httproute", + }, + }, + }, + }, + }, + } + + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + + pp := &PoliciesPrinter{ + Writer: &bytes.Buffer{}, + Clock: fakeClock, + } + + pp.PrintPolicies(policyManager.GetPolicies(), utils.OutputFormatTable) + got := pp.Writer.(*bytes.Buffer).String() + want := ` +NAME KIND TARGET REFS POLICY TYPE AGE +health-check-gateway HealthCheckPolicy.foo.com foo-gateway (Gateway) Inherited 20d +health-check-gatewayclass HealthCheckPolicy.foo.com foo-gatewayclass (GatewayClass), foo-gateway (Gateway), +2 more Inherited 6d +timeout-policy-httproute TimeoutPolicy.bar.com foo-httproute (HTTPRoute) Direct 13m +timeout-policy-namespace TimeoutPolicy.bar.com default (Namespace) Direct 5m +` + if diff := cmp.Diff(common.YamlString(want), common.YamlString(got), common.YamlStringTransformer); diff != "" { + t.Errorf("Print: Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got, want, diff) + } + + pp.Writer = &bytes.Buffer{} + pp.PrintPoliciesDescribeView(policyManager.GetPolicies()) + got = pp.Writer.(*bytes.Buffer).String() + want = ` +Name: health-check-gateway +Group: foo.com +Kind: HealthCheckPolicy +Inherited: "true" +Spec: + default: + key2: value-child-2 + key5: value-child-5 + override: + key1: value-child-1 + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: foo-gateway + namespace: default + + +Name: health-check-gatewayclass +Group: foo.com +Kind: HealthCheckPolicy +Inherited: "true" +Spec: + default: + key2: value-parent-2 + key4: value-parent-4 + override: + key1: value-parent-1 + key3: value-parent-3 + key5: value-parent-5 + targetRefs: + - group: gateway.networking.k8s.io + kind: GatewayClass + name: foo-gatewayclass + - group: gateway.networking.k8s.io + kind: Gateway + name: foo-gateway + namespace: default + - group: gateway.networking.k8s.io + kind: Gateway + name: bar-gateway + namespace: default + - group: gateway.networking.k8s.io + kind: Gateway + name: foo-gateway2 + namespace: default + + +Name: timeout-policy-httproute +Group: bar.com +Kind: TimeoutPolicy +Inherited: "false" +Spec: + condition: path=/def + seconds: 60 + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: foo-httproute + + +Name: timeout-policy-namespace +Group: bar.com +Kind: TimeoutPolicy +Inherited: "false" +Spec: + condition: path=/abc + seconds: 30 + targetRefs: + - kind: Namespace name: default ` if diff := cmp.Diff(common.YamlString(want), common.YamlString(got), common.YamlStringTransformer); diff != "" { @@ -290,11 +553,13 @@ func TestPoliciesPrinter_PrintCRDs(t *testing.T) { "key2": "value-child-2", "key5": "value-child-5", }, - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "Gateway", - "name": "foo-gateway", - "namespace": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "foo-gateway", + "namespace": "default", + }, }, }, }, @@ -330,9 +595,11 @@ func TestPoliciesPrinter_PrintCRDs(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/abc", "seconds": int64(30), - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "default", + }, }, }, }, @@ -405,11 +672,13 @@ func TestPoliciesPrinter_PrintCRDs_JsonYaml(t *testing.T) { "key2": "value-child-2", "key5": "value-child-5", }, - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "Gateway", - "name": "foo-gateway", - "namespace": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "foo-gateway", + "namespace": "default", + }, }, }, }, @@ -449,9 +718,11 @@ func TestPoliciesPrinter_PrintCRDs_JsonYaml(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/abc", "seconds": int64(30), - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "default", + }, }, }, }, @@ -651,11 +922,13 @@ func TestPolicyCrd_PrintDescribeView(t *testing.T) { "key2": "value-child-2", "key5": "value-child-5", }, - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "Gateway", - "name": "foo-gateway", - "namespace": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "foo-gateway", + "namespace": "default", + }, }, }, }, @@ -693,9 +966,11 @@ func TestPolicyCrd_PrintDescribeView(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/abc", "seconds": int64(30), - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "default", + }, }, }, }, diff --git a/gwctl/pkg/resourcediscovery/resourcemodel.go b/gwctl/pkg/resourcediscovery/resourcemodel.go index 831d75bcb8..abceef7484 100644 --- a/gwctl/pkg/resourcediscovery/resourcemodel.go +++ b/gwctl/pkg/resourcediscovery/resourcemodel.go @@ -143,64 +143,66 @@ func (rm *ResourceModel) addPolicyIfTargetExists(policies ...policymanager.Polic policy := policy policyNode := NewPolicyNode(&policy) - switch { - case policy.TargetRef().Group == gatewayv1.GroupName: - switch policy.TargetRef().Kind { - case "GatewayClass": - gwcID := GatewayClassID(policy.TargetRef().Name) - gatewayClassNode, ok := rm.GatewayClasses[gwcID] - if !ok { - klog.V(1).ErrorS(nil, "Skipping policy since targetRef GatewayClass does not exist in ResourceModel", "policy", policy.Name(), "gatewayClassID", gwcID) - continue + for _, targetRef := range policy.TargetRefs() { + switch { + case targetRef.Group == gatewayv1.GroupName: + switch targetRef.Kind { + case "GatewayClass": + gwcID := GatewayClassID(targetRef.Name) + gatewayClassNode, ok := rm.GatewayClasses[gwcID] + if !ok { + klog.V(1).ErrorS(nil, "Skipping policy since targetRef GatewayClass does not exist in ResourceModel", "policy", policy.Name(), "gatewayClassID", gwcID) + continue + } + rm.Policies[policyNode.ID()] = policyNode + policyNode.GatewayClass = gatewayClassNode + gatewayClassNode.Policies[policyNode.ID()] = policyNode + + case "Gateway": + gwID := GatewayID(targetRef.Namespace, targetRef.Name) + gatewayNode, ok := rm.Gateways[gwID] + if !ok { + klog.V(1).ErrorS(nil, "Skipping policy since targetRef Gateway does not exist in ResourceModel", "policy", policy.Name(), "gatewayID", gwID) + continue + } + rm.Policies[policyNode.ID()] = policyNode + policyNode.Gateway = gatewayNode + gatewayNode.Policies[policyNode.ID()] = policyNode + + case "HTTPRoute": + hrID := HTTPRouteID(targetRef.Namespace, targetRef.Name) + httpRouteNode, ok := rm.HTTPRoutes[hrID] + if !ok { + klog.V(1).ErrorS(nil, "Skipping policy since targetRef HTTPRoute does not exist in ResourceModel", "policy", policy.Name(), "httpRouteID", hrID) + continue + } + rm.Policies[policyNode.ID()] = policyNode + policyNode.HTTPRoute = httpRouteNode + httpRouteNode.Policies[policyNode.ID()] = policyNode } - rm.Policies[policyNode.ID()] = policyNode - policyNode.GatewayClass = gatewayClassNode - gatewayClassNode.Policies[policyNode.ID()] = policyNode - case "Gateway": - gwID := GatewayID(policy.TargetRef().Namespace, policy.TargetRef().Name) - gatewayNode, ok := rm.Gateways[gwID] + case targetRef.Group == corev1.GroupName && targetRef.Kind == "Namespace": + nsID := NamespaceID(targetRef.Name) + namespaceNode, ok := rm.Namespaces[nsID] if !ok { - klog.V(1).ErrorS(nil, "Skipping policy since targetRef Gateway does not exist in ResourceModel", "policy", policy.Name(), "gatewayID", gwID) + klog.V(1).ErrorS(nil, "Skipping policy since targetRef Namespace does not exist in ResourceModel", "policy", policy.Name(), "namespaceID", nsID) continue } rm.Policies[policyNode.ID()] = policyNode - policyNode.Gateway = gatewayNode - gatewayNode.Policies[policyNode.ID()] = policyNode + policyNode.Namespace = namespaceNode + namespaceNode.Policies[policyNode.ID()] = policyNode - case "HTTPRoute": - hrID := HTTPRouteID(policy.TargetRef().Namespace, policy.TargetRef().Name) - httpRouteNode, ok := rm.HTTPRoutes[hrID] + default: // Assume attached to backend and evaluate further. + bID := BackendID(targetRef.Group, targetRef.Kind, targetRef.Namespace, targetRef.Name) + backendNode, ok := rm.Backends[bID] if !ok { - klog.V(1).ErrorS(nil, "Skipping policy since targetRef HTTPRoute does not exist in ResourceModel", "policy", policy.Name(), "httpRouteID", hrID) + klog.V(1).ErrorS(nil, "Skipping policy since targetRef Backend does not exist in ResourceModel", "policy", policy.Name(), "backendID", bID) continue } rm.Policies[policyNode.ID()] = policyNode - policyNode.HTTPRoute = httpRouteNode - httpRouteNode.Policies[policyNode.ID()] = policyNode - } - - case policy.TargetRef().Group == corev1.GroupName && policy.TargetRef().Kind == "Namespace": - nsID := NamespaceID(policy.TargetRef().Name) - namespaceNode, ok := rm.Namespaces[nsID] - if !ok { - klog.V(1).ErrorS(nil, "Skipping policy since targetRef Namespace does not exist in ResourceModel", "policy", policy.Name(), "namespaceID", nsID) - continue - } - rm.Policies[policyNode.ID()] = policyNode - policyNode.Namespace = namespaceNode - namespaceNode.Policies[policyNode.ID()] = policyNode - - default: // Assume attached to backend and evaluate further. - bID := BackendID(policy.TargetRef().Group, policy.TargetRef().Kind, policy.TargetRef().Namespace, policy.TargetRef().Name) - backendNode, ok := rm.Backends[bID] - if !ok { - klog.V(1).ErrorS(nil, "Skipping policy since targetRef Backend does not exist in ResourceModel", "policy", policy.Name(), "backendID", bID) - continue + policyNode.Backend = backendNode + backendNode.Policies[policyNode.ID()] = policyNode } - rm.Policies[policyNode.ID()] = policyNode - policyNode.Backend = backendNode - backendNode.Policies[policyNode.ID()] = policyNode } } } diff --git a/gwctl/pkg/resourcediscovery/resourcemodel_test.go b/gwctl/pkg/resourcediscovery/resourcemodel_test.go index 1a7f55ded7..86c1b3cdcd 100644 --- a/gwctl/pkg/resourcediscovery/resourcemodel_test.go +++ b/gwctl/pkg/resourcediscovery/resourcemodel_test.go @@ -128,9 +128,11 @@ func TestResourceModel_calculateInheritedPolicies(t *testing.T) { "name": "timeout-policy-on-namespace", }, "spec": map[string]interface{}{ - "targetRef": map[string]interface{}{ - "kind": "Namespace", - "name": "default", + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "Namespace", + "name": "default", + }, }, }, }, @@ -162,10 +164,12 @@ func TestResourceModel_calculateInheritedPolicies(t *testing.T) { "namespace": "default", }, "spec": map[string]interface{}{ - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "HTTPRoute", - "name": "foo-httproute", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "HTTPRoute", + "name": "foo-httproute", + }, }, }, }, @@ -198,10 +202,12 @@ func TestResourceModel_calculateInheritedPolicies(t *testing.T) { "namespace": "default", }, "spec": map[string]interface{}{ - "targetRef": map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "HTTPRoute", - "name": "foo-httproute", + "targetRefs": []interface{}{ + map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "HTTPRoute", + "name": "foo-httproute", + }, }, }, }, From 09fe7a827ffcc6cd7b37918483e6de73bc313bce Mon Sep 17 00:00:00 2001 From: Dennis Zhou Date: Thu, 25 Jul 2024 18:21:29 +0800 Subject: [PATCH 2/4] chore:del TestPoliciesPrinter_Print_And_PrintDescribeView2 --- gwctl/pkg/printer/policies_test.go | 225 ----------------------------- 1 file changed, 225 deletions(-) diff --git a/gwctl/pkg/printer/policies_test.go b/gwctl/pkg/printer/policies_test.go index 951b075d02..4e4f656336 100644 --- a/gwctl/pkg/printer/policies_test.go +++ b/gwctl/pkg/printer/policies_test.go @@ -36,231 +36,6 @@ import ( ) func TestPoliciesPrinter_Print_And_PrintDescribeView(t *testing.T) { - fakeClock := testingclock.NewFakeClock(time.Now()) - objects := []runtime.Object{ - &apiextensionsv1.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "healthcheckpolicies.foo.com", - Labels: map[string]string{ - gatewayv1alpha2.PolicyLabelKey: "inherited", - }, - }, - Spec: apiextensionsv1.CustomResourceDefinitionSpec{ - Scope: apiextensionsv1.ClusterScoped, - Group: "foo.com", - Versions: []apiextensionsv1.CustomResourceDefinitionVersion{{Name: "v1"}}, - Names: apiextensionsv1.CustomResourceDefinitionNames{ - Plural: "healthcheckpolicies", - Kind: "HealthCheckPolicy", - }, - }, - }, - &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "foo.com/v1", - "kind": "HealthCheckPolicy", - "metadata": map[string]interface{}{ - "name": "health-check-gatewayclass", - "creationTimestamp": fakeClock.Now().Add(-6 * 24 * time.Hour).Format(time.RFC3339), - }, - "spec": map[string]interface{}{ - "override": map[string]interface{}{ - "key1": "value-parent-1", - "key3": "value-parent-3", - "key5": "value-parent-5", - }, - "default": map[string]interface{}{ - "key2": "value-parent-2", - "key4": "value-parent-4", - }, - "targetRefs": []interface{}{ - map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "GatewayClass", - "name": "foo-gatewayclass", - }, - }, - }, - }, - }, - &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "foo.com/v1", - "kind": "HealthCheckPolicy", - "metadata": map[string]interface{}{ - "name": "health-check-gateway", - "creationTimestamp": fakeClock.Now().Add(-20 * 24 * time.Hour).Format(time.RFC3339), - }, - "spec": map[string]interface{}{ - "override": map[string]interface{}{ - "key1": "value-child-1", - }, - "default": map[string]interface{}{ - "key2": "value-child-2", - "key5": "value-child-5", - }, - "targetRefs": []interface{}{ - map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "Gateway", - "name": "foo-gateway", - "namespace": "default", - }, - }, - }, - }, - }, - - &apiextensionsv1.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "timeoutpolicies.bar.com", - Labels: map[string]string{ - gatewayv1alpha2.PolicyLabelKey: "direct", - }, - }, - Spec: apiextensionsv1.CustomResourceDefinitionSpec{ - Scope: apiextensionsv1.ClusterScoped, - Group: "bar.com", - Versions: []apiextensionsv1.CustomResourceDefinitionVersion{{Name: "v1"}}, - Names: apiextensionsv1.CustomResourceDefinitionNames{ - Plural: "timeoutpolicies", - Kind: "TimeoutPolicy", - }, - }, - }, - &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "bar.com/v1", - "kind": "TimeoutPolicy", - "metadata": map[string]interface{}{ - "name": "timeout-policy-namespace", - "creationTimestamp": fakeClock.Now().Add(-5 * time.Minute).Format(time.RFC3339), - }, - "spec": map[string]interface{}{ - "condition": "path=/abc", - "seconds": int64(30), - "targetRefs": []interface{}{ - map[string]interface{}{ - "kind": "Namespace", - "name": "default", - }, - }, - }, - }, - }, - &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "bar.com/v1", - "kind": "TimeoutPolicy", - "metadata": map[string]interface{}{ - "name": "timeout-policy-httproute", - "creationTimestamp": fakeClock.Now().Add(-13 * time.Minute).Format(time.RFC3339), - }, - "spec": map[string]interface{}{ - "condition": "path=/def", - "seconds": int64(60), - "targetRefs": []interface{}{ - map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "HTTPRoute", - "name": "foo-httproute", - }, - }, - }, - }, - }, - } - - k8sClients := common.MustClientsForTest(t, objects...) - policyManager := utils.MustPolicyManagerForTest(t, k8sClients) - - pp := &PoliciesPrinter{ - Writer: &bytes.Buffer{}, - Clock: fakeClock, - } - - pp.PrintPolicies(policyManager.GetPolicies(), utils.OutputFormatTable) - got := pp.Writer.(*bytes.Buffer).String() - want := ` -NAME KIND TARGET REFS POLICY TYPE AGE -health-check-gateway HealthCheckPolicy.foo.com foo-gateway (Gateway) Inherited 20d -health-check-gatewayclass HealthCheckPolicy.foo.com foo-gatewayclass (GatewayClass) Inherited 6d -timeout-policy-httproute TimeoutPolicy.bar.com foo-httproute (HTTPRoute) Direct 13m -timeout-policy-namespace TimeoutPolicy.bar.com default (Namespace) Direct 5m -` - if diff := cmp.Diff(common.YamlString(want), common.YamlString(got), common.YamlStringTransformer); diff != "" { - t.Errorf("Print: Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got, want, diff) - } - - pp.Writer = &bytes.Buffer{} - pp.PrintPoliciesDescribeView(policyManager.GetPolicies()) - got = pp.Writer.(*bytes.Buffer).String() - want = ` -Name: health-check-gateway -Group: foo.com -Kind: HealthCheckPolicy -Inherited: "true" -Spec: - default: - key2: value-child-2 - key5: value-child-5 - override: - key1: value-child-1 - targetRefs: - - group: gateway.networking.k8s.io - kind: Gateway - name: foo-gateway - namespace: default - - -Name: health-check-gatewayclass -Group: foo.com -Kind: HealthCheckPolicy -Inherited: "true" -Spec: - default: - key2: value-parent-2 - key4: value-parent-4 - override: - key1: value-parent-1 - key3: value-parent-3 - key5: value-parent-5 - targetRefs: - - group: gateway.networking.k8s.io - kind: GatewayClass - name: foo-gatewayclass - - -Name: timeout-policy-httproute -Group: bar.com -Kind: TimeoutPolicy -Inherited: "false" -Spec: - condition: path=/def - seconds: 60 - targetRefs: - - group: gateway.networking.k8s.io - kind: HTTPRoute - name: foo-httproute - - -Name: timeout-policy-namespace -Group: bar.com -Kind: TimeoutPolicy -Inherited: "false" -Spec: - condition: path=/abc - seconds: 30 - targetRefs: - - kind: Namespace - name: default -` - if diff := cmp.Diff(common.YamlString(want), common.YamlString(got), common.YamlStringTransformer); diff != "" { - t.Errorf("PrintDescribeView: Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got, want, diff) - } -} - -func TestPoliciesPrinter_Print_And_PrintDescribeView2(t *testing.T) { fakeClock := testingclock.NewFakeClock(time.Now()) objects := []runtime.Object{ &apiextensionsv1.CustomResourceDefinition{ From d13c46cb137142d67ccf76dd77b9ed291c42d644 Mon Sep 17 00:00:00 2001 From: Dennis Zhou Date: Thu, 25 Jul 2024 18:47:20 +0800 Subject: [PATCH 3/4] chore:del convertPoliciesToRefsTable and fix lint test --- gwctl/pkg/printer/common.go | 43 ++----------------------------------- 1 file changed, 2 insertions(+), 41 deletions(-) diff --git a/gwctl/pkg/printer/common.go b/gwctl/pkg/printer/common.go index 50ba243e69..8e05b9bd84 100644 --- a/gwctl/pkg/printer/common.go +++ b/gwctl/pkg/printer/common.go @@ -21,7 +21,6 @@ import ( "fmt" "io" "os" - "sigs.k8s.io/gateway-api/gwctl/pkg/resourcediscovery" "sort" "strings" "text/tabwriter" @@ -31,6 +30,8 @@ import ( "k8s.io/utils/clock" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" + + "sigs.k8s.io/gateway-api/gwctl/pkg/resourcediscovery" ) // DescriberKV stores key-value pairs that are used with Describing a resource. @@ -153,46 +154,6 @@ func convertEventsSliceToTable(events []corev1.Event, clock clock.Clock) *Table return table } -// func convertPoliciesToRefsTable(policies []*resourcediscovery.PolicyNode, includeTarget bool) *Table { -// table := &Table{ -// ColumnNames: []string{"Type", "Name"}, -// UseSeparator: true, -// } -// 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{ -// policyType, // Type -// policyName, // Name -// } -// -// if includeTarget { -// row = append(row, -// targetKind, // Target Kind -// targetName, // Target Name -// ) -// } -// -// table.Rows = append(table.Rows, row) -// } -// return table -// } func convertPoliciesToRefsTable(policies []*resourcediscovery.PolicyNode, includeTarget bool) *Table { table := &Table{ ColumnNames: []string{"Type", "Name"}, From 7408571f56da5d3c59b49d5e577b0bef081aa25c Mon Sep 17 00:00:00 2001 From: Dennis Zhou Date: Fri, 26 Jul 2024 16:25:34 +0800 Subject: [PATCH 4/4] Add support for both targetRefs and targetRef fields in gwctl --- gwctl/pkg/policymanager/manager.go | 33 +++++++++++++++++++++++++++- gwctl/pkg/printer/gateways_test.go | 3 --- gwctl/pkg/printer/httproutes_test.go | 4 ---- gwctl/pkg/printer/policies_test.go | 30 ++++++++++--------------- 4 files changed, 44 insertions(+), 26 deletions(-) diff --git a/gwctl/pkg/policymanager/manager.go b/gwctl/pkg/policymanager/manager.go index 024b7e1601..60bef82899 100644 --- a/gwctl/pkg/policymanager/manager.go +++ b/gwctl/pkg/policymanager/manager.go @@ -235,12 +235,15 @@ func PolicyFromUnstructured(u unstructured.Unstructured, policyCRDs map[PolicyCr metav1.ObjectMeta `json:"metadata,omitempty"` Spec struct { TargetRefs []gatewayv1alpha2.NamespacedPolicyTargetReference `json:"targetRefs"` + TargetRef *gatewayv1alpha2.NamespacedPolicyTargetReference `json:"targetRef"` } } structuredPolicy := &genericPolicy{} if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), structuredPolicy); err != nil { return Policy{}, fmt.Errorf("failed to convert unstructured policy resource to structured: %v", err) } + + // Process targetRefs for _, targetRef := range structuredPolicy.Spec.TargetRefs { objRef := common.ObjRef{ Group: string(targetRef.Group), @@ -257,6 +260,24 @@ func PolicyFromUnstructured(u unstructured.Unstructured, policyCRDs map[PolicyCr result.targetRefs = append(result.targetRefs, objRef) } + // Process single targetRef (if present and targetRefs is empty) + if structuredPolicy.Spec.TargetRef != nil && len(result.targetRefs) == 0 { + targetRef := structuredPolicy.Spec.TargetRef + objRef := common.ObjRef{ + Group: string(targetRef.Group), + Kind: string(targetRef.Kind), + Name: string(targetRef.Name), + Namespace: u.GetNamespace(), + } + if objRef.Namespace == "" { + objRef.Namespace = u.GetNamespace() + } + if targetRef.Namespace != nil { + objRef.Namespace = string(*targetRef.Namespace) + } + result.targetRefs = append(result.targetRefs, objRef) + } + // Get the CRD corresponding to this policy object. policyCRD, ok := policyCRDs[result.PolicyCrdID()] if !ok { @@ -327,11 +348,20 @@ func (p Policy) Spec() map[string]interface{} { if err != nil || !ok { return nil } - result, ok := spec.(map[string]interface{}) if !ok { return nil } + + if targetRef, hasTargetRef := result["targetRef"]; hasTargetRef { + if _, hasTargetRefs := result["targetRefs"]; !hasTargetRefs { + result["targetRefs"] = []interface{}{targetRef} + } else { + result["targetRefs"] = append(result["targetRefs"].([]interface{}), targetRef) + } + delete(result, "targetRef") + } + return result } @@ -339,6 +369,7 @@ func (p Policy) EffectiveSpec() (map[string]interface{}, error) { if !p.IsInherited() { // No merging is required in case of Direct policies. result := p.Spec() + delete(result, "targetRefs") delete(result, "targetRef") return result, nil } diff --git a/gwctl/pkg/printer/gateways_test.go b/gwctl/pkg/printer/gateways_test.go index fb00f0de32..523adc2d59 100644 --- a/gwctl/pkg/printer/gateways_test.go +++ b/gwctl/pkg/printer/gateways_test.go @@ -534,9 +534,6 @@ EffectivePolicies: TimeoutPolicy.bar.com: condition: path=/abc seconds: 30 - targetRefs: - - kind: Namespace - name: default Events: Type Reason Age From Message ---- ------ --- ---- ------- diff --git a/gwctl/pkg/printer/httproutes_test.go b/gwctl/pkg/printer/httproutes_test.go index 57c7b06fe2..90713a5dbf 100644 --- a/gwctl/pkg/printer/httproutes_test.go +++ b/gwctl/pkg/printer/httproutes_test.go @@ -528,10 +528,6 @@ EffectivePolicies: TimeoutPolicy.bar.com: condition: path=/def seconds: 60 - targetRefs: - - group: gateway.networking.k8s.io - kind: HTTPRoute - name: foo-httproute Events: ` diff --git a/gwctl/pkg/printer/policies_test.go b/gwctl/pkg/printer/policies_test.go index 4e4f656336..cbdbde06b9 100644 --- a/gwctl/pkg/printer/policies_test.go +++ b/gwctl/pkg/printer/policies_test.go @@ -117,13 +117,11 @@ func TestPoliciesPrinter_Print_And_PrintDescribeView(t *testing.T) { "key2": "value-child-2", "key5": "value-child-5", }, - "targetRefs": []interface{}{ - map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "Gateway", - "name": "foo-gateway", - "namespace": "default", - }, + "targetRef": map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "foo-gateway", + "namespace": "default", }, }, }, @@ -157,11 +155,9 @@ func TestPoliciesPrinter_Print_And_PrintDescribeView(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/abc", "seconds": int64(30), - "targetRefs": []interface{}{ - map[string]interface{}{ - "kind": "Namespace", - "name": "default", - }, + "targetRef": map[string]interface{}{ + "kind": "Namespace", + "name": "default", }, }, }, @@ -177,12 +173,10 @@ func TestPoliciesPrinter_Print_And_PrintDescribeView(t *testing.T) { "spec": map[string]interface{}{ "condition": "path=/def", "seconds": int64(60), - "targetRefs": []interface{}{ - map[string]interface{}{ - "group": "gateway.networking.k8s.io", - "kind": "HTTPRoute", - "name": "foo-httproute", - }, + "targetRef": map[string]interface{}{ + "group": "gateway.networking.k8s.io", + "kind": "HTTPRoute", + "name": "foo-httproute", }, }, },