Support JSON and YAML output format in get commands (-o json)

[yashvardhan-kukreja]

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #2931 #2932

Does this PR introduce a user-facing change?:

It introduces the capability for the end-user to get the JSON/YAML output analogous to kubectl in the following way.

> ./bin/gwctl get gatewayclass -o yaml

apiVersion: gateway.networking.k8s.io/v1
kind: GatewayClass
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"gateway.networking.k8s.io/v1","kind":"GatewayClass","metadata":{"annotations":{},"name":"gatewayclass-observed-generation-bump"},"spec":{"controllerName":"example.net/gateway-controller","description":"old"}}
  creationTimestamp: "2024-04-03T12:23:58Z"
  generation: 1
  managedFields:
  - apiVersion: gateway.networking.k8s.io/v1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .: {}
          f:kubectl.kubernetes.io/last-applied-configuration: {}
      f:spec:
        .: {}
        f:controllerName: {}
        f:description: {}
    manager: kubectl-client-side-apply
    operation: Update
    time: "2024-04-03T12:23:58Z"
  name: gatewayclass-observed-generation-bump
  resourceVersion: "2428"
  uid: 2581bf37-7a76-4084-82b3-e26b5438f9e8
spec:
  controllerName: example.net/gateway-controller
  description: old
status:
  conditions:
  - lastTransitionTime: "1970-01-01T00:00:00Z"
    message: Waiting for controller
    reason: Waiting
    status: Unknown
    type: Accepted

❯ ./bin/gwctl get gatewayclass -o json
{
  "kind": "GatewayClass",
  "apiVersion": "gateway.networking.k8s.io/v1",
  "metadata": {
    "name": "gatewayclass-observed-generation-bump",
    "uid": "2581bf37-7a76-4084-82b3-e26b5438f9e8",
    "resourceVersion": "2428",
    "generation": 1,
    "creationTimestamp": "2024-04-03T12:23:58Z",
    "annotations": {
      "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"gateway.networking.k8s.io/v1\",\"kind\":\"GatewayClass\",\"metadata\":{\"annotations\":{},\"name\":\"gatewayclass-observed-generation-bump\"},\"spec\":{\"controllerName\":\"example.net/gateway-controller\",\"description\":\"old\"}}\n"
    },
    "managedFields": [
      {
        "manager": "kubectl-client-side-apply",
        "operation": "Update",
        "apiVersion": "gateway.networking.k8s.io/v1",
        "time": "2024-04-03T12:23:58Z",
        "fieldsType": "FieldsV1",
        "fieldsV1": {
          "f:metadata": {
            "f:annotations": {
              ".": {},
              "f:kubectl.kubernetes.io/last-applied-configuration": {}
            }
          },
          "f:spec": {
            ".": {},
            "f:controllerName": {},
            "f:description": {}
          }
        }
      }
    ]
  },
  "spec": {
    "controllerName": "example.net/gateway-controller",
    "description": "old"
  },
  "status": {
    "conditions": [
      {
        "type": "Accepted",
        "status": "Unknown",
        "lastTransitionTime": "1970-01-01T00:00:00Z",
        "reason": "Waiting",
        "message": "Waiting for controller"
      }
    ]
  }
}

[k8s-ci-robot]

Hi @yashvardhan-kukreja. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[yashvardhan-kukreja]

Pending:

• Tests
• PR Description

[yashvardhan-kukreja]

Hi @robscott @gauravkghildiyal , can you ptal at this PR?

PS: majority of the changes are just tests 😬

[gauravkghildiyal]

/assign
/cc @mlavacca
/ok-to-test

[yashvardhan-kukreja]

Hi folks, I seem to be facing this error under pull-gateway-api-test despite the tests running fine locally

