Skip to content

Add new ClusterDropdown component#36310

Merged
avatus merged 1 commit intomasterfrom
avatus/cluster_dropdown
Jan 11, 2024
Merged

Add new ClusterDropdown component#36310
avatus merged 1 commit intomasterfrom
avatus/cluster_dropdown

Conversation

@avatus
Copy link
Copy Markdown
Contributor

@avatus avatus commented Jan 5, 2024

This PR will add a ClusterDropdown component to all pages that actively use the cluster dropdown currently in the TopBar.
e buddy: https://github.com/gravitational/teleport.e/pull/3117

This is part 1 of a set of PRs to update the topbar. One of the bigger changes is that we are removing the current ClusterSelector from the TopBar and only having it on the pages that require it. This PR does not remove the current cluster selector in the top, thats in part 2.

Screenshot 2024-01-04 at 6 03 12 PM

The design of the component is meant to fit the "look" of other filters on the unified resources page. See here

The pages this affects are Unified Resources, Active Sessions, Audit Logs, and Session Recordings. The last 3 pages are supposed to have the same looking component for their dropdown as per design (there isn't actual designs for those pages, but this is via Kenny)

@avatus avatus requested review from kimlisa and rudream January 5, 2024 00:06
@avatus avatus added the no-changelog Indicates that a PR does not require a changelog entry label Jan 5, 2024
@github-actions github-actions Bot requested review from gzdunek and ravicious January 5, 2024 00:06
@avatus avatus removed request for gzdunek and ravicious January 5, 2024 00:07
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a cluster dropdown also in the web terminal view

also for the unified resources view, i think the cluster dropdown should be first before types

and gosh, the button is so tiny. i think in unified resource views, it kinda works because there are many small buttons surrounding it). everywhere else where there's lots of white space surrounding it, it's kinda hard to see. (i'm not asking for change, just ranting)

i'll try to test with a second cluster tomorrow (that's always a pain to set up)

Comment thread web/packages/shared/components/ClusterDropdown/ClusterDropdown.tsx Outdated
Comment thread web/packages/shared/components/ClusterDropdown/ClusterDropdown.tsx Outdated
Comment thread web/packages/shared/components/ClusterDropdown/ClusterDropdown.tsx Outdated
Comment thread web/packages/shared/components/ClusterDropdown/ClusterDropdown.tsx Outdated
Comment thread web/packages/shared/components/ClusterDropdown/ClusterDropdown.tsx Outdated
Comment thread web/packages/shared/components/UnifiedResources/FilterPanel.tsx Outdated
Comment thread web/packages/teleport/src/Audit/Audit.tsx Outdated
@avatus
Copy link
Copy Markdown
Contributor Author

avatus commented Jan 5, 2024

also for the unified resources view, i think the cluster dropdown should be first before types

design asks for "less frequently used filters go further to the right" so this is where it ended up as per Kenny.

@avatus
Copy link
Copy Markdown
Contributor Author

avatus commented Jan 9, 2024

We spoke about using a bool to render the component, but because the component uses useStickyClusterId we couldn't import it to Connect at all (the tests break) so I just passed it as a JSX.Element instead and conditionally render based on existence

Copy link
Copy Markdown
Contributor

@kimlisa kimlisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did some more testing. Is there a enterprise counterpart for this because I didn't see a cluster drop down for access request.

couple of other findings:

missed the web terminal view:
image

The loaders for non-unified resource pages should be after the cluster dropdown (otherwise it jumps around), also it should be disabled while processing unless we do abort controlling previous requests still pending
image

the loading state for unified resources, the cluster dropdown disappears . also while changing clusters, the previous filters disappear, i wonder if we should be preserving it? (the filter preservation can be like future fine tuning)
image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
value: string;
clusterId: string;

Copy link
Copy Markdown
Contributor Author

@avatus avatus Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also while changing clusters, the previous filters disappear, i wonder if we should be preserving it? (the filter preservation can be like future fine tuning)

