Repository: knative/client
Pull request #1428 (2021-08-14 02:12:01)

fix kn trigger list command to show correct v1/service sink output
## Description
If the sink of the trigger is a normal service, the  `kn trigger list` command will show it as ksvc, which is incorrect
<!-- Please add a short summary about what the pull request is going to bring on the table -->

## Changes

<!-- Please add list of more detailed changes. These changes should be reflected also in the commit messages -->

*
*
*

## Reference

<!-- Please add the corresponding issue number which this pull request is about to fix -->
Fixes #

<!--
Please add an entry to CHANGELOG.adoc file, too, as part of your Pull Request.

In the following cases, add a short description of PR to the unreleased section in CHANGELOG.adoc:

- 🎁 New feature
- 🐛 Bug fix
- ✨ Feature Update
- 🐣 Refactoring
- 🗑️ Remove feature or internal logic

See other entries in CHANGELOG.adoc as an example for how to add the entry, including a reference to the corresponding issue/PR

PLEASE DON'T ADD THAT LINE HERE IN THE PULL-REQUEST DESCRIPTION BUT DIRECTLY IN CHANGELOG.ADOC AND ADD CHANGELOG.ADOC AS PART OF YOUR PULL-REQUEST.
-->

<!--
To automatically lint go code in this pull request uncomment the line below. You get feedback as comments on your pull request then -->

<!--
/lint
-->


knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/1428#" title="Author self-approved">Abirdcfly</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **richieescarez** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @richieescarez` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/knative/client/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["richieescarez"]} -->
knative-prow-robot: Hi @Abirdcfly. Thanks for your PR.

I'm waiting for a [knative](https://github.com/orgs/knative/people) 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](https://github.com/orgs/knative/people) 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](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

knative-prow-robot: Welcome @Abirdcfly! It looks like this is your first PR to knative/client 🎉
Abirdcfly: /assign @richieescarez
itsmurugappan: /ok-to-test
knative-prow-robot: @Abirdcfly: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-client-unit-tests | 6c42449dfc8e6254fd848976ad276b03474c02b7 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_client/1428/pull-knative-client-unit-tests/1426545427284496384) | `/test pull-knative-client-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_client/1428). [Your PR dashboard](https://gubernator.knative.dev/pr/Abirdcfly).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: @Abirdcfly: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-client-unit-tests | 6c42449dfc8e6254fd848976ad276b03474c02b7 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_client/1428/pull-knative-client-unit-tests/1426545427284496384) | `/test pull-knative-client-unit-tests`
pull-knative-client-integration-tests-latest-release | 6c42449dfc8e6254fd848976ad276b03474c02b7 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_client/1428/pull-knative-client-integration-tests-latest-release/1426545427401936896) | `/test pull-knative-client-integration-tests-latest-release`

[Full PR test history](https://gubernator.knative.dev/pr/knative_client/1428). [Your PR dashboard](https://gubernator.knative.dev/pr/Abirdcfly).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/1428#" title="Author self-approved">Abirdcfly</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please ask for approval from **richieescarez** after the PR has been reviewed.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/knative/client/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["richieescarez"]} -->
itsmurugappan: looks like Group is not stabilized, more details here -> knative/eventing#5086. But if you include the version in the check, it would need to be changed when a new knative serving version comes.
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/client/pull/1428?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) Report
> Merging [#1428](https://codecov.io/gh/knative/client/pull/1428?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) (126e869) into [main](https://codecov.io/gh/knative/client/commit/2e714510c53369a415b98e56dc9a55ab487e81a9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) (2e71451) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/client/pull/1428/graphs/tree.svg?width=650&height=150&src=pr&token=bkTp6BzJiV&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)](https://codecov.io/gh/knative/client/pull/1428?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)

```diff
@@           Coverage Diff           @@
##             main    #1428   +/-   ##
=======================================
  Coverage   78.70%   78.70%           
=======================================
  Files         162      162           
  Lines        8368     8368           
=======================================
  Hits         6586     6586           
  Misses       1098     1098           
  Partials      684      684           
