From a3d69965cf67d1a4bc2bf693f90648f83dbc7066 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Sat, 4 May 2024 15:25:22 -0700 Subject: [PATCH 1/8] Remove unnecessary flags exposed by klog and only expose the verbosity (-v) flag. Get rid of global flag variables like kubeConfigPath. --- gwctl/cmd/root.go | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/gwctl/cmd/root.go b/gwctl/cmd/root.go index d808a19831..2956f6339e 100644 --- a/gwctl/cmd/root.go +++ b/gwctl/cmd/root.go @@ -31,23 +31,37 @@ import ( cmdutils "sigs.k8s.io/gateway-api/gwctl/pkg/utils" ) -var kubeConfigPath string - func newRootCmd() *cobra.Command { rootCmd := &cobra.Command{ Use: "gwctl", Short: "gwctl is a command-line tool for exploring Gateway API resources.", Long: `gwctl provides a familiar kubectl-like interface for navigating the Kubernetes Gateway API's multi-resource model, offering visibility into resource relationships and the policies that affect them.`, } - cobra.OnInitialize(initConfig) + + var kubeConfigPath string rootCmd.PersistentFlags().StringVar(&kubeConfigPath, "kubeconfig", "", "path to kubeconfig file (default is the KUBECONFIG environment variable and if it isn't set, falls back to $HOME/.kube/config)") - // initialize logging flags in a new flag set - // otherwise it conflicts with cobra's flags + // Initialize flags for klog. + // + // These are not directly added to the rootCmd since we ony want to expose the + // verbosity (-v) flag and not the rest. To achieve that, we'll define a + // separate verbosity flag whose value we'll propagate to the klogFlags. + var verbosity int + rootCmd.PersistentFlags().IntVarP(&verbosity, "v", "v", 0, "number for the log level verbosity (defaults to 0)") klogFlags := flag.NewFlagSet("klog", flag.ExitOnError) klog.InitFlags(klogFlags) - rootCmd.PersistentFlags().AddGoFlagSet(klogFlags) + cobra.OnInitialize(func() { + if kubeConfigPath == "" { + kubeConfigPath = os.Getenv("KUBECONFIG") + if kubeConfigPath == "" { + kubeConfigPath = path.Join(os.Getenv("HOME"), ".kube/config") + } + } + if err := klogFlags.Set("v", fmt.Sprintf("%v", verbosity)); err != nil { + fmt.Fprintf(os.Stderr, "Failed to configure verbosity for logging") + } + }) rootCmd.AddCommand(NewGetCommand()) rootCmd.AddCommand(NewDescribeCommand()) @@ -64,15 +78,6 @@ func Execute() { } } -func initConfig() { - if kubeConfigPath == "" { - kubeConfigPath = os.Getenv("KUBECONFIG") - if kubeConfigPath == "" { - kubeConfigPath = path.Join(os.Getenv("HOME"), ".kube/config") - } - } -} - func getParams(path string) *cmdutils.CmdParams { k8sClients, err := common.NewK8sClients(path) if err != nil { From 47ba51e4cbf828a7ccc83fe6757073ab9fa5a01e Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Wed, 8 May 2024 19:29:19 -0700 Subject: [PATCH 2/8] Remove duplication in Get and Describe commands. --- gwctl/cmd/describe.go | 224 ---------- gwctl/cmd/get.go | 201 --------- gwctl/cmd/root.go | 37 +- gwctl/cmd/subcommand.go | 411 ++++++++++++++++++ gwctl/pkg/printer/backends_test.go | 12 +- gwctl/pkg/printer/gatewayclasses_test.go | 36 +- gwctl/pkg/printer/gateways_test.go | 48 +- gwctl/pkg/printer/httproutes_test.go | 36 +- gwctl/pkg/printer/namespace_test.go | 36 +- gwctl/pkg/printer/policies_test.go | 24 +- .../pkg/resourcediscovery/discoverer_test.go | 42 +- gwctl/pkg/utils/types.go | 61 ++- 12 files changed, 611 insertions(+), 557 deletions(-) delete mode 100644 gwctl/cmd/describe.go delete mode 100644 gwctl/cmd/get.go create mode 100644 gwctl/cmd/subcommand.go diff --git a/gwctl/cmd/describe.go b/gwctl/cmd/describe.go deleted file mode 100644 index 5d44d2c6f0..0000000000 --- a/gwctl/cmd/describe.go +++ /dev/null @@ -1,224 +0,0 @@ -/* -Copyright 2023 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package cmd - -import ( - "fmt" - "os" - - "github.com/spf13/cobra" - - "sigs.k8s.io/gateway-api/gwctl/pkg/policymanager" - "sigs.k8s.io/gateway-api/gwctl/pkg/printer" - "sigs.k8s.io/gateway-api/gwctl/pkg/resourcediscovery" - "sigs.k8s.io/gateway-api/gwctl/pkg/utils" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/utils/clock" -) - -func NewDescribeCommand() *cobra.Command { - var namespaceFlag string - var allNamespacesFlag bool - var labelSelector string - - cmd := &cobra.Command{ - Use: "describe {policies|httproutes|gateways|gatewayclasses|backends|namespace|policycrd} RESOURCE_NAME", - Short: "Show details of a specific resource or group of resources", - Args: cobra.RangeArgs(1, 2), - Run: func(cmd *cobra.Command, args []string) { - params := getParams(kubeConfigPath) - runDescribe(cmd, args, params) - }, - } - cmd.Flags().StringVarP(&namespaceFlag, "namespace", "n", "default", "") - cmd.Flags().BoolVarP(&allNamespacesFlag, "all-namespaces", "A", false, "If present, list requested resources from all namespaces.") - cmd.Flags().StringVarP(&labelSelector, "selector", "l", "", "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2). Matching objects must satisfy all of the specified label constraints.") - - return cmd -} - -func runDescribe(cmd *cobra.Command, args []string, params *utils.CmdParams) { - kind := args[0] - - ns, err := cmd.Flags().GetString("namespace") - if err != nil { - fmt.Fprintf(os.Stderr, "failed to read flag \"namespace\": %v\n", err) - os.Exit(1) - } - - allNs, err := cmd.Flags().GetBool("all-namespaces") - if err != nil { - fmt.Fprintf(os.Stderr, "failed to read flag \"all-namespaces\": %v\n", err) - os.Exit(1) - } - - labelSelector, err := cmd.Flags().GetString("selector") - if err != nil { - fmt.Fprintf(os.Stderr, "failed to read flag \"selector\": %v\n", err) - os.Exit(1) - } - - if allNs { - ns = metav1.NamespaceAll - } - - 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{}} - gwPrinter := &printer.GatewaysPrinter{Writer: params.Out, Clock: clock.RealClock{}} - gwcPrinter := &printer.GatewayClassesPrinter{Writer: params.Out, Clock: clock.RealClock{}} - backendsPrinter := &printer.BackendsPrinter{Writer: params.Out, Clock: clock.RealClock{}} - namespacesPrinter := &printer.NamespacesPrinter{Writer: params.Out, Clock: clock.RealClock{}} - - switch kind { - case "policy", "policies": - var policyList []policymanager.Policy - if len(args) == 1 { - policyList = params.PolicyManager.GetPolicies() - } else { - var found bool - policy, found := params.PolicyManager.GetPolicy(ns + "/" + args[1]) - if !found && ns == "default" { - policy, found = params.PolicyManager.GetPolicy("/" + args[1]) - } - if found { - policyList = []policymanager.Policy{policy} - } - } - policiesPrinter.PrintPoliciesDescribeView(policyList) - - case "policycrd", "policycrds": - var policyCrdList []policymanager.PolicyCRD - if len(args) == 1 { - policyCrdList = params.PolicyManager.GetCRDs() - } else { - var found bool - policyCrd, found := params.PolicyManager.GetCRD(args[1]) - if !found { - fmt.Fprintf(os.Stderr, "failed to find PolicyCrd: %v\n", err) - os.Exit(1) - } - policyCrdList = []policymanager.PolicyCRD{policyCrd} - } - policiesPrinter.PrintPolicyCRDsDescribeView(policyCrdList) - - case "httproute", "httproutes": - selector, err := labels.Parse(labelSelector) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse label selector %q: %v\n", labelSelector, err) - os.Exit(1) - } - filter := resourcediscovery.Filter{ - Namespace: ns, - Labels: selector, - } - if len(args) > 1 { - filter.Name = args[1] - } - resourceModel, err := discoverer.DiscoverResourcesForHTTPRoute(filter) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to discover HTTPRoute resources: %v\n", err) - os.Exit(1) - } - httpRoutesPrinter.PrintDescribeView(resourceModel) - - case "gateway", "gateways": - selector, err := labels.Parse(labelSelector) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse label selector %q: %v\n", labelSelector, err) - os.Exit(1) - } - filter := resourcediscovery.Filter{ - Namespace: ns, - Labels: selector, - } - if len(args) > 1 { - filter.Name = args[1] - } - resourceModel, err := discoverer.DiscoverResourcesForGateway(filter) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to discover Gateway resources: %v\n", err) - os.Exit(1) - } - gwPrinter.PrintDescribeView(resourceModel) - - case "gatewayclass", "gatewayclasses": - selector, err := labels.Parse(labelSelector) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse label selector %q: %v\n", labelSelector, err) - os.Exit(1) - } - filter := resourcediscovery.Filter{ - Labels: selector, - } - if len(args) > 1 { - filter.Name = args[1] - } - resourceModel, err := discoverer.DiscoverResourcesForGatewayClass(filter) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to discover GatewayClass resources: %v\n", err) - os.Exit(1) - } - gwcPrinter.PrintDescribeView(resourceModel) - - case "backend", "backends": - selector, err := labels.Parse(labelSelector) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse label selector %q: %v\n", labelSelector, err) - os.Exit(1) - } - filter := resourcediscovery.Filter{ - Namespace: ns, - Labels: selector, - } - if len(args) > 1 { - filter.Name = args[1] - } - resourceModel, err := discoverer.DiscoverResourcesForBackend(filter) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to discover resources related to Backend: %v\n", err) - os.Exit(1) - } - backendsPrinter.PrintDescribeView(resourceModel) - - case "namespace", "namespaces", "ns": - selector, err := labels.Parse(labelSelector) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse label selector %q: %v\n", labelSelector, err) - os.Exit(1) - } - filter := resourcediscovery.Filter{ - Labels: selector, - } - if len(args) > 1 { - filter.Name = args[1] - } - - resourceModel, err := discoverer.DiscoverResourcesForNamespace(filter) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to discover Namespace resources: %v\n", err) - os.Exit(1) - } - namespacesPrinter.PrintDescribeView(resourceModel) - - default: - fmt.Fprintf(os.Stderr, "Unrecognized RESOURCE_TYPE\n") - } -} diff --git a/gwctl/cmd/get.go b/gwctl/cmd/get.go deleted file mode 100644 index 432b57985d..0000000000 --- a/gwctl/cmd/get.go +++ /dev/null @@ -1,201 +0,0 @@ -/* -Copyright 2023 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package cmd - -import ( - "fmt" - "os" - - "github.com/spf13/cobra" - - "k8s.io/apimachinery/pkg/labels" - "k8s.io/utils/clock" - - "sigs.k8s.io/gateway-api/gwctl/pkg/printer" - "sigs.k8s.io/gateway-api/gwctl/pkg/resourcediscovery" - "sigs.k8s.io/gateway-api/gwctl/pkg/utils" -) - -func NewGetCommand() *cobra.Command { - var namespaceFlag string - var allNamespacesFlag bool - var labelSelector string - var outputFormat string - - cmd := &cobra.Command{ - Use: "get {namespaces|gateways|gatewayclasses|policies|policycrds|httproutes} RESOURCE_NAME", - Short: "Display one or many resources", - Args: cobra.RangeArgs(1, 2), - Run: func(cmd *cobra.Command, args []string) { - params := getParams(kubeConfigPath) - runGet(cmd, args, params) - }, - } - cmd.Flags().StringVarP(&namespaceFlag, "namespace", "n", "default", "") - cmd.Flags().BoolVarP(&allNamespacesFlag, "all-namespaces", "A", false, "If present, list requested resources from all namespaces.") - cmd.Flags().StringVarP(&labelSelector, "selector", "l", "", "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2). Matching objects must satisfy all of the specified label constraints.") - cmd.Flags().StringVarP(&outputFormat, "output", "o", "", `Output format. Must be one of (yaml, json)`) - - return cmd -} - -func runGet(cmd *cobra.Command, args []string, params *utils.CmdParams) { - kind := args[0] - ns, err := cmd.Flags().GetString("namespace") - if err != nil { - fmt.Fprintf(os.Stderr, "failed to read flag \"namespace\": %v\n", err) - os.Exit(1) - } - - allNs, err := cmd.Flags().GetBool("all-namespaces") - if err != nil { - fmt.Fprintf(os.Stderr, "failed to read flag \"all-namespaces\": %v\n", err) - os.Exit(1) - } - - labelSelector, err := cmd.Flags().GetString("selector") - if err != nil { - fmt.Fprintf(os.Stderr, "failed to read flag \"selector\": %v\n", err) - os.Exit(1) - } - output, err := cmd.Flags().GetString("output") - if err != nil { - fmt.Fprintf(os.Stderr, "failed to read flag \"output\": %v\n", err) - os.Exit(1) - } - outputFormat, err := utils.ValidateAndReturnOutputFormat(output) - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) - } - - if allNs { - ns = "" - } - - discoverer := resourcediscovery.NewDiscoverer(params.K8sClients, params.PolicyManager) - realClock := clock.RealClock{} - - nsPrinter := &printer.NamespacesPrinter{Writer: params.Out, Clock: realClock} - gwPrinter := &printer.GatewaysPrinter{Writer: params.Out, Clock: realClock} - gwcPrinter := &printer.GatewayClassesPrinter{Writer: params.Out, Clock: realClock} - policiesPrinter := &printer.PoliciesPrinter{Writer: params.Out, Clock: realClock} - httpRoutesPrinter := &printer.HTTPRoutesPrinter{Writer: params.Out, Clock: realClock} - backendsPrinter := &printer.BackendsPrinter{Writer: params.Out, Clock: realClock} - - var resourceModel *resourcediscovery.ResourceModel - var printerImpl printer.Printer - - switch kind { - case "namespace", "namespaces", "ns": - selector, err := labels.Parse(labelSelector) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse label selector %q: %v\n", labelSelector, err) - os.Exit(1) - } - resourceModel, err = discoverer.DiscoverResourcesForNamespace(resourcediscovery.Filter{Labels: selector}) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to discover Namespace resources: %v\n", err) - os.Exit(1) - } - printerImpl = nsPrinter - - case "gateway", "gateways": - selector, err := labels.Parse(labelSelector) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse label selector %q: %v\n", labelSelector, err) - os.Exit(1) - } - filter := resourcediscovery.Filter{Namespace: ns, Labels: selector} - if len(args) > 1 { - filter.Name = args[1] - } - resourceModel, err = discoverer.DiscoverResourcesForGateway(filter) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to discover Gateway resources: %v\n", err) - os.Exit(1) - } - printerImpl = gwPrinter - - case "gatewayclass", "gatewayclasses": - selector, err := labels.Parse(labelSelector) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse label selector %q: %v\n", labelSelector, err) - os.Exit(1) - } - filter := resourcediscovery.Filter{Namespace: ns, Labels: selector} - if len(args) > 1 { - filter.Name = args[1] - } - resourceModel, err = discoverer.DiscoverResourcesForGatewayClass(filter) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to discover GatewayClass resources: %v\n", err) - os.Exit(1) - } - printerImpl = gwcPrinter - - case "policy", "policies": - list := params.PolicyManager.GetPolicies() - policiesPrinter.PrintPolicies(list, outputFormat) - return - - case "policycrd", "policycrds": - list := params.PolicyManager.GetCRDs() - policiesPrinter.PrintCRDs(list, outputFormat) - return - - case "httproute", "httproutes": - selector, err := labels.Parse(labelSelector) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse label selector %q: %v\n", labelSelector, err) - os.Exit(1) - } - filter := resourcediscovery.Filter{Namespace: ns, Labels: selector} - if len(args) > 1 { - filter.Name = args[1] - } - resourceModel, err = discoverer.DiscoverResourcesForHTTPRoute(filter) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to discover HTTPRoute resources: %v\n", err) - os.Exit(1) - } - printerImpl = httpRoutesPrinter - - case "backend", "backends": - selector, err := labels.Parse(labelSelector) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse label selector %q: %v\n", labelSelector, err) - os.Exit(1) - } - filter := resourcediscovery.Filter{Namespace: ns, Labels: selector} - if len(args) > 1 { - filter.Name = args[1] - } - resourceModel, err = discoverer.DiscoverResourcesForBackend(filter) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to discover backend resources: %v\n", err) - os.Exit(1) - } - backendsPrinter.Print(resourceModel) - return - - default: - fmt.Fprintf(os.Stderr, "Unrecognized RESOURCE_TYPE\n") - os.Exit(1) - } - printer.Print(printerImpl, resourceModel, outputFormat) -} diff --git a/gwctl/cmd/root.go b/gwctl/cmd/root.go index 2956f6339e..e85791bbe1 100644 --- a/gwctl/cmd/root.go +++ b/gwctl/cmd/root.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "context" "flag" "fmt" "os" @@ -26,8 +25,6 @@ import ( "github.com/spf13/cobra" "k8s.io/klog/v2" - "sigs.k8s.io/gateway-api/gwctl/pkg/common" - "sigs.k8s.io/gateway-api/gwctl/pkg/policymanager" cmdutils "sigs.k8s.io/gateway-api/gwctl/pkg/utils" ) @@ -63,8 +60,10 @@ func newRootCmd() *cobra.Command { } }) - rootCmd.AddCommand(NewGetCommand()) - rootCmd.AddCommand(NewDescribeCommand()) + factory := cmdutils.NewFactory(&kubeConfigPath) + + rootCmd.AddCommand(NewSubCommand(factory, os.Stdout, commandNameGet)) + rootCmd.AddCommand(NewSubCommand(factory, os.Stdout, commandNameDescribe)) return rootCmd } @@ -78,24 +77,18 @@ func Execute() { } } -func getParams(path string) *cmdutils.CmdParams { - k8sClients, err := common.NewK8sClients(path) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to create k8s clients: %v\n", err) - os.Exit(1) - } +func addNamespaceFlag(p *string, cmd *cobra.Command) { + cmd.Flags().StringVarP(p, "namespace", "n", "default", "") +} - policyManager := policymanager.New(k8sClients.DC) - if err := policyManager.Init(context.Background()); err != nil { - fmt.Fprintf(os.Stderr, "failed to initialize policy manager: %v\n", err) - os.Exit(1) - } +func addAllNamespacesFlag(p *bool, cmd *cobra.Command) { + cmd.Flags().BoolVarP(p, "all-namespaces", "A", false, "If present, list requested resources from all namespaces.") +} - params := &cmdutils.CmdParams{ - K8sClients: k8sClients, - PolicyManager: policyManager, - Out: os.Stdout, - } +func addLabelSelectorFlag(p *string, cmd *cobra.Command) { + cmd.Flags().StringVarP(p, "selector", "l", "", "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2). Matching objects must satisfy all of the specified label constraints.") +} - return params +func addOutputFormatFlag(p *string, cmd *cobra.Command) { + cmd.Flags().StringVarP(p, "output", "o", "", `Output format. Must be one of (yaml, json)`) } diff --git a/gwctl/cmd/subcommand.go b/gwctl/cmd/subcommand.go new file mode 100644 index 0000000000..f40c2ba26b --- /dev/null +++ b/gwctl/cmd/subcommand.go @@ -0,0 +1,411 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd + +import ( + "fmt" + "io" + "os" + + "github.com/spf13/cobra" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/utils/clock" + + "sigs.k8s.io/gateway-api/gwctl/pkg/policymanager" + "sigs.k8s.io/gateway-api/gwctl/pkg/printer" + "sigs.k8s.io/gateway-api/gwctl/pkg/resourcediscovery" + cmdutils "sigs.k8s.io/gateway-api/gwctl/pkg/utils" +) + +type commandName string + +const ( + commandNameGet commandName = "get" + commandNameDescribe commandName = "describe" +) + +func NewSubCommand(f cmdutils.Factory, out io.Writer, cmdName commandName) *cobra.Command { + var shortMsg string + if cmdName == commandNameGet { + shortMsg = "Display one or many resources" + } else { + shortMsg = "Show details of a specific resource or group of resources" + } + + cmd := &cobra.Command{ + Use: string(cmdName), + Short: shortMsg, + } + cmd.AddCommand(newCmdNamespaces(f, out, cmdName)) + cmd.AddCommand(newCmdGatewayClasses(f, out, cmdName)) + cmd.AddCommand(newCmdGateways(f, out, cmdName)) + cmd.AddCommand(newCmdHTTPRoutes(f, out, cmdName)) + cmd.AddCommand(newCmdBackends(f, out, cmdName)) + cmd.AddCommand(newCmdPolicies(f, out, cmdName)) + cmd.AddCommand(newCmdPolicyCRDs(f, out, cmdName)) + return cmd +} + +func newCmdNamespaces(f cmdutils.Factory, out io.Writer, cmdName commandName) *cobra.Command { + o := &getOrDescribeOptions{out: out, cmdName: cmdName} + cmd := &cobra.Command{ + Use: "namespaces", + Aliases: []string{"namespace", "ns"}, + Short: "Display one or more Namespaces", + Args: cobra.RangeArgs(0, 1), + Run: func(_ *cobra.Command, args []string) { + o.parse(args) + runGetOrDescribeNamespaces(f, o) + }, + } + addLabelSelectorFlag(&o.labelSelectorFlag, cmd) + if cmdName == commandNameGet { + addOutputFormatFlag(&o.outputFlag, cmd) + } + return cmd +} + +func newCmdGatewayClasses(f cmdutils.Factory, out io.Writer, cmdName commandName) *cobra.Command { + o := &getOrDescribeOptions{out: out, cmdName: cmdName} + cmd := &cobra.Command{ + Use: "gatewayclasses", + Aliases: []string{"gatewayclass"}, + Short: "Display one or more GatewayClasses", + Args: cobra.RangeArgs(0, 1), + Run: func(_ *cobra.Command, args []string) { + o.parse(args) + runGetOrDescribeGatewayClasses(f, o) + }, + } + addLabelSelectorFlag(&o.labelSelectorFlag, cmd) + if cmdName == commandNameGet { + addOutputFormatFlag(&o.outputFlag, cmd) + } + return cmd +} + +func newCmdGateways(f cmdutils.Factory, out io.Writer, cmdName commandName) *cobra.Command { + o := &getOrDescribeOptions{out: out, cmdName: cmdName} + cmd := &cobra.Command{ + Use: "gateways", + Aliases: []string{"gateway", "gw", "Gateways", "Gateway"}, + Short: "Display one or more Gateways", + Args: cobra.RangeArgs(0, 1), + Run: func(_ *cobra.Command, args []string) { + o.parse(args) + runGetOrDescribeGateways(f, o) + }, + } + addNamespaceFlag(&o.namespaceFlag, cmd) + addAllNamespacesFlag(&o.allNamespacesFlag, cmd) + addLabelSelectorFlag(&o.labelSelectorFlag, cmd) + if cmdName == commandNameGet { + addOutputFormatFlag(&o.outputFlag, cmd) + } + return cmd +} + +func newCmdHTTPRoutes(f cmdutils.Factory, out io.Writer, cmdName commandName) *cobra.Command { + o := &getOrDescribeOptions{out: out, cmdName: cmdName} + cmd := &cobra.Command{ + Use: "httproutes", + Aliases: []string{"httproute"}, + Short: "Display one or more HTTPRoutes", + Args: cobra.RangeArgs(0, 1), + Run: func(_ *cobra.Command, args []string) { + o.parse(args) + runGetOrDescribeHTTPRoutes(f, o) + }, + } + addNamespaceFlag(&o.namespaceFlag, cmd) + addAllNamespacesFlag(&o.allNamespacesFlag, cmd) + addLabelSelectorFlag(&o.labelSelectorFlag, cmd) + if cmdName == commandNameGet { + addOutputFormatFlag(&o.outputFlag, cmd) + } + return cmd +} + +func newCmdBackends(f cmdutils.Factory, out io.Writer, cmdName commandName) *cobra.Command { + o := &getOrDescribeOptions{out: out, cmdName: cmdName} + cmd := &cobra.Command{ + Use: "backends", + Aliases: []string{"backend"}, + Short: "Display one or more Backends", + Args: cobra.RangeArgs(0, 1), + Run: func(_ *cobra.Command, args []string) { + o.parse(args) + runGetOrDescribeBackends(f, o) + }, + } + addNamespaceFlag(&o.namespaceFlag, cmd) + addAllNamespacesFlag(&o.allNamespacesFlag, cmd) + addLabelSelectorFlag(&o.labelSelectorFlag, cmd) + if cmdName == commandNameGet { + addOutputFormatFlag(&o.outputFlag, cmd) + } + return cmd +} + +func newCmdPolicies(f cmdutils.Factory, out io.Writer, cmdName commandName) *cobra.Command { + o := &getOrDescribeOptions{out: out, cmdName: cmdName} + cmd := &cobra.Command{ + Use: "policies", + Aliases: []string{"policy"}, + Short: "Display one or more Policies", + Args: cobra.RangeArgs(0, 1), + Run: func(_ *cobra.Command, args []string) { + o.parse(args) + runGetOrDescribePolicies(f, o) + }, + } + if cmdName == commandNameGet { + addOutputFormatFlag(&o.outputFlag, cmd) + } + return cmd +} + +func newCmdPolicyCRDs(f cmdutils.Factory, out io.Writer, cmdName commandName) *cobra.Command { + o := &getOrDescribeOptions{out: out, cmdName: cmdName} + cmd := &cobra.Command{ + Use: "policycrds", + Aliases: []string{"policycrd"}, + Short: "Display one or more Policy CRDs", + Args: cobra.RangeArgs(0, 1), + Run: func(_ *cobra.Command, args []string) { + o.parse(args) + runGetOrDescribePolicyCRDs(f, o) + }, + } + if cmdName == commandNameGet { + addOutputFormatFlag(&o.outputFlag, cmd) + } + return cmd +} + +func runGetOrDescribeNamespaces(f cmdutils.Factory, o *getOrDescribeOptions) { + k8sClients, err := f.K8sClients() + handleErrOrExitWithMsg(err, "") + policyManager, err := f.PolicyManager() + handleErrOrExitWithMsg(err, "") + + discoverer := resourcediscovery.NewDiscoverer(k8sClients, policyManager) + resourceModel, err := discoverer.DiscoverResourcesForNamespace(o.toResourceDiscoveryFilter()) + handleErrOrExitWithMsg(err, "failed to discover Namespace resources") + + realClock := clock.RealClock{} + nsPrinter := &printer.NamespacesPrinter{Writer: o.out, Clock: realClock} + if o.cmdName == commandNameGet { + printer.Print(nsPrinter, resourceModel, o.outputFormat) + } else { + nsPrinter.PrintDescribeView(resourceModel) + } +} + +func runGetOrDescribeGatewayClasses(f cmdutils.Factory, o *getOrDescribeOptions) { + k8sClients, err := f.K8sClients() + handleErrOrExitWithMsg(err, "") + policyManager, err := f.PolicyManager() + handleErrOrExitWithMsg(err, "") + + discoverer := resourcediscovery.NewDiscoverer(k8sClients, policyManager) + resourceModel, err := discoverer.DiscoverResourcesForGatewayClass(o.toResourceDiscoveryFilter()) + handleErrOrExitWithMsg(err, "failed to discover GatewayClass resources") + + realClock := clock.RealClock{} + gwcPrinter := &printer.GatewayClassesPrinter{Writer: o.out, Clock: realClock} + if o.cmdName == commandNameGet { + printer.Print(gwcPrinter, resourceModel, o.outputFormat) + } else { + gwcPrinter.PrintDescribeView(resourceModel) + } +} + +func runGetOrDescribeGateways(f cmdutils.Factory, o *getOrDescribeOptions) { + k8sClients, err := f.K8sClients() + handleErrOrExitWithMsg(err, "") + policyManager, err := f.PolicyManager() + handleErrOrExitWithMsg(err, "") + + discoverer := resourcediscovery.NewDiscoverer(k8sClients, policyManager) + resourceModel, err := discoverer.DiscoverResourcesForGateway(o.toResourceDiscoveryFilter()) + handleErrOrExitWithMsg(err, "failed to discover Gateway resources") + + realClock := clock.RealClock{} + gwPrinter := &printer.GatewaysPrinter{Writer: o.out, Clock: realClock} + if o.cmdName == commandNameGet { + printer.Print(gwPrinter, resourceModel, o.outputFormat) + } else { + gwPrinter.PrintDescribeView(resourceModel) + } +} + +func runGetOrDescribeHTTPRoutes(f cmdutils.Factory, o *getOrDescribeOptions) { + k8sClients, err := f.K8sClients() + handleErrOrExitWithMsg(err, "") + policyManager, err := f.PolicyManager() + handleErrOrExitWithMsg(err, "") + + discoverer := resourcediscovery.NewDiscoverer(k8sClients, policyManager) + resourceModel, err := discoverer.DiscoverResourcesForHTTPRoute(o.toResourceDiscoveryFilter()) + handleErrOrExitWithMsg(err, "failed to discover HTTPRoute resources") + + realClock := clock.RealClock{} + httpRoutesPrinter := &printer.HTTPRoutesPrinter{Writer: o.out, Clock: realClock} + if o.cmdName == commandNameGet { + printer.Print(httpRoutesPrinter, resourceModel, o.outputFormat) + } else { + httpRoutesPrinter.PrintDescribeView(resourceModel) + } +} + +func runGetOrDescribeBackends(f cmdutils.Factory, o *getOrDescribeOptions) { + k8sClients, err := f.K8sClients() + handleErrOrExitWithMsg(err, "") + policyManager, err := f.PolicyManager() + handleErrOrExitWithMsg(err, "") + + discoverer := resourcediscovery.NewDiscoverer(k8sClients, policyManager) + resourceModel, err := discoverer.DiscoverResourcesForBackend(o.toResourceDiscoveryFilter()) + handleErrOrExitWithMsg(err, "failed to discover Backend resources") + + realClock := clock.RealClock{} + backendsPrinter := &printer.BackendsPrinter{Writer: o.out, Clock: realClock} + if o.cmdName == commandNameGet { + backendsPrinter.Print(resourceModel) + } else { + backendsPrinter.PrintDescribeView(resourceModel) + } +} + +func runGetOrDescribePolicies(f cmdutils.Factory, o *getOrDescribeOptions) { + policyManager, err := f.PolicyManager() + handleErrOrExitWithMsg(err, "") + + realClock := clock.RealClock{} + policiesPrinter := &printer.PoliciesPrinter{Writer: o.out, Clock: realClock} + + var policyList []policymanager.Policy + switch { + case o.resourceName == "": // Fetch all policies. + policyList = policyManager.GetPolicies() + default: // Fetch a single policy by its name. + var found bool + policy, found := policyManager.GetPolicy(o.namespace + "/" + o.resourceName) + if !found && o.resourceName == "default" { + policy, found = policyManager.GetPolicy("/" + o.resourceName) + } + if found { + policyList = []policymanager.Policy{policy} + } + } + + if o.cmdName == commandNameGet { + policiesPrinter.PrintPolicies(policyList, o.outputFormat) + } else { + policiesPrinter.PrintPoliciesDescribeView(policyList) + } +} + +func runGetOrDescribePolicyCRDs(f cmdutils.Factory, o *getOrDescribeOptions) { + policyManager, err := f.PolicyManager() + handleErrOrExitWithMsg(err, "") + + realClock := clock.RealClock{} + policiesPrinter := &printer.PoliciesPrinter{Writer: o.out, Clock: realClock} + + var policyCrdList []policymanager.PolicyCRD + if o.resourceName == "" { + policyCrdList = policyManager.GetCRDs() + } else { + var found bool + policyCrd, found := policyManager.GetCRD(o.resourceName) + if !found { + fmt.Fprintf(os.Stderr, "failed to find PolicyCrd: %v\n", err) + os.Exit(1) + } + policyCrdList = []policymanager.PolicyCRD{policyCrd} + } + + if o.cmdName == commandNameGet { + policiesPrinter.PrintCRDs(policyCrdList, o.outputFormat) + } else { + policiesPrinter.PrintPolicyCRDsDescribeView(policyCrdList) + } +} + +type getOrDescribeOptions struct { + cmdName commandName + + namespaceFlag string + allNamespacesFlag bool + labelSelectorFlag string + outputFlag string + + namespace string + resourceName string + labelSelector labels.Selector + outputFormat cmdutils.OutputFormat + + out io.Writer +} + +func (o *getOrDescribeOptions) parse(args []string) { + o.namespace = o.namespaceFlag + if o.allNamespacesFlag { + o.namespace = metav1.NamespaceAll + } + + if len(args) >= 1 { + o.resourceName = args[0] + } + + var err error + o.labelSelector, err = labels.Parse(o.labelSelectorFlag) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to parse label selector %q: %v\n", o.labelSelectorFlag, err) + os.Exit(1) + } + + o.outputFormat, err = cmdutils.ValidateAndReturnOutputFormat(o.outputFlag) + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + os.Exit(1) + } +} + +func (o *getOrDescribeOptions) toResourceDiscoveryFilter() resourcediscovery.Filter { + return resourcediscovery.Filter{ + Name: o.resourceName, + Namespace: o.namespace, + Labels: o.labelSelector, + } +} + +func handleErrOrExitWithMsg(err error, msg string) { + if err == nil { + return + } + var str string + if msg != "" { + str = msg + ": " + } + str += err.Error() + fmt.Fprintf(os.Stderr, "Error: %s\n", str) + os.Exit(1) +} diff --git a/gwctl/pkg/printer/backends_test.go b/gwctl/pkg/printer/backends_test.go index 8d6453d010..de74327ab3 100644 --- a/gwctl/pkg/printer/backends_test.go +++ b/gwctl/pkg/printer/backends_test.go @@ -179,10 +179,12 @@ func TestBackendsPrinter_Print(t *testing.T) { finalObjects = append(finalObjects, objects...) finalObjects = append(finalObjects, backendPolicies...) - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, finalObjects...)) + k8sClients := common.MustClientsForTest(t, finalObjects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForBackend(resourcediscovery.Filter{}) if err != nil { @@ -190,13 +192,13 @@ func TestBackendsPrinter_Print(t *testing.T) { } bp := &BackendsPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } bp.Print(resourceModel) - got := params.Out.(*bytes.Buffer).String() + got := buff.String() want := ` NAMESPACE NAME TYPE REFERRED BY ROUTES AGE POLICIES ns1 foo-svc-1 Service ns1/foo-httproute-1 3d 1 diff --git a/gwctl/pkg/printer/gatewayclasses_test.go b/gwctl/pkg/printer/gatewayclasses_test.go index 9d219d1085..ea74a169e1 100644 --- a/gwctl/pkg/printer/gatewayclasses_test.go +++ b/gwctl/pkg/printer/gatewayclasses_test.go @@ -101,10 +101,12 @@ func TestGatewayClassesPrinter_PrintTable(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForGatewayClass(resourcediscovery.Filter{}) if err != nil { @@ -112,12 +114,12 @@ func TestGatewayClassesPrinter_PrintTable(t *testing.T) { } gcp := &GatewayClassesPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } Print(gcp, resourceModel, utils.OutputFormatTable) - got := params.Out.(*bytes.Buffer).String() + got := buff.String() want := ` NAME CONTROLLER ACCEPTED AGE bar-com-internal-gateway-class bar.baz/internal-gateway-class True 365d @@ -268,10 +270,12 @@ Events: for _, tc := range testcases { tc := tc t.Run(tc.name, func(t *testing.T) { - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, tc.objects...)) + k8sClients := common.MustClientsForTest(t, tc.objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForGatewayClass(resourcediscovery.Filter{}) if err != nil { @@ -279,12 +283,12 @@ Events: } gcp := &GatewayClassesPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } gcp.PrintDescribeView(resourceModel) - got := params.Out.(*bytes.Buffer).String() + got := buff.String() if diff := cmp.Diff(common.YamlString(tc.want), common.YamlString(got), common.YamlStringTransformer); diff != "" { t.Errorf("Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got, tc.want, diff) } @@ -327,10 +331,12 @@ func TestGatewayClassesPrinter_PrintJsonYaml(t *testing.T) { gtwObject.APIVersion = *gtwApplyConfig.APIVersion gtwObject.Kind = *gtwApplyConfig.Kind - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, gtwObject)) + k8sClients := common.MustClientsForTest(t, gtwObject) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForGatewayClass(resourcediscovery.Filter{}) if err != nil { @@ -338,12 +344,12 @@ func TestGatewayClassesPrinter_PrintJsonYaml(t *testing.T) { } gcp := &GatewayClassesPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } Print(gcp, resourceModel, utils.OutputFormatJSON) - gotJSON := common.JSONString(params.Out.(*bytes.Buffer).String()) + gotJSON := common.JSONString(buff.String()) wantJSON := common.JSONString(fmt.Sprintf(` { "apiVersion": "v1", diff --git a/gwctl/pkg/printer/gateways_test.go b/gwctl/pkg/printer/gateways_test.go index d019e8a52c..21f255722f 100644 --- a/gwctl/pkg/printer/gateways_test.go +++ b/gwctl/pkg/printer/gateways_test.go @@ -175,10 +175,12 @@ func TestGatewaysPrinter_PrintTable(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForGateway(resourcediscovery.Filter{}) if err != nil { @@ -186,12 +188,12 @@ func TestGatewaysPrinter_PrintTable(t *testing.T) { } gp := &GatewaysPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } gp.PrintTable(resourceModel) - got := params.Out.(*bytes.Buffer).String() + got := buff.String() want := ` NAME CLASS ADDRESSES PORTS PROGRAMMED AGE abc-gateway-12345 internal-class 192.168.100.5 443,8080 False 20d @@ -365,10 +367,12 @@ func TestGatewaysPrinter_PrintDescribeView(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForGateway(resourcediscovery.Filter{}) if err != nil { @@ -376,12 +380,12 @@ func TestGatewaysPrinter_PrintDescribeView(t *testing.T) { } gp := &GatewaysPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } gp.PrintDescribeView(resourceModel) - got := params.Out.(*bytes.Buffer).String() + got := buff.String() want := ` Name: foo-gateway Namespace: "" @@ -482,10 +486,12 @@ func TestGatewaysPrinter_PrintJsonYaml(t *testing.T) { gcObject, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForGateway(resourcediscovery.Filter{}) if err != nil { @@ -493,12 +499,12 @@ func TestGatewaysPrinter_PrintJsonYaml(t *testing.T) { } gp := &GatewaysPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } Print(gp, resourceModel, utils.OutputFormatJSON) - gotJSON := common.JSONString(params.Out.(*bytes.Buffer).String()) + gotJSON := common.JSONString(buff.String()) wantJSON := common.JSONString(fmt.Sprintf(` { "apiVersion": "v1", @@ -648,10 +654,12 @@ func TestGatewaysPrinter_PrintYaml(t *testing.T) { gcObject, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForGateway(resourcediscovery.Filter{}) if err != nil { @@ -659,12 +667,12 @@ func TestGatewaysPrinter_PrintYaml(t *testing.T) { } gp := &GatewaysPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } Print(gp, resourceModel, utils.OutputFormatYAML) - got := common.YamlString(params.Out.(*bytes.Buffer).String()) + got := common.YamlString(buff.String()) want := common.YamlString(fmt.Sprintf(` apiVersion: v1 items: diff --git a/gwctl/pkg/printer/httproutes_test.go b/gwctl/pkg/printer/httproutes_test.go index 52d17b1ffb..a2e3f6d766 100644 --- a/gwctl/pkg/printer/httproutes_test.go +++ b/gwctl/pkg/printer/httproutes_test.go @@ -205,10 +205,12 @@ func TestHTTPRoutesPrinter_PrintTable(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForHTTPRoute(resourcediscovery.Filter{}) if err != nil { @@ -216,13 +218,13 @@ func TestHTTPRoutesPrinter_PrintTable(t *testing.T) { } hp := &HTTPRoutesPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } hp.PrintTable(resourceModel) - got := params.Out.(*bytes.Buffer).String() + got := buff.String() want := ` NAMESPACE NAME HOSTNAMES PARENT REFS AGE default foo-httproute-1 example.com,example2.com + 1 more 1 24h @@ -394,10 +396,12 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForHTTPRoute(resourcediscovery.Filter{}) if err != nil { @@ -405,12 +409,12 @@ func TestHTTPRoutesPrinter_PrintDescribeView(t *testing.T) { } hp := &HTTPRoutesPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } hp.PrintDescribeView(resourceModel) - got := params.Out.(*bytes.Buffer).String() + got := buff.String() want := ` Name: foo-httproute ParentRefs: @@ -492,10 +496,12 @@ func TestHTTPRoutesPrinter_PrintJsonYaml(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForHTTPRoute(resourcediscovery.Filter{}) @@ -504,12 +510,12 @@ func TestHTTPRoutesPrinter_PrintJsonYaml(t *testing.T) { } hp := &HTTPRoutesPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } Print(hp, resourceModel, utils.OutputFormatJSON) - gotJSON := common.JSONString(params.Out.(*bytes.Buffer).String()) + gotJSON := common.JSONString(buff.String()) wantJSON := common.JSONString(fmt.Sprintf(` { "apiVersion": "v1", diff --git a/gwctl/pkg/printer/namespace_test.go b/gwctl/pkg/printer/namespace_test.go index c3357d3106..68158573a1 100644 --- a/gwctl/pkg/printer/namespace_test.go +++ b/gwctl/pkg/printer/namespace_test.go @@ -75,10 +75,12 @@ func TestNamespacePrinter_PrintTable(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForNamespace(resourcediscovery.Filter{}) if err != nil { @@ -86,12 +88,12 @@ func TestNamespacePrinter_PrintTable(t *testing.T) { } nsp := &NamespacesPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } nsp.PrintTable(resourceModel) - got := params.Out.(*bytes.Buffer).String() + got := buff.String() want := ` NAME STATUS AGE default Active 46d @@ -215,10 +217,12 @@ func TestNamespacePrinter_PrintDescribeView(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForNamespace(resourcediscovery.Filter{}) if err != nil { @@ -226,12 +230,12 @@ func TestNamespacePrinter_PrintDescribeView(t *testing.T) { } nsp := &NamespacesPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } nsp.PrintDescribeView(resourceModel) - got := params.Out.(*bytes.Buffer).String() + got := buff.String() want := ` Name: development Labels: @@ -289,10 +293,12 @@ func TestNamespacesPrinter_PrintJsonYaml(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, nsObject)) + k8sClients := common.MustClientsForTest(t, nsObject) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + buff := &bytes.Buffer{} discoverer := resourcediscovery.Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForNamespace(resourcediscovery.Filter{}) if err != nil { @@ -300,12 +306,12 @@ func TestNamespacesPrinter_PrintJsonYaml(t *testing.T) { } nsp := &NamespacesPrinter{ - Writer: params.Out, + Writer: buff, Clock: fakeClock, } Print(nsp, resourceModel, utils.OutputFormatJSON) - gotJSON := common.JSONString(params.Out.(*bytes.Buffer).String()) + gotJSON := common.JSONString(buff.String()) wantJSON := common.JSONString(fmt.Sprintf(` { "apiVersion": "v1", diff --git a/gwctl/pkg/printer/policies_test.go b/gwctl/pkg/printer/policies_test.go index dc4bc9d60d..cc1420a482 100644 --- a/gwctl/pkg/printer/policies_test.go +++ b/gwctl/pkg/printer/policies_test.go @@ -163,14 +163,15 @@ func TestPoliciesPrinter_Print_And_PrintDescribeView(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) pp := &PoliciesPrinter{ Writer: &bytes.Buffer{}, Clock: fakeClock, } - pp.PrintPolicies(params.PolicyManager.GetPolicies(), utils.OutputFormatTable) + pp.PrintPolicies(policyManager.GetPolicies(), utils.OutputFormatTable) got := pp.Writer.(*bytes.Buffer).String() want := ` NAME KIND TARGET NAME TARGET KIND POLICY TYPE AGE @@ -184,7 +185,7 @@ timeout-policy-namespace TimeoutPolicy.bar.com default Namespac } pp.Writer = &bytes.Buffer{} - pp.PrintPoliciesDescribeView(params.PolicyManager.GetPolicies()) + pp.PrintPoliciesDescribeView(policyManager.GetPolicies()) got = pp.Writer.(*bytes.Buffer).String() want = ` Name: health-check-gateway @@ -338,13 +339,14 @@ func TestPoliciesPrinter_PrintCRDs(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) pp := &PoliciesPrinter{ Writer: &bytes.Buffer{}, Clock: fakeClock, } - pp.PrintCRDs(params.PolicyManager.GetCRDs(), utils.OutputFormatTable) + pp.PrintCRDs(policyManager.GetCRDs(), utils.OutputFormatTable) got := pp.Writer.(*bytes.Buffer).String() want := ` @@ -456,12 +458,13 @@ func TestPoliciesPrinter_PrintCRDs_JsonYaml(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) pp := &PoliciesPrinter{ Writer: &bytes.Buffer{}, Clock: fakeClock, } - pp.PrintCRDs(params.PolicyManager.GetCRDs(), utils.OutputFormatJSON) + pp.PrintCRDs(policyManager.GetCRDs(), utils.OutputFormatJSON) gotJSON := common.JSONString(pp.Writer.(*bytes.Buffer).String()) wantJSON := common.JSONString(fmt.Sprintf(`{ @@ -549,7 +552,7 @@ func TestPoliciesPrinter_PrintCRDs_JsonYaml(t *testing.T) { } pp.Writer = &bytes.Buffer{} - pp.PrintCRDs(params.PolicyManager.GetCRDs(), utils.OutputFormatYAML) + pp.PrintCRDs(policyManager.GetCRDs(), utils.OutputFormatYAML) gotYaml := common.YamlString(pp.Writer.(*bytes.Buffer).String()) wantYaml := common.YamlString(fmt.Sprintf(` @@ -699,11 +702,12 @@ func TestPolicyCrd_PrintDescribeView(t *testing.T) { }, } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) pp := &PoliciesPrinter{ Writer: &bytes.Buffer{}, } - pp.PrintPolicyCRDsDescribeView(params.PolicyManager.GetCRDs()) + pp.PrintPolicyCRDsDescribeView(policyManager.GetCRDs()) got := pp.Writer.(*bytes.Buffer).String() want := ` diff --git a/gwctl/pkg/resourcediscovery/discoverer_test.go b/gwctl/pkg/resourcediscovery/discoverer_test.go index 3b8157b9f1..46e1ff5e60 100644 --- a/gwctl/pkg/resourcediscovery/discoverer_test.go +++ b/gwctl/pkg/resourcediscovery/discoverer_test.go @@ -103,10 +103,11 @@ func TestDiscoverResourcesForGateway(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, tc.objects...)) + k8sClients := common.MustClientsForTest(t, tc.objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) discoverer := Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForGateway(tc.filter) @@ -311,10 +312,11 @@ func TestDiscoverResourcesForBackend(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, tc.objects...)) + k8sClients := common.MustClientsForTest(t, tc.objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) discoverer := Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } resourceModel, err := discoverer.DiscoverResourcesForBackend(tc.filter) @@ -369,10 +371,11 @@ func TestDiscoverResourcesForGatewayClass_LabelSelector(t *testing.T) { gatewayClass("foo-com-external-gateway-class", map[string]string{"app": "foo"}), gatewayClass("foo-com-internal-gateway-class", map[string]string{"app": "foo", "env": "internal"}), } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) discoverer := Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } labelSelector := "env=internal" selector, err := labels.Parse(labelSelector) @@ -446,10 +449,11 @@ func TestDiscoverResourcesForGateway_LabelSelector(t *testing.T) { gateway("gateway-2", map[string]string{"app": "foo", "env": "internal"}), } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) discoverer := Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } labelSelector := "env=internal" selector, err := labels.Parse(labelSelector) @@ -522,10 +526,11 @@ func TestDiscoverResourcesForHTTPRoute_LabelSelector(t *testing.T) { httpRoute("httproute-2", map[string]string{"app": "foo", "env": "internal"}), } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) discoverer := Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } labelSelector := "env=internal" @@ -572,10 +577,11 @@ func TestDiscoverResourcesForNamespace_LabelSelector(t *testing.T) { namespace("namespace-2", map[string]string{"app": "foo", "env": "internal"}), } - params := utils.MustParamsForTest(t, common.MustClientsForTest(t, objects...)) + k8sClients := common.MustClientsForTest(t, objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) discoverer := Discoverer{ - K8sClients: params.K8sClients, - PolicyManager: params.PolicyManager, + K8sClients: k8sClients, + PolicyManager: policyManager, } labelSelector := "env=internal" selector, err := labels.Parse(labelSelector) diff --git a/gwctl/pkg/utils/types.go b/gwctl/pkg/utils/types.go index 481b872a09..761f7027c4 100644 --- a/gwctl/pkg/utils/types.go +++ b/gwctl/pkg/utils/types.go @@ -17,11 +17,9 @@ limitations under the License. package utils import ( - "bytes" "context" "encoding/json" "fmt" - "io" "testing" "sigs.k8s.io/yaml" @@ -30,22 +28,61 @@ import ( "sigs.k8s.io/gateway-api/gwctl/pkg/policymanager" ) -type CmdParams struct { - K8sClients *common.K8sClients - PolicyManager *policymanager.PolicyManager - Out io.Writer +type Factory interface { + K8sClients() (*common.K8sClients, error) + PolicyManager() (*policymanager.PolicyManager, error) } -func MustParamsForTest(t *testing.T, fakeClients *common.K8sClients) *CmdParams { +type factoryImpl struct { + kubeConfigPath *string + + k8sClients *common.K8sClients + policyManager *policymanager.PolicyManager +} + +func NewFactory(kubeConfigPath *string) Factory { + return &factoryImpl{kubeConfigPath: kubeConfigPath} +} + +func (f *factoryImpl) K8sClients() (*common.K8sClients, error) { + if f.k8sClients != nil { + return f.k8sClients, nil + } + + if f.kubeConfigPath == nil { + return nil, fmt.Errorf("kubeConfigPath is nil") + } + + k8sClients, err := common.NewK8sClients(*f.kubeConfigPath) + if err != nil { + return nil, fmt.Errorf("failed to create k8s clients: %v", err) + } + f.k8sClients = k8sClients + return f.k8sClients, nil +} + +func (f *factoryImpl) PolicyManager() (*policymanager.PolicyManager, error) { + if f.policyManager != nil { + return f.policyManager, nil + } + k8sClients, err := f.K8sClients() + if err != nil { + return nil, err + } + policyManager := policymanager.New(k8sClients.DC) + if err := policyManager.Init(context.Background()); err != nil { + return nil, fmt.Errorf("failed to initialize policy manager: %v", err) + } + f.policyManager = policyManager + return f.policyManager, nil +} + +func MustPolicyManagerForTest(t *testing.T, fakeClients *common.K8sClients) *policymanager.PolicyManager { policyManager := policymanager.New(fakeClients.DC) if err := policyManager.Init(context.Background()); err != nil { t.Fatalf("failed to initialize PolicyManager: %v", err) } - return &CmdParams{ - K8sClients: fakeClients, - PolicyManager: policyManager, - Out: &bytes.Buffer{}, - } + return policyManager } type OutputFormat string From a6bcd8961dc566b92ead84c78b613d02c1ebc0e2 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Wed, 8 May 2024 22:49:38 -0700 Subject: [PATCH 3/8] Add missing discovery functions discoverGatewaysForGatewayClasses() and discoverBackendsForHTTPRoutes(). --- gwctl/pkg/resourcediscovery/discoverer.go | 125 ++++++++ .../pkg/resourcediscovery/discoverer_test.go | 294 ++++++++++++++++++ 2 files changed, 419 insertions(+) diff --git a/gwctl/pkg/resourcediscovery/discoverer.go b/gwctl/pkg/resourcediscovery/discoverer.go index 314664dd45..823c608b68 100644 --- a/gwctl/pkg/resourcediscovery/discoverer.go +++ b/gwctl/pkg/resourcediscovery/discoverer.go @@ -143,6 +143,7 @@ func (d Discoverer) DiscoverResourcesForGatewayClass(filter Filter) (*ResourceMo d.discoverEventsForGatewayClasses(ctx, resourceModel) + d.discoverGatewaysForGatewayClasses(ctx, resourceModel) d.discoverPolicies(resourceModel) return resourceModel, nil @@ -162,6 +163,7 @@ func (d Discoverer) DiscoverResourcesForGateway(filter Filter) (*ResourceModel, d.discoverEventsForGateways(ctx, resourceModel) d.discoverHTTPRoutesForGateways(ctx, resourceModel) + d.discoverBackendsForHTTPRoutes(ctx, resourceModel) d.discoverGatewayClassesForGateways(ctx, resourceModel) d.discoverNamespaces(ctx, resourceModel) d.discoverPolicies(resourceModel) @@ -186,6 +188,7 @@ func (d Discoverer) DiscoverResourcesForHTTPRoute(filter Filter) (*ResourceModel d.discoverEventsForHTTPRoutes(ctx, resourceModel) + d.discoverBackendsForHTTPRoutes(ctx, resourceModel) d.discoverGatewaysForHTTPRoutes(ctx, resourceModel) d.discoverGatewayClassesForGateways(ctx, resourceModel) d.discoverNamespaces(ctx, resourceModel) @@ -243,6 +246,31 @@ func (d Discoverer) DiscoverResourcesForNamespace(filter Filter) (*ResourceModel return resourceModel, nil } +// discoverGatewaysForGatewayClasses will add Gateways that use any GatewayClass +// in the resourceModel. +func (d Discoverer) discoverGatewaysForGatewayClasses(ctx context.Context, resourceModel *ResourceModel) { + gateways, err := d.fetchGateways(ctx, Filter{ /* every gateway */ }) + if err != nil { + klog.V(1).ErrorS(err, "Failed to fetch Gateways") + return + } + + for _, gateway := range gateways { + gatewayClassName := relations.FindGatewayClassNameForGateway(gateway) + gwcID := GatewayClassID(gatewayClassName) + _, ok := resourceModel.GatewayClasses[gwcID] + if !ok { + klog.V(1).InfoS("Skipping Gateway since it does not use any relevant GatewayClass", + "gateway", gateway.GetNamespace()+"/"+gateway.GetName(), + ) + continue + } + + resourceModel.addGateways(gateway) + resourceModel.connectGatewayWithGatewayClass(GatewayID(gateway.GetNamespace(), gateway.GetName()), gwcID) + } +} + // discoverGatewayClassesForGateways will add GatewayClasses associated with // Gateways in the resourceModel. func (d Discoverer) discoverGatewayClassesForGateways(ctx context.Context, resourceModel *ResourceModel) { @@ -428,6 +456,103 @@ func (d Discoverer) discoverHTTPRoutesForBackends(ctx context.Context, resourceM } } +// discoverBackendsForHTTPRoutes will add Backends that are referenced by any +// HTTPRoute in the resourceModel. +func (d Discoverer) discoverBackendsForHTTPRoutes(ctx context.Context, resourceModel *ResourceModel) { + // This will be a three step process: + // 1. Add ALL Backends to the resourceModel that are referenced by the + // HTTPRoutes. + // 2. Discover ReferenceGrants for those Backends. + // 3. Remove Backends from the resourceModel which are in a different namespace + // from the HTTPRoute and are not exposed through a ReferenceGrant. + + // Step 1 + for _, httpRouteNode := range resourceModel.HTTPRoutes { + for _, backendRef := range relations.FindBackendRefsForHTTPRoute(*httpRouteNode.HTTPRoute) { + // Check if the Backend already exists in the resourceModel + backendID := BackendID(backendRef.Group, backendRef.Kind, backendRef.Namespace, backendRef.Name) + if _, ok := resourceModel.Backends[backendID]; ok { + // Backend already exists in the resourceModel, skip re-fetching. + continue + } + + // Backend doesn't already exist so fetch and add it to the resourceModel. + backends, err := d.fetchBackends(ctx, Filter{Namespace: backendRef.Namespace, Name: backendRef.Name}) + if err != nil { + if apierrors.IsNotFound(err) { + err := ReferenceToNonExistentResourceError{ReferenceFromTo: ReferenceFromTo{ + ReferringObject: common.ObjRef{Kind: "HTTPRoute", Name: httpRouteNode.HTTPRoute.GetName(), Namespace: httpRouteNode.HTTPRoute.GetNamespace()}, + ReferredObject: backendRef, + }} + httpRouteNode.Errors = append(httpRouteNode.Errors, err) + klog.V(1).Info(err) + } else { + klog.V(1).ErrorS(err, "Error while fetching Backend for HTTPRoute", + "backend", fmt.Sprintf("%v/%v/%v", backendRef.Kind, backendRef.Namespace, backendRef.Name), + "httproute", httpRouteNode.HTTPRoute.GetNamespace()+"/"+httpRouteNode.HTTPRoute.GetName(), + ) + } + continue + } + resourceModel.addBackends(backends[0]) + } + } + + // Step 2 + d.discoverReferenceGrantsForBackends(ctx, resourceModel) + + // Step 3 + for _, httpRouteNode := range resourceModel.HTTPRoutes { + for _, backendRef := range relations.FindBackendRefsForHTTPRoute(*httpRouteNode.HTTPRoute) { + // Check if the Backend exists in the resourceModel + backendID := BackendID(backendRef.Group, backendRef.Kind, backendRef.Namespace, backendRef.Name) + backendNode, ok := resourceModel.Backends[backendID] + if !ok { + // Backend does not exist in the resourceModel (maybe because of some + // error), so skip processing this backend. + continue + } + + // Ensure that if this is a cross namespace reference, then it is accepted + // through some ReferenceGrant. + if httpRouteNode.HTTPRoute.GetNamespace() != backendRef.Namespace { + httpRouteRef := common.ObjRef{ + Group: httpRouteNode.HTTPRoute.GroupVersionKind().Group, + Kind: httpRouteNode.HTTPRoute.GroupVersionKind().Kind, + Name: httpRouteNode.HTTPRoute.GetName(), + Namespace: httpRouteNode.HTTPRoute.GetNamespace(), + } + var referenceAccepted bool + for _, referenceGrantNode := range backendNode.ReferenceGrants { + if relations.ReferenceGrantAccepts(*referenceGrantNode.ReferenceGrant, httpRouteRef) { + referenceAccepted = true + break + } + } + if !referenceAccepted { + err := ReferenceNotPermittedError{ReferenceFromTo: ReferenceFromTo{ + ReferringObject: common.ObjRef{Kind: "HTTPRoute", Name: httpRouteNode.HTTPRoute.GetName(), Namespace: httpRouteNode.HTTPRoute.GetNamespace()}, + ReferredObject: backendRef, + }} + httpRouteNode.Errors = append(httpRouteNode.Errors, err) + klog.V(1).Info(err) + continue + } + } + + // At this point, we know that either in the same namespace as the + // HTTPRoute, or is exposed through a ReferenceGrant. + resourceModel.connectHTTPRouteWithBackend(HTTPRouteID(httpRouteNode.HTTPRoute.GetNamespace(), httpRouteNode.HTTPRoute.GetName()), backendID) + } + } + // Remove Backends which are not connected to any HTTPRoute + for backendID, backendNode := range resourceModel.Backends { + if len(backendNode.HTTPRoutes) == 0 { + delete(resourceModel.Backends, backendID) + } + } +} + // discoverNamespaces adds Namespaces for resources that exist in the // resourceModel. func (d Discoverer) discoverNamespaces(ctx context.Context, resourceModel *ResourceModel) { diff --git a/gwctl/pkg/resourcediscovery/discoverer_test.go b/gwctl/pkg/resourcediscovery/discoverer_test.go index 46e1ff5e60..dbca5df668 100644 --- a/gwctl/pkg/resourcediscovery/discoverer_test.go +++ b/gwctl/pkg/resourcediscovery/discoverer_test.go @@ -35,6 +35,83 @@ import ( "sigs.k8s.io/gateway-api/gwctl/pkg/utils" ) +func TestDiscoverResourcesForGatewayClasses(t *testing.T) { + testcases := []struct { + name string + objects []runtime.Object + filter Filter + + wantGatewayClasses []string + wantGateways []apimachinerytypes.NamespacedName + }{ + { + name: "normal", + filter: Filter{Labels: labels.Everything()}, + objects: []runtime.Object{ + common.NamespaceForTest("default"), + common.NamespaceForTest("baz"), + &gatewayv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-gatewayclass", + }, + }, + &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar-gateway", + Namespace: "default", + }, + Spec: gatewayv1.GatewaySpec{ + GatewayClassName: "bar-gatewayclass", + }, + }, + &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz-gateway", + Namespace: "baz", + }, + Spec: gatewayv1.GatewaySpec{ + GatewayClassName: "foo-gatewayclass", + }, + }, + }, + wantGatewayClasses: []string{"foo-gatewayclass"}, + wantGateways: []apimachinerytypes.NamespacedName{ + {Namespace: "baz", Name: "baz-gateway"}, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + k8sClients := common.MustClientsForTest(t, tc.objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + discoverer := Discoverer{ + K8sClients: k8sClients, + PolicyManager: policyManager, + } + + resourceModel, err := discoverer.DiscoverResourcesForGatewayClass(tc.filter) + if err != nil { + t.Fatalf("Failed to construct resourceModel: %v", resourceModel) + } + + gotGatewayClasses := gatewayClassNamesFromResourceModel(resourceModel) + gotGateways := namespacedGatewaysFromResourceModel(resourceModel) + + if tc.wantGatewayClasses != nil { + if diff := cmp.Diff(tc.wantGatewayClasses, gotGatewayClasses, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("Unexpected diff in GatewayClasses; got=%v, want=%v;\ndiff (-want +got)=\n%v", gotGatewayClasses, tc.wantGatewayClasses, diff) + } + } + if tc.wantGateways != nil { + if diff := cmp.Diff(tc.wantGateways, gotGateways, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("Unexpected diff in Gateways; got=%v, want=%v;\ndiff (-want +got)=\n%v", gotGateways, tc.wantGateways, diff) + } + } + }) + } +} + func TestDiscoverResourcesForGateway(t *testing.T) { testcases := []struct { name string @@ -132,6 +209,215 @@ func TestDiscoverResourcesForGateway(t *testing.T) { } } +func TestDiscoverResourcesForHTTPRoute(t *testing.T) { + testcases := []struct { + name string + objects []runtime.Object + filter Filter + + wantHTTPRoutes []apimachinerytypes.NamespacedName + wantBackends []apimachinerytypes.NamespacedName + }{ + { + name: "normal", + filter: Filter{Labels: labels.Everything()}, + objects: []runtime.Object{ + common.NamespaceForTest("default"), + &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-httproute", + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + CommonRouteSpec: gatewayv1.CommonRouteSpec{}, + Rules: []gatewayv1.HTTPRouteRule{ + { + BackendRefs: []gatewayv1.HTTPBackendRef{ + { + BackendRef: gatewayv1.BackendRef{ + BackendObjectReference: gatewayv1.BackendObjectReference{ + Kind: common.PtrTo(gatewayv1.Kind("Service")), + Name: "foo-svc", + Port: common.PtrTo(gatewayv1.PortNumber(80)), + }, + }, + }, + }, + }, + }, + }, + }, + &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-svc", + Namespace: "default", + }, + }, + }, + wantHTTPRoutes: []apimachinerytypes.NamespacedName{ + {Namespace: "default", Name: "foo-httproute"}, + }, + wantBackends: []apimachinerytypes.NamespacedName{ + {Namespace: "default", Name: "foo-svc"}, + }, + }, + { + name: "backendref from different namespace should require referencegrant", + filter: Filter{Labels: labels.Everything()}, + objects: []runtime.Object{ + common.NamespaceForTest("default"), + common.NamespaceForTest("bar"), + &gatewayv1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{ + APIVersion: gatewayv1.GroupVersion.String(), + Kind: "HTTPRoute", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-httproute", + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + CommonRouteSpec: gatewayv1.CommonRouteSpec{}, + Rules: []gatewayv1.HTTPRouteRule{ + { + BackendRefs: []gatewayv1.HTTPBackendRef{ + { + BackendRef: gatewayv1.BackendRef{ + BackendObjectReference: gatewayv1.BackendObjectReference{ + Kind: common.PtrTo(gatewayv1.Kind("Service")), + Name: "bar-svc", + Namespace: common.PtrTo(gatewayv1.Namespace("bar")), // Different namespace than HTTPRoute. + Port: common.PtrTo(gatewayv1.PortNumber(80)), + }, + }, + }, + }, + }, + }, + }, + }, + &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "bar-svc", + Namespace: "bar", + }, + }, + }, + wantHTTPRoutes: []apimachinerytypes.NamespacedName{ + {Namespace: "default", Name: "foo-httproute"}, + }, + wantBackends: []apimachinerytypes.NamespacedName{}, + }, + { + name: "backendref from different namespace should get allowed with referencegrant", + filter: Filter{Labels: labels.Everything()}, + objects: []runtime.Object{ + common.NamespaceForTest("default"), + common.NamespaceForTest("bar"), + &gatewayv1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{ + APIVersion: gatewayv1.GroupVersion.String(), + Kind: "HTTPRoute", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-httproute", + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + CommonRouteSpec: gatewayv1.CommonRouteSpec{}, + Rules: []gatewayv1.HTTPRouteRule{ + { + BackendRefs: []gatewayv1.HTTPBackendRef{ + { + BackendRef: gatewayv1.BackendRef{ + BackendObjectReference: gatewayv1.BackendObjectReference{ + Kind: common.PtrTo(gatewayv1.Kind("Service")), + Name: "bar-svc", + Namespace: common.PtrTo(gatewayv1.Namespace("bar")), // Different namespace than HTTPRoute. + Port: common.PtrTo(gatewayv1.PortNumber(80)), + }, + }, + }, + }, + }, + }, + }, + }, + &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "bar-svc", + Namespace: "bar", + }, + }, + &gatewayv1beta1.ReferenceGrant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar-reference-grant", + Namespace: "bar", + }, + Spec: gatewayv1beta1.ReferenceGrantSpec{ + From: []gatewayv1beta1.ReferenceGrantFrom{{ + Group: gatewayv1.Group(gatewayv1.GroupVersion.Group), + Kind: "HTTPRoute", + Namespace: "default", + }}, + To: []gatewayv1beta1.ReferenceGrantTo{{ + Kind: "Service", + }}, + }, + }, + }, + wantHTTPRoutes: []apimachinerytypes.NamespacedName{ + {Namespace: "default", Name: "foo-httproute"}, + }, + wantBackends: []apimachinerytypes.NamespacedName{ + {Namespace: "bar", Name: "bar-svc"}, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + k8sClients := common.MustClientsForTest(t, tc.objects...) + policyManager := utils.MustPolicyManagerForTest(t, k8sClients) + discoverer := Discoverer{ + K8sClients: k8sClients, + PolicyManager: policyManager, + } + + resourceModel, err := discoverer.DiscoverResourcesForHTTPRoute(tc.filter) + if err != nil { + t.Fatalf("Failed to construct resourceModel: %v", resourceModel) + } + + gotHTTPRoutes := namespacedHTTPRoutesFromResourceModel(resourceModel) + gotBackends := namespacedBackendsFromResourceModel(resourceModel) + + if tc.wantHTTPRoutes != nil { + if diff := cmp.Diff(tc.wantHTTPRoutes, gotHTTPRoutes, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("Unexpected diff in HTTPRoutes; got=%v, want=%v;\ndiff (-want +got)=\n%v", gotHTTPRoutes, tc.wantHTTPRoutes, diff) + } + } + if tc.wantBackends != nil { + if diff := cmp.Diff(tc.wantBackends, gotBackends, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("Unexpected diff in Backends; got=%v, want=%v;\ndiff (-want +got)=\n%v", gotBackends, tc.wantBackends, diff) + } + } + }) + } +} + func TestDiscoverResourcesForBackend(t *testing.T) { testcases := []struct { name string @@ -604,6 +890,14 @@ func TestDiscoverResourcesForNamespace_LabelSelector(t *testing.T) { } } +func gatewayClassNamesFromResourceModel(r *ResourceModel) []string { + var gatewayClassNames []string + for _, gatewayClassNode := range r.GatewayClasses { + gatewayClassNames = append(gatewayClassNames, gatewayClassNode.GatewayClass.GetName()) + } + return gatewayClassNames +} + func namespacedGatewaysFromResourceModel(r *ResourceModel) []apimachinerytypes.NamespacedName { var gateways []apimachinerytypes.NamespacedName for _, gatewayNode := range r.Gateways { From 5b8ea2ce8e19f555b3e1e17067f33736cf0891ff Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Thu, 9 May 2024 00:48:49 -0700 Subject: [PATCH 4/8] Add --for flag for filtering resources based on an associated resource. For example: * gwctl get backends --for gateway/ns2/my-gateway * gwctl get gateways --for httproute/ns1/my-httproute * gwctl get gateways --for gatewayclass/foo-gatewayclass --- gwctl/cmd/root.go | 4 ++ gwctl/cmd/subcommand.go | 113 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/gwctl/cmd/root.go b/gwctl/cmd/root.go index e85791bbe1..a3ec6d865c 100644 --- a/gwctl/cmd/root.go +++ b/gwctl/cmd/root.go @@ -92,3 +92,7 @@ func addLabelSelectorFlag(p *string, cmd *cobra.Command) { func addOutputFormatFlag(p *string, cmd *cobra.Command) { cmd.Flags().StringVarP(p, "output", "o", "", `Output format. Must be one of (yaml, json)`) } + +func addForFlag(p *string, cmd *cobra.Command) { + cmd.Flags().StringVar(p, "for", "", `Filter results to only those related to the specified resource. Format: TYPE[/NAMESPACE]/NAME. Not specifying a NAMESPACE assumes the 'default' value. Examples: gateway/ns2/foo-gateway, httproute/bar-httproute, service/ns1/my-svc`) +} diff --git a/gwctl/cmd/subcommand.go b/gwctl/cmd/subcommand.go index f40c2ba26b..0c5614cafd 100644 --- a/gwctl/cmd/subcommand.go +++ b/gwctl/cmd/subcommand.go @@ -20,12 +20,15 @@ import ( "fmt" "io" "os" + "strings" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/utils/clock" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/gwctl/pkg/common" "sigs.k8s.io/gateway-api/gwctl/pkg/policymanager" "sigs.k8s.io/gateway-api/gwctl/pkg/printer" "sigs.k8s.io/gateway-api/gwctl/pkg/resourcediscovery" @@ -94,6 +97,7 @@ func newCmdGatewayClasses(f cmdutils.Factory, out io.Writer, cmdName commandName } addLabelSelectorFlag(&o.labelSelectorFlag, cmd) if cmdName == commandNameGet { + addForFlag(&o.forFlag, cmd) addOutputFormatFlag(&o.outputFlag, cmd) } return cmd @@ -115,6 +119,7 @@ func newCmdGateways(f cmdutils.Factory, out io.Writer, cmdName commandName) *cob addAllNamespacesFlag(&o.allNamespacesFlag, cmd) addLabelSelectorFlag(&o.labelSelectorFlag, cmd) if cmdName == commandNameGet { + addForFlag(&o.forFlag, cmd) addOutputFormatFlag(&o.outputFlag, cmd) } return cmd @@ -136,6 +141,7 @@ func newCmdHTTPRoutes(f cmdutils.Factory, out io.Writer, cmdName commandName) *c addAllNamespacesFlag(&o.allNamespacesFlag, cmd) addLabelSelectorFlag(&o.labelSelectorFlag, cmd) if cmdName == commandNameGet { + addForFlag(&o.forFlag, cmd) addOutputFormatFlag(&o.outputFlag, cmd) } return cmd @@ -157,6 +163,7 @@ func newCmdBackends(f cmdutils.Factory, out io.Writer, cmdName commandName) *cob addAllNamespacesFlag(&o.allNamespacesFlag, cmd) addLabelSelectorFlag(&o.labelSelectorFlag, cmd) if cmdName == commandNameGet { + addForFlag(&o.forFlag, cmd) addOutputFormatFlag(&o.outputFlag, cmd) } return cmd @@ -175,6 +182,7 @@ func newCmdPolicies(f cmdutils.Factory, out io.Writer, cmdName commandName) *cob }, } if cmdName == commandNameGet { + addForFlag(&o.forFlag, cmd) addOutputFormatFlag(&o.outputFlag, cmd) } return cmd @@ -224,7 +232,19 @@ func runGetOrDescribeGatewayClasses(f cmdutils.Factory, o *getOrDescribeOptions) handleErrOrExitWithMsg(err, "") discoverer := resourcediscovery.NewDiscoverer(k8sClients, policyManager) - resourceModel, err := discoverer.DiscoverResourcesForGatewayClass(o.toResourceDiscoveryFilter()) + emptyObjRef := common.ObjRef{} + var resourceModel *resourcediscovery.ResourceModel + if o.cmdName == commandNameGet && o.forObjRef != emptyObjRef { + switch o.forObjRef.Kind { + case "Gateway": + resourceModel, err = discoverer.DiscoverResourcesForGateway(o.forObjRefToResourceDiscoveryFilter()) + default: + fmt.Fprintf(os.Stderr, "Filtering by type %q is not supported for GatewayClasses", o.forObjRef.Kind) + os.Exit(1) + } + } else { + resourceModel, err = discoverer.DiscoverResourcesForGatewayClass(o.toResourceDiscoveryFilter()) + } handleErrOrExitWithMsg(err, "failed to discover GatewayClass resources") realClock := clock.RealClock{} @@ -243,7 +263,21 @@ func runGetOrDescribeGateways(f cmdutils.Factory, o *getOrDescribeOptions) { handleErrOrExitWithMsg(err, "") discoverer := resourcediscovery.NewDiscoverer(k8sClients, policyManager) - resourceModel, err := discoverer.DiscoverResourcesForGateway(o.toResourceDiscoveryFilter()) + emptyObjRef := common.ObjRef{} + var resourceModel *resourcediscovery.ResourceModel + if o.cmdName == commandNameGet && o.forObjRef != emptyObjRef { + switch o.forObjRef.Kind { + case "GatewayClass": + resourceModel, err = discoverer.DiscoverResourcesForGatewayClass(o.forObjRefToResourceDiscoveryFilter()) + case "HTTPRoute": + resourceModel, err = discoverer.DiscoverResourcesForHTTPRoute(o.forObjRefToResourceDiscoveryFilter()) + default: + fmt.Fprintf(os.Stderr, "Filtering by type %q is not supported for Gateways", o.forObjRef.Kind) + os.Exit(1) + } + } else { + resourceModel, err = discoverer.DiscoverResourcesForGateway(o.toResourceDiscoveryFilter()) + } handleErrOrExitWithMsg(err, "failed to discover Gateway resources") realClock := clock.RealClock{} @@ -262,7 +296,21 @@ func runGetOrDescribeHTTPRoutes(f cmdutils.Factory, o *getOrDescribeOptions) { handleErrOrExitWithMsg(err, "") discoverer := resourcediscovery.NewDiscoverer(k8sClients, policyManager) - resourceModel, err := discoverer.DiscoverResourcesForHTTPRoute(o.toResourceDiscoveryFilter()) + emptyObjRef := common.ObjRef{} + var resourceModel *resourcediscovery.ResourceModel + if o.cmdName == commandNameGet && o.forObjRef != emptyObjRef { + switch o.forObjRef.Kind { + case "Gateway": + resourceModel, err = discoverer.DiscoverResourcesForGateway(o.forObjRefToResourceDiscoveryFilter()) + case "Service": + resourceModel, err = discoverer.DiscoverResourcesForBackend(o.forObjRefToResourceDiscoveryFilter()) + default: + fmt.Fprintf(os.Stderr, "Filtering by type %q is not supported for HTTPRoutes", o.forObjRef.Kind) + os.Exit(1) + } + } else { + resourceModel, err = discoverer.DiscoverResourcesForHTTPRoute(o.toResourceDiscoveryFilter()) + } handleErrOrExitWithMsg(err, "failed to discover HTTPRoute resources") realClock := clock.RealClock{} @@ -281,7 +329,21 @@ func runGetOrDescribeBackends(f cmdutils.Factory, o *getOrDescribeOptions) { handleErrOrExitWithMsg(err, "") discoverer := resourcediscovery.NewDiscoverer(k8sClients, policyManager) - resourceModel, err := discoverer.DiscoverResourcesForBackend(o.toResourceDiscoveryFilter()) + emptyObjRef := common.ObjRef{} + var resourceModel *resourcediscovery.ResourceModel + if o.cmdName == commandNameGet && o.forObjRef != emptyObjRef { + switch o.forObjRef.Kind { + case "Gateway": + resourceModel, err = discoverer.DiscoverResourcesForGateway(o.forObjRefToResourceDiscoveryFilter()) + case "HTTPRoute": + resourceModel, err = discoverer.DiscoverResourcesForHTTPRoute(o.forObjRefToResourceDiscoveryFilter()) + default: + fmt.Fprintf(os.Stderr, "Filtering by type %q is not supported for Backends", o.forObjRef.Kind) + os.Exit(1) + } + } else { + resourceModel, err = discoverer.DiscoverResourcesForBackend(o.toResourceDiscoveryFilter()) + } handleErrOrExitWithMsg(err, "failed to discover Backend resources") realClock := clock.RealClock{} @@ -301,7 +363,10 @@ func runGetOrDescribePolicies(f cmdutils.Factory, o *getOrDescribeOptions) { policiesPrinter := &printer.PoliciesPrinter{Writer: o.out, Clock: realClock} var policyList []policymanager.Policy + emptyObjRef := common.ObjRef{} switch { + case o.cmdName == commandNameGet && o.forObjRef != emptyObjRef: // Fetch policies attached to some resource. + policyList = policyManager.PoliciesAttachedTo(policymanager.ObjRef(o.forObjRef)) case o.resourceName == "": // Fetch all policies. policyList = policyManager.GetPolicies() default: // Fetch a single policy by its name. @@ -356,11 +421,13 @@ type getOrDescribeOptions struct { allNamespacesFlag bool labelSelectorFlag string outputFlag string + forFlag string namespace string resourceName string labelSelector labels.Selector outputFormat cmdutils.OutputFormat + forObjRef common.ObjRef out io.Writer } @@ -387,6 +454,37 @@ func (o *getOrDescribeOptions) parse(args []string) { fmt.Fprintf(os.Stderr, "%v\n", err) os.Exit(1) } + + // Parse `--for` flag + if o.forFlag != "" { + parts := strings.Split(o.forFlag, "/") + if len(parts) < 2 || len(parts) > 3 { + fmt.Fprintf(os.Stderr, "invalid value used in --for flag; value must be in the format TYPE[/NAMESPACE]/NAME\n") + os.Exit(1) + } + if len(parts) == 2 { + o.forObjRef = common.ObjRef{Kind: parts[0], Namespace: metav1.NamespaceDefault, Name: parts[1]} + } else { + o.forObjRef = common.ObjRef{Kind: parts[0], Namespace: parts[1], Name: parts[2]} + } + switch strings.ToLower(o.forObjRef.Kind) { + case "gatewayclass", "gateawyclasses": + o.forObjRef.Group = gatewayv1.GroupVersion.Group + o.forObjRef.Kind = "GatewayClass" + o.forObjRef.Namespace = "" + case "gateway", "gateways": + o.forObjRef.Group = gatewayv1.GroupVersion.Group + o.forObjRef.Kind = "Gateway" + case "httproute", "httproutes": + o.forObjRef.Group = gatewayv1.GroupVersion.Group + o.forObjRef.Kind = "HTTPRoute" + case "service", "services": + o.forObjRef.Kind = "Service" + default: + fmt.Fprintf(os.Stderr, "invalid type provided in --for flag; type must be one of [gatewayclass, gateway, httproute, service]\n") + os.Exit(1) + } + } } func (o *getOrDescribeOptions) toResourceDiscoveryFilter() resourcediscovery.Filter { @@ -397,6 +495,13 @@ func (o *getOrDescribeOptions) toResourceDiscoveryFilter() resourcediscovery.Fil } } +func (o *getOrDescribeOptions) forObjRefToResourceDiscoveryFilter() resourcediscovery.Filter { + return resourcediscovery.Filter{ + Name: o.forObjRef.Name, + Namespace: o.forObjRef.Namespace, + } +} + func handleErrOrExitWithMsg(err error, msg string) { if err == nil { return From 1dbeb7c62d6ab33657babbc958408f5b48311ffe Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Tue, 14 May 2024 22:43:57 -0700 Subject: [PATCH 5/8] fixup! Remove duplication in Get and Describe commands. --- gwctl/cmd/root.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gwctl/cmd/root.go b/gwctl/cmd/root.go index a3ec6d865c..a6d26a72dc 100644 --- a/gwctl/cmd/root.go +++ b/gwctl/cmd/root.go @@ -25,7 +25,7 @@ import ( "github.com/spf13/cobra" "k8s.io/klog/v2" - cmdutils "sigs.k8s.io/gateway-api/gwctl/pkg/utils" + "sigs.k8s.io/gateway-api/gwctl/pkg/utils" ) func newRootCmd() *cobra.Command { @@ -60,7 +60,7 @@ func newRootCmd() *cobra.Command { } }) - factory := cmdutils.NewFactory(&kubeConfigPath) + factory := utils.NewFactory(&kubeConfigPath) rootCmd.AddCommand(NewSubCommand(factory, os.Stdout, commandNameGet)) rootCmd.AddCommand(NewSubCommand(factory, os.Stdout, commandNameDescribe)) From 5dc192d12ba320efd5b87f150c341d691edec8a1 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Tue, 14 May 2024 22:55:29 -0700 Subject: [PATCH 6/8] fixup! Add missing discovery functions discoverGatewaysForGatewayClasses() and discoverBackendsForHTTPRoutes(). --- gwctl/pkg/resourcediscovery/discoverer.go | 1 + 1 file changed, 1 insertion(+) diff --git a/gwctl/pkg/resourcediscovery/discoverer.go b/gwctl/pkg/resourcediscovery/discoverer.go index 823c608b68..d67db420b5 100644 --- a/gwctl/pkg/resourcediscovery/discoverer.go +++ b/gwctl/pkg/resourcediscovery/discoverer.go @@ -510,6 +510,7 @@ func (d Discoverer) discoverBackendsForHTTPRoutes(ctx context.Context, resourceM if !ok { // Backend does not exist in the resourceModel (maybe because of some // error), so skip processing this backend. + klog.V(3).ErrorS(nil, "Backend not found in the resourceModel", "backend", fmt.Sprintf("%v/%v/%v", backendRef.Kind, backendRef.Namespace, backendRef.Name)) continue } From 158bca4cdcb20fcae4f30499e6289a850649c7a9 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Tue, 14 May 2024 23:24:32 -0700 Subject: [PATCH 7/8] fixup! Remove duplication in Get and Describe commands. --- gwctl/pkg/utils/types.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/gwctl/pkg/utils/types.go b/gwctl/pkg/utils/types.go index 761f7027c4..74423d5ac6 100644 --- a/gwctl/pkg/utils/types.go +++ b/gwctl/pkg/utils/types.go @@ -28,7 +28,19 @@ import ( "sigs.k8s.io/gateway-api/gwctl/pkg/policymanager" ) +// Factory encapsulates the common clients and structures which are needed for +// the execution of some command. type Factory interface { + // INTERNAL COMMENT: + // - The reason for an interface here is to be able to inject this dependency + // during unit tests. + // - The reason for the "factory" pattern is to delay the construction of the + // objects for when the commands get run (as opposed to when the commands + // are registered in Cobra). This is required because during registration of + // the commands, we will need to inject this dependency, but we cannot + // construct the dependency because it depends on some flag values which are + // only known during the runtime of the command. + K8sClients() (*common.K8sClients, error) PolicyManager() (*policymanager.PolicyManager, error) } From 2092ffd112ebd9b385b395a3bdec8d71028419b6 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Thu, 16 May 2024 14:27:43 -0700 Subject: [PATCH 8/8] fixup! Add --for flag for filtering resources based on an associated resource. --- gwctl/cmd/subcommand.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gwctl/cmd/subcommand.go b/gwctl/cmd/subcommand.go index 0c5614cafd..01a7d8edcf 100644 --- a/gwctl/cmd/subcommand.go +++ b/gwctl/cmd/subcommand.go @@ -366,7 +366,7 @@ func runGetOrDescribePolicies(f cmdutils.Factory, o *getOrDescribeOptions) { emptyObjRef := common.ObjRef{} switch { case o.cmdName == commandNameGet && o.forObjRef != emptyObjRef: // Fetch policies attached to some resource. - policyList = policyManager.PoliciesAttachedTo(policymanager.ObjRef(o.forObjRef)) + policyList = policyManager.PoliciesAttachedTo(o.forObjRef) case o.resourceName == "": // Fetch all policies. policyList = policyManager.GetPolicies() default: // Fetch a single policy by its name.