Change change set --confirm flag to --no-confirm

[andyfase]

Currently when using the sync option change-sets are produced but by default they are immediately applied with no ability to review the change-sets. The option --confirm exists but its optional and often will be forgotten about and thus stacks updated without time for review.

This seems counter intuitive to the purpose of change sets which exist so that safe, reviewed changes can be made. This PR reverses the flag - changing it to --no-confirm hence by default cfn-cli stack sync will wait for user confirmation before applying the change-set

Understand this may be a controversial PR but I honestly believe its the way 99% of users would expect change-sets to work.

[Kotaimen]

hi @andyfase , I agree it's better default to --confirm, however this changes break the cli compatibility of existing deployments. I'm suggesting adding --confirm and --no-confirmnow and change to--confirm` as default after one or two releases.

[andyfase]

Hey @Kotaimen happy to have both flags but which takes priority?

Currently we have --confirm defaults to False --no-confirm I assume would also default to False

Hence If both not included what should occur? An alternative to changing the Flag is just to default --confirm to True, which would have the same impact if no flags are provided to wait for user-confirmation (the overall goal). Perhaps do that first? and then change the flag to --no-confirm later on?

Either option would impact the behaviour going forward, those that are using cfn-cli now without a flag would now have to confirm the change-set.

Let me know your thoughts - happy to adjust PR accordingly.

[GlieseRay]

I am suggesting the following way:

1. Keep default behavior and --confirm for now. Add --no-confirm (default to false) but always set it to true for now.
2. Show warning message for users who is using --confirm and tell them that we are going to deprecate the --confirm option and change default behavior. They need to explicitly add --no-confirm if they are using current default behavior in the future.
3. Some releases later, deprecate --confirm and change default behavior.

What do you think?

[andyfase]

Hey sorry @GlieseRay was away for a few days.

Happy to have both flags. So just to confirm your suggesting --no-confirm is added and set to True which for now can be overridden by the --confirm flag if set to True (default False). Is that correct? is so i'll change and update the PR.

For the warning message I would think you would need one for all usage of sync as the behaviour will change in later releases both both cases, if your not using the --confirm flag and you want no confirmation you will have to add one later i.e. --no-confirm and if you are using the flag (because you want user confirmation) you will need to remove the confirm flag whenever the later release is

Hence suggest Warning for sync in general?

[GlieseRay]

@andyfase Yes Step1: keep default behavior and add --no-confirm default to true, --confirm will override --no-confirm Step2: Warning user that he need to add --no-confirm explicitly if they want to keep existing behavior in the future Step3: Remove --confirm and set --no-confirm default to false, so that in the future confirmation is required by default.

Hope I made it clear. :-)