diff --git a/pkg/registry/apis/provisioning/resources/dualwriter.go b/pkg/registry/apis/provisioning/resources/dualwriter.go index f30920d4e8a..a1719d16ece 100644 --- a/pkg/registry/apis/provisioning/resources/dualwriter.go +++ b/pkg/registry/apis/provisioning/resources/dualwriter.go @@ -86,6 +86,8 @@ func (r *DualReadWriter) Delete(ctx context.Context, path string, ref string, me // Delete the file in the grafana database if ref == "" { + // FIXME: empty folders with no repository files will remain in the system + // until the next reconciliation. err = parsed.Client.Delete(ctx, parsed.Obj.GetName(), metav1.DeleteOptions{}) if apierrors.IsNotFound(err) { err = nil // ignorable @@ -193,19 +195,11 @@ func (r *DualReadWriter) CreateResource(ctx context.Context, path string, ref st // Directly update the grafana database // Behaves the same running sync after writing + // FIXME: to make sure if behaves in the same way as in sync, we should + // we should refactor the code to use the same function. if ref == "" { - // FIXME: we are not creating the folder path - // TODO: will existing also be present here? for update? - if parsed.Existing == nil { - parsed.Upsert, err = parsed.Client.Create(ctx, parsed.Obj, metav1.CreateOptions{}) - if err != nil { - parsed.Errors = append(parsed.Errors, err) - } - } else { - parsed.Upsert, err = parsed.Client.Update(ctx, parsed.Obj, metav1.UpdateOptions{}) - if err != nil { - parsed.Errors = append(parsed.Errors, err) - } + if err := r.writeParsed(ctx, path, parsed); err != nil { + parsed.Errors = append(parsed.Errors, err) } } @@ -257,21 +251,36 @@ func (r *DualReadWriter) UpdateResource(ctx context.Context, path string, ref st // Directly update the grafana database // Behaves the same running sync after writing + // FIXME: to make sure if behaves in the same way as in sync, we should + // we should refactor the code to use the same function. if ref == "" { - // FIXME: we are not creating the folder path - // FIXME: I don't like this parsed strategy here - if parsed.Existing == nil { - parsed.Upsert, err = parsed.Client.Create(ctx, parsed.Obj, metav1.CreateOptions{}) - if err != nil { - parsed.Errors = append(parsed.Errors, err) - } - } else { - parsed.Upsert, err = parsed.Client.Update(ctx, parsed.Obj, metav1.UpdateOptions{}) - if err != nil { - parsed.Errors = append(parsed.Errors, err) - } + if err := r.writeParsed(ctx, path, parsed); err != nil { + parsed.Errors = append(parsed.Errors, err) } } return parsed, err } + +// writeParsed write parsed resource to the repository and grafana database +func (r *DualReadWriter) writeParsed(ctx context.Context, path string, parsed *ParsedResource) error { + if _, err := r.folders.EnsureFolderPathExist(ctx, path); err != nil { + return fmt.Errorf("ensure folder path exists: %w", err) + } + + // FIXME: I don't like this parsed strategy here + var err error + if parsed.Existing == nil { + parsed.Upsert, err = parsed.Client.Create(ctx, parsed.Obj, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("create resource: %w", err) + } + } else { + parsed.Upsert, err = parsed.Client.Update(ctx, parsed.Obj, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("update resource: %w", err) + } + } + + return nil +} diff --git a/pkg/registry/apis/provisioning/resources/id.go b/pkg/registry/apis/provisioning/resources/id.go index b9d14734c10..66ebc82329b 100644 --- a/pkg/registry/apis/provisioning/resources/id.go +++ b/pkg/registry/apis/provisioning/resources/id.go @@ -92,12 +92,12 @@ func appendHashSuffix(hashKey, repositoryName string) func(string) string { } // Will pick a name based on the hashed repository and path -func NamesFromHashedRepoPath(repo string, fpath string) (string, string) { +func FileNameFromHashedRepoPath(repo string, fpath string) string { // Remove the extension: we don't want the extension to impact the ID. This lets the user change between all supported formats. fpath = safepath.RemoveExt(fpath) hasher := appendHashSuffix(fpath, repo) - return hasher(safepath.Base(fpath)), hasher(strings.Trim(safepath.Dir(fpath), "/")) + return hasher(safepath.Base(fpath)) } // Folder contains the data for a folder we use in provisioning. diff --git a/pkg/registry/apis/provisioning/resources/id_test.go b/pkg/registry/apis/provisioning/resources/id_test.go index 77c319b72a1..8aa681b876f 100644 --- a/pkg/registry/apis/provisioning/resources/id_test.go +++ b/pkg/registry/apis/provisioning/resources/id_test.go @@ -67,18 +67,11 @@ func TestAppendHashSuffix(t *testing.T) { } } -func TestNamesFromHashedRepoPath(t *testing.T) { - dashName, folderName := NamesFromHashedRepoPath("xyz", "path/to/folder/dashboard.json") +func TestFileNameFromHashedRepoPath(t *testing.T) { + dashName := FileNameFromHashedRepoPath("xyz", "path/to/folder/dashboard.json") assert.Equal(t, "dashboard-fy2kflbmskvt6u-9uecoahd1ekwbb7", dashName, "dashboard name of dashboard.json") - assert.Equal(t, "path-to-folder-3ehfurpmbvs4yxfp7a0r2uskr", folderName, "folder name of dashboard.json") // We only want 40 characters because UIDs support no more. When we get rid of legacy storage, we can extend the support to 253 character long strings. assert.LessOrEqual(t, len(dashName), 40, "dashName after hashing needs to be <=40 chars long") - assert.LessOrEqual(t, len(folderName), 40, "folderName after hashing needs to be <=40 chars long") - - name1, f1 := NamesFromHashedRepoPath("xyz", "path/to/folder.json") - name2, f2 := NamesFromHashedRepoPath("xyz", "path/to/folder") - assert.Equal(t, name1, name2, "folder.json should have same object name as folder (no extension)") - assert.Equal(t, f1, f2, "folder.json and folder should have same folder name as each other") } func TestParseFolderID(t *testing.T) { diff --git a/pkg/registry/apis/provisioning/resources/parser.go b/pkg/registry/apis/provisioning/resources/parser.go index e37227bc4a1..c38c638a37d 100644 --- a/pkg/registry/apis/provisioning/resources/parser.go +++ b/pkg/registry/apis/provisioning/resources/parser.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/utils" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/safepath" ) var ( @@ -169,11 +170,15 @@ func (r *Parser) Parse(ctx context.Context, info *repository.FileInfo, validate // Calculate name+folder from the file path if info.Path != "" { - objName, folderName := NamesFromHashedRepoPath(r.repo.Name, info.Path) - parsed.Meta.SetFolder(folderName) + objName := FileNameFromHashedRepoPath(r.repo.Name, info.Path) if obj.GetName() == "" { obj.SetName(objName) // use the name saved in config } + + dirPath := safepath.Dir(info.Path) + if dirPath != "" { + parsed.Meta.SetFolder(ParseFolder(dirPath, r.repo.Name).ID) + } } obj.SetUID("") // clear identifiers obj.SetResourceVersion("") // clear identifiers diff --git a/pkg/registry/apis/provisioning/resources/resources.go b/pkg/registry/apis/provisioning/resources/resources.go index 6a7660deebe..b1899a4865d 100644 --- a/pkg/registry/apis/provisioning/resources/resources.go +++ b/pkg/registry/apis/provisioning/resources/resources.go @@ -189,7 +189,7 @@ func (r *ResourcesManager) RemoveResourceFromFile(ctx context.Context, path stri objName := obj.GetName() if objName == "" { // Find the referenced file - objName, _ = NamesFromHashedRepoPath(r.repo.Config().Name, path) + objName = FileNameFromHashedRepoPath(r.repo.Config().Name, path) } client, _, err := r.clients.ForKind(*gvk)