Closed PatMyron closed 1 year ago
Thanks @PatMyron!
We have a lot to learn around AWS, so pointers like this are much appreciated. As for the PR, I'd prefer to see the existing logic for this updated to use the us-east-1
region rather than having an entirely new cron job file added.
Main reason I added a new cron job was to also open a PR with https://github.com/peter-evans/create-pull-request instead of just failing when there are new resource types
@PatMyron that seems sensible. Would you consider updating the current job to include that? We'd need to drop the exit code from the script as well so it won't fail on changes. Thanks!
EDIT: Oh, and of course, update the region 😄
Updated the region in the pre-existing logic, but it's been failing since opa
isn't pre-installed like jq
on ubuntu-latest
Also thinking the couple jq
lines are a more general example for other CloudFormation Hooks to copy as well
Oh, that's embarrassing 😅 I had forgotten to add the setup-opa
task to that job (it was there for the tests only). I've now corrected that. Since we're maintainers of the OPA project after all, we tend to prefer dogfooding our own tools rather than relying on external ones, at least unless there's some particular reason not to.
Another "interesting" observation I made while playing around with the Java version of a hook is that they've capped the size of the config file there to 100K... meaning I had to remove some resource types even when using us-west one :( I wonder if they'll do that for Python too eventually?
Shame I didn't see that documented, but makes sense there'd be some limit.. and I'd be surprised if it's intentionally different between languages
Can also reduce whitespace indentation with jq
. Simply reducing indentation from 2 to 1 should remove ~1/4 of JSON file size while still preserving indentation formatting. Beyond that, another ~1/4 can be knocked off by removing all whitespace if it grows too much before wildcard support
Shame I didn't see that documented anywhere, but makes sense there'd be some limit.. and I'd be surprised if it's intentionally different between languages
Agreed, although I hope we can get rid of it so we won't need to retort to tricks like minimizing the conf 😅 (although that's smart, I hadn't thought of that). Maybe @jimmyraywv can shed some light on that issue.
@PatMyron and @anderseknert below are the only quotas I see wrt hooks:
Category | Quota |
---|---|
Hooks per account | 100 |
Hooks per resource | 100 |
Versions per hook | 100 |
Hook configuration size | 300KB |
I will try to gain additional insight into any language differences.
That's useful, thanks Jimmy 👍 Probably just a mistake made in the Java hook then. I mean, at least to me I can't see a reason why they'd differ between languages.
Can just get rid of this whole workflow now that wildcards are supported:
Oooh, nice! I had missed that entirely. Do you want to update the PR, or should I fix our resource configuration to use this?
feel free to take it from here, I'll close this PR since it's no longer needed: https://github.com/StyraInc/opa-aws-cloudformation-hook/pull/43
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-resource-specification.html
https://github.com/iann0036/cfn-hooks/pull/1, #6, https://github.com/StyraInc/opa-aws-cloudformation-hook/pull/28
automated PR strictly added 654+ lines of resource types when tested in my fork: https://github.com/PatMyron/opa-aws-cloudformation-hook/pull/1