Skip to content
This repository was archived by the owner on Jan 8, 2024. It is now read-only.

cli/uninstall: Add more validation to requested platform for uninstall #2052

Merged
merged 7 commits into from
Aug 17, 2021

Conversation

briancain
Copy link
Member

Prior to this commit, the CLI uninstall command assumed that the
platform requested matched the server platform. This could be confusing
if a user has both a docker platform server and a local server from
waypoint server run. This commit adds some extra validation with the
platform requested by loading the server platform and taking into
account what platform was requested.

This commit also changes the hard requirement that the uninstall
command needs a platform flag. If a server context has a platform set,
and no platform flag was requested, use that platform instead. Note
that the platform flag, if requested, always overrides what the server
context has. This is because some early versions of Waypoint do
not set the Platform context. If the flag and server platform conflict,
we'll print a note to say we're honoring the platform flag requested.

Fixes #2046

Screenshot of an example when no server platform was set or requested, and then the warning for if no server platform was set but a platform flag was requested:

Screenshot from 2021-08-13 16-33-30

Prior to this commit, the CLI uninstall command assumed that the
platform requested matched the server platform. This could be confusing
if a user has both a docker platform server and a local server from
`waypoint server run`. This commit adds some extra validation with the
platform requested by loading the server platform and taking into
account what platform was requested.

This commit also changes the hard requirement that the uninstall
command needs a platform flag. If a server context has a platform set,
and no platform flag was requested, use that platform instead. Note
that the platform flag, if requested, always overrides what the server
context has. This is because some early versions of Waypoint do
not set the Platform context. If the flag and server platform conflift,
we'll print a note to say we're honoring the platform flag requested.

Fixes #2046
@briancain briancain added this to the 0.5.x milestone Aug 13, 2021
@briancain briancain requested a review from a team August 13, 2021 23:34
Copy link
Contributor

@krantzinator krantzinator left a comment

Choose a reason for hiding this comment

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

Didn't test it manually on account of me checking this real quick before I'm offline tomorrow, but the code looks 👍🏻 !

Copy link
Contributor

@evanphx evanphx left a comment

Choose a reason for hiding this comment

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

A nil check and a question about if we should check that the platform value agrees.

@briancain briancain force-pushed the bug/cli/uninstall-validate-context-before-uninstall branch from 3af4dba to 32e1968 Compare August 17, 2021 15:43
@briancain briancain requested a review from evanphx August 17, 2021 15:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

waypoint server uninstall can delete the wrong context
4 participants