Skip to content

Fix prompts showing before required flag errors #2219

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

Merged
merged 1 commit into from
May 30, 2025

Conversation

screspod
Copy link
Member

@screspod screspod commented May 30, 2025

What this PR does / why we need it:

Currently, when users run the install command with missing required flags (like --license), they may be prompted for optional values (like creating an admin console password if one was not provided through the --admin-console-password flag) before seeing validation errors. This creates a frustrating experience where users complete prompts unnecessarily.

This PR moves thepreRunInstall call from cobra's PreRunE to RunE to get the following order of execution and give precedence to required flag errors

Current Behavior
PreRunE
preRunInstall (prompts happen here)
Cobra's flag validation
RunE

New Behavior
PreRunE (removed, preRunInstall was the only thing in it)
Cobra's flag validation
RunE
preRunInstall (prompts happen here)

Which issue(s) this PR fixes:

https://app.shortcut.com/replicated/story/124248/cli-install-command-shows-prompts-before-required-flag-validation-errors

Does this PR require a test?

NONE

Does this PR require a release note?

NONE

Does this PR require documentation?

NONE

Copy link

This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID.

Online Installer:

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci/appver-dev-3c3a1d2" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Airgap Installer (may take a few minutes before the airgap bundle is built):

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci-airgap/appver-dev-3c3a1d2?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

@screspod screspod changed the title Fix prompts showing before required flags errors Fix prompts showing before required flag errors May 30, 2025
@screspod screspod marked this pull request as ready for review May 30, 2025 22:36
@screspod screspod merged commit 16a878d into main May 30, 2025
124 of 131 checks passed
@screspod screspod deleted the screspod/sc-124248/fix-prompts-showing-before-errors branch May 30, 2025 22:37
@screspod screspod restored the screspod/sc-124248/fix-prompts-showing-before-errors branch June 4, 2025 22:41
@screspod screspod deleted the screspod/sc-124248/fix-prompts-showing-before-errors branch June 4, 2025 22:43
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.

2 participants