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.

Sign up for GitHub

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

Jump to bottom

feature: Update command warning message for Fir spaces #3171

Merged
merged 9 commits into from
Jan 13, 2025
Merged

feature: Update command warning message for Fir spaces #3171

merged 9 commits into from
Jan 13, 2025

Conversation

justinwilaby
Copy link
Contributor

@justinwilaby justinwilaby commented Jan 10, 2025 •
edited
Loading

This PR provides an updated warning message to users that the ipv4 and ipv6 addresses previously associated with their soon to be destroyed space will be re-used and to take appropriate security precautions.

to test:

  1. delete a space
  2. observe warning message

CX Review Request Link

@justinwilaby justinwilaby requested a review from a team as a code owner January 10, 2025 17:05
eablack
eablack reviewed Jan 10, 2025
View reviewed changes
Copy link
Contributor

@eablack eablack left a comment

Choose a reason for hiding this comment

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

Mostly looks good to me. Just one comment about using the color package instead of going directly to chalk.

packages/cli/src/commands/spaces/destroy.ts Outdated Show resolved Hide resolved
@justinwilaby justinwilaby force-pushed the feature/update-spaces-destroy branch from f713c7e to 318cf36 Compare January 10, 2025 17:57
@justinwilaby justinwilaby force-pushed the feature/update-spaces-destroy branch from 318cf36 to c34642e Compare January 10, 2025 17:57
@justinwilaby @SandyPantsLai
Update packages/cli/src/commands/spaces/destroy.ts
837ab88 
Co-authored-by: Sandy Lai <sandy.lai@salesforce.com>
@justinwilaby @SandyPantsLai
Update packages/cli/src/commands/spaces/destroy.ts
f979894 
Co-authored-by: Sandy Lai <sandy.lai@salesforce.com>
@justinwilaby @SandyPantsLai
Update packages/cli/src/commands/spaces/destroy.ts
55b2472 
Co-authored-by: Sandy Lai <sandy.lai@salesforce.com>
@justinwilaby justinwilaby force-pushed the feature/update-spaces-destroy branch from 228dc7e to a017077 Compare January 10, 2025 19:41
sbosio
sbosio approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@sbosio sbosio left a comment

Choose a reason for hiding this comment

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

Great work! I just have one doubt around what IP address families are assigned to each space generation. I believe cedar only uses IPv4 addresses, but does fir spaces only have IPv6 outbound addresses, or might they have mixed IPv4/IPv6? If it's the latter, then the message is correct.

@justinwilaby justinwilaby merged commit 43d9164 into main Jan 13, 2025
8 checks passed
@justinwilaby justinwilaby deleted the feature/update-spaces-destroy branch January 13, 2025 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eablack eablack eablack left review comments

@SandyPantsLai SandyPantsLai SandyPantsLai approved these changes

@sbosio sbosio sbosio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@justinwilaby @eablack @sbosio @SandyPantsLai