Add ruler tenant federation - #7828
SungJin1212 wants to merge 5 commits into
Conversation
Sorry, something went wrong.
friedrichg
left a comment
There was a problem hiding this comment.
It's a good start. I have only one nit
Sorry, something went wrong.
friedrichg
left a comment
There was a problem hiding this comment.
Looks actually really good. One question only I found
Sorry, something went wrong.
| // not own them, e.g. because the feature was disabled after they were stored. | ||
| func (r *DefaultMultiTenantManager) filterFederatedRuleGroups(userID string, groups rulespb.RuleGroupList) rulespb.RuleGroupList { | ||
| ownerErr := r.federatedRules.checkOwner(userID) | ||
| if ownerErr == nil { |
There was a problem hiding this comment.
If checkOwner returns no error we should not exit the function: the owner being
allowed to have federated rule groups says nothing about whether these particular
source tenants are still acceptable under the current tenant federation config.
We should validate the source tenants here too. (Probably using r.federatedRules.validateSourceTenants)
Imagine scenarios where regex-matcher-enabled or max-tenant has changed
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for catching it. I validated the source tenants in filterFederatedRuleGroups.
Sorry, something went wrong.
This comment was marked as duplicate.
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
… and regex constraints Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
There was a problem hiding this comment.
Probably the only thing missing is that all tenants are allowed to access all tenants. Someone could create a recording rule to read any other tenant.
Some configuration in the tenants overrides could authorize only specific accesses. it can be done in a follow up PR
Sorry, something went wrong.
Implement the federated ruler proposal. A rule group listing tenants in its src_tenants field is evaluated against those tenants, while the resulting series, ALERTS series and notifications are written to the tenant owning the rule group.
This feature behind -ruler.enable-federated-rules and restrict the owning tenants with -ruler.allowed-federated-tenants and -ruler.disallowed-federated-tenants.
Which issue(s) this PR fixes:
Fixes #
Checklist