```


| [Impacted Files](https://codecov.io/gh/knative/client/pull/1428?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) | Coverage Δ | |
|---|---|---|
| [pkg/kn/commands/flags/sink.go](https://codecov.io/gh/knative/client/pull/1428/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-cGtnL2tuL2NvbW1hbmRzL2ZsYWdzL3NpbmsuZ28=) | `87.50% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/client/pull/1428?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/client/pull/1428?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative). Last update [2e71451...126e869](https://codecov.io/gh/knative/client/pull/1428?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).

itsmurugappan: /lgtm
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/1428#" title="Author self-approved">Abirdcfly</a>*, *<a href="https://github.com/knative/client/pull/1428#pullrequestreview-732708331" title="Approved">dsimansk</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative/client/blob/main/OWNERS)~~ [dsimansk]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/kn/commands/flags/sink.go b/pkg/kn/commands/flags/sink.go
index ec008c34..e0cabb6e 100644
--- a/pkg/kn/commands/flags/sink.go
+++ b/pkg/kn/commands/flags/sink.go
@@ -149,7 +149,7 @@ func parseSink(sink string) (string, string, string) {
 // SinkToString prepares a sink for list output
 func SinkToString(sink duckv1.Destination) string {
 	if sink.Ref != nil {
-		if sink.Ref.Kind == "Service" {
+		if sink.Ref.Kind == "Service" && strings.HasPrefix(sink.Ref.APIVersion, sinkMappings["ksvc"].Group) {
 			return fmt.Sprintf("ksvc:%s", sink.Ref.Name)
 		} else {
 			return fmt.Sprintf("%s:%s", strings.ToLower(sink.Ref.Kind), sink.Ref.Name)


Test patch:
diff --git a/pkg/kn/commands/flags/sink_test.go b/pkg/kn/commands/flags/sink_test.go
index 5564a681..3f63529d 100644
--- a/pkg/kn/commands/flags/sink_test.go
+++ b/pkg/kn/commands/flags/sink_test.go
@@ -194,6 +194,13 @@ func TestSinkToString(t *testing.T) {
 			Name:       "default"}}
 	expected = "broker:default"
 	assert.Equal(t, expected, SinkToString(sink))
+	sink = duckv1.Destination{
+		Ref: &duckv1.KReference{Kind: "Service",
+			APIVersion: "v1",
+			Namespace:  "my-namespace",
+			Name:       "mysvc"}}
+	expected = "service:mysvc"
+	assert.Equal(t, expected, SinkToString(sink))
 
 	uri := "http://target.example.com"
 	targetExampleCom, err := apis.ParseURL(uri)
diff --git a/pkg/kn/commands/source/duck/multisourcelist_test.go b/pkg/kn/commands/source/duck/multisourcelist_test.go
index 1e2fc507..dad6c43f 100644
--- a/pkg/kn/commands/source/duck/multisourcelist_test.go
+++ b/pkg/kn/commands/source/duck/multisourcelist_test.go
@@ -83,8 +83,9 @@ func newSourceUnstructuredObjWithSink(name, apiVersion, kind string) *unstructur
 			"spec": map[string]interface{}{
 				"sink": map[string]interface{}{
 					"ref": map[string]interface{}{
-						"kind": "Service",
-						"name": "foo",
+						"apiVersion": "serving.knative.dev/v1",
+						"kind":       "Service",
+						"name":       "foo",
 					},
 				},
 			},
diff --git a/pkg/kn/commands/source/list_test.go b/pkg/kn/commands/source/list_test.go
index ce7c2fe8..38744441 100644
--- a/pkg/kn/commands/source/list_test.go
+++ b/pkg/kn/commands/source/list_test.go
@@ -188,8 +188,9 @@ func newSourceUnstructuredObj(name, apiVersion, kind string) *unstructured.Unstr
 			"spec": map[string]interface{}{
 				"sink": map[string]interface{}{
 					"ref": map[string]interface{}{
-						"kind": "Service",
-						"name": "foo",
+						"apiVersion": "serving.knative.dev/v1",
+						"kind":       "Service",
+						"name":       "foo",
 					},
 				},
 			},