Bug 1826808: added endpoint to fetch duck typed knative event source crds #4945

abhi-kn · 2020-04-07T14:43:18Z

Fixes:
https://issues.redhat.com/browse/ODC-3398

Analysis / Root cause:
Need a new end point in console that specifically returns metadata for all knative event source CRDs. This shall allow an unprivileged user to access this information without read access to CRDs.

Solution Description:
Added an end point /api/console/knative-event-sources to fetch all duck typed event source CRDs. Updated ResourceLister handler to propagate response error status as is & also added a ResponseFilter function to filter any response before propagation.
In order to allow an unprivileged user to get this information without read access to CRDs, new RBAC has been added in console-operator to authorise service account to list CRDs. Ref PR: openshift/console-operator#413

Screen shots / Gifs for design review:
N/A

Unit test coverage report:
N/A

Test setup:
Need to build backend & console-operator to test above API
Note: In off-cluster mode, user token shall be used & hence user need to be authorised to verify this API. Whereas for in-cluster mode, service account shall be used & it has RBAC to authorise for unprivileged user.

Browser conformance:
N/A

abhi-kn · 2020-04-07T15:31:51Z

/assign @spadgett

abhi-kn · 2020-04-14T17:45:36Z

/assign @christianvogt

abhi-kn · 2020-04-15T09:33:06Z

/assign @invincibleJai

invincibleJai · 2020-04-15T10:48:52Z

Verified locally and in-cluster mode

in in-cluster mode

it works for un-privileged user
it works for kubeadmin

in off-cluster (localhost)

kubeadmin works as expected
for non Admin (un privileged user) won't work out of the box

@abhi-kn Thanks , i was able to verify the changes

spadgett · 2020-04-15T17:45:38Z

It would be better if we could only return the group/version/kind in the response instead of the entire resource. That way we don't leak additional information about the CRD that you wouldn't otherwise be able to see.

andrewballantyne · 2020-04-16T19:42:43Z

/kind feature

abhi-kn · 2020-04-17T19:32:05Z

It would be better if we could only return the group/version/kind in the response instead of the entire resource. That way we don't leak additional information about the CRD that you wouldn't otherwise be able to see.

Added ResponseFilter function to prune partial metadata data before propagating CRD response.

pkg/server/resource_lister.go

pkg/server/server.go

pkg/serverutils/crdutils.go

spadgett · 2020-04-17T20:07:42Z

@benjaminapetersen it might be good for you to look since you know go better

abhi-kn · 2020-04-18T04:38:07Z

/retest

abhi-kn · 2020-04-18T07:28:05Z

/retest

pkg/serverutils/knative_event_source_utils.go

pkg/server/server.go

pkg/serverutils/knative_event_source_utils.go

openshift-ci-robot · 2020-04-22T15:19:41Z

@abhi-kn: This pull request references Bugzilla bug 1826808, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1826808: added endpoint to fetch duck typed knative event source crds

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.

pkg/serverutils/knative_event_source_utils.go

cmd/bridge/main.go

abhi-kn · 2020-04-23T21:25:45Z

/retest

abhi-kn · 2020-04-24T02:36:39Z

/retest

abhi-kn · 2020-04-24T04:29:14Z

/retest

pkg/server/resource_lister.go

spadgett

/approve
/lgtm

spadgett · 2020-04-24T14:03:47Z

/bugzilla refresh

openshift-ci-robot · 2020-04-24T14:03:51Z

@spadgett: This pull request references Bugzilla bug 1826808, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

/bugzilla refresh

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.

openshift-ci-robot · 2020-04-24T14:03:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhi-kn, jhadvig, spadgett, stlaz

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:

~~OWNERS~~ [spadgett]

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

benjaminapetersen · 2020-04-24T14:57:58Z

pkg/knative/utils.go

+	Storage bool   `json:"storage" protobuf:"varint,3,opt,name=storage"`
+}
+
+// EventSourceSpec describes how a user wants their resource to appear


Nit, you could put the types in a types.go and the utils in a separate file, but not a big deal unless you keep adding to this.

Agree. Hope its fine if I consider it in next change set!

openshift-ci-robot · 2020-04-24T16:52:30Z

@abhi-kn: All pull requests linked via external trackers have merged: openshift/console#4945. Bugzilla bug 1826808 has been moved to the MODIFIED state.

In response to this:

Bug 1826808: added endpoint to fetch duck typed knative event source crds

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.

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. component/backend Related to backend labels Apr 7, 2020

openshift-ci-robot requested review from jhadvig and rhamilto April 7, 2020 14:44

openshift-ci-robot assigned spadgett Apr 7, 2020

abhi-kn force-pushed the event_source_crd branch from 2591466 to 388330a Compare April 13, 2020 21:15

abhi-kn mentioned this pull request Apr 13, 2020

Bug 1826716: RBAC to authorise service account to list CRDs openshift/console-operator#413

Merged

abhi-kn force-pushed the event_source_crd branch 2 times, most recently from a00f66e to b604805 Compare April 14, 2020 15:49

abhi-kn changed the title ~~[WIP] added endpoint to fetch duck typed event source crds~~ added endpoint to fetch duck typed knative event source crds Apr 14, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 14, 2020

