-
Notifications
You must be signed in to change notification settings - Fork 486
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
common/relabel: update validation rules from Prometheus #5566
Conversation
Signed-off-by: Paschalis Tsilias <[email protected]>
@@ -146,16 +146,13 @@ func (rc *Config) Validate() error { | |||
if rc.Modulus == 0 && rc.Action == HashMod { | |||
return fmt.Errorf("relabel configuration for hashmod requires non-zero modulus") | |||
} | |||
if (rc.Action == Replace || rc.Action == KeepEqual || rc.Action == DropEqual || rc.Action == HashMod || rc.Action == Lowercase || rc.Action == Uppercase) && rc.TargetLabel == "" { | |||
if (rc.Action == Replace || rc.Action == HashMod || rc.Action == Lowercase || rc.Action == Uppercase || rc.Action == KeepEqual || rc.Action == DropEqual) && rc.TargetLabel == "" { |
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.
Re-ordering so it's easier to scan if the Prometheus code has changed something.
if (rc.Action == KeepEqual || rc.Action == DropEqual) && rc.SourceLabels == nil { | ||
return fmt.Errorf("relabel configuration for %s action requires 'source_labels' value", rc.Action) | ||
} |
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.
This never existed upstream, not sure why I'd added it tbh.
return fmt.Errorf("%q is invalid 'target_label' for %s action", rc.TargetLabel, rc.Action) | ||
} | ||
if (rc.Action == Lowercase || rc.Action == Uppercase) && rc.Replacement != DefaultRelabelConfig.Replacement { | ||
if (rc.Action == Lowercase || rc.Action == Uppercase || rc.Action == KeepEqual || rc.Action == DropEqual) && rc.Replacement != DefaultRelabelConfig.Replacement { |
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.
We were missing these two for this validation.
Signed-off-by: Paschalis Tsilias <[email protected]>
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.
LGTM, thanks!
PR Description
This PR updates the validation rules for all
*.relabel
blocks to be aligned with the validations in upstream Prometheus's UnmarshalYAML method.Which issue(s) this PR fixes
Fixes #4837
Notes to the Reviewer
I don't think there's something else to sync for loki.relabel since it's not really using much of Promtail code
PR Checklist