Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve apiVersion in plugin models #87639

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/pr-go-workspace-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
echo "Changes detected:"
git diff
echo "Please run 'go work sync' and commit the changes."
echo "If there is a change in enterprise dependencies, please update pkg/extensions/main.go.
echo "If there is a change in enterprise dependencies, please update pkg/extensions/main.go."
exit 1
fi

Expand All @@ -46,6 +46,6 @@ jobs:
echo "Changes detected:"
git diff
echo "Please run 'make build-go' and commit the changes."
echo "If there is a change in enterprise dependencies, please update pkg/extensions/main.go.
echo "If there is a change in enterprise dependencies, please update pkg/extensions/main.go."
exit 1
fi
5 changes: 5 additions & 0 deletions docs/sources/developers/plugins/plugin.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,11 @@
}
}
}
},
"apiVersion": {
"type": "string",
"description": "[internal only] The API version for the plugin. Used for Datasource API servers. This metadata is temporary and will be removed in the future.",
"pattern": "^v([\\d]+)(?:(alpha|beta)([\\d]+))?$"
}
}
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ require (
github.com/grafana/grafana-azure-sdk-go/v2 v2.0.3 // @grafana/partner-datasources
github.com/grafana/grafana-google-sdk-go v0.1.0 // @grafana/partner-datasources
github.com/grafana/grafana-openapi-client-go v0.0.0-20231213163343-bd475d63fb79 // @grafana/grafana-backend-group
github.com/grafana/grafana-plugin-sdk-go v0.229.0 // @grafana/plugins-platform-backend
github.com/grafana/grafana-plugin-sdk-go v0.230.0 // @grafana/plugins-platform-backend
github.com/grafana/grafana/pkg/apimachinery v0.0.0-20240226124929-648abdbd0ea4 // @grafana/grafana-app-platform-squad
github.com/grafana/grafana/pkg/apiserver v0.0.0-20240226124929-648abdbd0ea4 // @grafana/grafana-app-platform-squad
// This needs to be here for other projects that import grafana/grafana
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2176,8 +2176,8 @@ github.com/grafana/grafana-google-sdk-go v0.1.0/go.mod h1:Vo2TKWfDVmNTELBUM+3lkr
github.com/grafana/grafana-openapi-client-go v0.0.0-20231213163343-bd475d63fb79 h1:r+mU5bGMzcXCRVAuOrTn54S80qbfVkvTdUJZfSfTNbs=
github.com/grafana/grafana-openapi-client-go v0.0.0-20231213163343-bd475d63fb79/go.mod h1:wc6Hbh3K2TgCUSfBC/BOzabItujtHMESZeFk5ZhdxhQ=
github.com/grafana/grafana-plugin-sdk-go v0.114.0/go.mod h1:D7x3ah+1d4phNXpbnOaxa/osSaZlwh9/ZUnGGzegRbk=
github.com/grafana/grafana-plugin-sdk-go v0.229.0 h1:0IeuzscdAYcDVsXvIatqdObff4oTkebsXWTtGT4X9bA=
github.com/grafana/grafana-plugin-sdk-go v0.229.0/go.mod h1:6V6ikT4ryva8MrAp7Bdz5fTJx3/ztzKvpMJFfpzr4CI=
github.com/grafana/grafana-plugin-sdk-go v0.230.0 h1:Y4IL+eT1jXqTCctlNzdCvxAozpBZ8xEsRGWjGAVRXxo=
github.com/grafana/grafana-plugin-sdk-go v0.230.0/go.mod h1:6V6ikT4ryva8MrAp7Bdz5fTJx3/ztzKvpMJFfpzr4CI=
github.com/grafana/grafana/pkg/apimachinery v0.0.0-20240226124929-648abdbd0ea4 h1:hpyusz8c3yRFoJPlA0o34rWnsLbaOOBZleqRhFBi5Lg=
github.com/grafana/grafana/pkg/apimachinery v0.0.0-20240226124929-648abdbd0ea4/go.mod h1:vrRQJuNprTWqwm6JPxHf3BoTJhvO15QMEjQ7Q/YUOnI=
github.com/grafana/grafana/pkg/apiserver v0.0.0-20240226124929-648abdbd0ea4 h1:tIbI5zgos92vwJ8lV3zwHwuxkV03GR3FGLkFW9V5LxY=
Expand Down
141 changes: 141 additions & 0 deletions go.work.sum

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions pkg/api/datasources.go
Original file line number Diff line number Diff line change
Expand Up @@ -840,6 +840,7 @@ func (hs *HTTPServer) convertModelToDtos(ctx context.Context, ds *datasources.Da
SecureJsonFields: map[string]bool{},
Version: ds.Version,
ReadOnly: ds.ReadOnly,
APIVersion: ds.APIVersion,
}

if hs.pluginStore != nil {
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/dtos/datasource.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ type DataSource struct {
Version int `json:"version"`
ReadOnly bool `json:"readOnly"`
AccessControl accesscontrol.Metadata `json:"accessControl,omitempty"`

// The apiVersion (and schema) when the datasource was saved
// This value links the JsonData and SecureJsonFields with an explicit version
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remove the comment if it helps 🤷🏻

But I keep getting confused which one indicates where it is running vs where it was saved!

APIVersion string `json:"apiVersion,omitempty"`
}

type DataSourceListItemDTO struct {
Expand Down
23 changes: 23 additions & 0 deletions pkg/api/dtos/plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type PluginSetting struct {
SignatureType plugins.SignatureType `json:"signatureType"`
SignatureOrg string `json:"signatureOrg"`
AngularDetected bool `json:"angularDetected"`
APIServer *APIServerInfo `json:"apiserver,omitempty"`
}

type PluginListItem struct {
Expand All @@ -49,6 +50,28 @@ type PluginListItem struct {
AccessControl accesscontrol.Metadata `json:"accessControl,omitempty"`
AngularDetected bool `json:"angularDetected"`
IAM *pfs.IAM `json:"iam,omitempty"`
APIServer *APIServerInfo `json:"apiserver,omitempty"`
}

type APIServerInfo struct {
Group string `json:"group"`
Versions []string `json:"versions"`
PreferedVersion string `json:"preferredVersion"`
}

func GetPluginAPIServerInfo(pluginID, apiVersion string) *APIServerInfo {
if apiVersion == "" {
return nil
}
group, err := plugins.GetDatasourceGroupNameFromPluginID(pluginID)
if err != nil {
return nil
}
return &APIServerInfo{
Group: group,
Versions: []string{apiVersion},
PreferedVersion: apiVersion,
}
}

type PluginList []PluginListItem
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ func (hs *HTTPServer) GetPluginList(c *contextmodel.ReqContext) response.Respons
SignatureOrg: pluginDef.SignatureOrg,
AccessControl: pluginsMetadata[pluginDef.ID],
AngularDetected: pluginDef.Angular.Detected,
APIServer: dtos.GetPluginAPIServerInfo(pluginDef.ID, pluginDef.APIVersion),
}

if hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagExternalServiceAccounts) {
Expand Down Expand Up @@ -208,6 +209,7 @@ func (hs *HTTPServer) GetPluginSettingByID(c *contextmodel.ReqContext) response.
SignatureOrg: plugin.SignatureOrg,
SecureJsonFields: map[string]bool{},
AngularDetected: plugin.Angular.Detected,
APIServer: dtos.GetPluginAPIServerInfo(plugin.ID, plugin.APIVersion),
}

if plugin.IsApp() {
Expand Down
11 changes: 9 additions & 2 deletions pkg/apimachinery/apis/common/v0alpha1/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@ func NewResourceInfo(group, version, resourceName, singularName, kind string,
return ResourceInfo{group, version, resourceName, singularName, shortName, kind, newObj, newList}
}

func (info *ResourceInfo) WithGroupAndShortName(group string, shortName string) ResourceInfo {
func (info *ResourceInfo) WithGroupAndShortName(group, version, shortName string) ResourceInfo {
if version == "" {
version = info.version
}
return ResourceInfo{
group: group,
version: info.version,
version: version,
resourceName: info.resourceName,
singularName: info.singularName,
kind: info.kind,
Expand Down Expand Up @@ -58,6 +61,10 @@ func (info *ResourceInfo) TypeMeta() metav1.TypeMeta {
}
}

func (info *ResourceInfo) APIVersion() string {
return info.version
}

func (info *ResourceInfo) GroupVersion() schema.GroupVersion {
return schema.GroupVersion{
Group: info.group,
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.21.10
require (
github.com/bwmarrin/snowflake v0.3.0
github.com/gorilla/mux v1.8.1
github.com/grafana/grafana-plugin-sdk-go v0.229.0
github.com/grafana/grafana-plugin-sdk-go v0.230.0
github.com/grafana/grafana/pkg/apimachinery v0.0.0-20240409140820-518d3341d58f
github.com/stretchr/testify v1.9.0
golang.org/x/mod v0.15.0
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=
github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/grafana/grafana-plugin-sdk-go v0.229.0 h1:0IeuzscdAYcDVsXvIatqdObff4oTkebsXWTtGT4X9bA=
github.com/grafana/grafana-plugin-sdk-go v0.230.0 h1:Y4IL+eT1jXqTCctlNzdCvxAozpBZ8xEsRGWjGAVRXxo=
github.com/grafana/grafana/pkg/apimachinery v0.0.0-20240409140820-518d3341d58f h1:+CK3tH3XrAAqx5urmVqpgSxMrL2MlpTOnLVSU4w4IjY=
github.com/grafana/grafana/pkg/apimachinery v0.0.0-20240409140820-518d3341d58f/go.mod h1:ZxIaCOlDmFupiL55aLU+Qp7O1dgwkDMBAQBK7wnEVBg=
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 h1:UH//fgunKIs4JdUbpDl1VZCDaL56wXCB/5+wF6uHfaI=
Expand Down
35 changes: 35 additions & 0 deletions pkg/plugins/manager/pipeline/validation/steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package validation
import (
"context"
"errors"
"fmt"
"regexp"
"slices"
"time"

Expand Down Expand Up @@ -115,3 +117,36 @@ func (a *AngularDetector) Validate(ctx context.Context, p *plugins.Plugin) error
p.Angular.HideDeprecation = slices.Contains(a.cfg.HideAngularDeprecation, p.ID)
return nil
}

// APIVersionValidation implements a ValidateFunc for validating plugin API versions.
type APIVersionValidation struct {
}

// APIVersionValidationStep returns a new ValidateFunc for validating plugin signatures.
func APIVersionValidationStep() ValidateFunc {
sv := &APIVersionValidation{}
return sv.Validate
}

// Validate validates the plugin signature. If a signature error is encountered, the error is recorded with the
// pluginerrs.ErrorTracker.
func (v *APIVersionValidation) Validate(ctx context.Context, p *plugins.Plugin) error {
if p.APIVersion != "" {
if !p.Backend {
return fmt.Errorf("plugin %s has an API version but is not a backend plugin", p.ID)
}
// Eventually, all backend plugins will be supported
if p.Type != plugins.TypeDataSource {
return fmt.Errorf("plugin %s has an API version but is not a datasource plugin", p.ID)
}
m, err := regexp.MatchString(`^v([\d]+)(?:(alpha|beta)([\d]+))?$`, p.APIVersion)
if err != nil {
return fmt.Errorf("failed to verify apiVersion %s: %v", p.APIVersion, err)
}
if !m {
return fmt.Errorf("plugin %s has an invalid API version %s", p.ID, p.APIVersion)
}
}

return nil
}
75 changes: 75 additions & 0 deletions pkg/plugins/manager/pipeline/validation/steps_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package validation

import (
"context"
"testing"

"github.com/grafana/grafana/pkg/plugins"
"github.com/stretchr/testify/require"
)

func TestAPIVersionValidation(t *testing.T) {
s := APIVersionValidationStep()

tests := []struct {
name string
plugin *plugins.Plugin
err bool
}{
{
name: "valid plugin",
plugin: &plugins.Plugin{
JSONData: plugins.JSONData{
Backend: true,
Type: plugins.TypeDataSource,
APIVersion: "v0alpha1",
},
},
err: false,
},
{
name: "invalid plugin - not backend",
plugin: &plugins.Plugin{
JSONData: plugins.JSONData{
Backend: false,
Type: plugins.TypeDataSource,
APIVersion: "v0alpha1",
},
},
err: true,
},
{
name: "invalid plugin - not datasource",
plugin: &plugins.Plugin{
JSONData: plugins.JSONData{
Backend: true,
Type: plugins.TypeApp,
APIVersion: "v0alpha1",
},
},
err: true,
},
{
name: "invalid plugin - invalid API version",
plugin: &plugins.Plugin{
JSONData: plugins.JSONData{
Backend: true,
Type: plugins.TypeDataSource,
APIVersion: "invalid",
},
},
err: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := s(context.Background(), tt.plugin)
if tt.err {
require.Error(t, err)
} else {
require.NoError(t, err)
}
})
}
}
3 changes: 3 additions & 0 deletions pkg/plugins/plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ type JSONData struct {

// App Service Auth Registration
IAM *pfs.IAM `json:"iam,omitempty"`

// API Version: Temporary field while plugins don't expose a OpenAPI schema
APIVersion string `json:"apiVersion,omitempty"`
}

func ReadPluginJSON(reader io.Reader) (JSONData, error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/promlib/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/grafana/grafana/pkg/promlib
go 1.21.10

require (
github.com/grafana/grafana-plugin-sdk-go v0.229.0
github.com/grafana/grafana-plugin-sdk-go v0.230.0
github.com/json-iterator/go v1.1.12
github.com/patrickmn/go-cache v2.1.0+incompatible
github.com/prometheus/client_golang v1.19.0
Expand Down
2 changes: 1 addition & 1 deletion pkg/promlib/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e h1:JKmoR8x90Iww1
github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
github.com/grafana/grafana-plugin-sdk-go v0.229.0 h1:0IeuzscdAYcDVsXvIatqdObff4oTkebsXWTtGT4X9bA=
github.com/grafana/grafana-plugin-sdk-go v0.230.0 h1:Y4IL+eT1jXqTCctlNzdCvxAozpBZ8xEsRGWjGAVRXxo=
github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db h1:7aN5cccjIqCLTzedH7MZzRZt5/lsAHch6Z3L2ZGn5FA=
github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db/go.mod h1:M5qHK+eWfAv8VR/265dIuEpL3fNfeC21tXXp9itM24A=
github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.0.1 h1:qnpSQwGEnkcRpTqNOIR6bJbR0gAorgP9CSALpRcKoAA=
Expand Down
15 changes: 10 additions & 5 deletions pkg/registry/apis/datasource/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func RegisterAPIService(
}

for _, ds := range all {
if !slices.Contains(ids, ds.ID) {
if !slices.Contains(ids, ds.ID) && ds.APIVersion == "" {
continue // skip this one
}

Expand Down Expand Up @@ -106,7 +106,7 @@ func NewDataSourceAPIBuilder(
datasources PluginDatasourceProvider,
contextProvider PluginContextWrapper,
accessControl accesscontrol.AccessControl) (*DataSourceAPIBuilder, error) {
ri, err := resourceFromPluginID(plugin.ID)
ri, err := resourceFromPluginID(plugin.ID, plugin.APIVersion)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -158,12 +158,15 @@ func (b *DataSourceAPIBuilder) InstallSchema(scheme *runtime.Scheme) error {
return scheme.SetVersionPriority(gv)
}

func resourceFromPluginID(pluginID string) (common.ResourceInfo, error) {
func resourceFromPluginID(pluginID string, apiVersion string) (common.ResourceInfo, error) {
group, err := plugins.GetDatasourceGroupNameFromPluginID(pluginID)
if err != nil {
return common.ResourceInfo{}, err
}
return datasource.GenericConnectionResourceInfo.WithGroupAndShortName(group, pluginID+"-connection"), nil
if apiVersion == "" {
apiVersion = "v0alpha1"
}
return datasource.GenericConnectionResourceInfo.WithGroupAndShortName(group, apiVersion, pluginID+"-connection"), nil
}

func (b *DataSourceAPIBuilder) GetAPIGroupInfo(
Expand Down Expand Up @@ -224,7 +227,9 @@ func (b *DataSourceAPIBuilder) getPluginContext(ctx context.Context, uid string)
if err != nil {
return backend.PluginContext{}, err
}
return b.contextProvider.PluginContextForDataSource(ctx, instance)
pctx, err := b.contextProvider.PluginContextForDataSource(ctx, instance)
pctx.APIVersion = b.connectionResourceInfo.APIVersion()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^^^ this is the interesting part

This tells the backend what apiversion was requested.

NOTE: pctx.DataSourceInstanceSettings.APIVersion may be different! it should be the version when the datasource settings were saved

return pctx, err
}

func (b *DataSourceAPIBuilder) GetOpenAPIDefinitions() openapi.GetOpenAPIDefinitions {
Expand Down
1 change: 1 addition & 0 deletions pkg/services/datasources/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ var (
ErrDatasourceIsReadOnly = errors.New("data source is readonly, can only be updated from configuration")
ErrDataSourceNameInvalid = errutil.ValidationFailed("datasource.nameInvalid", errutil.WithPublicMessage("Invalid datasource name."))
ErrDataSourceURLInvalid = errutil.ValidationFailed("datasource.urlInvalid", errutil.WithPublicMessage("Invalid datasource url."))
ErrDataSourceAPIVersionInvalid = errutil.ValidationFailed("datasource.apiVersionInvalid", errutil.WithPublicMessage("Invalid datasource apiVersion."))
)
3 changes: 3 additions & 0 deletions pkg/services/datasources/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ type DataSource struct {
SecureJsonData map[string][]byte `json:"secureJsonData"`
ReadOnly bool `json:"readOnly"`
UID string `json:"uid" xorm:"uid"`
APIVersion string `json:"apiVersion" xorm:"api_version"`
// swagger:ignore
IsPrunable bool `xorm:"is_prunable"`

Expand Down Expand Up @@ -163,6 +164,7 @@ type AddDataSourceCommand struct {
JsonData *simplejson.Json `json:"jsonData"`
SecureJsonData map[string]string `json:"secureJsonData"`
UID string `json:"uid"`
APIVersion string `json:"apiVersion"`
// swagger:ignore
IsPrunable bool

Expand All @@ -189,6 +191,7 @@ type UpdateDataSourceCommand struct {
SecureJsonData map[string]string `json:"secureJsonData"`
Version int `json:"version"`
UID string `json:"uid"`
APIVersion string `json:"apiVersion"`
// swagger:ignore
IsPrunable bool

Expand Down