Provisioning: Allow configurable min interval (#111920)
This commit is contained in:
@@ -75,11 +75,6 @@ func ValidateRepository(repo Repository) field.ErrorList {
|
||||
"The target type is required when sync is enabled"))
|
||||
}
|
||||
|
||||
if cfg.Spec.Sync.Enabled && cfg.Spec.Sync.IntervalSeconds < 10 {
|
||||
list = append(list, field.Invalid(field.NewPath("spec", "sync", "intervalSeconds"),
|
||||
cfg.Spec.Sync.IntervalSeconds, fmt.Sprintf("Interval must be at least %d seconds", 10)))
|
||||
}
|
||||
|
||||
// Reserved names (for now)
|
||||
reserved := []string{"classic", "sql", "SQL", "plugins", "legacy", "new", "job", "github", "s3", "gcs", "file", "new", "create", "update", "delete"}
|
||||
if slices.Contains(reserved, cfg.Name) {
|
||||
|
||||
@@ -74,28 +74,6 @@ func TestValidateRepository(t *testing.T) {
|
||||
require.Contains(t, errors.ToAggregate().Error(), "spec.sync.target: Required value")
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "sync interval too low",
|
||||
repository: func() *MockRepository {
|
||||
m := NewMockRepository(t)
|
||||
m.On("Config").Return(&provisioning.Repository{
|
||||
Spec: provisioning.RepositorySpec{
|
||||
Title: "Test Repo",
|
||||
Sync: provisioning.SyncOptions{
|
||||
Enabled: true,
|
||||
Target: "test",
|
||||
IntervalSeconds: 5,
|
||||
},
|
||||
},
|
||||
})
|
||||
m.On("Validate").Return(field.ErrorList{})
|
||||
return m
|
||||
}(),
|
||||
expectedErrs: 1,
|
||||
validateError: func(t *testing.T, errors field.ErrorList) {
|
||||
require.Contains(t, errors.ToAggregate().Error(), "spec.sync.intervalSeconds: Invalid value")
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "reserved name",
|
||||
repository: func() *MockRepository {
|
||||
@@ -191,11 +169,10 @@ func TestValidateRepository(t *testing.T) {
|
||||
m.On("Validate").Return(field.ErrorList{})
|
||||
return m
|
||||
}(),
|
||||
expectedErrs: 4, // Updated from 3 to 4 to match actual errors:
|
||||
expectedErrs: 3,
|
||||
// 1. missing title
|
||||
// 2. sync target missing
|
||||
// 3. sync interval too low
|
||||
// 4. reserved name
|
||||
// 3. reserved name
|
||||
},
|
||||
{
|
||||
name: "branch workflow for non-github repository",
|
||||
@@ -447,18 +424,6 @@ func TestFromFieldError(t *testing.T) {
|
||||
expectedType: metav1.CauseTypeFieldValueRequired,
|
||||
expectedDetail: "a repository title must be given",
|
||||
},
|
||||
{
|
||||
name: "invalid field error",
|
||||
fieldError: &field.Error{
|
||||
Type: field.ErrorTypeInvalid,
|
||||
Field: "spec.sync.intervalSeconds",
|
||||
Detail: "Interval must be at least 10 seconds",
|
||||
},
|
||||
expectedCode: http.StatusBadRequest,
|
||||
expectedField: "spec.sync.intervalSeconds",
|
||||
expectedType: metav1.CauseTypeFieldValueInvalid,
|
||||
expectedDetail: "Interval must be at least 10 seconds",
|
||||
},
|
||||
{
|
||||
name: "not supported field error",
|
||||
fieldError: &field.Error{
|
||||
|
||||
@@ -2229,3 +2229,8 @@ allowed_targets = instance|folder
|
||||
# Whether image rendering is allowed for dashboard previews.
|
||||
# Requires image rendering service to be configured.
|
||||
allow_image_rendering = true
|
||||
|
||||
# The minimum sync interval that can be set for a repository. This is how often the controller
|
||||
# will check if there has been any changes to the repository not propagated by a webhook.
|
||||
# The minimum value is 10 seconds.
|
||||
min_sync_interval = 10s
|
||||
|
||||
@@ -92,6 +92,7 @@ type APIBuilder struct {
|
||||
|
||||
allowedTargets []provisioning.SyncTargetType
|
||||
allowImageRendering bool
|
||||
minSyncInterval time.Duration
|
||||
|
||||
features featuremgmt.FeatureToggles
|
||||
usageStats usagestats.Service
|
||||
@@ -144,6 +145,7 @@ func NewAPIBuilder(
|
||||
allowedTargets []provisioning.SyncTargetType,
|
||||
restConfigGetter func(context.Context) (*clientrest.Config, error),
|
||||
allowImageRendering bool,
|
||||
minSyncInterval time.Duration,
|
||||
registry prometheus.Registerer,
|
||||
newStandaloneClientFactoryFunc func(loopbackConfigProvider apiserver.RestConfigProvider) resources.ClientFactory, // optional, only used for standalone apiserver
|
||||
) *APIBuilder {
|
||||
@@ -156,6 +158,11 @@ func NewAPIBuilder(
|
||||
parsers := resources.NewParserFactory(clients)
|
||||
resourceLister := resources.NewResourceListerForMigrations(unified, legacyMigrator, storageStatus)
|
||||
|
||||
// do not allow minsync interval to be less than 10
|
||||
if minSyncInterval <= 10*time.Second {
|
||||
minSyncInterval = 10 * time.Second
|
||||
}
|
||||
|
||||
b := &APIBuilder{
|
||||
onlyApiServer: onlyApiServer,
|
||||
tracer: tracer,
|
||||
@@ -175,6 +182,7 @@ func NewAPIBuilder(
|
||||
allowedTargets: allowedTargets,
|
||||
restConfigGetter: restConfigGetter,
|
||||
allowImageRendering: allowImageRendering,
|
||||
minSyncInterval: minSyncInterval,
|
||||
registry: registry,
|
||||
}
|
||||
|
||||
@@ -261,6 +269,7 @@ func RegisterAPIService(
|
||||
allowedTargets,
|
||||
nil, // will use loopback instead
|
||||
cfg.ProvisioningAllowImageRendering,
|
||||
cfg.ProvisioningMinSyncInterval,
|
||||
reg,
|
||||
nil,
|
||||
)
|
||||
@@ -587,6 +596,11 @@ func (b *APIBuilder) Validate(ctx context.Context, a admission.Attributes, o adm
|
||||
"sync target is not supported"))
|
||||
}
|
||||
|
||||
if cfg.Spec.Sync.Enabled && cfg.Spec.Sync.IntervalSeconds < int64(b.minSyncInterval.Seconds()) {
|
||||
list = append(list, field.Invalid(field.NewPath("spec", "sync", "intervalSeconds"),
|
||||
cfg.Spec.Sync.IntervalSeconds, fmt.Sprintf("Interval must be at least %d seconds", int64(b.minSyncInterval.Seconds()))))
|
||||
}
|
||||
|
||||
if !b.allowImageRendering && cfg.Spec.GitHub != nil && cfg.Spec.GitHub.GenerateDashboardPreviews {
|
||||
list = append(list,
|
||||
field.Invalid(field.NewPath("spec", "generateDashboardPreviews"),
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
package provisioning
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
apierrors "k8s.io/apimachinery/pkg/api/errors"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"k8s.io/apiserver/pkg/admission"
|
||||
"k8s.io/apiserver/pkg/authentication/user"
|
||||
|
||||
"github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1"
|
||||
"github.com/grafana/grafana/apps/provisioning/pkg/repository"
|
||||
|
||||
"github.com/stretchr/testify/mock"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestAPIBuilderValidate(t *testing.T) {
|
||||
factory := repository.NewMockFactory(t)
|
||||
mockRepo := repository.NewMockConfigRepository(t)
|
||||
mockRepo.EXPECT().Validate().Return(nil)
|
||||
factory.EXPECT().Build(mock.Anything, mock.Anything).Return(mockRepo, nil)
|
||||
b := &APIBuilder{
|
||||
repoFactory: factory,
|
||||
allowedTargets: []v0alpha1.SyncTargetType{v0alpha1.SyncTargetTypeFolder},
|
||||
allowImageRendering: false,
|
||||
minSyncInterval: 30 * time.Second,
|
||||
}
|
||||
|
||||
t.Run("min sync interval is less than 10 seconds", func(t *testing.T) {
|
||||
cfg := &v0alpha1.Repository{
|
||||
Spec: v0alpha1.RepositorySpec{
|
||||
Title: "repo",
|
||||
Type: v0alpha1.GitHubRepositoryType,
|
||||
Sync: v0alpha1.SyncOptions{Enabled: true, Target: v0alpha1.SyncTargetTypeFolder, IntervalSeconds: 5},
|
||||
},
|
||||
}
|
||||
mockRepo.EXPECT().Config().Return(cfg)
|
||||
|
||||
obj := newRepoObj("repo1", "default", cfg.Spec, v0alpha1.RepositoryStatus{})
|
||||
err := b.Validate(context.Background(), newAttributes(obj, nil, admission.Create), nil)
|
||||
require.Error(t, err)
|
||||
require.True(t, apierrors.IsInvalid(err))
|
||||
})
|
||||
|
||||
t.Run("image rendering is not enabled", func(t *testing.T) {
|
||||
cfg2 := &v0alpha1.Repository{
|
||||
Spec: v0alpha1.RepositorySpec{
|
||||
Title: "repo",
|
||||
Type: v0alpha1.GitHubRepositoryType,
|
||||
Sync: v0alpha1.SyncOptions{Enabled: false, Target: v0alpha1.SyncTargetTypeFolder},
|
||||
GitHub: &v0alpha1.GitHubRepositoryConfig{URL: "https://github.com/acme/repo", Branch: "main", GenerateDashboardPreviews: true},
|
||||
},
|
||||
}
|
||||
mockRepo.EXPECT().Config().Return(cfg2)
|
||||
|
||||
obj := newRepoObj("repo2", "default", cfg2.Spec, v0alpha1.RepositoryStatus{})
|
||||
err := b.Validate(context.Background(), newAttributes(obj, nil, admission.Create), nil)
|
||||
require.Error(t, err)
|
||||
require.True(t, apierrors.IsInvalid(err))
|
||||
})
|
||||
|
||||
t.Run("sync target is not supported", func(t *testing.T) {
|
||||
cfg3 := &v0alpha1.Repository{
|
||||
Spec: v0alpha1.RepositorySpec{
|
||||
Title: "repo",
|
||||
Type: v0alpha1.GitHubRepositoryType,
|
||||
Sync: v0alpha1.SyncOptions{Enabled: true, Target: v0alpha1.SyncTargetTypeInstance},
|
||||
},
|
||||
}
|
||||
mockRepo.EXPECT().Config().Return(cfg3)
|
||||
|
||||
obj := newRepoObj("repo3", "default", cfg3.Spec, v0alpha1.RepositoryStatus{})
|
||||
err := b.Validate(context.Background(), newAttributes(obj, nil, admission.Create), nil)
|
||||
require.Error(t, err)
|
||||
require.True(t, apierrors.IsInvalid(err))
|
||||
})
|
||||
}
|
||||
|
||||
func newRepoObj(name string, ns string, spec v0alpha1.RepositorySpec, status v0alpha1.RepositoryStatus) *v0alpha1.Repository {
|
||||
return &v0alpha1.Repository{
|
||||
TypeMeta: metav1.TypeMeta{APIVersion: v0alpha1.APIVERSION, Kind: "Repository"},
|
||||
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns},
|
||||
Spec: spec,
|
||||
Status: status,
|
||||
}
|
||||
}
|
||||
|
||||
func newAttributes(obj, old runtime.Object, op admission.Operation) admission.Attributes {
|
||||
return admission.NewAttributesRecord(
|
||||
obj,
|
||||
old,
|
||||
v0alpha1.RepositoryResourceInfo.GroupVersionKind(),
|
||||
"default",
|
||||
func() string {
|
||||
if obj != nil {
|
||||
return obj.(*v0alpha1.Repository).Name
|
||||
}
|
||||
if old != nil {
|
||||
return old.(*v0alpha1.Repository).Name
|
||||
}
|
||||
return ""
|
||||
}(),
|
||||
v0alpha1.RepositoryResourceInfo.GroupVersionResource(),
|
||||
"",
|
||||
op,
|
||||
nil,
|
||||
false,
|
||||
&user.DefaultInfo{},
|
||||
)
|
||||
}
|
||||
@@ -138,6 +138,7 @@ type Cfg struct {
|
||||
ProvisioningDisableControllers bool
|
||||
ProvisioningAllowedTargets []string
|
||||
ProvisioningAllowImageRendering bool
|
||||
ProvisioningMinSyncInterval time.Duration
|
||||
ProvisioningRepositoryTypes []string
|
||||
ProvisioningLokiURL string
|
||||
ProvisioningLokiUser string
|
||||
@@ -2125,6 +2126,7 @@ func (cfg *Cfg) readProvisioningSettings(iniFile *ini.File) error {
|
||||
cfg.ProvisioningAllowedTargets = []string{"instance", "folder"}
|
||||
}
|
||||
cfg.ProvisioningAllowImageRendering = iniFile.Section("provisioning").Key("allow_image_rendering").MustBool(true)
|
||||
cfg.ProvisioningMinSyncInterval = iniFile.Section("provisioning").Key("min_sync_interval").MustDuration(10 * time.Second)
|
||||
|
||||
// Read job history configuration
|
||||
cfg.ProvisioningLokiURL = valueAsString(iniFile.Section("provisioning"), "loki_url", "")
|
||||
|
||||
Reference in New Issue
Block a user