Fix --addon-database/--addon-redis flags being no-ops - #164
Merged
Conversation
Wire up the boolean --addon-database and --addon-redis flags so they are no longer dead. Previously these flags were registered in cmd/create.go but no struct field in AppStackParameters was bound to them, so passing them alone had no effect. Changes: - Add DatabaseAddonEnabled and RedisAddonEnabled bool fields to AppStackParameters with cfnignore:"-" so they are excluded from CloudFormation parameter serialization (cli-only fields). - AskForDatabase / AskForRedis now default the enable prompt to true when either the bool flag is set OR the stack name is already non-empty. The "no" branch also clears the bool flag to prevent resolveAddonStacks from re-enabling it after the user explicitly declined. - Add resolveAddonStacks (called from SetInternalFields) which runs in both interactive and non-interactive flows. When --addon-database is set without --addon-database-name: if exactly one database exists on the cluster it is selected automatically; if multiple exist, the error lists the choices and instructs the user to pass --addon-database-name; if none exist, the error directs to `apppack create database`. Same logic for --addon-redis / --addon-redis-name / `apppack create redis`. - Add cfnignore tag support to StructToCloudformationParameters so fields marked cfnignore:"-" are excluded from the CloudFormation parameter list. - Update --addon-database / --addon-redis help text in cmd/create.go to make the "create the instance first" ordering constraint explicit and document when --addon-database-name is needed. - Add table-driven unit tests for selectDatabaseStack, selectRedisStack, and the enable-default logic. Closes #163
Contributor
Author
Live tested on apppack-sandbox ✅Built this branch and ran it against the
Multi-instance and zero-instance error paths are covered by the unit tests (couldn't exercise them live with only one instance of each on the sandbox cluster). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wires up the boolean
--addon-databaseand--addon-redisflags oncreate app/create pipeline, which were previously registered but never read.Changes:
DatabaseAddonEnabled/RedisAddonEnabledbool fields toAppStackParameters(cfnignore:"-"excludes them from CloudFormation parameters)AskForDatabase/AskForRedisdefault to yes when either the bool flag is set or a stack name is already configured; the no branch clears the flagresolveAddonStackshelper (called fromSetInternalFields) auto-selects when exactly one instance exists, errors with the list when multiple exist, and directs toapppack create database/apppack create rediswhen none existcfnignoretag support toStructToCloudformationParametersfor CLI-only fields--addon-database-nameis neededselectDatabaseStack,selectRedisStack, and the enable-default logicCloses #163