search

remind101 / empire

A PaaS built on top of Amazon EC2 Container Service (ECS)
BSD 2-Clause "Simplified" License
2.69k stars 159 forks source link

Workaround new long ARNs #1165

Closed tyrken closed 3 years ago

tyrken commented 4 years ago

This patch should work around the new long ARNs that AWS are creating by default now (not next year as I had thought in https://github.com/remind101/empire/issues/1156). It seems if you don't explicitly opt-out of the long ARNs you get them now, but you can opt out again manually.

We're actually using this server-side fix rebased to an older empire version, but it should also work in latest master I hope...

sonarcloud[bot] commented 4 years ago

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

oliviagunton commented 3 years ago

Hello all (hi @aengelas, nice to see a familiar face here!)

Any chance we could get this approved and merged? AWS is removing the ability to opt out of the long ARN format at the end of this month.

aengelas commented 3 years ago

Hey Olivia :-) Unfortunately, CircleCI won't build this because no one was following it it :-/ I fixed that; @tyrken any chance you can rebase and re-push this PR to trigger a new CircleCI build?

If we don't get a response, I can make a new a new PR with the changes.

isobit commented 3 years ago

Looks good to me, but I just wanted to point out that this change means empire extracts <cluster>/<name> as service/task names from ARNs rather than just <name>. Luckily the ECS API accepts that for serviceName etc., but as far as I can tell that's not documented.

For example, ListTasksPages(Cluster: "cluster", ServiceName: "acme-inc-web") will become ListTasksPages(Cluster: "cluster", ServiceName: "cluster/acme-inc-web")

This isn't strictly necessary but it might also be worth updating the tests in cloudformation_test.go to use the long ARN format instead, since that'll be the only format going forward.