POC/Provisioning: Allow saving valid, but unhealthy configurations (#97899)

Co-authored-by: Mariell Hoversholm <mariell.hoversholm@grafana.com>

pkg/registry/apis/provisioning/register.go

@@ -5,7 +5,6 @@ import (
 	"crypto/hmac"
 	"crypto/sha256"
 	"encoding/hex"
-	"errors"
 	"fmt"
 	"log/slog"
 	"net/url"
@@ -48,9 +47,8 @@ const (
 )
 
 var (
-	_                          builder.APIGroupBuilder = (*ProvisioningAPIBuilder)(nil)
-	_                          RepoGetter              = (*ProvisioningAPIBuilder)(nil)
-	ErrLocalRepositoryDisabled                         = errors.New("the local repository type has been disabled due to having no permitted local paths")
+	_ builder.APIGroupBuilder = (*ProvisioningAPIBuilder)(nil)
+	_ RepoGetter              = (*ProvisioningAPIBuilder)(nil)
 )
 
 // This is used just so wire has something unique to return
@@ -247,10 +245,7 @@ func (b *ProvisioningAPIBuilder) asRepository(ctx context.Context, obj runtime.O
 func (b *ProvisioningAPIBuilder) AsRepository(ctx context.Context, r *provisioning.Repository) (repository.Repository, error) {
 	switch r.Spec.Type {
 	case provisioning.LocalRepositoryType:
-		if len(b.localFileResolver.PermittedPrefixes) == 0 {
-			return nil, ErrLocalRepositoryDisabled
-		}
-		return repository.NewLocal(r, b.localFileResolver)
+		return repository.NewLocal(r, b.localFileResolver), nil
 	case provisioning.GitHubRepositoryType:
 		baseURL, err := url.Parse(b.urlProvider(r.GetNamespace()))
 		if err != nil {

pkg/registry/apis/provisioning/repository/local.go

@@ -81,24 +81,22 @@ func (r *LocalFolderResolver) LocalPath(p string) (string, error) {
 var _ Repository = (*localRepository)(nil)
 
 type localRepository struct {
-	config *provisioning.Repository
+	config   *provisioning.Repository
+	resolver *LocalFolderResolver
 
 	// validated path that can be read if not empty
 	path string
 }
 
-func NewLocal(config *provisioning.Repository, resolver *LocalFolderResolver) (*localRepository, error) {
+func NewLocal(config *provisioning.Repository, resolver *LocalFolderResolver) *localRepository {
 	r := &localRepository{
-		config: config,
+		config:   config,
+		resolver: resolver,
 	}
 	if config.Spec.Local != nil {
-		var err error
-		r.path, err = resolver.LocalPath(config.Spec.Local.Path)
-		if err != nil {
-			return nil, err
-		}
+		r.path, _ = resolver.LocalPath(config.Spec.Local.Path)
 	}
-	return r, nil
+	return r
 }
 
 func (r *localRepository) Config() *provisioning.Repository {
@@ -107,14 +105,6 @@ func (r *localRepository) Config() *provisioning.Repository {
 
 // Validate implements provisioning.Repository.
 func (r *localRepository) Validate() (fields field.ErrorList) {
-	if r.config.Spec.Type != provisioning.LocalRepositoryType {
-		fields = append(fields, &field.Error{
-			Type:   field.ErrorTypeInvalid,
-			Field:  "spec.type",
-			Detail: "Local repository requires spec.type=local",
-		})
-	}
-
 	cfg := r.config.Spec.Local
 	if cfg == nil {
 		fields = append(fields, &field.Error{
@@ -128,9 +118,6 @@ func (r *localRepository) Validate() (fields field.ErrorList) {
 	if cfg.Path == "" {
 		fields = append(fields, field.Required(field.NewPath("spec", "local", "path"),
 			"must enter a path to local file"))
-	} else if r.path == "" {
-		fields = append(fields, field.Invalid(field.NewPath("spec", "local", "path"),
-			cfg.Path, "configured path is not allowed, see system allow list"))
 	}
 
 	return fields
@@ -139,23 +126,34 @@ func (r *localRepository) Validate() (fields field.ErrorList) {
 // Test implements provisioning.Repository.
 // NOTE: Validate has been called (and passed) before this function should be called
 func (r *localRepository) Test(ctx context.Context, logger *slog.Logger) (*provisioning.TestResults, error) {
-	if r.path == "" {
+	if r.config.Spec.Local.Path == "" {
 		return &provisioning.TestResults{
 			Code:    http.StatusBadRequest,
 			Success: false,
 			Errors: []string{
-				fmt.Sprintf("invalid path: %s", r.config.Spec.Local.Path),
+				"no path is configured",
 			},
 		}, nil
 	}
 
-	_, err := os.Stat(r.path)
+	_, err := r.resolver.LocalPath(r.config.Spec.Local.Path)
+	if err != nil {
+		return &provisioning.TestResults{
+			Code:    http.StatusBadRequest,
+			Success: false,
+			Errors: []string{
+				err.Error(),
+			},
+		}, nil
+	}
+
+	_, err = os.Stat(r.path)
 	if errors.Is(err, os.ErrNotExist) {
 		return &provisioning.TestResults{
 			Code:    http.StatusBadRequest,
 			Success: false,
 			Errors: []string{
-				fmt.Sprintf("file not found: %s", r.config.Spec.Local.Path),
+				fmt.Sprintf("directory not found: %s", r.config.Spec.Local.Path),
 			},
 		}, nil
 	}
@@ -172,6 +170,10 @@ func (r *localRepository) validateRequest(ref string) error {
 		return apierrors.NewBadRequest("local repository does not support ref")
 	}
 	if r.path == "" {
+		_, err := r.resolver.LocalPath(r.config.Spec.Local.Path)
+		if err != nil {
+			return err
+		}
 		return &apierrors.StatusError{
 			ErrStatus: metav1.Status{
 				Message: "the service is missing a root path",

pkg/registry/apis/provisioning/repository/local_test.go

@@ -0,0 +1,54 @@
+package repository
+
+import (
+	"context"
+	"log/slog"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
+)
+
+func TestLocalResolver(t *testing.T) {
+	resolver := &LocalFolderResolver{
+		PermittedPrefixes: []string{
+			"github/testdata",
+		},
+		HomePath: "",
+	}
+	_, err := resolver.LocalPath("github/testdata")
+	require.NoError(t, err)
+
+	_, err = resolver.LocalPath("something")
+	require.Error(t, err)
+}
+
+func TestLocal(t *testing.T) {
+	t.Run("invalid path", func(t *testing.T) {
+		r := NewLocal(&v0alpha1.Repository{
+			Spec: v0alpha1.RepositorySpec{
+				Local: &v0alpha1.LocalRepositoryConfig{
+					Path: "invalid/path",
+				},
+			},
+		}, &LocalFolderResolver{})
+
+		// Did not resolve a local path
+		require.Equal(t, "", r.path)
+
+		// The correct fields are set
+		require.Nil(t, r.Validate())
+
+		expected := "the path given ('invalid/path') is invalid for a local repository (no permitted prefixes were configured)"
+
+		rsp, err := r.Test(context.Background(), slog.Default())
+		require.NoError(t, err)
+		require.Equal(t, false, rsp.Success)
+		require.Equal(t, []string{expected}, rsp.Errors)
+
+		// We get the same error when trying to read a file
+		_, err = r.Read(context.Background(), slog.Default(), "path/to/file", "")
+		require.Equal(t, expected, err.Error())
+	})
+}

pkg/registry/apis/provisioning/types.go

@@ -12,5 +12,7 @@ type RepoGetter interface {
 	GetRepository(ctx context.Context, name string) (repository.Repository, error)
 
 	// Given a repository configuration, return it as a repository instance
+	// This will only error for un-recoverable system errors
+	// the repository instance may or may not be valid/healthy
 	AsRepository(ctx context.Context, cfg *provisioning.Repository) (repository.Repository, error)
 }