# sigs.k8s.io/gateway-api/gwctl/pkg/printer [sigs.k8s.io/gateway-api/gwctl/pkg/printer.test]
pkg/printer/gatewayclasses_test.go:308:33: undefined: apisv1alpha2.GatewayClass
pkg/printer/gatewayclasses_test.go:3[97](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_gateway-api/2940/pull-gateway-api-test/1777974347768532992#1:build-log.txt%3A97):33: undefined: apisv1alpha2.GatewayClass
pkg/printer/gateways_test.go:460:32: undefined: apisv1alpha2.Gateway
pkg/printer/gateways_test.go:583:32: undefined: apisv1alpha2.Gateway
pkg/printer/httproutes_test.go:531:27: undefined: apisv1alpha2.HTTPRoute
pkg/printer/httproutes_test.go:640:27: undefined: apisv1alpha2.HTTPRoute
FAIL	sigs.k8s.io/gateway-api/gwctl/pkg/printer [build failed]

Seems like an unexpected issue with the import apisv1alpha2 "sigs.k8s.io/gateway-api/apis/applyconfiguration/apis/v1alpha2"

Can anyone of you help me with this?

[mlavacca]

Thanks for this PR, @yashsingh74! In addition to the comments I left in the review, I have one broader comment:

I see a lot of code duplication in this PR: the logic to group and sort GatewayClasses, Gateways, HTTPRoutes is almost the same (and I think it should be 100% the same, I left a couple of comments about it). Do we really need to have resource-specific printers or can we have only one printer that deals with client.Object? We could have a common logic that sorts the objects based on Namespace and Name and print it out.

In addition, the difference between json and yaml functions is very tiny; we could just add a parameter to the print function signature to tell whether the output needs to be json or yaml, instead of having output-specific functions.

[yashvardhan-kukreja]

Hi @mlavacca , I just did a major refactor introducing a some factories and interfaces, and got rid of a majority of the code duplication.
The code is much more extensible now.

Please take a look.

Thank you

[yashvardhan-kukreja]

/test pull-gateway-api-test

[yashvardhan-kukreja]

The expectation with something like gwctl get gateways -o json ALWAYS returning a list (even if the list only contains one element) is useful when such commands are further chained with things like jq or equivalent.

@gauravkghildiyal I was under the impression that we might wanna keep things compliant with how kubectl works i.e. returning a list for non-one-elements sized items, else returning the items itself.

But if you're explicitly calling this out, we can just straightaway returning a list only instead

[yashvardhan-kukreja]

@gauravkghildiyal addressed your comments

[gauravkghildiyal]

The expectation with something like gwctl get gateways -o json ALWAYS returning a list (even if the list only contains one element) is useful when such commands are further chained with things like jq or equivalent.

@gauravkghildiyal I was under the impression that we might wanna keep things compliant with how kubectl works i.e. returning a list for non-one-elements sized items, else returning the items itself.

But if you're explicitly calling this out, we can just straightaway returning a list only instead

I think kubectl as well behaves in that same manner I'm describing. Anyways, I really don't want to add more logic to this PR and keep it on hold. Let's tackle it separately.

❯ kubectl create ns my-ns
namespace/my-ns created

❯ kubectl create secret generic my-secret -n my-ns
secret/my-secret created

❯ kubectl get secret -n my-ns
NAME        TYPE     DATA   AGE
my-secret   Opaque   0      8s

❯ kubectl get secret -n my-ns -o yaml
apiVersion: v1
items:
- apiVersion: v1
  kind: Secret
  metadata:
    creationTimestamp: "2024-04-30T18:06:22Z"
    name: my-secret
    namespace: my-ns
    resourceVersion: "4005867"
    uid: 2ff8f11c-baed-45d9-8de1-2d0fb9add3d7
  type: Opaque
kind: List
metadata:
  resourceVersion: ""

[gauravkghildiyal]

(I think this should be the last, sorry for not calling this out earlier)

[gauravkghildiyal]

Can we please squash the separate tests for PrintJSON and PrintYAML into one, since they should mostly be the same with the same setup, with the only difference in the assertion/comparision of output.

[yashvardhan-kukreja]

Sure, sorry for leaving them like this, they were a consequence of the previous design of separate JSON and YAML printers.

[yashvardhan-kukreja]

(I think this should be the last, sorry for not calling this out earlier)

No worries at all, feel absolutely free to call out any concerns you'd like to mention. I would be happy to resolve them.
And sorry for making you go through so much back and forth, I should've started off with a better written PR 😅

Anyway, highly appreciate your reviews.
Thank you!

[yashvardhan-kukreja]

@gauravkghildiyal squashed the tests too

[gauravkghildiyal]

Thank you for the patience!

/lgtm

[gauravkghildiyal]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gauravkghildiyal, yashvardhan-kukreja

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• gwctl/OWNERS [gauravkghildiyal]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[gauravkghildiyal]

/lgtm

[yashvardhan-kukreja]

@gauravkghildiyal just squashed the commits and rebased them on top of HEAD. Can you lgtm again?

[gauravkghildiyal]

/lgtm