openshift-ci-robot assigned christianvogt Apr 14, 2020

openshift-ci-robot assigned invincibleJai Apr 15, 2020

openshift-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 16, 2020

abhi-kn force-pushed the event_source_crd branch 2 times, most recently from 9229c59 to 491dde0 Compare April 17, 2020 19:28

spadgett reviewed Apr 17, 2020

View reviewed changes

abhi-kn force-pushed the event_source_crd branch 2 times, most recently from 916b400 to e03c6f1 Compare April 17, 2020 23:41

jhadvig requested changes Apr 20, 2020

View reviewed changes

abhi-kn changed the title ~~added endpoint to fetch duck typed knative event source crds~~ Bug 1826808: added endpoint to fetch duck typed knative event source crds Apr 22, 2020

openshift-ci-robot added bugzilla/unspecified bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Apr 22, 2020

benjaminapetersen reviewed Apr 22, 2020

View reviewed changes

pkg/serverutils/knative_event_source_utils.go Outdated Show resolved Hide resolved

benjaminapetersen reviewed Apr 22, 2020

View reviewed changes

cmd/bridge/main.go Show resolved Hide resolved

abhi-kn force-pushed the event_source_crd branch 2 times, most recently from 61c7b18 to d82b834 Compare April 23, 2020 19:51

stlaz suggested changes Apr 24, 2020

View reviewed changes

pkg/server/resource_lister.go Outdated Show resolved Hide resolved

pkg/server/resource_lister.go Outdated Show resolved Hide resolved

abhi-kn added 2 commits April 24, 2020 13:46

added endpoint to fetch duck typed event source crds

02f67f4

added ResponseFilter function to prune CRD response

a8aaf06

abhi-kn force-pushed the event_source_crd branch from d82b834 to a8aaf06 Compare April 24, 2020 08:16

stlaz approved these changes Apr 24, 2020

View reviewed changes

spadgett approved these changes Apr 24, 2020

View reviewed changes

spadgett removed the kind/feature Categorizes issue or PR as related to a new feature. label Apr 24, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 24, 2020

openshift-ci-robot removed the bugzilla/unspecified label Apr 24, 2020

openshift-ci-robot added the bugzilla/medium label Apr 24, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 24, 2020

benjaminapetersen reviewed Apr 24, 2020

View reviewed changes

openshift-merge-robot merged commit 2d2e411 into openshift:master Apr 24, 2020

spadgett added this to the v4.5 milestone Apr 24, 2020

abhi-kn mentioned this pull request May 5, 2020

Bug 1830841: updates api-endpoint to fetch event sources and update default sources #5270

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1826808: added endpoint to fetch duck typed knative event source crds #4945

Bug 1826808: added endpoint to fetch duck typed knative event source crds #4945

abhi-kn commented Apr 7, 2020 •

edited

abhi-kn commented Apr 7, 2020

abhi-kn commented Apr 14, 2020

abhi-kn commented Apr 15, 2020

invincibleJai commented Apr 15, 2020 •

edited

spadgett commented Apr 15, 2020

andrewballantyne commented Apr 16, 2020

abhi-kn commented Apr 17, 2020

spadgett commented Apr 17, 2020

abhi-kn commented Apr 18, 2020

abhi-kn commented Apr 18, 2020

openshift-ci-robot commented Apr 22, 2020

abhi-kn commented Apr 23, 2020

abhi-kn commented Apr 24, 2020

abhi-kn commented Apr 24, 2020

spadgett left a comment

spadgett commented Apr 24, 2020

openshift-ci-robot commented Apr 24, 2020

openshift-ci-robot commented Apr 24, 2020

benjaminapetersen Apr 24, 2020

abhi-kn Apr 24, 2020

openshift-ci-robot commented Apr 24, 2020

Bug 1826808: added endpoint to fetch duck typed knative event source crds #4945

Bug 1826808: added endpoint to fetch duck typed knative event source crds #4945

Conversation

abhi-kn commented Apr 7, 2020 • edited

abhi-kn commented Apr 7, 2020

abhi-kn commented Apr 14, 2020

abhi-kn commented Apr 15, 2020

invincibleJai commented Apr 15, 2020 • edited

spadgett commented Apr 15, 2020

andrewballantyne commented Apr 16, 2020

abhi-kn commented Apr 17, 2020

spadgett commented Apr 17, 2020

abhi-kn commented Apr 18, 2020

abhi-kn commented Apr 18, 2020

openshift-ci-robot commented Apr 22, 2020

abhi-kn commented Apr 23, 2020

abhi-kn commented Apr 24, 2020

abhi-kn commented Apr 24, 2020

spadgett left a comment

Choose a reason for hiding this comment

spadgett commented Apr 24, 2020

openshift-ci-robot commented Apr 24, 2020

openshift-ci-robot commented Apr 24, 2020

benjaminapetersen Apr 24, 2020

Choose a reason for hiding this comment

abhi-kn Apr 24, 2020

Choose a reason for hiding this comment

openshift-ci-robot commented Apr 24, 2020

abhi-kn commented Apr 7, 2020 •

edited

invincibleJai commented Apr 15, 2020 •

edited