WebShared: Update how request checkout handles kube resource related errors #48168

kimlisa · 2024-10-30T19:29:53Z

The backend error messaging has changed, so this PR just updates how the web UI processes that error.

It also fixes a subtle bug where, after a request was created (or when user cancels the request), the specifiable fields (eg: suggested reviewers and especially the selected resource roles) were retained, so when user tries to create a new checkout, the resource roles fetch can fail, and the following dry run fetch referred to the previous requests values of selected roles, which resulted in incorrect error message referring to the wrong roles.

Screenshots:

with tooltip, if the error message gets long (happens with wildcard expanded into all kinds supported):

without tooltip (reasonably short error message)

aws-amplify-us-west-2 · 2024-10-30T19:31:46Z

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-48168.d3pp5qlev8mo18.amplifyapp.com

...ckages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.story.tsx

rudream · 2024-10-30T19:39:23Z

...ckages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.story.tsx

+      isResourceRequest={true}
+      fetchResourceRequestRolesAttempt={{
+        status: 'failed',
+        statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting to some or all of the requested Kubernetes resources. allowed kinds for each requestable roles: test-role-1: [deployment]`,


Suggested change

statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting to some or all of the requested Kubernetes resources. allowed kinds for each requestable roles: test-role-1: [deployment]`,

statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting to some or all of the requested Kubernetes resources. Allowed kinds for each requestable roles: test-role-1: [deployment]`,

this is a backend error and i was told in one of my backend PRs that error messages are not capitalized...

kimlisa · 2024-10-31T00:35:47Z

...ckages/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx

i had to remove the required checking b/c theres now no clear way to tell if it's required or not.

eg: one role can enforce namespaces only, another role can enforce pods only. it'll be confusing to assume that all kube_cluster resources selected has access to namespaces and mark it required (letting user try to create request and backend return an error is better)

flyinghermit

lgtm.

flyinghermit · 2024-10-31T23:28:10Z

...ckages/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx

@@ -144,7 +143,7 @@ export function KubeNamespaceSelector({
  return (
    <Box width="100%" mb={-3}>
      <StyledSelect
-        label={`Namespaces${namespaceRequired ? ' (required)' : ''}:`}
+        label={`Namespaces:`}


nit: is : necessary?

flyinghermit · 2024-10-31T23:31:49Z

web/packages/shared/components/AccessRequests/NewRequest/kube.ts

+ * Parses error messsage (in an expected format) to see if
+ * in the message it's a type of request.kubernetes_resources
+ * related error, then check if namespace or kube_cluster is mentioned
+ * in the "allowed kinds" portion of the error message.


Suggested change

* Parses error messsage (in an expected format) to see if

* in the message it's a type of request.kubernetes_resources

* related error, then check if namespace or kube_cluster is mentioned

* in the "allowed kinds" portion of the error message.

* Parses error message (in an expected format returned from the backend) to see if

* the message is a type of request.kubernetes_resources (link backend code or add error sample below) related error.

* If error matches, it then checks if namespace or kube_cluster is mentioned

* in the "allowed kinds" portion of the error message.

flyinghermit · 2024-10-31T23:33:27Z

web/packages/shared/components/AccessRequests/NewRequest/kube.ts

+    if (
+      errMsg.includes('request.kubernetes_resources') &&
+      errMsg.includes('allowed kinds')
+    ) {
+      let splitMsgParts = [];
+      if (errMsg.includes('requested roles: ')) {
+        splitMsgParts = errMsg.split('requested roles: ');
+      } else if (errMsg.includes('requestable roles: ')) {
+        splitMsgParts = errMsg.split('requestable roles: ');


This is really brutal way to guess error type. Not suggesting we change anything here but someday, we should come up with error codes that can be meaningfully shared amongst backend and frontend.

…errors

public-teleport-github-review-bot · 2024-11-01T05:25:33Z

@kimlisa See the table below for backport results.

Branch	Result
branch/v16	Failed
branch/v17	Create PR

ravicious

I started reviewing this on Thursday but then I got distracted with something else. 🫥

ravicious · 2024-10-31T15:15:11Z

web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx

@@ -316,14 +315,14 @@ export function RequestCheckout<T extends PendingListItem>({
                >
                  tsh request search
                </ExternalLink>{' '}
-                command that will help you construct the request.


I think it should still say "command" no?

oh what the.... i have minor UX's stuff i need to address, so i'll add it back

…errors (#48168) * WebShared: Update how request checkout handles kube resource related errors * Fix bug where after create/cancel, specifiable fields were retained * Remove single toggler for kube resources * Address CR

…ces during access requests (#48413) * Web: add support for requesting for kube namespaces (#47345) * Teleterm: add support for access requesting kube namespaces (#47347) * WebShared: Update how request checkout handles kube resource related errors (#48168) * WebShared: Update how request checkout handles kube resource related errors * Fix bug where after create/cancel, specifiable fields were retained * Remove single toggler for kube resources * Address CR * Update snaps * Backport fixes - teleport version v16 and less uses react select version 3 which required to add manual support for initially fetching namespaces on select dropdown - hover tooltip design diffs - field select design diffs

github-actions bot requested review from ravicious and rudream October 30, 2024 19:30

github-actions bot added size/md ui labels Oct 30, 2024

kimlisa added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v16 backport/branch/v17 labels Oct 30, 2024

rudream approved these changes Oct 30, 2024

View reviewed changes

kimlisa commented Oct 31, 2024

View reviewed changes

kimlisa force-pushed the lisa/update-web-ui branch 2 times, most recently from 0bea852 to 81d06e9 Compare October 31, 2024 04:54

flyinghermit approved these changes Oct 31, 2024

View reviewed changes

r0mant approved these changes Oct 31, 2024

View reviewed changes

public-teleport-github-review-bot bot removed the request for review from ravicious October 31, 2024 23:54

kimlisa added 4 commits October 31, 2024 22:07

WebShared: Update how request checkout handles kube resource related …

d033b53

…errors

Fix bug where after create/cancel, specifiable fields were retained

7411038

Remove single toggler for kube resources

17e54e8

Address CR

b24b5fa

kimlisa force-pushed the lisa/update-web-ui branch from 81d06e9 to b24b5fa Compare November 1, 2024 05:07

kimlisa enabled auto-merge November 1, 2024 05:07

kimlisa added this pull request to the merge queue Nov 1, 2024

Merged via the queue into master with commit 3491836 Nov 1, 2024
39 checks passed

kimlisa deleted the lisa/update-web-ui branch November 1, 2024 05:24

kimlisa mentioned this pull request Nov 1, 2024

[v17] WebShared: Update how request checkout handles kube resource related errors #48259

Merged

ravicious reviewed Nov 4, 2024

View reviewed changes

kimlisa mentioned this pull request Nov 5, 2024

[v16] Add Connect and Web UI support for selecting Kubernetes namespaces during access requests #48413

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WebShared: Update how request checkout handles kube resource related errors #48168

WebShared: Update how request checkout handles kube resource related errors #48168

kimlisa commented Oct 30, 2024 •

edited

Loading

aws-amplify-us-west-2 bot commented Oct 30, 2024

rudream Oct 30, 2024

kimlisa Oct 31, 2024

kimlisa Oct 31, 2024 •

edited

Loading

flyinghermit left a comment

flyinghermit Oct 31, 2024

flyinghermit Oct 31, 2024

flyinghermit Oct 31, 2024

public-teleport-github-review-bot bot commented Nov 1, 2024

ravicious left a comment

ravicious Oct 31, 2024

kimlisa Nov 4, 2024

	statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting to some or all of the requested Kubernetes resources. allowed kinds for each requestable roles: test-role-1: [deployment]`,
	statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting to some or all of the requested Kubernetes resources. Allowed kinds for each requestable roles: test-role-1: [deployment]`,

WebShared: Update how request checkout handles kube resource related errors #48168

WebShared: Update how request checkout handles kube resource related errors #48168

Conversation

kimlisa commented Oct 30, 2024 • edited Loading

aws-amplify-us-west-2 bot commented Oct 30, 2024

rudream Oct 30, 2024

Choose a reason for hiding this comment

kimlisa Oct 31, 2024

Choose a reason for hiding this comment

kimlisa Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

flyinghermit left a comment

Choose a reason for hiding this comment

flyinghermit Oct 31, 2024

Choose a reason for hiding this comment

flyinghermit Oct 31, 2024

Choose a reason for hiding this comment

flyinghermit Oct 31, 2024

Choose a reason for hiding this comment

public-teleport-github-review-bot bot commented Nov 1, 2024

ravicious left a comment

Choose a reason for hiding this comment

ravicious Oct 31, 2024

Choose a reason for hiding this comment

kimlisa Nov 4, 2024

Choose a reason for hiding this comment

kimlisa commented Oct 30, 2024 •

edited

Loading

kimlisa Oct 31, 2024 •

edited

Loading