feat: Add 'capability' schema with model and ts#559
feat: Add 'capability' schema with model and ts#559leecalcote wants to merge 1 commit intomasterfrom
Conversation
Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
Summary of ChangesHello @leecalcote, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a foundational 'capability' feature, establishing a structured and extensible mechanism for defining and managing provider-specific capabilities, user interface extensions, and restricted access within the Meshery platform. By leveraging OpenAPI for API definition and code generation, it ensures a consistent and type-safe approach to integrating these new configurations across both backend (Go) and frontend (TypeScript) components, enabling dynamic and granular control over Meshery's features and UI based on various provider contexts. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new capability schema, generating the corresponding Go models and TypeScript types from an OpenAPI specification. It also includes a JSON template for capabilities and integrates new API endpoints into the RTK Query client. The changes are well-structured, but there are a few key areas for improvement. The generated Go code contains significant duplication due to inlined anonymous structs, which should be addressed. Critically, the TypeScript types for the new RTK Query endpoints are manually copied instead of being imported, posing a significant maintainability risk. Additionally, there's a minor naming inconsistency in the OpenAPI schema's JSON tag that should be corrected. Addressing these points will greatly improve the maintainability and robustness of the new code.
| export type GetCapabilityApiResponse = /** status 200 Provider capability */ { | ||
| /** Type of provider (e.g., remote, local) */ | ||
| providerType?: string; | ||
| /** Version of the extensions package */ | ||
| packageVersion?: string; | ||
| /** URL to download the extensions package */ | ||
| packageUrl?: string; | ||
| /** Name of the provider */ | ||
| providerName?: string; | ||
| /** List of provider feature descriptions */ | ||
| providerDescription?: string[]; | ||
| /** Extension points for Meshery UI and backend */ | ||
| extensions?: { | ||
| navigator?: { | ||
| title?: string; | ||
| onClickCallback?: number; | ||
| /** Href configuration for navigator extensions */ | ||
| href?: { | ||
| uri?: string; | ||
| external?: boolean | null; | ||
| }; | ||
| component?: string; | ||
| icon?: string; | ||
| link?: boolean | null; | ||
| show?: boolean | null; | ||
| type?: string; | ||
| /** Component set for MeshMap permissions */ | ||
| allowedTo?: { | ||
| /** Designer component permissions */ | ||
| designer?: { | ||
| design?: boolean; | ||
| application?: boolean; | ||
| filter?: boolean; | ||
| save?: boolean; | ||
| new?: boolean; | ||
| saveAs?: boolean; | ||
| validate?: boolean; | ||
| deploy?: boolean; | ||
| undeploy?: boolean; | ||
| }; | ||
| visualizer?: boolean; | ||
| }; | ||
| isBeta?: boolean | null; | ||
| }[]; | ||
| userPrefs?: { | ||
| component?: string; | ||
| type?: string; | ||
| }[]; | ||
| graphql?: { | ||
| component?: string; | ||
| path?: string; | ||
| type?: string; | ||
| }[]; | ||
| account?: { | ||
| title?: string; | ||
| onClickCallback?: number; | ||
| /** Href configuration for account extensions */ | ||
| href?: { | ||
| uri?: string; | ||
| external?: boolean | null; | ||
| }; | ||
| component?: string; | ||
| link?: boolean | null; | ||
| show?: boolean | null; | ||
| type?: string; | ||
| }[]; | ||
| collaborator?: { | ||
| component?: string; | ||
| type?: string; | ||
| }[]; | ||
| }; | ||
| capabilities?: { | ||
| feature?: string; | ||
| endpoint?: string; | ||
| }[]; | ||
| /** Restricted access configuration for Meshery UI */ | ||
| restrictedAccess?: { | ||
| isMesheryUIRestricted?: boolean; | ||
| /** Meshery UI component capability */ | ||
| allowedComponents?: { | ||
| /** Navigator component visibility settings */ | ||
| navigator?: { | ||
| dashboard?: boolean; | ||
| performance?: boolean; | ||
| conformance?: boolean; | ||
| extensions?: boolean; | ||
| toggler?: boolean; | ||
| help?: boolean; | ||
| /** Service mesh adapter visibility settings */ | ||
| lifecycle?: { | ||
| istio?: boolean; | ||
| citrix?: boolean; | ||
| consul?: boolean; | ||
| cilium?: boolean; | ||
| appMesh?: boolean; | ||
| kuma?: boolean; | ||
| linkerd?: boolean; | ||
| nginx?: boolean; | ||
| nsm?: boolean; | ||
| }; | ||
| /** Configuration component visibility settings */ | ||
| configuration?: { | ||
| designs?: boolean; | ||
| applications?: boolean; | ||
| filters?: boolean; | ||
| }; | ||
| }; | ||
| /** Header component visibility settings */ | ||
| header?: { | ||
| contextSwitcher?: boolean; | ||
| settings?: boolean; | ||
| notifications?: boolean; | ||
| profile?: boolean; | ||
| }; | ||
| }; | ||
| }; | ||
| }; | ||
| export type GetCapabilityApiArg = { | ||
| /** Operating system for package URL */ | ||
| os?: string; | ||
| /** Whether this is a playground build */ | ||
| playground?: boolean; | ||
| }; | ||
| export type GetCapabilityByVersionApiResponse = /** status 200 Provider capability */ { | ||
| /** Type of provider (e.g., remote, local) */ | ||
| providerType?: string; | ||
| /** Version of the extensions package */ | ||
| packageVersion?: string; | ||
| /** URL to download the extensions package */ | ||
| packageUrl?: string; | ||
| /** Name of the provider */ | ||
| providerName?: string; | ||
| /** List of provider feature descriptions */ | ||
| providerDescription?: string[]; | ||
| /** Extension points for Meshery UI and backend */ | ||
| extensions?: { | ||
| navigator?: { | ||
| title?: string; | ||
| onClickCallback?: number; | ||
| /** Href configuration for navigator extensions */ | ||
| href?: { | ||
| uri?: string; | ||
| external?: boolean | null; | ||
| }; | ||
| component?: string; | ||
| icon?: string; | ||
| link?: boolean | null; | ||
| show?: boolean | null; | ||
| type?: string; | ||
| /** Component set for MeshMap permissions */ | ||
| allowedTo?: { | ||
| /** Designer component permissions */ | ||
| designer?: { | ||
| design?: boolean; | ||
| application?: boolean; | ||
| filter?: boolean; | ||
| save?: boolean; | ||
| new?: boolean; | ||
| saveAs?: boolean; | ||
| validate?: boolean; | ||
| deploy?: boolean; | ||
| undeploy?: boolean; | ||
| }; | ||
| visualizer?: boolean; | ||
| }; | ||
| isBeta?: boolean | null; | ||
| }[]; | ||
| userPrefs?: { | ||
| component?: string; | ||
| type?: string; | ||
| }[]; | ||
| graphql?: { | ||
| component?: string; | ||
| path?: string; | ||
| type?: string; | ||
| }[]; | ||
| account?: { | ||
| title?: string; | ||
| onClickCallback?: number; | ||
| /** Href configuration for account extensions */ | ||
| href?: { | ||
| uri?: string; | ||
| external?: boolean | null; | ||
| }; | ||
| component?: string; | ||
| link?: boolean | null; | ||
| show?: boolean | null; | ||
| type?: string; | ||
| }[]; | ||
| collaborator?: { | ||
| component?: string; | ||
| type?: string; | ||
| }[]; | ||
| }; | ||
| capabilities?: { | ||
| feature?: string; | ||
| endpoint?: string; | ||
| }[]; | ||
| /** Restricted access configuration for Meshery UI */ | ||
| restrictedAccess?: { | ||
| isMesheryUIRestricted?: boolean; | ||
| /** Meshery UI component capability */ | ||
| allowedComponents?: { | ||
| /** Navigator component visibility settings */ | ||
| navigator?: { | ||
| dashboard?: boolean; | ||
| performance?: boolean; | ||
| conformance?: boolean; | ||
| extensions?: boolean; | ||
| toggler?: boolean; | ||
| help?: boolean; | ||
| /** Service mesh adapter visibility settings */ | ||
| lifecycle?: { | ||
| istio?: boolean; | ||
| citrix?: boolean; | ||
| consul?: boolean; | ||
| cilium?: boolean; | ||
| appMesh?: boolean; | ||
| kuma?: boolean; | ||
| linkerd?: boolean; | ||
| nginx?: boolean; | ||
| nsm?: boolean; | ||
| }; | ||
| /** Configuration component visibility settings */ | ||
| configuration?: { | ||
| designs?: boolean; | ||
| applications?: boolean; | ||
| filters?: boolean; | ||
| }; | ||
| }; | ||
| /** Header component visibility settings */ | ||
| header?: { | ||
| contextSwitcher?: boolean; | ||
| settings?: boolean; | ||
| notifications?: boolean; | ||
| profile?: boolean; | ||
| }; | ||
| }; | ||
| }; | ||
| }; | ||
| export type GetCapabilityByVersionApiArg = { | ||
| /** Meshery version string */ | ||
| mesheryVersion: string; | ||
| /** Operating system for package URL */ | ||
| os?: string; | ||
| /** Whether this is a playground build */ | ||
| playground?: boolean; | ||
| }; |
There was a problem hiding this comment.
The API response and argument types for getCapability and getCapabilityByVersion are manually defined here. This creates significant code duplication, as these types are already generated in typescript/generated/v1beta1/capability/Capability.ts. Manually defining them here makes the code harder to maintain and prone to errors, as any changes in the OpenAPI schema will not be automatically reflected in these types.
Please remove these manual type definitions and import them from the generated file instead. You would add the import at the top of the file:
import { components, operations } from "../../generated/v1beta1/capability/Capability";And then define the types like this:
export type GetCapabilityApiResponse = components["schemas"]["Capability"];
export type GetCapabilityApiArg = operations["getCapability"]["parameters"]["query"];
export type GetCapabilityByVersionApiResponse = components["schemas"]["Capability"];
export type GetCapabilityByVersionApiArg = operations["getCapabilityByVersion"]["parameters"]["path"] & operations["getCapabilityByVersion"]["parameters"]["query"];This will remove over 200 lines of duplicated code and make the API client robust to schema changes.
| type Capability struct { | ||
| Capabilities *[]struct { | ||
| Endpoint *string `json:"endpoint,omitempty" yaml:"endpoint,omitempty"` | ||
| Feature *string `json:"feature,omitempty" yaml:"feature,omitempty"` | ||
| } `json:"capabilities,omitempty" yaml:"capabilities,omitempty"` | ||
|
|
||
| // Extensions Extension points for Meshery UI and backend | ||
| Extensions *struct { | ||
| Account *[]struct { | ||
| Component *string `json:"component,omitempty" yaml:"component,omitempty"` | ||
|
|
||
| // Href Href configuration for account extensions | ||
| Href *struct { | ||
| External *bool `json:"external,omitempty" yaml:"external,omitempty"` | ||
| Uri *string `json:"uri,omitempty" yaml:"uri,omitempty"` | ||
| } `json:"href,omitempty" yaml:"href,omitempty"` | ||
| Link *bool `json:"link,omitempty" yaml:"link,omitempty"` | ||
| OnClickCallback *int `json:"on_click_callback,omitempty" yaml:"on_click_callback,omitempty"` | ||
| Show *bool `json:"show,omitempty" yaml:"show,omitempty"` | ||
| Title *string `json:"title,omitempty" yaml:"title,omitempty"` | ||
| Type *string `json:"type,omitempty" yaml:"type,omitempty"` | ||
| } `json:"account,omitempty" yaml:"account,omitempty"` | ||
| Collaborator *[]struct { | ||
| Component *string `json:"component,omitempty" yaml:"component,omitempty"` | ||
| Type *string `json:"type,omitempty" yaml:"type,omitempty"` | ||
| } `json:"collaborator,omitempty" yaml:"collaborator,omitempty"` | ||
| Graphql *[]struct { | ||
| Component *string `json:"component,omitempty" yaml:"component,omitempty"` | ||
| Path *string `json:"path,omitempty" yaml:"path,omitempty"` | ||
| Type *string `json:"type,omitempty" yaml:"type,omitempty"` | ||
| } `json:"graphql,omitempty" yaml:"graphql,omitempty"` | ||
| Navigator *[]struct { | ||
| // AllowedTo Component set for MeshMap permissions | ||
| AllowedTo *struct { | ||
| // Designer Designer component permissions | ||
| Designer *struct { | ||
| Application *bool `json:"application,omitempty" yaml:"application,omitempty"` | ||
| Deploy *bool `json:"deploy,omitempty" yaml:"deploy,omitempty"` | ||
| Design *bool `json:"design,omitempty" yaml:"design,omitempty"` | ||
| Filter *bool `json:"filter,omitempty" yaml:"filter,omitempty"` | ||
| New *bool `json:"new,omitempty" yaml:"new,omitempty"` | ||
| Save *bool `json:"save,omitempty" yaml:"save,omitempty"` | ||
| SaveAs *bool `json:"saveAs,omitempty" yaml:"saveAs,omitempty"` | ||
| Undeploy *bool `json:"unDeploy,omitempty" yaml:"unDeploy,omitempty"` | ||
| Validate *bool `json:"validate,omitempty" yaml:"validate,omitempty"` | ||
| } `json:"designer,omitempty" yaml:"designer,omitempty"` | ||
| Visualizer *bool `json:"visualizer,omitempty" yaml:"visualizer,omitempty"` | ||
| } `json:"allowedTo,omitempty" yaml:"allowedTo,omitempty"` | ||
| Component *string `json:"component,omitempty" yaml:"component,omitempty"` | ||
|
|
||
| // Href Href configuration for navigator extensions | ||
| Href *struct { | ||
| External *bool `json:"external,omitempty" yaml:"external,omitempty"` | ||
| Uri *string `json:"uri,omitempty" yaml:"uri,omitempty"` | ||
| } `json:"href,omitempty" yaml:"href,omitempty"` | ||
| Icon *string `json:"icon,omitempty" yaml:"icon,omitempty"` | ||
| IsBeta *bool `json:"isBeta,omitempty" yaml:"isBeta,omitempty"` | ||
| Link *bool `json:"link,omitempty" yaml:"link,omitempty"` | ||
| OnClickCallback *int `json:"on_click_callback,omitempty" yaml:"on_click_callback,omitempty"` | ||
| Show *bool `json:"show,omitempty" yaml:"show,omitempty"` | ||
| Title *string `json:"title,omitempty" yaml:"title,omitempty"` | ||
| Type *string `json:"type,omitempty" yaml:"type,omitempty"` | ||
| } `json:"navigator,omitempty" yaml:"navigator,omitempty"` | ||
| UserPrefs *[]struct { | ||
| Component *string `json:"component,omitempty" yaml:"component,omitempty"` | ||
| Type *string `json:"type,omitempty" yaml:"type,omitempty"` | ||
| } `json:"user_prefs,omitempty" yaml:"user_prefs,omitempty"` | ||
| } `json:"extensions,omitempty" yaml:"extensions,omitempty"` | ||
|
|
||
| // PackageUrl URL to download the extensions package | ||
| PackageUrl *string `json:"package_url,omitempty" yaml:"package_url,omitempty"` | ||
|
|
||
| // PackageVersion Version of the extensions package | ||
| PackageVersion *string `json:"package_version,omitempty" yaml:"package_version,omitempty"` | ||
|
|
||
| // ProviderDescription List of provider feature descriptions | ||
| ProviderDescription *[]string `json:"provider_description,omitempty" yaml:"provider_description,omitempty"` | ||
|
|
||
| // ProviderName Name of the provider | ||
| ProviderName *string `json:"provider_name,omitempty" yaml:"provider_name,omitempty"` | ||
|
|
||
| // ProviderType Type of provider (e.g., remote, local) | ||
| ProviderType *string `json:"provider_type,omitempty" yaml:"provider_type,omitempty"` | ||
|
|
||
| // RestrictedAccess Restricted access configuration for Meshery UI | ||
| RestrictedAccess *struct { | ||
| // AllowedComponents Meshery UI component capability | ||
| AllowedComponents *struct { | ||
| // Header Header component visibility settings | ||
| Header *struct { | ||
| ContextSwitcher *bool `json:"contextSwitcher,omitempty" yaml:"contextSwitcher,omitempty"` | ||
| Notifications *bool `json:"notifications,omitempty" yaml:"notifications,omitempty"` | ||
| Profile *bool `json:"profile,omitempty" yaml:"profile,omitempty"` | ||
| Settings *bool `json:"settings,omitempty" yaml:"settings,omitempty"` | ||
| } `json:"header,omitempty" yaml:"header,omitempty"` | ||
|
|
||
| // Navigator Navigator component visibility settings | ||
| Navigator *struct { | ||
| // Configuration Configuration component visibility settings | ||
| Configuration *struct { | ||
| Applications *bool `json:"applications,omitempty" yaml:"applications,omitempty"` | ||
| Designs *bool `json:"designs,omitempty" yaml:"designs,omitempty"` | ||
| Filters *bool `json:"filters,omitempty" yaml:"filters,omitempty"` | ||
| } `json:"configuration,omitempty" yaml:"configuration,omitempty"` | ||
| Conformance *bool `json:"conformance,omitempty" yaml:"conformance,omitempty"` | ||
| Dashboard *bool `json:"dashboard,omitempty" yaml:"dashboard,omitempty"` | ||
| Extensions *bool `json:"extensions,omitempty" yaml:"extensions,omitempty"` | ||
| Help *bool `json:"help,omitempty" yaml:"help,omitempty"` | ||
|
|
||
| // Lifecycle Service mesh adapter visibility settings | ||
| Lifecycle *struct { | ||
| AppMesh *bool `json:"appMesh,omitempty" yaml:"appMesh,omitempty"` | ||
| Cilium *bool `json:"cilium,omitempty" yaml:"cilium,omitempty"` | ||
| Citrix *bool `json:"citrix,omitempty" yaml:"citrix,omitempty"` | ||
| Consul *bool `json:"consul,omitempty" yaml:"consul,omitempty"` | ||
| Istio *bool `json:"istio,omitempty" yaml:"istio,omitempty"` | ||
| Kuma *bool `json:"kuma,omitempty" yaml:"kuma,omitempty"` | ||
| Linkerd *bool `json:"linkerd,omitempty" yaml:"linkerd,omitempty"` | ||
| Nginx *bool `json:"nginx,omitempty" yaml:"nginx,omitempty"` | ||
| Nsm *bool `json:"nsm,omitempty" yaml:"nsm,omitempty"` | ||
| } `json:"lifecycle,omitempty" yaml:"lifecycle,omitempty"` | ||
| Performance *bool `json:"performance,omitempty" yaml:"performance,omitempty"` | ||
| Toggler *bool `json:"toggler,omitempty" yaml:"toggler,omitempty"` | ||
| } `json:"navigator,omitempty" yaml:"navigator,omitempty"` | ||
| } `json:"allowedComponents,omitempty" yaml:"allowedComponents,omitempty"` | ||
| IsMesheryUIRestricted *bool `json:"isMesheryUIRestricted" yaml:"isMesheryUIRestricted"` | ||
| } `json:"restrictedAccess,omitempty" yaml:"restrictedAccess,omitempty"` | ||
| } |
There was a problem hiding this comment.
The generated Go code contains many inlined anonymous structs (e.g., within Capability.Extensions, Capability.Capabilities, Capability.RestrictedAccess) instead of using the named struct types that are also generated in the same file (e.g., CapabilityExtension, CapabilityGeneralCapability, RestrictedAccess). This leads to significant code duplication and makes the generated code verbose and difficult to maintain.
For example, the Capability struct should ideally be defined like this to promote reusability and readability:
type Capability struct {
Capabilities *[]CapabilityGeneralCapability `json:"capabilities,omitempty" yaml:"capabilities,omitempty"`
Extensions *CapabilityExtension `json:"extensions,omitempty" yaml:"extensions,omitempty"`
PackageUrl *string `json:"package_url,omitempty" yaml:"package_url,omitempty"`
PackageVersion *string `json:"package_version,omitempty" yaml:"package_version,omitempty"`
ProviderDescription *[]string `json:"provider_description,omitempty" yaml:"provider_description,omitempty"`
ProviderName *string `json:"provider_name,omitempty" yaml:"provider_name,omitempty"`
ProviderType *string `json:"provider_type,omitempty" yaml:"provider_type,omitempty"`
RestrictedAccess *RestrictedAccess `json:"restrictedAccess,omitempty" yaml:"restrictedAccess,omitempty"`
}This seems to be an issue with how oapi-codegen is processing the OpenAPI schema, as the schema correctly uses $ref. Please investigate the generator's behavior or configuration to produce cleaner, non-duplicated Go structs.
| x-oapi-codegen-extra-tags: | ||
| json: unDeploy,omitempty | ||
| yaml: unDeploy,omitempty |
There was a problem hiding this comment.
The JSON tag for the undeploy property is unDeploy, which uses inconsistent casing compared to other properties like saveAs. For consistency and to follow standard camelCase conventions, it's recommended to use undeploy.
x-oapi-codegen-extra-tags:
json: undeploy,omitempty
yaml: undeploy,omitempty1 participant
Signed-off-by: Lee Calcote lee.calcote@layer5.io