agreed that we should probably preserve the filter when clusters change as it is sort of used as its own "filter" now. but i'll add that in a separate PR. It shouldn't preserve the exact filters, as we'd want to reset any kind of startKey or whatever, so this would be a special case

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avatus
Copy link
Copy Markdown
Contributor Author

avatus commented Jan 9, 2024

Moved the dropdown above indicators, and cached the results of the clusters fetch to prevent the menu from hiding on re-render (and updated tests accordingly)

@avatus avatus requested a review from kimlisa January 9, 2024 21:14
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry i hsould've been more clear on access request, the user groups looks like you can change cluster too?? but new cluster dropdown is missing:

image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should add an abort controller here on option change. in the unified resource changing filter mid way cancels previous requests, we should do the same for cluster filter.

when one cluster is loading, changing to another one will briefly flash the previous results before the new one (in my case my other cluster was down, so it briefly flashed me a red error banner before loading the new response)

Copy link
Copy Markdown
Contributor Author

@avatus avatus Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry i hsould've been more clear on access request, the user groups looks like you can change cluster too?? but new cluster dropdown is missing:

This branch is a bit out of date, but we don't have a user groups dropdown anymore. It's no longer an option to select from the user groups table on master/v14

i think we should add an abort controller here on option change.

sure!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm so dumb. so the abort controller didn't fix what i was seeing (b/c duh it's cached..) the flash i'm getting it's from the non-fetch-cluster api call that fails (eg: get sessions with a cluster that's down). that would mean we'd have to add aborters everywhere but i don't think it's that a big deal to change atm

Comment thread web/packages/shared/components/ClusterDropdown/ClusterDropdown.tsx Outdated
Comment thread web/packages/teleport/src/services/clusters/clusters.ts Outdated
@kimlisa
Copy link
Copy Markdown
Contributor

kimlisa commented Jan 10, 2024

ignore this comment if kenny okayed it, but I think changing the font size to 12px and adding a colon looks more cohesive

image

@avatus
Copy link
Copy Markdown
Contributor Author

avatus commented Jan 10, 2024

ignore this comment if kenny okayed it, but I think changing the font size to 12px and adding a colon looks more cohesive

that looks great. This specific part wasn't part of the design so I can implement that feedback 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* onError is required because this dropdown can be placed on any page, it does not display it's own error
* onError is required because this dropdown can be placed on any page, it does not display its own error

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move the onError('') on line 78 here so that it also clears out the error before running a custom onLoad.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the custom onLoad so I'll skip this but very great idea if we kept it in.

@avatus avatus requested a review from kimlisa January 10, 2024 22:02
@avatus avatus force-pushed the avatus/cluster_dropdown branch from af02a49 to 3edb26d Compare January 11, 2024 17:35
@avatus avatus enabled auto-merge January 11, 2024 17:38
This replaces the current ClusterSelector in the TopBar and adds
the functionality to ClusterDropdown component on relevant pages
@avatus avatus force-pushed the avatus/cluster_dropdown branch from 3edb26d to 03b5030 Compare January 11, 2024 17:56
@avatus avatus added this pull request to the merge queue Jan 11, 2024
Merged via the queue into master with commit 851d1e9 Jan 11, 2024
@avatus avatus deleted the avatus/cluster_dropdown branch January 11, 2024 18:16
zmb3 pushed a commit that referenced this pull request Jan 12, 2024
This replaces the current ClusterSelector in the TopBar and adds
the functionality to ClusterDropdown component on relevant pages
github-merge-queue Bot pushed a commit that referenced this pull request Jan 13, 2024
* Refactor desktop player

Adopt an approach similar to the SSH player (more standard react
calls and less event emitting). This also updates the progress
bar to be more of a "dumb" component that just renders state,
and leaves the smoothing out of progress updates to the client.

In addition, this adds support for seeking by dragging the
progress bar to a particular portion of the video.

Fixes #17199

* Prevents EOF from being reported as a tdp.Notification error at the end of every session

* Removes duplicated code, handles errors in handleRDPFastPathPDU, ensures we always spit out some message if an error notification pops up in the UI

* Add dynamic/ prefix to server info labels (#36219)

This change:
- Adds the dynamic/ prefix to labels from a server_info resource created with tctl
- Forbids labels with the dynamic/ prefix form being used in deny rules for new roles.
  Existing roles will generate warnings in tctl as well as a cluster alert.

* Address deprecation TODOs for or before v15. (#36473)

* Convert insecure-drop to drop for unsupported clients (#35803)

This change converts the insecure-drop host user creation mode
to drop for clients that don't support it.

* discovery: remove update if discovery group differs (#36472)

* discovery: remove update if discovery group differs

This PR removes code marked to be removed in Teleport 15 that updated
unconditionally kubernetes and databases that were discovered using a
different discovery group.

Until this PR, if the `onCreate` function returned `trace.AlreadyExists`
error, the resource ended up being updated without any condition. After
this PR, the resource is only updated if the existing resource has an
empty discovery group. This behavior is kept to ensure that users that
migrate from bad configs don't need to delete resources manually.

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>

* Update discovery.go

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* handle code review suggestions

---------

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* Refactor Kubernetes Exec sessions upgrade logic (#36325)

* Refactor Kubernetes Exec sessions upgrade logic

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>

* handle code review suggestions

* Update lib/kube/proxy/forwarder.go

Co-authored-by: Anton Miniailo <anton@goteleport.com>

* Update lib/kube/proxy/forwarder.go

Co-authored-by: Anton Miniailo <anton@goteleport.com>

* handle code review suggestions

---------

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Co-authored-by: Anton Miniailo <anton@goteleport.com>

* Deduplicate yarn.lock (#36560)

* Deduplicate yarn.lock
* Fix types in ProgressBar

* docs: updates to tsh connect your client (#36526)

* docs: updates to tsh connect your client

* remove extra dash

* Add backend code for listing EKS clusters through AWS OIDC integration. (#36489)

* Add backend code for listing EKS clusters through AWS OIDC integration.

* Remove leftover commented code.

Co-authored-by: Marco André Dinis <marco.dinis@goteleport.com>

* Add godoc.

* Remove unneeded intialization of a parameter.

* Reduce nesting.

* Rename ExtraLabels to JoinLabels.

---------

Co-authored-by: Marco André Dinis <marco.dinis@goteleport.com>

* Update devbox. (#36193)

* Update AWS account ID used in `update-ami-ids` GHA workflow (#36556)

PR #36153 updated the AWS account ID used for pulling AMIs, but the GHA
workflow was not updated to use this updated AWS account ID.

Ref #34282.

* Fix accesslist `tctl` (#36531)

* Support unmarshalling accesslists manifests wihtout next_audit_date

* Support `tctl get access_lists`

* Fix accesslist reference

* accountrecovery.go: Unconditionally delete the token after use (#36527)

A previous conditional was allowing a replay attack on the recovery process.  Although discovery of this token is a high bar for an attacker, we should be able to unconditionally delete this token after it's used.

* Add app gateways to Connect (#36393)

* Allow creating app gateways in tshd

* Add UI for document gateway app

* Show apps in connections

This is a copy & paste from other connection kinds.

* Capture app protocol

* Start app proxy when clicking on 'Connect' in app

* Remove `removeAppGateway`

* `appUri` -> `targetUri`

* Add TCP and HTTP constants

* Add CLI command for HTTP apps

* Add `makeAppGateway`

* Specify `handleChangePort` dependencies correctly

* Remove `doc.gateway_app` and `connection.app`, instead differentiate gateways by URI

* Correctly report protocol usage

* Mention that AWS apps are supported in tsh

* Rename constants and add godoc

* Add a TODO comment about dialogs for connecting to unsupported apps

* `onRun` -> `onButtonClick`, `runButtonText` -> `buttonText`

* Show a notification after copying to clipboard

* Make the message about unsupported gateways more precise

* Revert mistakenly removed `document.targetUri` from `getResourceUri`

* Remove 'Local app proxy' header

* Post-merge fixes

* Use proper component for the 'offline gateway' state

* Support all gateway types in relogin UI

* Fix JSdoc comment

* Add a TODO comment about the docs link

---------

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>

* Allow configuration of Okta access list importing. (#36569)

Configuration options for enabling/disabling Okta access list importing have
been added. This defaults to false. This has been added to both to Okta plugin
settings and the standalone Okta service.

Co-authored-by: Trent Clarke <trent@goteleport.com>

* Fix example mysql grant all command (#36519)

Fixing example mysql grant all command, because current one fails with `ERROR 1102 (42000): Incorrect database name ' % '` because of extra spaces around `%`

* lib/teleterm app access: Add middleware for handling expired certs (#36520)

* Add middleware for app gateways

* Accommodate for app gateways in integration tests

The previous version of tests depended on receiving helpers.TeleInstance
from higher up. It was then used to generate valid and expired user certs,
as well as to get client.TeleportClient.

proxy.Suite (Kube tests) and dbhelpers.DatabasePack (db tests) expose
helpers.TeleInstance so it wasn't a problem. However, appaccess.Pack,
which we're going to use for app access tests, does not expose it.

To work around that, we introduce two new fields to gatewayCertRenewalParams,
tc (which accepts client.TeleportClient) and generateAndSetupUserCreds.
These two fields get rid of the dependency on helpers.TeleInstance.

* Add integration tests for app gateways

* Switch to the new account settings screen (#36525)

* Migrate `RotateCertAuthority` to gRPC (#36536)

* Add RotateCertAuthority to gRPC TrustService.

* Add gRPC server implementation.

* Move RotateRequest type to api/types.

* Mark deprecated HTTP rotate endpoint for deletion.

* Add client implemenation and HTTP fallback.

* Update go-oidc to get final go-jose v2 -> v3 updates (#36514)

* Update go-oidc to get final go-jose v2 updates

This updates our replaced go-oidc fork to use a tag with go-jose updated to v3: gravitational/go-oidc#19

This update removes the final usage of v2, and fully addresses the GHSA-2c7c-3mj9-8fqh DoS.

* Update gopkg.in/go-jose/go-jose.v2 to 2.6.2 to get p2c DoS fix

* Add ClusterDropdown component (#36310)

This replaces the current ClusterSelector in the TopBar and adds
the functionality to ClusterDropdown component on relevant pages

* fix: Verify MFA device locks during authentication (#36471)

* Test authn and password change with a locked user

* Verify MFA device locks during authentication

* Configure a LockWatcher in the passwordSuite setup

* Appease linter

* Update Toggle component styles (#36535)

* Reintroduces the changes in #33273 which were erroneously deleted in #33273 (#36538)

* Route to server by public addr (#36584)

This change fixes a bug where tsh ssh could not dial a node with
its public address.

* Add EKS Discover into web testplan. (#36578)

* Remove unused static token endpoints. (#36545)

* Ensure player finishes at 100%

* Fix style for disabled progress bar

* Remove unused file

* fix import order

---------

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Co-authored-by: Isaiah Becker-Mayer <isaiah@goteleport.com>
Co-authored-by: Andrew Burke <31974658+atburke@users.noreply.github.com>
Co-authored-by: Brian Joerger <bjoerger@goteleport.com>
Co-authored-by: Tiago Silva <tiago.silva@goteleport.com>
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
Co-authored-by: Anton Miniailo <anton@goteleport.com>
Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
Co-authored-by: Steven Martin <steven@goteleport.com>
Co-authored-by: Marco André Dinis <marco.dinis@goteleport.com>
Co-authored-by: Michael Wilson <mike@mdwn.dev>
Co-authored-by: Reed Loden <reed@goteleport.com>
Co-authored-by: Hugo Shaka <hugo.hervieux@goteleport.com>
Co-authored-by: Mike Jensen <jentfoo@users.noreply.github.com>
Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com>
Co-authored-by: Trent Clarke <trent@goteleport.com>
Co-authored-by: Taras <9948629+taraspos@users.noreply.github.com>
Co-authored-by: Bartosz Leper <bartosz.leper@goteleport.com>
Co-authored-by: Michael <michael.myers@goteleport.com>
Co-authored-by: Alan Parra <alan.parra@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/md ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants