-
Notifications
You must be signed in to change notification settings - Fork 119
[cozystack-api] Configure dynamic api via custom resources #1230
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
Conversation
""" WalkthroughThis change migrates the configuration of Cozystack resource definitions from a static ConfigMap to dynamic loading via Kubernetes Custom Resource Definitions (CRDs). It introduces new Go API types, CRD manifests, and code for listing and mapping CRDs into internal configuration. Associated deep copy functions are generated, and static config loading logic is removed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API Server
participant Controller
participant K8s Cluster
User->>K8s Cluster: Apply CozystackResourceDefinition CRs
Controller->>K8s Cluster: List CozystackResourceDefinition CRs
K8s Cluster-->>Controller: Return CR list
Controller->>Controller: Map CRs to internal config structs
Controller->>API Server: Start with dynamic config
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
💤 Files with no reviewable changes (3)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @kvaps, 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 new Custom Resource Definition (CRD) named CozystackResourceDefinition
to the Cozystack API. This new resource aims to provide a structured and extensible way to define and manage application and release configurations within the Cozystack ecosystem, likely facilitating the automated deployment and management of applications via Helm charts.
Highlights
- New CRD Introduction: A new Kubernetes Custom Resource Definition (CRD) named
CozystackResourceDefinition
is introduced to thecozystack.io
API group underv1alpha1
. This CRD will allow users to define custom application and release configurations. - Schema Definition: The Go types for the
CozystackResourceDefinition
are defined, including itsSpec
which encapsulatesApplication
andRelease
configurations. This provides a structured way to specify application metadata and deployment details. - Application Configuration: The
Application
part of the spec includes fields likeKind
,OpenAPISchema
,Plural
, andSingular
names, suggesting its use for defining application-specific metadata for UI and API integration. - Release Management Integration: The
Release
configuration within the CRD's spec integrates with Helm, allowing specification of the Helm chartName
,SourceRef
(with defaultKind
as 'HelmRepository' andNamespace
as 'cozy-public'),Labels
, and aPrefix
for release names. - Automated Code Generation: Necessary deepcopy methods and OpenAPI schema definitions are automatically generated and updated for the new CRD types, ensuring proper Kubernetes object handling and API server validation.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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 or fill out our survey to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new CozystackResourceDefinition
CRD. The implementation is a good start, but there are a few critical issues that need to be addressed before merging. I've found a misnamed field that violates Kubernetes conventions, an incorrect type registration in the init()
function that will cause runtime errors, and missing OpenAPI definitions for the new types. Please address these points and regenerate the necessary files.
} | ||
|
||
func init() { | ||
SchemeBuilder.Register(&Workload{}, &WorkloadList{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This init()
function is registering Workload
and WorkloadList
instead of the new CozystackResourceDefinition
and CozystackResourceDefinitionList
types. This will prevent the scheme from recognizing the new types, leading to runtime errors when the controller tries to handle them.
func init() {
SchemeBuilder.Register(&CozystackResourceDefinition{}, &CozystackResourceDefinitionList{})
}
metav1.TypeMeta `json:",inline"` | ||
metav1.ObjectMeta `json:"metadata,omitempty"` | ||
|
||
Status CozystackResourceDefinitionSpec `json:"spec,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to Kubernetes API conventions, the field containing the desired state of a resource should be named Spec
, not Status
. The Status
field is reserved for the observed state of the resource. The current naming is confusing and could lead to maintenance issues.
Status CozystackResourceDefinitionSpec `json:"spec,omitempty"` | |
Spec CozystackResourceDefinitionSpec `json:"spec,omitempty"` |
"github.com/cozystack/cozystack/pkg/apis/apps/v1alpha1.Application": schema_pkg_apis_apps_v1alpha1_Application(ref), | ||
"github.com/cozystack/cozystack/pkg/apis/apps/v1alpha1.ApplicationList": schema_pkg_apis_apps_v1alpha1_ApplicationList(ref), | ||
"github.com/cozystack/cozystack/pkg/apis/apps/v1alpha1.ApplicationStatus": schema_pkg_apis_apps_v1alpha1_ApplicationStatus(ref), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OpenAPI definitions for the new CozystackResourceDefinition
types are missing from this generated file. This is required for features like server-side validation and kubectl explain
to work correctly for your new CRD.
Please run make generate
to update all generated files, including this one, and commit the changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api/v1alpha1/cozystackresourcedefinitions_types.go (1)
42-44
: Registration fixed correctly.The
init()
function now properly registersCozystackResourceDefinition
andCozystackResourceDefinitionList
types with the scheme, addressing the previous critical issue.
🧹 Nitpick comments (3)
pkg/cmd/server/start.go (3)
143-143
: Remove or clarify the TODO commentThe TODO comment appears to be outdated since the implementation below actually does fetch
CozystackResourceDefinition
resources. Either remove this comment or update it to reflect what still needs to be done.- // TODO get CozystackResourceDefinition
149-152
: Consider making kubeconfig path configurableThe empty string passed to
BuildConfigFromFlags
assumes in-cluster configuration. Consider making this configurable for development and testing scenarios.Consider adding a flag to specify the kubeconfig path:
- cfg, err := clientcmd.BuildConfigFromFlags("", "") + kubeconfigPath := "" // Could be populated from a flag + cfg, err := clientcmd.BuildConfigFromFlags("", kubeconfigPath)
173-173
: Implement shortNames supportThe
ShortNames
field is hardcoded as an empty array with a TODO comment. This feature would be valuable for kubectl usability.Would you like me to help implement the shortNames feature or create an issue to track this enhancement?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pkg/generated/openapi/zz_generated.openapi.go
is excluded by!**/generated/**
📒 Files selected for processing (10)
.gitignore
(1 hunks)api/v1alpha1/cozystackresourcedefinitions_types.go
(1 hunks)api/v1alpha1/zz_generated.deepcopy.go
(2 hunks)hack/update-codegen.sh
(1 hunks)packages/system/cozystack-api/templates/configmap.yaml
(0 hunks)packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml
(1 hunks)packages/system/cozystack-api/templates/deployment.yaml
(0 hunks)packages/system/cozystack-controller/templates/crds/cozystack.io_cozystackresourcedefinitions.yaml
(1 hunks)pkg/cmd/server/start.go
(4 hunks)pkg/config/config.go
(0 hunks)
💤 Files with no reviewable changes (3)
- packages/system/cozystack-api/templates/deployment.yaml
- pkg/config/config.go
- packages/system/cozystack-api/templates/configmap.yaml
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: lllamnyp
PR: cozystack/cozystack#1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is `kuberneteses.apps.cozystack.io`, not `kubernetes.apps.cozystack.io`. This is defined in the API schema even though it's not grammatically perfect.
packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml (4)
Learnt from: lllamnyp
PR: #1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is kuberneteses.apps.cozystack.io
, not kubernetes.apps.cozystack.io
. This is defined in the API schema even though it's not grammatically perfect.
Learnt from: NickVolynkin
PR: #1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The cozy-lib.resources.sanitize
function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
Learnt from: NickVolynkin
PR: #1216
File: packages/apps/virtual-machine/values.yaml:0-0
Timestamp: 2025-07-26T18:12:05.641Z
Learning: In the cozystack repository, for the virtual-machine app's resources.sockets parameter, the value is intentionally kept as a string in values.yaml despite being documented as {int} type, because the string-to-integer parsing happens elsewhere in the system pipeline rather than at the YAML validation level.
Learnt from: lllamnyp
PR: #1239
File: packages/extra/seaweedfs/templates/external/cosi-bucket-class.yaml:3-15
Timestamp: 2025-07-24T13:21:17.388Z
Learning: In COSI (Container Object Storage Interface), BucketAccessClass resources have fields like driverName and authenticationType at the top level, not under a spec block. Only BucketClass resources require a spec block for fields like driverName and deletionPolicy.
api/v1alpha1/cozystackresourcedefinitions_types.go (4)
Learnt from: lllamnyp
PR: #1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is kuberneteses.apps.cozystack.io
, not kubernetes.apps.cozystack.io
. This is defined in the API schema even though it's not grammatically perfect.
Learnt from: NickVolynkin
PR: #1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The cozy-lib.resources.sanitize
function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
Learnt from: NickVolynkin
PR: #1216
File: packages/apps/virtual-machine/values.yaml:31-33
Timestamp: 2025-07-26T18:01:52.557Z
Learning: In cozystack's schema generator annotation format, when documenting fields of array items, use the singular form of the item type rather than array notation. For example, for a parameter gpus {[]gpu}
, use @field gpu.name
rather than @field gpus[].name
to refer to the name field of each GPU object in the array.
Learnt from: lllamnyp
PR: #1239
File: packages/extra/seaweedfs/templates/external/cosi-bucket-class.yaml:3-15
Timestamp: 2025-07-24T13:21:17.388Z
Learning: In COSI (Container Object Storage Interface), BucketAccessClass resources have fields like driverName and authenticationType at the top level, not under a spec block. Only BucketClass resources require a spec block for fields like driverName and deletionPolicy.
pkg/cmd/server/start.go (1)
Learnt from: lllamnyp
PR: #1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is kuberneteses.apps.cozystack.io
, not kubernetes.apps.cozystack.io
. This is defined in the API schema even though it's not grammatically perfect.
packages/system/cozystack-controller/templates/crds/cozystack.io_cozystackresourcedefinitions.yaml (2)
Learnt from: lllamnyp
PR: #1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is kuberneteses.apps.cozystack.io
, not kubernetes.apps.cozystack.io
. This is defined in the API schema even though it's not grammatically perfect.
Learnt from: NickVolynkin
PR: #1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The cozy-lib.resources.sanitize
function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
🧬 Code Graph Analysis (3)
api/v1alpha1/cozystackresourcedefinitions_types.go (1)
pkg/apis/apps/v1alpha1/register.go (1)
RegisterDynamicTypes
(67-84)
pkg/cmd/server/start.go (4)
pkg/apis/apps/v1alpha1/register.go (2)
AddToScheme
(45-45)Resource
(62-64)api/v1alpha1/cozystackresourcedefinitions_types.go (2)
CozystackResourceDefinitionList
(36-40)SourceRef
(60-69)pkg/config/config.go (6)
ResourceConfig
(20-22)Resource
(25-28)ApplicationConfig
(31-37)ReleaseConfig
(40-44)ChartConfig
(47-50)SourceRefConfig
(53-57)cmd/cozystack-controller/main.go (1)
init
(52-58)
api/v1alpha1/zz_generated.deepcopy.go (3)
api/v1alpha1/cozystackresourcedefinitions_types.go (7)
CozystackResourceDefinition
(26-31)CozystackResourceDefinitionApplication
(71-80)CozystackResourceDefinitionChart
(53-58)SourceRef
(60-69)CozystackResourceDefinitionList
(36-40)CozystackResourceDefinitionRelease
(82-89)CozystackResourceDefinitionSpec
(46-51)pkg/generated/applyconfiguration/apps/v1alpha1/application.go (1)
Application
(39-46)pkg/apis/apps/v1alpha1/zz_generated.deepcopy.go (4)
DeepCopyInto
(31-42)DeepCopyInto
(96-106)DeepCopyInto
(63-75)DeepCopy
(45-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (18)
.gitignore (1)
80-81
: LGTM!Adding
tmp/
to.gitignore
is a standard practice for excluding temporary files from version control.hack/update-codegen.sh (1)
35-37
: LGTM!The additional code generation for the
api
directory is necessary to support the newCozystackResourceDefinition
types and follows the same pattern as the existing code generation.packages/system/cozystack-controller/templates/crds/cozystack.io_cozystackresourcedefinitions.yaml (1)
1-116
: CRD definition looks well-structuredThe CRD follows Kubernetes conventions and includes appropriate schema validation. The default values for
sourceRef.kind
("HelmRepository") andsourceRef.namespace
("cozy-public") are sensible defaults that align with the Helm-based deployment model.pkg/cmd/server/start.go (1)
144-191
: Good implementation of dynamic configuration loadingThe migration from static config loading to dynamic CRD-based configuration is well-implemented. The error handling is comprehensive, and the conversion logic properly maps CRD fields to the internal configuration structure.
packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml (3)
369-369
: Verify empty prefix is intentional for certain resourcesSeveral resources (monitoring, etcd, ingress, seaweedfs, bootbox, info) have an empty
prefix
field. This means their release names won't have a prefix, which could lead to naming conflicts.Please confirm this is intentional. If these are singleton resources per namespace, the empty prefix might be appropriate. Otherwise, consider adding prefixes for consistency and to avoid potential naming conflicts.
Also applies to: 390-390, 411-411, 432-432, 453-453, 474-474
1-483
: Well-structured resource definitionsThe resource definitions are comprehensive and follow a consistent pattern. The use of Helm template functions for loading OpenAPI schemas is appropriate, and the separation between cozystack-apps and cozystack-extra repositories is clear.
10-10
: All referenced OpenAPI schema files existAll 23 JSON schemas referenced in
packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml
underopenapi-schemas/
have been verified present inpackages/system/cozystack-api/openapi-schemas
. No further action required.api/v1alpha1/cozystackresourcedefinitions_types.go (4)
23-31
: LGTM! Proper Kubernetes CRD structure.The
CozystackResourceDefinition
type correctly follows Kubernetes API conventions with embeddedTypeMeta
andObjectMeta
, and properly uses aSpec
field for desired state configuration.
33-40
: LGTM! Correct list type implementation.The
CozystackResourceDefinitionList
follows standard Kubernetes list type conventions with proper metadata embedding and items field.
46-69
: Well-designed struct definitions.The spec structures are well-organized with clear field names, proper JSON tags, and appropriate kubebuilder defaults. The default values for
SourceRef
(HelmRepository kind and cozy-public namespace) align well with the cozystack ecosystem.
71-89
: Comprehensive and well-structured definitions.The
Application
andRelease
structs effectively capture all necessary configuration for dynamic resource definitions. The optionalLabels
field withomitempty
tag and the clear field separation between application metadata and Helm release configuration demonstrate good API design.api/v1alpha1/zz_generated.deepcopy.go (7)
28-52
: LGTM! Correct deep copy implementation.The autogenerated deep copy methods for
CozystackResourceDefinition
properly handle embedded metadata and nested spec structures, ensuring safe object copying for Kubernetes API operations.
54-67
: LGTM! Appropriate deep copy for string-only struct.The deep copy methods for
CozystackResourceDefinitionApplication
correctly use shallow copying since the struct contains only string fields.
69-83
: LGTM! Correct handling of nested struct.The deep copy methods correctly handle the
SourceRef
field with direct assignment, which is appropriate sinceSourceRef
contains only string fields.
85-115
: LGTM! Proper list deep copy implementation.The deep copy methods for
CozystackResourceDefinitionList
correctly handle the embedded metadata and properly iterate through the Items slice to ensure each element is deeply copied.
117-138
: LGTM! Proper map deep copying.The deep copy methods correctly handle the
Labels
map field with appropriate nil checking and key-value pair copying, ensuring the map is properly deep copied.
140-155
: LGTM! Correct handling of nested structs.The deep copy methods appropriately use direct copying for the
Application
field andDeepCopyInto
for theRelease
field, correctly accounting for the different deep copy requirements of each nested struct.
178-191
: LGTM! Simple struct deep copy.The deep copy methods for
SourceRef
correctly use shallow copying since the struct contains only string fields.
packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml (1)
1-483
: Missing OpenAPI schema files for CozystackResourceDefinitionsIt looks like every
.Files.Get "openapi-schemas/*.json"
reference inpackages/system/cozystack-api/templates/cozystack-resource-definitions.yaml
points at a directory that doesn’t exist in the chart. As a result, all of these schemas are missing and the CRD templates will fail to render.Please address the following:
• Confirm or create an
openapi-schemas
folder alongside yourtemplates/
directory in the chart.
• Add the expected JSON files:
- bucket.json
- clickhouse.json
- http-cache.json
- nats.json
- tcp-balancer.json
- virtual-machine.json
- vpn.json
- mysql.json
- tenant.json
- kubernetes.json
- redis.json
- rabbitmq.json
- postgres.json
- ferretdb.json
- kafka.json
- vm-disk.json
- vm-instance.json
- monitoring.json
- etcd.json
- ingress.json
- seaweedfs.json
- bootbox.json
- info.json
• Or, if the JSON files live elsewhere, update each
.Files.Get "openapi-schemas/…"
path to the correct location.Fix the missing files or paths so that Helm can successfully load the OpenAPI schemas.
♻️ Duplicate comments (1)
packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml (1)
282-282
: Fix incorrect plural form for FerretDB.The plural form should be "ferretdbs" not "ferretdb" to maintain consistency with Kubernetes naming conventions.
- plural: ferretdb + plural: ferretdbs
🧹 Nitpick comments (1)
pkg/cmd/server/start.go (1)
143-191
: Review the dynamic CRD loading implementation.The Complete() method has been significantly refactored to load resource definitions dynamically from Kubernetes CRDs. The implementation looks correct, but there are a few considerations:
- Error handling: Good error wrapping with context-specific messages
- TODO comment: Line 173 mentions implementing shortnames - consider tracking this
- Resource conversion: The mapping from CRD spec to internal ResourceConfig appears complete
The TODO comment on line 173 indicates shortnames are not yet implemented. Would you like me to generate a follow-up issue to track this enhancement?
Additionally, consider verifying that all expected CozystackResourceDefinition resources exist in the cluster during startup to provide early feedback if configurations are missing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pkg/generated/openapi/zz_generated.openapi.go
is excluded by!**/generated/**
📒 Files selected for processing (10)
.gitignore
(1 hunks)api/v1alpha1/cozystackresourcedefinitions_types.go
(1 hunks)api/v1alpha1/zz_generated.deepcopy.go
(2 hunks)hack/update-codegen.sh
(1 hunks)packages/system/cozystack-api/templates/configmap.yaml
(0 hunks)packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml
(1 hunks)packages/system/cozystack-api/templates/deployment.yaml
(0 hunks)packages/system/cozystack-controller/templates/crds/cozystack.io_cozystackresourcedefinitions.yaml
(1 hunks)pkg/cmd/server/start.go
(4 hunks)pkg/config/config.go
(0 hunks)
💤 Files with no reviewable changes (3)
- packages/system/cozystack-api/templates/deployment.yaml
- pkg/config/config.go
- packages/system/cozystack-api/templates/configmap.yaml
✅ Files skipped from review due to trivial changes (1)
- api/v1alpha1/cozystackresourcedefinitions_types.go
🚧 Files skipped from review as they are similar to previous changes (4)
- hack/update-codegen.sh
- .gitignore
- packages/system/cozystack-controller/templates/crds/cozystack.io_cozystackresourcedefinitions.yaml
- api/v1alpha1/zz_generated.deepcopy.go
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: lllamnyp
PR: cozystack/cozystack#1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is `kuberneteses.apps.cozystack.io`, not `kubernetes.apps.cozystack.io`. This is defined in the API schema even though it's not grammatically perfect.
pkg/cmd/server/start.go (1)
Learnt from: lllamnyp
PR: #1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is kuberneteses.apps.cozystack.io
, not kubernetes.apps.cozystack.io
. This is defined in the API schema even though it's not grammatically perfect.
packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml (5)
Learnt from: lllamnyp
PR: #1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is kuberneteses.apps.cozystack.io
, not kubernetes.apps.cozystack.io
. This is defined in the API schema even though it's not grammatically perfect.
Learnt from: NickVolynkin
PR: #1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The cozy-lib.resources.sanitize
function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
Learnt from: NickVolynkin
PR: #1216
File: packages/apps/virtual-machine/values.yaml:0-0
Timestamp: 2025-07-26T18:12:05.641Z
Learning: In the cozystack repository, for the virtual-machine app's resources.sockets parameter, the value is intentionally kept as a string in values.yaml despite being documented as {int} type, because the string-to-integer parsing happens elsewhere in the system pipeline rather than at the YAML validation level.
Learnt from: NickVolynkin
PR: #1216
File: packages/apps/virtual-machine/values.yaml:31-33
Timestamp: 2025-07-26T18:01:52.557Z
Learning: In cozystack's schema generator annotation format, when documenting fields of array items, use the singular form of the item type rather than array notation. For example, for a parameter gpus {[]gpu}
, use @field gpu.name
rather than @field gpus[].name
to refer to the name field of each GPU object in the array.
Learnt from: lllamnyp
PR: #1239
File: packages/extra/seaweedfs/templates/external/cosi-bucket-class.yaml:3-15
Timestamp: 2025-07-24T13:21:17.388Z
Learning: In COSI (Container Object Storage Interface), BucketAccessClass resources have fields like driverName and authenticationType at the top level, not under a spec block. Only BucketClass resources require a spec block for fields like driverName and deletionPolicy.
🪛 YAMLlint (1.37.1)
packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml
[error] 10-10: syntax error: expected chomping or indentation indicators, but found '{'
(syntax)
🔇 Additional comments (3)
pkg/cmd/server/start.go (2)
28-28
: LGTM! Imports align with the dynamic configuration approach.The new imports are appropriate for the transition from static config files to dynamic CRD-based configuration:
corev1alpha1
for the new CozystackResourceDefinition typesruntime
for scheme managementclientcmd
for Kubernetes client configurationclient
from controller-runtime for CRD operationsAlso applies to: 34-34, 43-43, 47-47
58-58
: LGTM! Client field addition supports dynamic configuration.Adding the Kubernetes client to AppsServerOptions enables dynamic retrieval of CozystackResourceDefinition resources from the cluster, replacing the previous static configuration approach.
packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml (1)
198-198
: LGTM! Kubernetes plural form is correct.The plural form "kuberneteses" for the Kubernetes resource is intentionally used despite not being grammatically perfect, as defined in the API schema. This aligns with the established convention in the codebase.
packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml
Outdated
Show resolved
Hide resolved
d7a0881
to
59049e5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/system/cozystack-api/templates/rbac.yaml (1)
15-17
: Wildcard"*"
does not cover sub-resources; confirm if status/scale updates are required
"*"
matches top-level resources only. If any component needs topatch
orupdate
thestatus
(or other sub-resources) ofCozystackResourceDefinition
, you will also need either
resources: ["*/status"]
or the broader"*/*"
pattern.Double-check the controller code before rollout to avoid unexpected 403s.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pkg/generated/openapi/zz_generated.openapi.go
is excluded by!**/generated/**
📒 Files selected for processing (11)
.gitignore
(1 hunks)api/v1alpha1/cozystackresourcedefinitions_types.go
(1 hunks)api/v1alpha1/zz_generated.deepcopy.go
(2 hunks)hack/update-codegen.sh
(1 hunks)packages/system/cozystack-api/templates/configmap.yaml
(0 hunks)packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml
(1 hunks)packages/system/cozystack-api/templates/deployment.yaml
(0 hunks)packages/system/cozystack-api/templates/rbac.yaml
(1 hunks)packages/system/cozystack-controller/templates/crds/cozystack.io_cozystackresourcedefinitions.yaml
(1 hunks)pkg/cmd/server/start.go
(4 hunks)pkg/config/config.go
(0 hunks)
💤 Files with no reviewable changes (3)
- packages/system/cozystack-api/templates/deployment.yaml
- packages/system/cozystack-api/templates/configmap.yaml
- pkg/config/config.go
✅ Files skipped from review due to trivial changes (1)
- api/v1alpha1/cozystackresourcedefinitions_types.go
🚧 Files skipped from review as they are similar to previous changes (6)
- .gitignore
- hack/update-codegen.sh
- pkg/cmd/server/start.go
- packages/system/cozystack-controller/templates/crds/cozystack.io_cozystackresourcedefinitions.yaml
- api/v1alpha1/zz_generated.deepcopy.go
- packages/system/cozystack-api/templates/cozystack-resource-definitions.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: lllamnyp
PR: cozystack/cozystack#1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is `kuberneteses.apps.cozystack.io`, not `kubernetes.apps.cozystack.io`. This is defined in the API schema even though it's not grammatically perfect.
packages/system/cozystack-api/templates/rbac.yaml (2)
Learnt from: lllamnyp
PR: #1161
File: packages/apps/virtual-machine/templates/dashboard-resourcemap.yaml:6-12
Timestamp: 2025-07-10T12:14:27.197Z
Learning: Kubernetes RBAC rules with resourceNames work correctly for list/watch verbs. When resourceNames is specified in an RBAC rule, it properly restricts access to only those named resources, even for list and watch operations. Examples: kubectl get resource resourcename -w
watches for changes on a single resource, and kubectl get resource --field-selector .metadata.name=resourcename
lists a specific resource. The Kubernetes API server correctly distinguishes such requests from their less specific counterparts.
Learnt from: lllamnyp
PR: #1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is kuberneteses.apps.cozystack.io
, not kubernetes.apps.cozystack.io
. This is defined in the API schema even though it's not grammatically perfect.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
What this PR does
This patch introduces a new CRD to manage the configuration of the Cozystack extension API server. Configuration previously done with a single ConfigMap containing a list of objects is now decomposed into a number of custom resources. Platform administrators receive a better UX for defining their own custom Cozystack managed applications in addition to the existing apps shipped with the default Cozystack installation.
Release note
Summary by CodeRabbit
New Features
Improvements
Removals