Provisioning: Ensure path to file exists in Files endpoints (#103343)

* Create ensure folder path exists

* Fix issue with folder names generated in 2 different ways
pull/103466/head
Roberto Jiménez Sánchez 4 months ago committed by GitHub
parent 17d075d81d
commit 800aa7827d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 57
      pkg/registry/apis/provisioning/resources/dualwriter.go
  2. 4
      pkg/registry/apis/provisioning/resources/id.go
  3. 11
      pkg/registry/apis/provisioning/resources/id_test.go
  4. 9
      pkg/registry/apis/provisioning/resources/parser.go
  5. 2
      pkg/registry/apis/provisioning/resources/resources.go

@ -86,6 +86,8 @@ func (r *DualReadWriter) Delete(ctx context.Context, path string, ref string, me
// Delete the file in the grafana database // Delete the file in the grafana database
if ref == "" { 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{}) err = parsed.Client.Delete(ctx, parsed.Obj.GetName(), metav1.DeleteOptions{})
if apierrors.IsNotFound(err) { if apierrors.IsNotFound(err) {
err = nil // ignorable err = nil // ignorable
@ -193,19 +195,11 @@ func (r *DualReadWriter) CreateResource(ctx context.Context, path string, ref st
// Directly update the grafana database // Directly update the grafana database
// Behaves the same running sync after writing // 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 == "" { if ref == "" {
// FIXME: we are not creating the folder path if err := r.writeParsed(ctx, path, parsed); err != nil {
// TODO: will existing also be present here? for update? parsed.Errors = append(parsed.Errors, err)
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)
}
} }
} }
@ -257,21 +251,36 @@ func (r *DualReadWriter) UpdateResource(ctx context.Context, path string, ref st
// Directly update the grafana database // Directly update the grafana database
// Behaves the same running sync after writing // 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 == "" { if ref == "" {
// FIXME: we are not creating the folder path if err := r.writeParsed(ctx, path, parsed); err != nil {
// FIXME: I don't like this parsed strategy here parsed.Errors = append(parsed.Errors, err)
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)
}
} }
} }
return parsed, 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
}

@ -92,12 +92,12 @@ func appendHashSuffix(hashKey, repositoryName string) func(string) string {
} }
// Will pick a name based on the hashed repository and path // 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. // 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) fpath = safepath.RemoveExt(fpath)
hasher := appendHashSuffix(fpath, repo) 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. // Folder contains the data for a folder we use in provisioning.

@ -67,18 +67,11 @@ func TestAppendHashSuffix(t *testing.T) {
} }
} }
func TestNamesFromHashedRepoPath(t *testing.T) { func TestFileNameFromHashedRepoPath(t *testing.T) {
dashName, folderName := NamesFromHashedRepoPath("xyz", "path/to/folder/dashboard.json") dashName := FileNameFromHashedRepoPath("xyz", "path/to/folder/dashboard.json")
assert.Equal(t, "dashboard-fy2kflbmskvt6u-9uecoahd1ekwbb7", dashName, "dashboard name of 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. // 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(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) { func TestParseFolderID(t *testing.T) {

@ -20,6 +20,7 @@ import (
"github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/apimachinery/utils"
provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" 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/repository"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/safepath"
) )
var ( var (
@ -169,11 +170,15 @@ func (r *Parser) Parse(ctx context.Context, info *repository.FileInfo, validate
// Calculate name+folder from the file path // Calculate name+folder from the file path
if info.Path != "" { if info.Path != "" {
objName, folderName := NamesFromHashedRepoPath(r.repo.Name, info.Path) objName := FileNameFromHashedRepoPath(r.repo.Name, info.Path)
parsed.Meta.SetFolder(folderName)
if obj.GetName() == "" { if obj.GetName() == "" {
obj.SetName(objName) // use the name saved in config 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.SetUID("") // clear identifiers
obj.SetResourceVersion("") // clear identifiers obj.SetResourceVersion("") // clear identifiers

@ -189,7 +189,7 @@ func (r *ResourcesManager) RemoveResourceFromFile(ctx context.Context, path stri
objName := obj.GetName() objName := obj.GetName()
if objName == "" { if objName == "" {
// Find the referenced file // Find the referenced file
objName, _ = NamesFromHashedRepoPath(r.repo.Config().Name, path) objName = FileNameFromHashedRepoPath(r.repo.Config().Name, path)
} }
client, _, err := r.clients.ForKind(*gvk) client, _, err := r.clients.ForKind(*gvk)

Loading…
Cancel
Save