public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore: add validations to test endpoint (#111622)

Browse Source 
* chore: add validations to test endpoint

* Validate path

---------

Co-authored-by: Clarity-89 <homes89@ukr.net>
This commit is contained in:
Costa Alexoglou
2025-09-25 15:10:13 +00:00
committed by GitHub
co-authored by Clarity-89
parent 054e12b1ac
commit 31ae013e8d
8 changed files with 185 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/provisioning/pkg/repository/test.go
+30 -1
View File
@@ -12,7 +12,16 @@ import (
provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1"
)
// RepositoryValidator interface for validating repositories against existing ones
type RepositoryValidator interface {
VerifyAgainstExistingRepositories(ctx context.Context, cfg *provisioning.Repository) *field.Error
}
func TestRepository(ctx context.Context, repo Repository) (*provisioning.TestResults, error) {
return TestRepositoryWithValidator(ctx, repo, nil)
}
func TestRepositoryWithValidator(ctx context.Context, repo Repository, validator RepositoryValidator) (*provisioning.TestResults, error) {
errors := ValidateRepository(repo)
if len(errors) > 0 {
rsp := &provisioning.TestResults{
@@ -30,7 +39,27 @@ func TestRepository(ctx context.Context, repo Repository) (*provisioning.TestRes
return rsp, nil
}
return repo.Test(ctx)
rsp, err := repo.Test(ctx)
if err != nil {
return nil, err
}
if rsp.Success && validator != nil {
cfg := repo.Config()
if validationErr := validator.VerifyAgainstExistingRepositories(ctx, cfg); validationErr != nil {
rsp = &provisioning.TestResults{
Success: false,
Code: http.StatusUnprocessableEntity,
Errors: []provisioning.ErrorDetails{{
Type: metav1.CauseType(validationErr.Type),
Field: validationErr.Field,
Detail: validationErr.Detail,
}},
}
}
}
return rsp, nil
}
func ValidateRepository(repo Repository) field.ErrorList {
pkg/registry/apis/provisioning/register.go
+12 -105
View File
@@ -6,14 +6,12 @@ import (
"fmt"
"net/http"
"net/url"
"path/filepath"
"slices"
"strings"
"time"
"github.com/prometheus/client_golang/prometheus"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/internalversion"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
@@ -21,7 +19,6 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
genericapiserver "k8s.io/apiserver/pkg/server"
clientrest "k8s.io/client-go/rest"
@@ -478,7 +475,7 @@ func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupI
storage[provisioning.RepositoryResourceInfo.StoragePath("status")] = repositoryStatusStorage
// TODO: Add some logic so that the connectors can registered themselves and we don't have logic all over the place
storage[provisioning.RepositoryResourceInfo.StoragePath("test")] = NewTestConnector(b, b.repoFactory, b)
storage[provisioning.RepositoryResourceInfo.StoragePath("test")] = NewTestConnector(b)
storage[provisioning.RepositoryResourceInfo.StoragePath("files")] = NewFilesConnector(b, b.parsers, b.clients, b.access)
storage[provisioning.RepositoryResourceInfo.StoragePath("refs")] = NewRefsConnector(b)
storage[provisioning.RepositoryResourceInfo.StoragePath("resources")] = &listConnector{
@@ -622,7 +619,7 @@ func (b *APIBuilder) Validate(ctx context.Context, a admission.Attributes, o adm
}
// Exit early if we have already found errors
targetError := b.verifyAgainstExistingRepositories(cfg)
targetError := b.VerifyAgainstExistingRepositories(ctx, cfg)
if targetError != nil {
return invalidRepositoryError(a.GetName(), field.ErrorList{targetError})
}
@@ -636,105 +633,8 @@ func invalidRepositoryError(name string, list field.ErrorList) error {
name, list)
}
func (b *APIBuilder) getRepositoriesInNamespace(ctx context.Context) ([]provisioning.Repository, error) {
var allRepositories []provisioning.Repository
continueToken := ""
for {
obj, err := b.store.List(ctx, &internalversion.ListOptions{
Limit: 100,
Continue: continueToken,
})
if err != nil {
return nil, err
}
repositoryList, ok := obj.(*provisioning.RepositoryList)
if !ok {
return nil, fmt.Errorf("expected repository list")
}
allRepositories = append(allRepositories, repositoryList.Items...)
continueToken = repositoryList.GetContinue()
if continueToken == "" {
break
}
}
return allRepositories, nil
}
// TODO: move this to a more appropriate place. Probably controller/validation.go
func (b *APIBuilder) verifyAgainstExistingRepositories(cfg *provisioning.Repository) *field.Error {
ctx, _, err := identity.WithProvisioningIdentity(context.Background(), cfg.Namespace)
if err != nil {
return &field.Error{Type: field.ErrorTypeInternal, Detail: err.Error()}
}
all, err := b.getRepositoriesInNamespace(request.WithNamespace(ctx, cfg.Namespace))
if err != nil {
return field.Forbidden(field.NewPath("spec"),
"Unable to verify root target: "+err.Error())
}
if cfg.Spec.Sync.Target == provisioning.SyncTargetTypeInstance {
// Instance sync can only be created if NO other repositories exist
for _, v := range all {
if v.Name != cfg.Name {
return field.Forbidden(field.NewPath("spec", "sync", "target"),
"Instance repository can only be created when no other repositories exist. Found: "+v.Name)
}
}
} else {
// Folder sync cannot be created if an instance repository exists
for _, v := range all {
if v.Spec.Sync.Target == provisioning.SyncTargetTypeInstance && v.Name != cfg.Name {
return field.Forbidden(field.NewPath("spec", "sync", "target"),
"Cannot create folder repository when instance repository exists: "+v.Name)
}
}
}
// If repo is git, ensure no other repository is defined with a child path
if cfg.Spec.Type.IsGit() {
for _, v := range all {
// skip itself
if cfg.Name == v.Name {
continue
}
if v.URL() == cfg.URL() {
if v.Path() == cfg.Path() {
return field.Forbidden(field.NewPath("spec", string(cfg.Spec.Type), "path"),
fmt.Sprintf("%s: %s", ErrRepositoryDuplicatePath.Error(), v.Name))
}
relPath, err := filepath.Rel(v.Path(), cfg.Path())
if err != nil {
return field.Forbidden(field.NewPath("spec", string(cfg.Spec.Type), "path"), "failed to evaluate path: "+err.Error())
}
// https://pkg.go.dev/path/filepath#Rel
// Rel will return "../" if the relative paths are not related
if !strings.HasPrefix(relPath, "../") {
return field.Forbidden(field.NewPath("spec", string(cfg.Spec.Type), "path"),
fmt.Sprintf("%s: %s", ErrRepositoryParentFolderConflict.Error(), v.Name))
}
}
}
}
// Count repositories excluding the current one being created/updated
count := 0
for _, v := range all {
if v.Name != cfg.Name {
count++
}
}
if count >= 10 {
return field.Forbidden(field.NewPath("spec"),
"Maximum number of 10 repositories reached")
}
return nil
func (b *APIBuilder) VerifyAgainstExistingRepositories(ctx context.Context, cfg *provisioning.Repository) *field.Error {
return VerifyAgainstExistingRepositories(ctx, b.store, cfg)
}
func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartHookFunc, error) {
@@ -786,7 +686,10 @@ func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartH
}
// Create the repository resources factory
usageMetricCollector := usage.MetricCollector(b.tracer, b.getRepositoriesInNamespace, b.unified)
repositoryListerWrapper := func(ctx context.Context) ([]provisioning.Repository, error) {
return GetRepositoriesInNamespace(ctx, b.store)
}
usageMetricCollector := usage.MetricCollector(b.tracer, repositoryListerWrapper, b.unified)
b.usageStats.RegisterMetricsFunc(usageMetricCollector)
metrics := jobs.RegisterJobMetrics(b.registry)
@@ -1373,6 +1276,10 @@ func (b *APIBuilder) GetRepository(ctx context.Context, name string) (repository
return b.asRepository(ctx, obj, nil)
}
func (b *APIBuilder) GetRepoFactory() repository.Factory {
return b.repoFactory
}
func (b *APIBuilder) GetHealthyRepository(ctx context.Context, name string) (repository.Repository, error) {
repo, err := b.GetRepository(ctx, name)
if err != nil {
pkg/registry/apis/provisioning/routes.go
+1 -1
View File
@@ -150,7 +150,7 @@ func (b *APIBuilder) handleSettings(w http.ResponseWriter, r *http.Request) {
}
// TODO: check if lister could list too many repositories or resources
all, err := b.getRepositoriesInNamespace(request.WithNamespace(r.Context(), u.GetNamespace()))
all, err := GetRepositoriesInNamespace(request.WithNamespace(r.Context(), u.GetNamespace()), b.store)
if err != nil {
errhttp.Write(r.Context(), err, w)
return
pkg/registry/apis/provisioning/test.go
+14 -9
View File
@@ -28,21 +28,26 @@ type HealthCheckerProvider interface {
GetHealthChecker() *controller.HealthChecker
}
type ConnectorDependencies interface {
RepoGetter
HealthCheckerProvider
repository.RepositoryValidator
GetRepoFactory() repository.Factory
}
type testConnector struct {
getter RepoGetter
factory repository.Factory
healthProvider HealthCheckerProvider
validator repository.RepositoryValidator
}
func NewTestConnector(
getter RepoGetter,
factory repository.Factory,
healthProvider HealthCheckerProvider,
) *testConnector {
func NewTestConnector(deps ConnectorDependencies) *testConnector {
return &testConnector{
factory: factory,
getter: getter,
healthProvider: healthProvider,
factory: deps.GetRepoFactory(),
getter: deps,
healthProvider: deps,
validator: deps,
}
}
@@ -181,7 +186,7 @@ func (s *testConnector) Connect(ctx context.Context, name string, opts runtime.O
}
} else {
// Testing temporary repository - just run test without status update
rsp, err = repository.TestRepository(ctx, repo)
rsp, err = repository.TestRepositoryWithValidator(ctx, repo, s.validator)
if err != nil {
responder.Error(err)
return
pkg/registry/apis/provisioning/validation.go
+124
View File
@@ -0,0 +1,124 @@
package provisioning
import (
"context"
"fmt"
"path/filepath"
"strings"
"k8s.io/apimachinery/pkg/apis/meta/internalversion"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/endpoints/request"
provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1"
"github.com/grafana/grafana/pkg/apimachinery/identity"
)
// RepositoryLister interface for listing repositories
type RepositoryLister interface {
List(ctx context.Context, options *internalversion.ListOptions) (runtime.Object, error)
}
// GetRepositoriesInNamespace retrieves all repositories in a given namespace
func GetRepositoriesInNamespace(ctx context.Context, store RepositoryLister) ([]provisioning.Repository, error) {
var allRepositories []provisioning.Repository
continueToken := ""
for {
obj, err := store.List(ctx, &internalversion.ListOptions{
Limit: 100,
Continue: continueToken,
})
if err != nil {
return nil, err
}
repositoryList, ok := obj.(*provisioning.RepositoryList)
if !ok {
return nil, fmt.Errorf("expected repository list")
}
allRepositories = append(allRepositories, repositoryList.Items...)
continueToken = repositoryList.GetContinue()
if continueToken == "" {
break
}
}
return allRepositories, nil
}
// VerifyAgainstExistingRepositories validates a repository configuration against existing repositories
func VerifyAgainstExistingRepositories(ctx context.Context, store RepositoryLister, cfg *provisioning.Repository) *field.Error {
ctx, _, err := identity.WithProvisioningIdentity(ctx, cfg.Namespace)
if err != nil {
return &field.Error{Type: field.ErrorTypeInternal, Detail: err.Error()}
}
all, err := GetRepositoriesInNamespace(request.WithNamespace(ctx, cfg.Namespace), store)
if err != nil {
return field.Forbidden(field.NewPath("spec"),
"Unable to verify root target: "+err.Error())
}
if cfg.Spec.Sync.Target == provisioning.SyncTargetTypeInstance {
// Instance sync can only be created if NO other repositories exist
for _, v := range all {
if v.Name != cfg.Name {
return field.Forbidden(field.NewPath("spec", "sync", "target"),
"Instance repository can only be created when no other repositories exist. Found: "+v.Name)
}
}
} else {
// Folder sync cannot be created if an instance repository exists
for _, v := range all {
if v.Spec.Sync.Target == provisioning.SyncTargetTypeInstance && v.Name != cfg.Name {
return field.Forbidden(field.NewPath("spec", "sync", "target"),
"Cannot create folder repository when instance repository exists: "+v.Name)
}
}
}
// If repo is git, ensure no other repository is defined with a child path
if cfg.Spec.Type.IsGit() {
for _, v := range all {
// skip itself
if cfg.Name == v.Name {
continue
}
if v.URL() == cfg.URL() {
if v.Path() == cfg.Path() {
return field.Invalid(field.NewPath("spec", string(cfg.Spec.Type), "path"),
cfg.Path(),
fmt.Sprintf("%s: %s", ErrRepositoryDuplicatePath.Error(), v.Name))
}
relPath, err := filepath.Rel(v.Path(), cfg.Path())
if err != nil {
return field.Invalid(field.NewPath("spec", string(cfg.Spec.Type), "path"), cfg.Path(), "failed to evaluate path: "+err.Error())
}
// https://pkg.go.dev/path/filepath#Rel
// Rel will return "../" if the relative paths are not related
if !strings.HasPrefix(relPath, "../") {
return field.Invalid(field.NewPath("spec", string(cfg.Spec.Type), "path"), cfg.Path(),
fmt.Sprintf("%s: %s", ErrRepositoryParentFolderConflict.Error(), v.Name))
}
}
}
}
// Count repositories excluding the current one being created/updated
count := 0
for _, v := range all {
if v.Name != cfg.Name {
count++
}
}
if count >= 10 {
return field.Forbidden(field.NewPath("spec"),
"Maximum number of 10 repositories reached")
}
return nil
}
pkg/tests/apis/provisioning/health_test.go
+3 -1
View File
@@ -22,7 +22,9 @@ func TestIntegrationHealth(t *testing.T) {
ctx := context.Background()
repo := "test-repo-health"
helper.CreateRepo(t, TestRepo{
Name: repo,
Name: repo,
Target: "folder",
ExpectedFolders: 1,
})
// Verify the health status before calling the endpoint
pkg/tests/apis/provisioning/repository_test.go
-1
View File
@@ -267,7 +267,6 @@ func TestIntegrationProvisioning_RepositoryValidation(t *testing.T) {
if test.expectError != nil {
require.Error(t, err, "Expected error for repository with path: %s", test.path)
require.ErrorContains(t, err, test.expectError.Error(), "Error should contain expected message for path: %s", test.path)
var statusError *apierrors.StatusError
if errors.As(err, &statusError) {
public/app/features/provisioning/utils/getFormErrors.ts
+1
View File
@@ -17,6 +17,7 @@ export const getFormErrors = (errors: ErrorDetails[]): FormErrorTuple => {
'local.path',
'github.branch',
'github.url',
'github.path',
'secure.token',
'gitlab.branch',
'gitlab.url',