Use aws-codedeploy-hook. Remove preHook.ts and postHook.ts #1644
Conversation
🦋 Changeset detectedLatest commit: 49d7a55 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Thanks for tackling this one!
Any chance you could add a changeset?
which would allow the template consumer to do more specific smoke tests like checking for existence of a dynamodb table.
To your note on this, the idea of these pre-hooks is that they don't contain the logic themselves, but instead ask the lambda function being deployed to check. If you wanted to ensure dynamodb existence, that could be part of the worker's smokeTest function.
| @@ -15,3 +16,11 @@ new AppStack(app, 'appStack', { | |||
| // 'seek:system:name': 'TODO: add system name', | |||
| }, | |||
| }); | |||
|
|
|||
| // eslint-disable-next-line no-new | |||
| const hookStack = new HookStack(app, 'hookStack'); | |||
There was a problem hiding this comment.
This is an interesting one; if we're recommending deploying per-application, it probably needs a unique stack name / function names / etc baked in. I'm unsure on the best approach here!
There was a problem hiding this comment.
There's normally a from function you can use to reference existing resources but not sure that solves where the hooks are deployed
There was a problem hiding this comment.
Which function name are you referring to? Even if we allow HookStack to be created with different stack name (it is now hardcoded to aws-codedeploy-hooks), it will still fail if we try to deploy multiple HookStacks because lambda names are not unique. I feel like I am missing something here
Also, sorry that I am commenting after work hours, please enjoy your evening! 😄
There was a problem hiding this comment.
There was a problem hiding this comment.
I think just purely because it reduces the complexity of the template I'm happy to merge as is
There was a problem hiding this comment.
@AaronMoat, @samchungy. I have added that comment and also created a changeset. Using changeset for the first time. Let me know if I am not doing that right
There was a problem hiding this comment.
Thank you! @samchungy @72636c might be worth a look from you as well
Co-authored-by: Aaron Moat <2937187+AaronMoat@users.noreply.github.com>
Co-authored-by: Aaron Moat <2937187+AaronMoat@users.noreply.github.com>
|
Gonna force merge and hope for the best 🙈 |
Overview
Use
aws-codeploy-hooksand remove existingcodedeploycode. Resolves #1640Note
I am unsure if this approach should be used to replace current implementation in
skuba.skubacurrently createspreandposthooks that are not account-global which would allow the template consumer to do more specific smoke tests like checking for existence of adynamodbtable.By using
aws-codedeploy-hooksas it is now we lose that ability. Happy to iterateaws-codedeploy-hooksto support that or update this PR if I am just missing how to do that