-
-
Notifications
You must be signed in to change notification settings - Fork 323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ddos protection: Discover resources outside us-east-1 #1040
ddos protection: Discover resources outside us-east-1 #1040
Conversation
…lter implementations
func Test_Services_Have_Filters_In_V1_and_V2(t *testing.T) { | ||
for _, service := range config.SupportedServices { | ||
namespace := service.Namespace | ||
t.Run(fmt.Sprintf("%s has filter definitions in v1 and v2", namespace), func(t *testing.T) { | ||
v1Filters, v1Exists := v1.ServiceFilters[namespace] | ||
v2Filters, v2Exists := v2.ServiceFilters[namespace] | ||
|
||
require.Equal(t, v1Exists, v2Exists, "Service filters are only implemented for v1 or v2 but should be implemented for both") | ||
|
||
v1FilterFuncNil := v1Filters.FilterFunc == nil | ||
v2FilterFuncNil := v2Filters.FilterFunc == nil | ||
assert.Equal(t, v1FilterFuncNil, v2FilterFuncNil, "FilterFunc is only implemented for v1 or v2 but should be implemented for both") | ||
|
||
v1ResourceFuncNil := v1Filters.ResourceFunc == nil | ||
v2ResourceFuncNil := v2Filters.ResourceFunc == nil | ||
assert.Equal(t, v1ResourceFuncNil, v2ResourceFuncNil, "ResourceFunc is only implemented for v1 or v2 but should be implemented for both") | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I almost forgot to add a v1 implementation before opening this PR so I added this test for sanity. The side effect is that I had to export theServiceFilters
. We can always hide them again when sdk v1 is no longer supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 🚀 🚀
AWS Shield (
AWS/DDoSProtection
namespace in cloudwatch) has some non-standard scenarios when it comes to resources and metrics in cloudwatch. Shield allows you to create "global" protections and those global resources exist in us-east-1. Metrics for the global protection also end up in us-east-1 as noted by, https://docs.aws.amazon.com/waf/latest/developerguide/monitoring-cloudwatch.html.Shield Advanced reports metrics in the US East (N. Virginia) Region, us-east-1 for the following:
Resource discovery for
AWS/DDoSProtection
currently discovers the protection groups using the ResourceFilter"shield:protection"
. This setup allows for discovery of the global protection resources and any metrics associated with them. Attempting to pullAWS/DDoSProtection
metrics outside of us-east-1 is not possible because resource discovery does not work outside of us-east-1. The problem with this is thatAWS/DDoSProtection
also publishes metrics for all protected resources. It's unlikely those resources all exist in us-east-1 so you cannot pull anyAWS/DDoSProtection
metrics outside of us-east-1 as noted by, #800.After talking with @cristiangreco a bit about options we landed on using a custom
ResourceFunc
forAWS/DDoSProtection
. The implmentation uses theshield.ListProtections
API to discover protected resources outside of us-east-1. These resources are added to the collection ofTaggedResources
. The ARN is the protected resource arn and there's a tag for the protection arn the resource is associated with.This solves the initial issues as well as giving a path to associate metrics. Most of the
AWS/DDoSProtection
metrics include a Dimension,ResourceArn
, which is joinable to the new series produced by the customResourceFunc
. Using some label replace it's possible to join back to the protection info metric and see protection tags as well,Ideally, we could create a more simplified
aws_ddosprotection_info
series to avoid this but YACE wasn't built with this scenario in mind. I'll make a follow up ticket to describe the challenge and possible solution as solving this initial problem is complicated enough by itself.Resolves: #800