Skip to content

Validate user-configurable overrides#2682

Merged
yvrhdn merged 6 commits intografana:mainfrom
yvrhdn:kvrhdn/overrides-validation
Jul 24, 2023
Merged

Validate user-configurable overrides#2682
yvrhdn merged 6 commits intografana:mainfrom
yvrhdn:kvrhdn/overrides-validation

Conversation

@yvrhdn
Copy link
Copy Markdown
Contributor

@yvrhdn yvrhdn commented Jul 20, 2023

What this PR does:
Validate the user-configurable overrides before storing them in the backend.

I've also split off overrides-api into a separate module to make matters more explicit. This also opens the way to running the override-api as a separate component.

Note this will conflict slightly with #2681. I can address merge conflicts when either PR gets merged.

Which issue(s) this PR fixes:
Fixes #

Checklist

Copy link
Copy Markdown
Contributor

@mapno mapno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread cmd/tempo/app/overrides_validation.go Outdated

if limits.Forwarders != nil {
for _, f := range *limits.Forwarders {
if !slices.Contains(validForwarders, f) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: a map would be more efficient

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I've updated the implementation :)

@yvrhdn yvrhdn merged commit 0e65852 into grafana:main Jul 24, 2023
@yvrhdn yvrhdn deleted the kvrhdn/overrides-validation branch July 24, 2023 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants