Repository navigation
fix(updater): check the deploy role recorded at prepare, not DescribeChangeSet - #478
Merged
Merged
Conversation
…ChangeSet DescribeChangeSet's output has no RoleARN field, so apply compared None with the deploy role and refused every in-app Confirm and apply, even for change sets created with RoleARN. Prepare now records the role ARN it passed to CreateChangeSet on the update's entry (role_arn), and apply compares it with DEPLOY_ROLE_ARN. Entries from older updaters without role_arn count as made with the role when apply is set (only the role makes it true). Apply also checks the described change set id is the entry's. The review summary's role flag comes from the entry too. The test fake now refuses to return RoleARN, matching the real API.
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
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.
Summary
Every in-app Confirm and apply of a prepared stack update was refused with "this update was prepared without the deploy role", even when the updater had
DEPLOY_ROLE_ARNand the change set was created withRoleARN. Apply readRoleARNfromDescribeChangeSet, whose output has no such field (botocore's model lists: Capabilities, ChangeSetId, ChangeSetName, Changes, CreationTime, DeploymentMode, Description, ExecutionStatus, ImportExistingResources, IncludeNestedStacks, NextToken, NotificationARNs, OnStackFailure, Parameters, ParentChangeSetId, RollbackConfiguration, RootChangeSetId, StackDriftStatus, StackId, StackName, Status, StatusReason, Tags). So the check comparedNonewith the role and could never pass. The tests' fake returnedRoleARN, which hid it.Changes
native_proposerecords the role ARN it passes toCreateChangeSeton the update's entry asrole_arn(next to the existingapply).prepared_role(entry): the recorded role; for entries from older updaters with norole_arn,apply: truecounts as made with the deploy role (only the role makesapplytrue).native_applycompares that withDEPLOY_ROLE_ARNand refuses when the entry has no role or a different one. It also checks the describedChangeSetIdis the entry's.ExecuteChangeSetalready uses the role given at creation.change_set_summary'sroleflag comes from the entry rather than the missingRoleARNfield.FakeCfn.describe_change_setnow asserts it never returnsRoleARN(the real shape), and the test setups no longer set it. New tests: a change set prepared with the role applies; an older entry withapplyset and norole_arnapplies; an entry recording no role or another role is refused; a mismatched change set id is refused.Verification
python3 -m unittest discover -s lambda/tests(the CI Lambda job): 314 tests OK.handler.pyand these tests, 6 apply tests intest_updater_nativefail ("prepared without the deploy role"), every in-app apply included. That is the failure seen live.