Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add version incompatibility warning to Connect #51832

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ravicious
Copy link
Member

@ravicious ravicious commented Feb 4, 2025

Closes #32555.

This PR refactors the version compatibility detection logic from tsh so that it can be reused in Connect. Then we use it to display a version compatibility warning in Connect when logging in to a cluster.

changelog: Added version compatibility warnings to Teleport Connect when logging in to a cluster

compatibility-warning

@gzdunek
Copy link
Contributor

gzdunek commented Feb 27, 2025

@ravicious, I added the UI:
image

In the end I didn't use the tsh copy, it was too long.
If something doesn't seem right in the UI, feel free to adjust it.

size="small"
onClick={props.disableVersionCheck}
>
<Cog size="small" mr={1} />
Copy link
Contributor

Choose a reason for hiding this comment

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

I wanted it to indicate that it's a config option :p

as="a"
href={buildDownloadUrl(props.platform)}
target="_blank"
<ActionButton
Copy link
Member Author

Choose a reason for hiding this comment

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

@gzdunek I've adjusted the buttons slightly so that they look the same way as primaryAction/secondaryAction from Alert. The cog icon was a nice touch, but with the new buttons there wasn't enough space for it.

I also removed the wildcards from the warning about the client being too old and updated the UIs to get rid of the -aa suffix when showing min version.

compatibility-warning

Copy link
Member Author

Choose a reason for hiding this comment

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

Next time we can think of adding some kind of a prop to alert to control where the action buttons are shown, as we now at least have two places for that. Though maybe that's something we should consult with the design team.

@ravicious ravicious marked this pull request as ready for review February 28, 2025 13:58
@ravicious ravicious added backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry labels Feb 28, 2025
@ravicious ravicious removed the request for review from rudream February 28, 2025 13:59
@ravicious ravicious removed the no-changelog Indicates that a PR does not require a changelog entry label Feb 28, 2025
@ravicious ravicious requested a review from gzdunek February 28, 2025 14:04
@@ -119,6 +122,9 @@ export function ClusterLoginPresentation({
clearLoginAttempt={clearLoginAttempt}
shouldPromptSsoStatus={shouldPromptSsoStatus}
passwordlessLoginState={passwordlessLoginState}
shouldSkipVersionCheck={shouldSkipVersionCheck}
disableVersionCheck={disableVersionCheck}
platform={platform}
Copy link
Member Author

Choose a reason for hiding this comment

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

There's a fair share of prop drilling, but this is because useClusterLogin doesn't use a context and the login modal is written in the old style.

Copy link

github-actions bot commented Feb 28, 2025

Amplify deployment status

Branch Commit Job ID Status Preview Updated (UTC)
r7s/version-compat HEAD 1 ✅SUCCEED r7s-version-compat 2025-02-28 14:25:36

Copy link
Contributor

@gzdunek gzdunek left a comment

Choose a reason for hiding this comment

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

🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add version incompatibility warnings to Connect
2 participants