Add faker for the pass through api to enable regular user testing #806

lucferbux · 2022-11-24T13:18:34Z

Closes: #949

Description

This PR adds support for impersonating pass-through API calls with a regular user token.
The main goal here is just to avoid disabling backend functionality in development which needs special admin roles (admin settings, regular jupyter notebooks, dsg project creation...) but enabling pass-through requests with a regular user token.

How Has This Been Tested?

To enable this:

Get a regular user Openshift username and password
Add the DEV_IMPERSONATE_USER and DEV_IMPERSONATE_PASSWORD variables to your env variables file with the regular user's username and password as the values.
Click on your username name in the upper right corner of the dashboard
Select Start impersonate
This way you can replicate some bugs like this:

Merge criteria:

The commits are squashed in a cohesive manner and have meaningful messages.
Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has manually tested the changes and verified that the changes work

andrewballantyne

Neat idea. We should probably log a dev ticket, low priority, to handle thinking on how to make the dev experience better all together. Might be able to avoid granting stupid permissions to do this.

This may also just come for free when we kill the old backend.

backend/src/utils/directCallUtils.ts

backend/src/utils/constants.ts

backend/src/utils/directCallUtils.ts

docs/SDK.md

backend/src/utils/directCallUtils.ts

lucferbux · 2022-11-24T20:21:41Z

Neat idea. We should probably log a dev ticket, low priority, to handle thinking on how to make the dev experience better all together. Might be able to avoid granting stupid permissions to do this.

This may also just come for free when we kill the old backend.

@andrewballantyne Yes for sure, I'm creating a new ticket for this, thanks for the feedback (resolved in my branch since I was working offline this evening).

andrewballantyne · 2022-12-14T20:03:37Z

Thinking more on this situation -- I wonder if we could set something up that would allow us to start the server with a cluster-admin (aka mimic a service account starting it on cluster) and then somehow making another more basic user to be the one logged in. The one in the top right, the one making all the "oauth" (not really because it does not exist locally) calls, etc, etc

lucferbux · 2022-12-15T09:22:37Z

Thinking more on this situation -- I wonder if we could set something up that would allow us to start the server with a cluster-admin (aka mimic a service account starting it on cluster) and then somehow making another more basic user to be the one logged in. The one in the top right, the one making all the "oauth" (not really because it does not exist locally) calls, etc, etc

Yeah, I've been thinking about this too, I think it can be done. There are three main points right now:

Pass through API faker: I think that this PR covers most of it.
Mimic local permissions to the same as the Dashboard Serviceaccount: This will be very helpful to catch all the permission errors we have with permissions in local development.
Fake user for backend: I think this is suuuuper easy to implement, I can have that in this PR if you want @andrewballantyne or maybe in a follow up PR. [EDIT] I've added support for this scenario, you can check this now

lucferbux · 2023-01-13T16:06:48Z

To sum up, with this PR we are now having:

Faker for pass-through API k8s calls
Faker for backend API calls

It would be really nice to support this in the future:

Ability to mimic dashboard serviceaccount permissions for the backend

What do you think, @andrewballantyne ?

DaoDaoNoCode · 2023-01-17T14:29:05Z

Can you rebase this PR? So I don't need to run npm install every time when I switch to this branch to test.

andrewballantyne

Still testing the functionality -- few code review comments.

backend/src/routes/api/status/statusUtils.ts

backend/src/devFlags.ts

backend/src/utils/directCallUtils.ts

backend/src/utils/constants.ts

docs/SDK.md

backend/src/routes/api/status/statusUtils.ts

DaoDaoNoCode · 2023-02-15T21:12:35Z

This PR currently covers most of the situations which directly make API calls to k8s resources. However, it could have some problems when making API calls to the external routes (like prometheus), the Impersonate-User field doesn't work, it will still use the bearer token of the login user.

lucferbux · 2023-02-28T10:22:51Z

/assign @andrewballantyne

andrewballantyne

I think we need to do one more iteration to get this into merging state.

Only downfall I see that is not addressed in my comments is when I change the server code, it does not update my client back to a state of understanding that the impersonation was lost. This is probably fine -- we don't do a lot of server dev, and we have #964 coming.

backend/src/routes/api/dev-impersonate/index.ts

backend/src/utils/directCallUtils.ts

lucferbux · 2023-03-01T09:25:40Z

@andrewballantyne I resolved your comments since @DaoDaoNoCode update the PR, but I just want to add that we still fall short in one scenario, but I think is no longer relevant.

We still use the primary logged user as the rbac for the backend calls, so we still don't have a 1:1 relation to the dashboard's serviceaccount. This is still an issue if we are developing any backend feature (it might work in local as we have cluster admin but then it won't work deployed because the dashboard role doesn't have those permissions).

