Plugins: Add apiserver info to settings when it exists #88041
Conversation
ryantxu
added
no-backport
| @@ -28,7 +28,7 @@ type PluginSetting struct { | |||
| SignatureType plugins.SignatureType `json:"signatureType"` | |||
| SignatureOrg string `json:"signatureOrg"` | |||
| AngularDetected bool `json:"angularDetected"` | |||
| APIVersion string `json:"apiVersion"` | |||
There was a problem hiding this comment.
the backend is not a single apiVersion -- it may have a few and a preferred version
There was a problem hiding this comment.
why not simplify this just using the preferredVersion here? In which case the frontend would be interested in knowing all the versions the backend supports?
Also, why duplicating this information? This is currently available in the endpoing /apis for the datasource backend endpoint.
There was a problem hiding this comment.
replied already here: #87639 (comment)
Not sure if the frontend needs to handle this complexity (multiple api versions) but it's syntactically correct so if we want to move forward with this it can be okay
There was a problem hiding this comment.
Alternatively we can keep this out of the plugins DTO entirely -- I want to make sure we do not read it to mean more than it does. The issue I have with the simplified version is that it encourages thinking about frontend+backend as one deployment rather than two things: client and server
There was a problem hiding this comment.
we need to have something here if we want to set the value in queries / configs. The frontend will use the preferredVersion anyway and use it if we add the whole api server info. I don't see a use case for the fronted to use the rest of the information so why adding it?
There was a problem hiding this comment.
What do we need to move this forward? Agreement on either only including preferred version or plus api group and versions?
Given this is new/experimental can we start with just adding what we actually need right now and iterate figuring out the rest? As long as we don't officially support any new api response field we can add/remove/change how many times we want/need.
There was a problem hiding this comment.
As far as I understand from this comment: #87961 (comment) we may want to just remove the apiVersion from the /api/* endpoints and leave it for the new ones under /apis/* 🤷
|
@andresmgot -- See #87639 (comment) -- that thread got lost when I made a new PR here. |
| @@ -28,7 +28,7 @@ type PluginSetting struct { | |||
| SignatureType plugins.SignatureType `json:"signatureType"` | |||
| SignatureOrg string `json:"signatureOrg"` | |||
| AngularDetected bool `json:"angularDetected"` | |||
| APIVersion string `json:"apiVersion"` | |||
There was a problem hiding this comment.
why not simplify this just using the preferredVersion here? In which case the frontend would be interested in knowing all the versions the backend supports?
Also, why duplicating this information? This is currently available in the endpoing /apis for the datasource backend endpoint.
| @@ -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() | |||
There was a problem hiding this comment.
This can probably be in a different PR? It's unrelated to having apiserver info in the plugin DTO.
There was a problem hiding this comment.
👍🏻 -- i can split it out soon, already a few too many open PRs for hte same topic 🤣
This replaces #87639
This updates the settings model so it is more clearly a indication of where the apiserver is running with which versions:
