Skip to content
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

Provisioning: Validate that dashboard providers have unique names #22898

Merged
merged 5 commits into from
Apr 14, 2020

Conversation

youshy
Copy link
Contributor

@youshy youshy commented Mar 19, 2020

What this PR does / why we need it:

This PR introduces a check if an array of *DashboardsAsConfig has only unique names

Which issue(s) this PR fixes:

Fixes #20421

(this is a clean version of #21676 which I've messed up beautifully)

@marefr :)

@youshy youshy requested a review from a team as a code owner March 19, 2020 14:23
@marefr marefr requested review from marefr and aknuds1 and removed request for a team March 19, 2020 14:26
@marefr marefr added area/backend pr/external This PR is from external contributor labels Mar 19, 2020
Copy link
Contributor

@bergquist bergquist left a comment

Choose a reason for hiding this comment

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

Looks good!

But I think we need a better way to handle this error when starting grafana. This is the error message I get when I have duplicate dashboard provisioners.

EROR[03-20|22:12:09] Failed to provision dashboard            logger=provisioning error="Failed to create provisioner: Failed to read dashboards config: could not parse provisioning config file: dev.yaml error: dashboards name \"gdev dashboards\" is not unique"
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x1482ce0]

goroutine 151 [running]:
github.com/grafana/grafana/pkg/services/provisioning.(*provisioningServiceImpl).Run(0xc00053a000, 0x1de04c0, 0xc000056140, 0x0, 0x0)
        /home/carl/go/src/github.com/grafana/grafana/pkg/services/provisioning/provisioning.go:89 +0x170
main.(*Server).Run.func1(0x0, 0x0)
        /home/carl/go/src/github.com/grafana/grafana/pkg/cmd/grafana-server/server.go:129 +0x60
golang.org/x/sync/errgroup.(*Group).Go.func1(0xc000398450, 0xc000a17200)
        /home/carl/go/src/github.com/grafana/grafana/vendor/golang.org/x/sync/errgroup/errgroup.go:57 +0x59
created by golang.org/x/sync/errgroup.(*Group).Go
        /home/carl/go/src/github.com/grafana/grafana/vendor/golang.org/x/sync/errgroup/errgroup.go:54 +0x66

Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

Just saw a typo.

pkg/services/provisioning/dashboards/types.go Outdated Show resolved Hide resolved
Co-Authored-By: Arve Knudsen <arve.knudsen@gmail.com>
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

LGTM code-wise

@marefr marefr requested a review from bergquist April 1, 2020 12:32
@marefr
Copy link
Member

marefr commented Apr 7, 2020

@bergquist pushed a fix for handling of error when starting grafana which will make Grafana server stop.

@bergquist bergquist self-assigned this Apr 8, 2020
@bergquist bergquist changed the title Fix: check if DashboardsAsConfig has unique names Provisioning: Validate that dashboard providers have unique names Apr 14, 2020
@bergquist bergquist added this to the 7.0 milestone Apr 14, 2020
Copy link
Contributor

@bergquist bergquist left a comment

Choose a reason for hiding this comment

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

LGTM!

@bergquist
Copy link
Contributor

Thank you for contributing to Grafana! :)

@bergquist bergquist merged commit 9f2af71 into grafana:master Apr 14, 2020
@youshy youshy deleted the 20421_fix_provisioner_name_clean branch April 20, 2020 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show an error when different provisioning providers have the same name
4 participants