It's similar to the prometheus workaround, something that we can easily test now as we can deploy images built by that PR. One easy workaround would be create a user that mimics the same permissions as the dashboard serviceaccount, but I would do it outside this PR in addition of the script that Andrew has for creating users in clusters.

openshift-ci · 2023-03-01T19:17:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne

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~~ [andrewballantyne]

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

more workbenches a11y updates fix dsp modal fix more select issues prettier Add faker for the pass through api to enable regular user testing (opendatahub-io#806) * Add faker for the pass through api to enable regular user testing * Add faker for backend openshift user * add impersonate function for developers * address comments and modify the doc * Update to get the access token by making API call * update doc * add error message * Fix lint issues * address comments --------- Co-authored-by: Juntao Wang <juntwang@redhat.com> Storybook test integration (opendatahub-io#974) * added storybook boilerplate added a mock and decorator added tests fixed deps for storybook linter old tests dont work on jest 28, reverting linter error fix fix build errors fixed deps * trimmed mocks fixed tests * fix webpack changes * added a11y test to test-runner fix a11y errors in settings (opendatahub-io#979) varname rename bodyText to ariaLabel add aria label to select add aria-label to Select fix a11y violation in cluster storage modal and list view

more workbenches a11y updates fix dsp modal fix more select issues prettier remove unused import revert id update fix project modal wip Workbenches, need image more workbenches a11y updates fix dsp modal fix more select issues prettier Add faker for the pass through api to enable regular user testing (opendatahub-io#806) * Add faker for the pass through api to enable regular user testing * Add faker for backend openshift user * add impersonate function for developers * address comments and modify the doc * Update to get the access token by making API call * update doc * add error message * Fix lint issues * address comments --------- Co-authored-by: Juntao Wang <juntwang@redhat.com> Storybook test integration (opendatahub-io#974) * added storybook boilerplate added a mock and decorator added tests fixed deps for storybook linter old tests dont work on jest 28, reverting linter error fix fix build errors fixed deps * trimmed mocks fixed tests * fix webpack changes * added a11y test to test-runner fix a11y errors in settings (opendatahub-io#979) varname rename bodyText to ariaLabel add aria label to select add aria-label to Select fix a11y violation in cluster storage modal and list view package-lock.json package-lock.json update DetailsSection.tsx remove aria-labelledby Table PR suggestion from Gage lint and revert Stack removal

more workbenches a11y updates fix dsp modal fix more select issues prettier remove unused import revert id update fix project modal revert stack wip Workbenches, need image more workbenches a11y updates fix dsp modal fix more select issues prettier Add faker for the pass through api to enable regular user testing (opendatahub-io#806) * Add faker for the pass through api to enable regular user testing * Add faker for backend openshift user * add impersonate function for developers * address comments and modify the doc * Update to get the access token by making API call * update doc * add error message * Fix lint issues * address comments --------- Co-authored-by: Juntao Wang <juntwang@redhat.com> Storybook test integration (opendatahub-io#974) * added storybook boilerplate added a mock and decorator added tests fixed deps for storybook linter old tests dont work on jest 28, reverting linter error fix fix build errors fixed deps * trimmed mocks fixed tests * fix webpack changes * added a11y test to test-runner fix a11y errors in settings (opendatahub-io#979) varname rename bodyText to ariaLabel add aria label to select add aria-label to Select fix a11y violation in cluster storage modal and list view package-lock.json package-lock.json fix a11y in data connections and models/model servers update DetailsSection to match changes on other branch update Table file fix pagination axe error PR feedback from Andrew change aria label text on password input

…endatahub-io#806) * Add faker for the pass through api to enable regular user testing * Add faker for backend openshift user * add impersonate function for developers * address comments and modify the doc * Update to get the access token by making API call * update doc * add error message * Fix lint issues * address comments --------- Co-authored-by: Juntao Wang <juntwang@redhat.com>

lucferbux requested review from andrewballantyne and DaoDaoNoCode November 24, 2022 13:18

openshift-ci bot requested a review from maroroman November 24, 2022 13:18

lucferbux force-pushed the add-pass-through-api-faker branch from fab30e1 to 2783b38 Compare November 24, 2022 13:29

andrewballantyne requested changes Nov 24, 2022

View reviewed changes

openshift-ci bot assigned andrewballantyne Nov 24, 2022

lucferbux force-pushed the add-pass-through-api-faker branch from 2783b38 to 0a2405f Compare November 24, 2022 20:16

lucferbux requested a review from andrewballantyne November 24, 2022 20:20

This was referenced Nov 25, 2022

[Investigation]: Improve Dev Workflows/Tooling #810

Closed

Fix bug limiting access to global view to regular users: #812

Merged

andrewballantyne mentioned this pull request Dec 14, 2022

[Bug]: Basic Users Cannot Get PVC Fill Status #862

Closed

1 task

lucferbux force-pushed the add-pass-through-api-faker branch from 0a2405f to 1a238f7 Compare December 15, 2022 09:16

This was referenced Jan 13, 2023

Add dynamic spec to servingruntime objects #888

Merged

Add role for servingruntime configmap opendatahub-io/odh-manifests#715

Closed

[UPSTREAM] Add dynamic spec to servingruntime objects red-hat-data-services/odh-dashboard#256

Merged

DaoDaoNoCode force-pushed the add-pass-through-api-faker branch from b8f007d to f9d9b9f Compare January 17, 2023 19:20

lucferbux force-pushed the add-pass-through-api-faker branch from f9d9b9f to ba41aa2 Compare January 20, 2023 16:15

DaoDaoNoCode force-pushed the add-pass-through-api-faker branch from ba41aa2 to f9d9b9f Compare January 20, 2023 16:19

andrewballantyne requested changes Feb 3, 2023

View reviewed changes

DaoDaoNoCode force-pushed the add-pass-through-api-faker branch from f9d9b9f to ca2fcd8 Compare February 3, 2023 20:56

DaoDaoNoCode force-pushed the add-pass-through-api-faker branch from ca2fcd8 to a865d17 Compare February 15, 2023 19:19

DaoDaoNoCode force-pushed the add-pass-through-api-faker branch from 2cb51ba to e071192 Compare February 17, 2023 18:45

lucferbux requested a review from andrewballantyne February 23, 2023 16:11

lucferbux force-pushed the add-pass-through-api-faker branch from e071192 to cadd375 Compare February 23, 2023 17:16

openshift-ci bot assigned DaoDaoNoCode Feb 23, 2023

openshift-ci bot added the lgtm label Feb 23, 2023

lucferbux and others added 8 commits February 28, 2023 11:10

Add faker for the pass through api to enable regular user testing

ed4cd7d

Add faker for backend openshift user

61acd13

add impersonate function for developers

88470d0

address comments and modify the doc

1aff0d5

Update to get the access token by making API call

19097bf

update doc

841923b

add error message

9a1b8d8

Fix lint issues

25bff47

lucferbux force-pushed the add-pass-through-api-faker branch from cadd375 to 25bff47 Compare February 28, 2023 10:22

openshift-ci bot removed the lgtm label Feb 28, 2023

andrewballantyne requested changes Feb 28, 2023

View reviewed changes

backend/src/routes/api/dev-impersonate/index.ts Outdated Show resolved Hide resolved

backend/src/utils/directCallUtils.ts Outdated Show resolved Hide resolved

address comments

6cc52b1

lucferbux requested a review from andrewballantyne March 1, 2023 11:10

andrewballantyne approved these changes Mar 1, 2023

View reviewed changes

openshift-ci bot added the lgtm label Mar 1, 2023

openshift-ci bot added the approved label Mar 1, 2023

openshift-merge-robot merged commit 74188de into opendatahub-io:main Mar 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add faker for the pass through api to enable regular user testing #806

Add faker for the pass through api to enable regular user testing #806

lucferbux commented Nov 24, 2022 •

edited

Loading

andrewballantyne left a comment

lucferbux commented Nov 24, 2022

andrewballantyne commented Dec 14, 2022

lucferbux commented Dec 15, 2022 •

edited

Loading

lucferbux commented Jan 13, 2023

DaoDaoNoCode commented Jan 17, 2023

andrewballantyne left a comment

DaoDaoNoCode commented Feb 15, 2023

lucferbux commented Feb 28, 2023

andrewballantyne left a comment

lucferbux commented Mar 1, 2023

openshift-ci bot commented Mar 1, 2023

Add faker for the pass through api to enable regular user testing #806

Add faker for the pass through api to enable regular user testing #806

Conversation

lucferbux commented Nov 24, 2022 • edited Loading

Description

How Has This Been Tested?

Merge criteria:

andrewballantyne left a comment

Choose a reason for hiding this comment

lucferbux commented Nov 24, 2022

andrewballantyne commented Dec 14, 2022

lucferbux commented Dec 15, 2022 • edited Loading

lucferbux commented Jan 13, 2023

DaoDaoNoCode commented Jan 17, 2023

andrewballantyne left a comment

Choose a reason for hiding this comment

DaoDaoNoCode commented Feb 15, 2023

lucferbux commented Feb 28, 2023

andrewballantyne left a comment

Choose a reason for hiding this comment

lucferbux commented Mar 1, 2023

openshift-ci bot commented Mar 1, 2023

lucferbux commented Nov 24, 2022 •

edited

Loading

lucferbux commented Dec 15, 2022 •

edited

Loading