chore(deps): upgrade @guardian/cdk to latest (8.2.0) #155
Conversation
akash1810
force-pushed
the
aa-gu-cdk-3.0.0
branch
from
9e0ed8b to
12e2e21
Compare
akash1810
changed the title
Looks like some @guardian/cdk constructs are not applying the App tag. I suspect since guardian/cdk#326. Until that is fixed, we can safely, manually apply it to all constructs in tree from `this` as it's a single app stack.
akash1810
force-pushed
the
aa-gu-cdk-3.0.0
branch
from
12e2e21 to
3ffc5bd
Compare
akash1810
changed the title
| /* | ||
| Looks like some @guardian/cdk constructs are not applying the App tag. | ||
| I suspect since https://github.com/guardian/cdk/pull/326. | ||
| Until that is fixed, we can safely, manually apply it to all constructs in tree from `this` as it's a single app stack. | ||
| TODO: remove this once @guardian/cdk has been fixed. | ||
| */ | ||
| AppIdentity.taggedConstruct(PrismAccess.app, this); |
There was a problem hiding this comment.
This can be seen in the snapshot diff on the first commit.
| @@ -573,7 +573,7 @@ dpkg -i /prism/prism.deb", | |||
| }, | |||
| "Type": "AWS::IAM::Role", | |||
| }, | |||
| "LoadBalancer": Object { | |||
| "LoadBalancerBE9EEC3A": Object { | |||
There was a problem hiding this comment.
@akash1810 - I think we'll need to set existingLogicalId on the load balancer to avoid a replacement here? I'm a bit confused as to how this logical id has been kept until now, as overrideId doesn't currently appear to be set 😕
There was a problem hiding this comment.
Argh! Good spot! Will add existingLogicalId.
I think this is the result of guardian/cdk#440. Looking at the removed tests, it looks like if migratedFromCloudFormation was true on the GuStack, the construct's logicalId was not auto-generated; and the value passed into the constructor was used.
I think this is another +1 for guardian/cdk#364 and encapsulating this logic in one place.
I'd also have hoped to see the message in the console, but then I realised I've been using Jest locally which sets NODE_ENV to test - oops guardian/cdk#397!
| "Properties": Object { | ||
| "GroupDescription": "Allow all outbound traffic on port 443", | ||
| "GroupDescription": "Allow all outbound HTTPS traffic", |
There was a problem hiding this comment.
This will result in a replacement 😢 . I wonder if guardian/cdk could send a warning on this event? Though maybe the major version number change is enough of a signal?
There was a problem hiding this comment.
Hmm, good point. We have (at least) two different types of breaking change:
- We've changed the API and you'll need to update some code, but it shouldn't cause any breaking changes to the underlying infrastructure
- We've changed something which will have a significant impact on the underlying infrastructure (e.g. replacing a security group, updating load balancer version)
I think everyone will be familiar with type 1, but I wonder if a major version bump will be enough to prepare them for changes of type 2... hopefully these will be rare events once we have our production-ready version though!
It looks like we were relying on odd behaviour before. See: - guardian/cdk#440 - guardian/cdk@0d0eba7#diff-ddcd012c9ad844faf4c3b865a658f5e3e600d62ad88228ca022798def7c56c9dL33
akash1810
force-pushed
the
aa-gu-cdk-3.0.0
branch
from
a7102d3 to
c2edecd
Compare
|
This change updates security groups. This is normally tricky when they are in-use by instances as the instances need to be cycled out of service. This manifests itself like this in the CFN event log:
When this happens, I usually perform an app only deploy to cycle the current instances out and launch new ones. I think I did this too early on CODE, which lead to a few issues. That is, I didn't wait for the
I've deployed the CFN change manually to PROD, waiting for the |
Ah this makes sense now! Thanks for explaining! |
What does this change?
Updates to @guardian/cdk v8.2.0 🎉 . This has got a number of breaking changes (full diff here).
We'd need to manually apply the CFN template as the following parameters have been renamed (hopefully for the last time!):
How to test
n/a
How can we measure success?
Keeping up to date.
Have we considered potential risks?
n/a
Images
n/a