That is inconvenient to use. Should be much shorter with something like :
I think we already do have a Label type somewhere. I think it would be more appropriate here than String. That's true for all the label related fields in this file.
I guess operator is not any string? Probably should be an enum too.
Not a String, should be a path type of some sort. I think we already handle this somewhere else correctly.
Yeah ok, I see what you did there.
But this is not a good use case for Macros. Macros are a last resort when the type semantics are already correct and we can't rely on the regular rust type…
OK that makes sense. Why not implement Display? I guess the format is not the expected one?
Also this file really deserves some tests to make sure the config output is correct.
What's the use case for this? I don't see this used in this pr.
Is this really Dell specific?
Looks pretty good overall. Some minor refactoring comments but the rest is great!
For clarity, this is fine-ish to be in the kube_prometheus mod because it is specifically for pvc alerts but I think alert definitions should be in another module called just prometheus. kube_prometheus is for stuff specific to deploying prometheus on k8s. It is very possible to have a prometheus deployed somewhere else (AWS managed or grafana cloud maybe) which scrapes k8s targets and will want this alert.
Nothing in this file seems Dell specific?