diff --git a/pkg/registry/apis/collections/datasources.go b/pkg/registry/apis/collections/datasources.go deleted file mode 100644 index 9b932a2e26b..00000000000 --- a/pkg/registry/apis/collections/datasources.go +++ /dev/null @@ -1,77 +0,0 @@ -package collections - -import ( - "context" - "fmt" - - collections "github.com/grafana/grafana/apps/collections/pkg/apis/collections/v1alpha1" - grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/validation/field" - "k8s.io/apiserver/pkg/registry/rest" -) - -var _ grafanarest.Storage = (*datasourceStorage)(nil) - -type datasourceStorage struct { - grafanarest.Storage -} - -func (s *datasourceStorage) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { - // TODO run our own validation here - - dsStack, ok := obj.(*collections.DataSourceStack) - if !ok { - return nil, fmt.Errorf("expected a datasource stack object") - } - - list := field.ErrorList{} - - // Check that the modes are valid - - // get the keys from the template - template := dsStack.Spec.Template - templateKeys := make([]string, 0, len(template)) - for key := range template { - templateKeys = append(templateKeys, key) - } - - // for each mode, check that the keys are in the template - modes := dsStack.Spec.Modes - - // if a key is not in the template, return an error - for _, mode := range modes { - for key := range mode.Definition { - if indexOf(templateKeys, key) == -1 { - list = append(list, field.Invalid(field.NewPath("spec", "modes", mode.Name, "definition", key), key, fmt.Sprintf("key %s is not in the template", key))) - - } - } - } - - if len(list) > 0 { - return nil, apierrors.NewInvalid(collections.DatasourceStacksResourceInfo.GroupVersionKind().GroupKind(), dsStack.Name, list) - } - - // TODO find any keys missing from the definition, or is that OK? - - // TODO Check that each data source reference is valid - - return s.Storage.Create(ctx, obj, createValidation, options) -} - -func (d *datasourceStorage) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) { - // the objInfo is not obviosuly simply to perform validation on, it feels like we should be performing validation elsewhere. - return d.Storage.Update(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options) -} - -func indexOf(slice []string, item string) int { - for i, v := range slice { - if v == item { - return i - } - } - return -1 -} diff --git a/pkg/registry/apis/collections/datasources_validator.go b/pkg/registry/apis/collections/datasources_validator.go new file mode 100644 index 00000000000..f02bb3f6e47 --- /dev/null +++ b/pkg/registry/apis/collections/datasources_validator.go @@ -0,0 +1,75 @@ +package collections + +import ( + "context" + "fmt" + + collections "github.com/grafana/grafana/apps/collections/pkg/apis/collections/v1alpha1" + "github.com/grafana/grafana/pkg/services/apiserver/builder" + "k8s.io/apiserver/pkg/admission" +) + +var _ builder.APIGroupValidation = (*DatasourceStacksValidator)(nil) + +type DatasourceStacksValidator struct{} + +func GetDatasourceStacksValidator() builder.APIGroupValidation { + return &DatasourceStacksValidator{} +} + +func (v *DatasourceStacksValidator) Validate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) { + if a.GetKind().Kind != collections.DatasourceStacksResourceInfo.GroupVersionKind().Kind { + return nil + } + + obj := a.GetObject() + if obj == nil { + return fmt.Errorf("object is nil (%s %s)", a.GetName(), a.GetKind().GroupVersion().String()) + } + + operation := a.GetOperation() + + if operation != admission.Create && operation != admission.Update { + return nil + } + + cast, ok := obj.(*collections.DataSourceStack) + if !ok { + return fmt.Errorf("object is not of type *collections.DataSourceStack (%s %s)", a.GetName(), a.GetKind().GroupVersion().String()) + } + + // get the keys from the template + template := cast.Spec.Template + templateKeys := map[string]bool{} + + // keys must be unique + for key := range template { + if _, ok := templateKeys[key]; ok { + return fmt.Errorf("template keys must be unique. key '%s' already exists (%s %s)", key, a.GetName(), a.GetKind().GroupVersion().String()) + } + templateKeys[key] = true + } + + // template names must be unique + templateNames := map[string]bool{} + for name := range template { + if _, ok := templateNames[name]; ok { + return fmt.Errorf("template names must be unique. name '%s' already exists (%s %s)", name, a.GetName(), a.GetKind().GroupVersion().String()) + } + templateNames[name] = true + } + + // for each mode, check that the keys are in the template + modes := cast.Spec.Modes + + // if a key is not in the template, return an error + for _, mode := range modes { + for key := range mode.Definition { + if _, ok := templateKeys[key]; !ok { + return fmt.Errorf("key %s is not in the DataSourceStack template. The template keys are %v (%s %s)", key, templateKeys, a.GetName(), a.GetKind().GroupVersion().String()) + } + } + } + + return nil +} diff --git a/pkg/registry/apis/collections/datasources_validator_test.go b/pkg/registry/apis/collections/datasources_validator_test.go new file mode 100644 index 00000000000..238eb5490e7 --- /dev/null +++ b/pkg/registry/apis/collections/datasources_validator_test.go @@ -0,0 +1,79 @@ +package collections_test + +import ( + "context" + "testing" + + collectionsv1alpha1 "github.com/grafana/grafana/apps/collections/pkg/apis/collections/v1alpha1" + "github.com/grafana/grafana/pkg/registry/apis/collections" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" +) + +func TestDataSourceValidator_Validate(t *testing.T) { + validator := &collections.DatasourceStacksValidator{} + ctx := context.Background() + + tests := []struct { + name string + operation admission.Operation + object runtime.Object + expectError bool + errorMsg string + }{ + { + name: "should return no error for invalid kind", + operation: admission.Delete, + object: &collectionsv1alpha1.Stars{}, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + attrs := &FakeAdmissionAttributes{ + Operation: tt.operation, + Object: tt.object, + Name: "test-datasourcestack", + Kind: schema.GroupVersionKind{Group: "collections.grafana.app", Version: "v1alpha1", Kind: "DataSourceStack"}, + } + + err := validator.Validate(ctx, attrs, nil) + + if tt.expectError { + assert.Error(t, err) + if tt.errorMsg != "" { + assert.Contains(t, err.Error(), tt.errorMsg) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +type FakeAdmissionAttributes struct { + admission.Attributes + Operation admission.Operation + Object runtime.Object + Name string + Kind schema.GroupVersionKind +} + +func (m *FakeAdmissionAttributes) GetOperation() admission.Operation { + return m.Operation +} + +func (m *FakeAdmissionAttributes) GetObject() runtime.Object { + return m.Object +} + +func (m *FakeAdmissionAttributes) GetName() string { + return m.Name +} + +func (m *FakeAdmissionAttributes) GetKind() schema.GroupVersionKind { + return m.Kind +} diff --git a/pkg/registry/apis/collections/register.go b/pkg/registry/apis/collections/register.go index 55fee8954c6..bf812c97268 100644 --- a/pkg/registry/apis/collections/register.go +++ b/pkg/registry/apis/collections/register.go @@ -7,6 +7,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/registry/rest" genericapiserver "k8s.io/apiserver/pkg/server" @@ -31,13 +32,15 @@ import ( ) var ( - _ builder.APIGroupBuilder = (*APIBuilder)(nil) - _ builder.APIGroupMutation = (*APIBuilder)(nil) + _ builder.APIGroupBuilder = (*APIBuilder)(nil) + _ builder.APIGroupMutation = (*APIBuilder)(nil) + _ builder.APIGroupValidation = (*APIBuilder)(nil) ) type APIBuilder struct { - authorizer authorizer.Authorizer - legacyStars *legacy.DashboardStarsStorage + authorizer authorizer.Authorizer + legacyStars *legacy.DashboardStarsStorage + datasourceStacksValidator builder.APIGroupValidation } func RegisterAPIService( @@ -56,6 +59,7 @@ func RegisterAPIService( sql := legacy.NewLegacySQL(legacysql.NewDatabaseProvider(db)) builder := &APIBuilder{ + datasourceStacksValidator: GetDatasourceStacksValidator(), authorizer: &utils.AuthorizeFromName{ Resource: map[string][]utils.ResourceOwner{ "stars": {utils.UserResourceOwner}, @@ -116,16 +120,19 @@ func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupI // no need for dual writer for a kind that does not exist in the legacy database resourceInfo := collections.DatasourceStacksResourceInfo datasourcesStorage, err := grafanaregistry.NewRegistryStore(opts.Scheme, resourceInfo, opts.OptsGetter) - datasources := &datasourceStorage{Storage: datasourcesStorage} - if err != nil { - return err - } - storage[resourceInfo.StoragePath()] = datasources + storage[resourceInfo.StoragePath()] = datasourcesStorage apiGroupInfo.VersionedResourcesStorageMap[collections.APIVersion] = storage return nil } +func (b *APIBuilder) Validate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) { + if a.GetKind().Group == collections.DatasourceStacksResourceInfo.GroupResource().Group { + return b.datasourceStacksValidator.Validate(ctx, a, o) + } + return nil +} + func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer { return authorizer.AuthorizerFunc(