chore: update dependencies and refactor API types for resource manage… - #34
Conversation
…ment - Added k8s.io/api as a dependency to support new API types. - Refactored ModuleSpec and SimpleAppSpec to replace runtime.RawExtension with specific Kubernetes API types for better clarity and type safety. - Updated CRD definitions for LimitRange and ResourceQuota to reflect changes in the underlying types. - Enhanced deepcopy functions to accommodate the new type structures. - Removed outdated comments and improved descriptions in the CRDs for better understanding.
Summary of ChangesHello, 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 significantly enhances the type safety and clarity of the project's Kubernetes Custom Resources by replacing generic Highlights
Changelog
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 is a valuable refactoring that significantly improves type safety and clarity by replacing runtime.RawExtension with specific, strongly-typed Kubernetes API objects for various resources. This change enhances CRD validation and overall maintainability. The dependency updates and modifications to the deepcopy functions are consistent with this goal. I've identified one issue in the go.sum file that could affect build stability, which should be straightforward to resolve.
| golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM= | ||
| google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= | ||
| google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= | ||
| gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= |
There was a problem hiding this comment.
The go.sum file appears to have been left in an inconsistent state. While this line (the content hash for gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405) was removed, the corresponding /go.mod entry on the next line was not. A go.sum file should have both the module content hash and the go.mod hash for each version, or neither. This inconsistency can cause build or verification failures.
Please run go mod tidy to automatically clean up the go.sum file and ensure its consistency.
There was a problem hiding this comment.
Pull request overview
This PR updates the API surface of several CRDs to use strongly-typed Kubernetes API specs (instead of runtime.RawExtension), adds the required Kubernetes dependency, and regenerates CRD/deepcopy artifacts accordingly.
Changes:
- Add
k8s.io/apidependency to support embedding core/apps Kubernetes spec types in CRDs. - Refactor
WorkspaceSpecandSimpleAppSpecto usecorev1/appsv1spec structs (and update generated deepcopy code). - Update the
WorkspaceCRD schema to reflect the new structural schemas forlimitRangeandresourceQuota(and remove outdated CRD descriptions/comments elsewhere).
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tenant/v1alpha1/workspace_types.go |
Switch Workspace quota/limit fields to typed corev1 specs. |
tenant/v1alpha1/zz_generated.deepcopy.go |
Regenerate deepcopy to match new typed fields and metav1 conditions. |
config/crd/bases/tenant.otterscale.io_workspaces.yaml |
Update Workspace CRD OpenAPI schema for LimitRangeSpec / ResourceQuotaSpec. |
apps/v1alpha1/simpleapp_types.go |
Switch SimpleApp embedded specs to typed appsv1/corev1 types. |
apps/v1alpha1/zz_generated.deepcopy.go |
Regenerate deepcopy to match new typed fields and metav1 conditions. |
addons/v1alpha1/module_types.go |
Remove outdated comment text around Helm values field behavior. |
config/crd/bases/addons.otterscale.io_modules.yaml |
Remove outdated CRD description text for Module values. |
go.mod |
Add k8s.io/api (plus indirect dependency adjustments). |
go.sum |
Record new module sums from the dependency update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Values overrides the default Helm chart values for Helm-based modules. | ||
| // Only applicable when the referenced ModuleTemplate uses a HelmChart template. | ||
| // Ignored for Kustomization-based modules. | ||
| // Accepts arbitrary JSON; RawExtension is used instead of apiextensionsv1.JSON | ||
| // to avoid pulling in the k8s.io/apiextensions-apiserver dependency. | ||
| // +kubebuilder:pruning:PreserveUnknownFields | ||
| // +optional | ||
| Values *runtime.RawExtension `json:"values,omitempty"` |
There was a problem hiding this comment.
The PR description says ModuleSpec was refactored to replace runtime.RawExtension with typed Kubernetes API types, but ModuleSpec.Values is still a *runtime.RawExtension. Either update the PR description to reflect the actual change, or complete the refactor for ModuleSpec (and regenerate CRDs/deepcopy as needed).
| // ResourceQuota defines the compute resource constraints (CPU, Memory, etc.) applied to the underlying namespace. | ||
| // The actual schema is composed at runtime by the Schema RPC from the core/v1 ResourceQuota. | ||
| // +kubebuilder:pruning:PreserveUnknownFields | ||
| // +optional | ||
| ResourceQuota *runtime.RawExtension `json:"resourceQuota,omitempty"` | ||
| ResourceQuota *corev1.ResourceQuotaSpec `json:"resourceQuota,omitempty"` | ||
|
|
||
| // LimitRange defines the default resource limits and requests for pods in the workspace. | ||
| // The actual schema is composed at runtime by the Schema RPC from the core/v1 LimitRange. | ||
| // +kubebuilder:pruning:PreserveUnknownFields | ||
| // +optional | ||
| LimitRange *runtime.RawExtension `json:"limitRange,omitempty"` | ||
| LimitRange *corev1.LimitRangeSpec `json:"limitRange,omitempty"` |
There was a problem hiding this comment.
Changing WorkspaceSpec.resourceQuota and .limitRange from RawExtension to corev1 specs is a wire-format/schema tightening (unknown fields previously accepted will now be rejected/pruned). If existing clusters may already have Workspace objects, please add/describe a migration strategy (e.g., version bump + conversion webhook or documented upgrade/migration steps) to avoid breaking upgrades.
| // Deployment defines the pod template, replicas, and update strategy | ||
| // for the application workload. | ||
| // +required | ||
| Deployment runtime.RawExtension `json:"deployment"` | ||
| Deployment appsv1.DeploymentSpec `json:"deployment"` | ||
|
|
||
| // Service defines the Service configuration. | ||
| // If specified, a Service will be created. | ||
| // The actual schema is composed at runtime by the Schema RPC from the core/v1 Service. | ||
| // +kubebuilder:pruning:PreserveUnknownFields | ||
| // If specified, a Service will be created to expose the application. | ||
| // +optional | ||
| Service *runtime.RawExtension `json:"service,omitempty"` | ||
| Service *corev1.ServiceSpec `json:"service,omitempty"` | ||
|
|
||
| // PersistentVolumeClaim defines the PersistentVolumeClaim configuration. | ||
| // If specified, a PersistentVolumeClaim will be created. | ||
| // The actual schema is composed at runtime by the Schema RPC from the core/v1 PersistentVolumeClaim. | ||
| // +kubebuilder:pruning:PreserveUnknownFields | ||
| // If specified, a PersistentVolumeClaim will be created for persistent storage. | ||
| // +optional | ||
| PersistentVolumeClaim *runtime.RawExtension `json:"persistentVolumeClaim,omitempty"` | ||
| PersistentVolumeClaim *corev1.PersistentVolumeClaimSpec `json:"persistentVolumeClaim,omitempty"` |
There was a problem hiding this comment.
Changing SimpleAppSpec.deployment/service/persistentVolumeClaim from RawExtension to typed Kubernetes specs is a backward-incompatible schema tightening for existing CRs/clients. If this API is already in use, consider a versioned migration path (new version + conversion) or document upgrade steps to ensure existing SimpleApps remain valid.
| limitRange: | ||
| description: |- | ||
| LimitRange defines the default resource limits and requests for pods in the workspace. | ||
| The actual schema is composed at runtime by the Schema RPC from the core/v1 LimitRange. | ||
| description: LimitRange defines the default resource limits and requests | ||
| for pods in the workspace. | ||
| properties: | ||
| limits: | ||
| description: Limits is the list of LimitRangeItem objects that | ||
| are enforced. |
There was a problem hiding this comment.
This CRD change removes preserve-unknown-fields and introduces structural schemas for limitRange/resourceQuota. That can make previously-stored Workspace objects invalid if they contained extra/unrecognized fields under these blocks. Ensure the release/upgrade notes include a migration or validation plan for existing clusters before applying the updated CRD.
| resourceQuota: | ||
| description: |- | ||
| ResourceQuota defines the compute resource constraints (CPU, Memory, etc.) applied to the underlying namespace. | ||
| The actual schema is composed at runtime by the Schema RPC from the core/v1 ResourceQuota. | ||
| description: ResourceQuota defines the compute resource constraints | ||
| (CPU, Memory, etc.) applied to the underlying namespace. | ||
| properties: | ||
| hard: | ||
| additionalProperties: | ||
| anyOf: |
There was a problem hiding this comment.
This CRD change removes preserve-unknown-fields and introduces a strict schema for resourceQuota. If any existing Workspace objects relied on arbitrary keys under resourceQuota, they will fail validation after the CRD update. Please ensure upgrade guidance/migration is in place (or keep unknown-field preservation if backward compatibility is required).
…ment