diff --git a/pkg/components/simplejson/simplejson.go b/pkg/components/simplejson/simplejson.go index d7759ac3c2b..a9e2b4c3819 100644 --- a/pkg/components/simplejson/simplejson.go +++ b/pkg/components/simplejson/simplejson.go @@ -12,6 +12,7 @@ import ( "errors" "fmt" "log" + "reflect" ) // returns the current implementation version @@ -117,6 +118,23 @@ func (j *Json) Interface() any { return j.data } +// Check if the underlying data is empty +func (j *Json) IsEmpty() bool { + if j.data == nil { + return true + } + v := reflect.ValueOf(j.data) + switch v.Kind() { + case reflect.Slice, reflect.Array, reflect.Map, reflect.String: + if v.Len() == 0 { + return true + } + default: + return false + } + return false +} + // Encode returns its marshaled data as `[]byte` func (j *Json) Encode() ([]byte, error) { return j.MarshalJSON() diff --git a/pkg/components/simplejson/simplejson_test.go b/pkg/components/simplejson/simplejson_test.go index efc786bc745..53acd4b556a 100644 --- a/pkg/components/simplejson/simplejson_test.go +++ b/pkg/components/simplejson/simplejson_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSimplejson(t *testing.T) { @@ -272,3 +273,45 @@ func TestMustJson(t *testing.T) { MustJson([]byte(`{`)) }) } + +func TestEmpty(t *testing.T) { + testCases := []struct { + name string + input any + empty bool + }{ + { + name: "empty map (any)", + input: map[string]any{}, + empty: true, + }, { + name: "empty map (string)", + input: map[string]string{}, + empty: true, + }, { + name: "empty array (any)", + input: []any{}, + empty: true, + }, { + name: "empty array (string)", + input: []string{}, + empty: true, + }, { + name: "empty string", + input: "", + empty: true, + }, { + name: "non empty string", + input: "hello", + }, { + name: "key value", + input: map[string]any{"key": "value"}, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + js := NewFromAny(tc.input) + require.Equal(t, tc.empty, js.IsEmpty()) + }) + } +} diff --git a/pkg/registry/apis/datasource/converter.go b/pkg/registry/apis/datasource/converter.go index a507d431422..9a1154c462a 100644 --- a/pkg/registry/apis/datasource/converter.go +++ b/pkg/registry/apis/datasource/converter.go @@ -56,19 +56,33 @@ func (r *converter) asDataSource(ds *datasources.DataSource) (*datasourceV0.Data SetBasicAuthUser(ds.BasicAuthUser). SetWithCredentials(ds.WithCredentials). SetIsDefault(ds.IsDefault). - SetReadOnly(ds.ReadOnly). - SetJSONData(ds.JsonData) + SetReadOnly(ds.ReadOnly) + if ds.JsonData != nil && !ds.JsonData.IsEmpty() { + obj.Spec.SetJSONData(ds.JsonData.Interface()) + } + + rv := int64(0) if !ds.Created.IsZero() { obj.CreationTimestamp = metav1.NewTime(ds.Created) + rv = ds.Created.UnixMilli() } + + // Only mark updated if the times have actually changed if !ds.Updated.IsZero() { - obj.ResourceVersion = fmt.Sprintf("%d", ds.Updated.UnixMilli()) - obj.Annotations = map[string]string{ - utils.AnnoKeyUpdatedTimestamp: ds.Updated.Format(time.RFC3339), + rv = ds.Updated.UnixMilli() + delta := rv - obj.CreationTimestamp.UnixMilli() + if delta > 1500 { + obj.Annotations = map[string]string{ + utils.AnnoKeyUpdatedTimestamp: ds.Updated.UTC().Format(time.RFC3339), + } } } + if rv > 0 { + obj.ResourceVersion = strconv.FormatInt(rv, 10) + } + if ds.APIVersion != "" { obj.APIVersion = fmt.Sprintf("%s/%s", r.group, ds.APIVersion) } diff --git a/pkg/registry/apis/datasource/legacy_store.go b/pkg/registry/apis/datasource/legacy_store.go index b4ee3b4b7eb..00342c043b8 100644 --- a/pkg/registry/apis/datasource/legacy_store.go +++ b/pkg/registry/apis/datasource/legacy_store.go @@ -2,19 +2,23 @@ package datasource import ( "context" + "errors" "fmt" "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/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/registry/rest" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/apis/datasource/v0alpha1" "github.com/grafana/grafana/pkg/infra/metrics/metricutil" + "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/storage/legacysql/dualwrite" ) @@ -106,7 +110,16 @@ func (s *legacyStorage) Create(ctx context.Context, obj runtime.Object, createVa } } - return s.datasources.CreateDataSource(ctx, ds) + obj, err := s.datasources.CreateDataSource(ctx, ds) + if err != nil { + switch { + case errors.Is(err, datasources.ErrDataSourceNameExists): + return nil, apierrors.NewInvalid(s.resourceInfo.GroupVersionKind().GroupKind(), ds.Name, field.ErrorList{ + field.Invalid(field.NewPath("spec", "title"), ds.Spec.Title(), "a datasource with this title already exists")}) + } + return nil, err + } + return obj, nil } // Update implements rest.Updater. diff --git a/pkg/registry/apis/datasource/openapi.go b/pkg/registry/apis/datasource/openapi.go index ab63c29e819..3962827c259 100644 --- a/pkg/registry/apis/datasource/openapi.go +++ b/pkg/registry/apis/datasource/openapi.go @@ -13,6 +13,18 @@ func (b *DataSourceAPIBuilder) PostProcessOpenAPI(oas *spec3.OpenAPI) (*spec3.Op // The plugin description oas.Info.Description = b.pluginJSON.Info.Description + // Add plugin information + info := map[string]any{ + "id": b.pluginJSON.ID, + } + if b.pluginJSON.Info.Version != "" { + info["version"] = b.pluginJSON.Info.Version + } + if b.pluginJSON.Info.Build.Time > 0 { + info["build"] = b.pluginJSON.Info.Build.Time + } + oas.Info.AddExtension("x-grafana-plugin", info) + // The root api URL root := "/apis/" + b.datasourceResourceInfo.GroupVersion().String() + "/" diff --git a/pkg/registry/apis/datasource/testdata/convert-dto-empty-to-resource.json b/pkg/registry/apis/datasource/testdata/convert-dto-empty-to-resource.json index 4b35a3e1da8..ef0be260dc5 100644 --- a/pkg/registry/apis/datasource/testdata/convert-dto-empty-to-resource.json +++ b/pkg/registry/apis/datasource/testdata/convert-dto-empty-to-resource.json @@ -3,6 +3,7 @@ "name": "unique-identifier", "namespace": "org-0", "uid": "YpaSG5GQAdxtLZtF6BqQWCeYXOhbVi5C4Cg4oILnJC0X", + "resourceVersion": "1015203600000", "generation": 8, "creationTimestamp": "2002-03-04T01:00:00Z", "labels": { @@ -10,7 +11,6 @@ } }, "spec": { - "jsonData": null, "title": "Display name" } } \ No newline at end of file diff --git a/pkg/registry/apis/datasource/testdata/convert-resource-empty-to-cmd-update-roundtrip.json b/pkg/registry/apis/datasource/testdata/convert-resource-empty-to-cmd-update-roundtrip.json index a8c49ba7637..2daa65ebca8 100644 --- a/pkg/registry/apis/datasource/testdata/convert-resource-empty-to-cmd-update-roundtrip.json +++ b/pkg/registry/apis/datasource/testdata/convert-resource-empty-to-cmd-update-roundtrip.json @@ -5,7 +5,6 @@ "uid": "boDNh7zU3nXj46rOXIJI7r44qaxjs8yy9I9dOj1MyBoX" }, "spec": { - "jsonData": null, "title": "Hello testdata" } } \ No newline at end of file diff --git a/pkg/services/apiserver/builder/openapi.go b/pkg/services/apiserver/builder/openapi.go index 4786e3d3c0f..dc76d87afa7 100644 --- a/pkg/services/apiserver/builder/openapi.go +++ b/pkg/services/apiserver/builder/openapi.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "maps" + "net/http" "strings" "sync" @@ -231,11 +232,9 @@ func getOpenAPIPostProcessor(version string, builders []APIGroupBuilder, gvs []s parent := copy.Paths.Paths[path[:idx+6]] if parent != nil && parent.Get != nil { for _, op := range GetPathOperations(spec) { - if op != nil && op.Extensions != nil { - action, ok := op.Extensions.GetString("x-kubernetes-action") - if ok && action == "connect" { - op.Tags = parent.Get.Tags - } + action, ok := op.Extensions.GetString("x-kubernetes-action") + if ok && action == "connect" { + op.Tags = parent.Get.Tags } } } @@ -281,15 +280,32 @@ func getOpenAPIPostProcessor(version string, builders []APIGroupBuilder, gvs []s } } -func GetPathOperations(path *spec3.Path) []*spec3.Operation { - return []*spec3.Operation{ - path.Get, - path.Head, - path.Delete, - path.Patch, - path.Post, - path.Put, - path.Trace, - path.Options, +// GetPathOperations returns the set of non-nil operations defined on a path +func GetPathOperations(path *spec3.Path) map[string]*spec3.Operation { + ops := make(map[string]*spec3.Operation) + if path.Get != nil { + ops[http.MethodGet] = path.Get } + if path.Head != nil { + ops[http.MethodHead] = path.Head + } + if path.Delete != nil { + ops[http.MethodDelete] = path.Delete + } + if path.Post != nil { + ops[http.MethodPost] = path.Post + } + if path.Put != nil { + ops[http.MethodPut] = path.Put + } + if path.Patch != nil { + ops[http.MethodPatch] = path.Patch + } + if path.Trace != nil { + ops[http.MethodTrace] = path.Trace + } + if path.Options != nil { + ops[http.MethodOptions] = path.Options + } + return ops } diff --git a/pkg/services/apiserver/builder/openapi_test.go b/pkg/services/apiserver/builder/openapi_test.go new file mode 100644 index 00000000000..746ae949e9f --- /dev/null +++ b/pkg/services/apiserver/builder/openapi_test.go @@ -0,0 +1,76 @@ +package builder + +import ( + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "k8s.io/kube-openapi/pkg/spec3" +) + +func TestOpenAPI_GetPathOperations(t *testing.T) { + testCases := []struct { + name string + input *spec3.Path + expect []string // the methods we should see + exclude []string // the methods we should never see + }{ + { + name: "some operations", + input: &spec3.Path{ + PathProps: spec3.PathProps{ + Get: &spec3.Operation{OperationProps: spec3.OperationProps{Summary: "get"}}, + Post: &spec3.Operation{OperationProps: spec3.OperationProps{Summary: "post"}}, + Delete: &spec3.Operation{OperationProps: spec3.OperationProps{Summary: "delete"}}, + }, + }, + expect: []string{"GET", "POST", "DELETE"}, + exclude: []string{"PUT", "PATCH", "OPTIONS", "HEAD", "TRACE"}, + }, + { + name: "all operations", + input: &spec3.Path{ + PathProps: spec3.PathProps{ + Get: &spec3.Operation{OperationProps: spec3.OperationProps{Summary: "get"}}, + Post: &spec3.Operation{OperationProps: spec3.OperationProps{Summary: "post"}}, + Delete: &spec3.Operation{OperationProps: spec3.OperationProps{Summary: "delete"}}, + Put: &spec3.Operation{OperationProps: spec3.OperationProps{Summary: "put"}}, + Patch: &spec3.Operation{OperationProps: spec3.OperationProps{Summary: "patch"}}, + Options: &spec3.Operation{OperationProps: spec3.OperationProps{Summary: "options"}}, + Head: &spec3.Operation{OperationProps: spec3.OperationProps{Summary: "head"}}, + Trace: &spec3.Operation{OperationProps: spec3.OperationProps{Summary: "trace"}}, + }, + }, + expect: []string{"GET", "POST", "DELETE", "PUT", "PATCH", "OPTIONS", "HEAD", "TRACE"}, + exclude: []string{}, + }, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + expect := make(map[string]bool) + for _, k := range tt.expect { + expect[k] = true + } + + for k, op := range GetPathOperations(tt.input) { + require.NotNil(t, op) + require.Equal(t, strings.ToLower(k), op.Summary) + + if !expect[k] { + if slices.Contains(tt.expect, k) { + require.Fail(t, "method returned multiple times", k) + } else { + require.Fail(t, "unexpected method", k) + } + } + delete(expect, k) + require.NotContains(t, tt.exclude, k, "exclude") + } + + if len(expect) > 0 { + require.Fail(t, "missing expected method", expect) + } + }) + } +} diff --git a/pkg/tests/apis/helper.go b/pkg/tests/apis/helper.go index 0f2cb95cc50..cf6e9791884 100644 --- a/pkg/tests/apis/helper.go +++ b/pkg/tests/apis/helper.go @@ -14,7 +14,6 @@ import ( "testing" "time" - appsdk_k8s "github.com/grafana/grafana-app-sdk/k8s" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/errors" @@ -27,9 +26,10 @@ import ( "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" "k8s.io/client-go/rest" + "k8s.io/kube-openapi/pkg/spec3" + appsdk_k8s "github.com/grafana/grafana-app-sdk/k8s" githubConnection "github.com/grafana/grafana/apps/provisioning/pkg/connection/github" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/configprovider" @@ -880,15 +880,29 @@ func VerifyOpenAPISnapshots(t *testing.T, dir string, gv schema.GroupVersion, h Method: http.MethodGet, Path: path, User: h.Org1.Admin, - }, &AnyResource{}) + }, &spec3.OpenAPI{}) require.NotNil(t, rsp.Response) if rsp.Response.StatusCode != 200 { require.Failf(t, "Not OK", "Code[%d] %s", rsp.Response.StatusCode, string(rsp.Body)) } + var err error + body := rsp.Body + + // Clear the plugin version and build stamp from snapshot + if v, ok := rsp.Result.Info.Extensions["x-grafana-plugin"]; ok && v != nil { + if pluginInfo, ok := v.(map[string]any); ok { + delete(pluginInfo, "version") + delete(pluginInfo, "build") + + body, err = rsp.Result.MarshalJSON() + require.NoError(t, err) + } + } + var prettyJSON bytes.Buffer - err := json.Indent(&prettyJSON, rsp.Body, "", " ") + err = json.Indent(&prettyJSON, body, "", " ") require.NoError(t, err) pretty := prettyJSON.String() @@ -897,7 +911,7 @@ func VerifyOpenAPISnapshots(t *testing.T, dir string, gv schema.GroupVersion, h // nolint:gosec // We can ignore the gosec G304 warning since this is a test and the function is only called with explicit paths - body, err := os.ReadFile(fpath) + body, err = os.ReadFile(fpath) if err == nil { if !assert.JSONEq(t, string(body), pretty) { t.Logf("openapi spec has changed: %s", path) diff --git a/pkg/tests/apis/openapi_snapshots/grafana-testdata-datasource.datasource.grafana.app-v0alpha1.json b/pkg/tests/apis/openapi_snapshots/grafana-testdata-datasource.datasource.grafana.app-v0alpha1.json index d1eada8834c..7c1ea6b0535 100644 --- a/pkg/tests/apis/openapi_snapshots/grafana-testdata-datasource.datasource.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/grafana-testdata-datasource.datasource.grafana.app-v0alpha1.json @@ -2,7 +2,10 @@ "openapi": "3.0.0", "info": { "description": "Generates test data in different forms", - "title": "grafana-testdata-datasource.datasource.grafana.app/v0alpha1" + "title": "grafana-testdata-datasource.datasource.grafana.app/v0alpha1", + "x-grafana-plugin": { + "id": "grafana-testdata-datasource" + } }, "paths": { "/apis/grafana-testdata-datasource.datasource.grafana.app/v0alpha1/": { @@ -64,8 +67,8 @@ "x-kubernetes-action": "connect", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "QueryDataResponse" + "kind": "QueryDataResponse", + "version": "v0alpha1" } }, "parameters": [ @@ -215,8 +218,8 @@ "x-kubernetes-action": "list", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "DataSource" + "kind": "DataSource", + "version": "v0alpha1" } }, "post": { @@ -319,8 +322,8 @@ "x-kubernetes-action": "post", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "DataSource" + "kind": "DataSource", + "version": "v0alpha1" } }, "delete": { @@ -468,8 +471,8 @@ "x-kubernetes-action": "deletecollection", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "DataSource" + "kind": "DataSource", + "version": "v0alpha1" } }, "parameters": [ @@ -521,8 +524,8 @@ "x-kubernetes-action": "get", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "DataSource" + "kind": "DataSource", + "version": "v0alpha1" } }, "put": { @@ -610,8 +613,8 @@ "x-kubernetes-action": "put", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "DataSource" + "kind": "DataSource", + "version": "v0alpha1" } }, "delete": { @@ -702,8 +705,8 @@ "x-kubernetes-action": "delete", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "DataSource" + "kind": "DataSource", + "version": "v0alpha1" } }, "patch": { @@ -810,8 +813,8 @@ "x-kubernetes-action": "patch", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "DataSource" + "kind": "DataSource", + "version": "v0alpha1" } }, "parameters": [ @@ -868,8 +871,8 @@ "x-kubernetes-action": "connect", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "HealthCheckResult" + "kind": "HealthCheckResult", + "version": "v0alpha1" } }, "parameters": [ @@ -926,8 +929,8 @@ "x-kubernetes-action": "connect", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "QueryDataResponse" + "kind": "QueryDataResponse", + "version": "v0alpha1" } }, "parameters": [ @@ -975,8 +978,8 @@ "x-kubernetes-action": "connect", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "Status" + "kind": "Status", + "version": "v0alpha1" } }, "parameters": [ @@ -1024,8 +1027,8 @@ "x-kubernetes-action": "connect", "x-kubernetes-group-version-kind": { "group": "grafana-testdata-datasource.datasource.grafana.app", - "version": "v0alpha1", - "kind": "QueryDataRequest" + "kind": "QueryDataRequest", + "version": "v0alpha1" } }, "parameters": [