Add CatalogAPI #342
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR introduces a template-based architecture for resource exports, replacing per-resource dynamic discovery with module/collection-centric workflows. Core changes include renaming Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant UI as Backend UI
participant Manager as Backend Manager
participant K8s as Kubernetes API
rect rgb(200, 220, 255)
Note over User,K8s: Old Per-Resource Flow (v0.5)
User->>UI: Request resources
UI->>Manager: getBackendDynamicResource
Manager->>K8s: List APIServiceExports
K8s-->>Manager: Per-resource schemas
Manager-->>UI: Individual resource data
UI->>UI: Build per-resource cards
UI-->>User: Export list with per-resource Bind
end
rect rgb(200, 255, 220)
Note over User,K8s: New Template-Based Flow (v0.6)
User->>UI: Request resources
UI->>Manager: listCollectionTemplates
Manager->>K8s: List Collections
K8s-->>Manager: Collection list
Manager->>K8s: Get Templates for each Collection
K8s-->>Manager: Template specs with resources & claims
Manager-->>UI: Template data (Description, Resources, PermissionClaims)
UI->>UI: Build module cards from templates
UI-->>User: Module grid with Module-level Bind
end
rect rgb(255, 220, 200)
Note over User,K8s: Binding Flow
User->>UI: Bind Module
UI->>Manager: GetTemplates(templateName)
Manager->>K8s: Get specific Template
K8s-->>Manager: Template spec
Manager-->>UI: APIServiceExportRequestSpec from Template
UI->>UI: Create single APIServiceBinding
UI->>K8s: Apply APIServiceBinding
K8s-->>UI: Binding created/reused
UI-->>User: Success (X resources bound)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Rationale: The PR spans multiple heterogeneous areas (API schema changes, backend refactoring, UI redesign, CLI simplification, and codegen) with substantive logic modifications. While generated code is high-volume, the core business logic changes in backend handlers, controller updates for NamedResources, and CRD migrations require careful reasoning per area. The template-to-resources mapping, UISchema structure changes, and unified binding workflow represent densely interconnected logic changes across the system. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
mjudeikis
force-pushed
the
mjudeikis/catalog.api
branch
2 times, most recently
from
a66c8f5 to
39d8023
Compare
mjudeikis
force-pushed
the
mjudeikis/catalog.api
branch
from
39d8023 to
d76d14e
Compare
|
Ran into another issue with "provider driven namespaces & resources". As its now blocked by kcp-dev/kcp#3653, continue work until its unblocked and will start splitting it out after. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
deploy/crd/kube-bind.io_apiservicebindings.yaml (1)
384-405: Update example YAML file to use new field name.The breaking field rename has been correctly implemented in the schema (CRD) and Go code, and migration documentation is in place. However, the example file
contrib/kcp/deploy/examples/apiserviceexport-namespaced.yamlat line 24 still uses the oldnamedResource:(singular) field name, which will fail validation against the updated schema.Update line 24 in the example file from:
namedResource:to:
namedResources:The rest of the example uses valid structure—it only needs the field name corrected.
deploy/crd/kube-bind.io_apiserviceexports.yaml (1)
427-429: Validation message does not match ruleRule enforces
metadata.name == <plural>.<group>, but message says “informerScope is immutable”.Apply this diff:
- - message: informerScope is immutable - rule: self.metadata.name == self.spec.names.plural+"."+self.spec.group + - message: metadata.name must equal spec.names.plural+"."+spec.group + rule: self.metadata.name == self.spec.names.plural+"."+self.spec.group
🧹 Nitpick comments (15)
backend/template/resources.gohtml (1)
10-11: Consider pinning Bootstrap Icons to a specific version.The Bootstrap Icons CDN link uses version 1.7.2, which is several years old (released ~2021). Consider either:
- Updating to the latest stable version for bug fixes and new icons
- Adding a subresource integrity (SRI) hash for the specified version
- <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/bootstrap-icons@1.7.2/font/bootstrap-icons.css"> + <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/bootstrap-icons@1.11.3/font/bootstrap-icons.css" integrity="sha384-..." crossorigin="anonymous">You can generate the SRI hash at https://www.srihash.org/
pkg/konnector/controllers/cluster/namespacelifecycle/namespacelifecycle_controller.go (1)
237-254: Clarify intended behavior for namespace cleanup on APIServiceNamespace deletion.When an APIServiceNamespace is deleted (lines 239-254), the code checks if the corresponding Namespace exists but then does not delete it, stating "we can't be sure who owns it" (line 251). This could leave orphaned namespaces in the consumer cluster.
Is this the intended behavior? Consider:
- If the APIServiceNamespace had the
ObjectOwnerLabel, that information is now lost (since the object is deleted), making it impossible to determine ownership- This conservative approach prevents accidental deletion but may require manual cleanup
Suggested alternatives:
- Use a finalizer on APIServiceNamespace to determine ownership before deletion
- Add owner references or labels to the Namespace itself to track ownership
- Document this behavior and provide a cleanup tool for operators
Do you want to track this as a known limitation, or should we implement one of the alternatives?
sdk/apis/catalog/v1alpha1/collection_types.go (1)
71-78: Consider adding validation for module name format.The
ModuleReference.Namefield is marked as required but doesn't specify format validation. Should it validate that the name references an actual Module resource?Consider adding:
- Pattern validation to match Kubernetes resource name constraints
- A CEL validation rule to check that the referenced Module exists (if cross-resource validation is supported in your environment)
type ModuleReference struct { // name is the name of the module. // // +required // +kubebuilder:validation:Required + // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$` + // +kubebuilder:validation:MaxLength=253 Name string `json:"name"` }Validating the reference exists would require a validating webhook or CEL rule, which may be out of scope for this PR.
contrib/kcp/deploy/resources/apiresourceschema-apiservicebindings.kube-bind.io.yaml (1)
377-399: Pluralization is correct; fix descriptions and consider list keys + optional CEL.
- Descriptions still say “NamedResource” (singular). Align wording with the plural field.
- For deterministic merges/dedup, make the list a map keyed by name/namespace.
- If selector.labelSelector and selector.namedResources are intended to be mutually exclusive, add a CEL rule (optional).
Apply within this block:
- namedResources: - description: NamedResource is a shorthand for selecting a - single resource by name and namespace. + namedResources: + description: NamedResources is a shorthand for selecting specific + resources by name and namespace. + x-kubernetes-list-type: map + x-kubernetes-list-map-keys: + - name + - namespace items: - description: NamedResource selects a specific resource by - name and namespace. + description: A NamedResource item selects a specific resource by + name and namespace.If mutually exclusive with labelSelector, add under selector (outside this range):
# under: selector: x-kubernetes-validations: - rule: "has(self.labelSelector) != has(self.namedResources)" message: "Provide either labelSelector or namedResources, not both."Please confirm exclusivity intent; I can send a full patch accordingly.
pkg/konnector/controllers/cluster/claimedresources/claimedresources_reconciler.go (1)
186-187: LGTM! Good refactoring to use public constant.Replacing the hardcoded label key
"kube-bind.io/owner"with the public constantkubebindv1alpha2.ObjectOwnerLabelimproves maintainability and eliminates potential typos. The behavior remains unchanged.Also applies to: 195-196, 238-238, 251-251
docs/content/usage/index.md (1)
125-125: Minor grammar issue: hyphenate compound adjective.As per the static analysis hint, "Label Selector Based" should be hyphenated when used as a compound adjective before a noun.
Apply this diff:
-### Label Selector Based Claims +### Label-Selector-Based Claimsbackend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go (2)
376-405: Optional: rename param to avoid shadowing package name.Rename
cache cache.CachetoinfCache cache.Cachefor readability and to avoid shadowing the imported package.-func (r *reconciler) ensureAPIServiceNamespaces(ctx context.Context, cl client.Client, cache cache.Cache, req *kubebindv1alpha2.APIServiceExportRequest) error { +func (r *reconciler) ensureAPIServiceNamespaces(ctx context.Context, cl client.Client, infCache cache.Cache, req *kubebindv1alpha2.APIServiceExportRequest) error { - err := cache.Get(ctx, client.ObjectKey{Namespace: apiServiceNamespace.Namespace, Name: apiServiceNamespace.Name}, currentAPIServiceNamespace) + err := infCache.Get(ctx, client.ObjectKey{Namespace: apiServiceNamespace.Namespace, Name: apiServiceNamespace.Name}, currentAPIServiceNamespace)
73-76: Consider status/condition for namespace provisioning.Add a condition (e.g., NamespacesReady) to reflect progress/errors per requested namespace.
If desired, I can draft the condition plumbing.
deploy/crd/catalog.kube-bind.io_collections.yaml (1)
58-71: Enforce uniqueness for spec.modules via list-map keysTo prevent duplicate module references and get merge-friendly patches, model
modulesas a map-by-name.Apply this diff:
modules: description: modules is a list of module references that are part of this collection. items: description: ModuleReference references a Module by name. properties: name: description: name is the name of the module. type: string required: - name type: object - minItems: 1 - type: array + minItems: 1 + type: array + x-kubernetes-list-type: map + x-kubernetes-list-map-keys: + - namecli/pkg/kubectl/bind-apiservice/plugin/servicebindings.go (1)
54-55: Avoid hardcoding kube-bind namespace for the secret (if configurable)If the secret’s namespace can vary across environments, avoid a hard-coded check; compare against the intended namespace source, or make it an option.
Consider:
- Pass the expected namespace into this function, or
- Compare against
request.Spec.Namespaces(if present in v1alpha2) or another config source.deploy/crd/kube-bind.io_apiserviceexports.yaml (1)
567-589: Pluralized fieldnamedResourcesstill uses singular wording; add list-map keys for dedupeUpdate description to plural and consider enforcing uniqueness by (namespace,name).
Apply this diff:
- namedResources: - description: NamedResource is a shorthand for selecting - a single resource by name and namespace. + namedResources: + description: NamedResources is a shorthand for selecting + specific resources by name and namespace. items: description: NamedResource selects a specific resource by name and namespace. properties: @@ type: array + x-kubernetes-list-type: map + x-kubernetes-list-map-keys: + - namespace + - namedeploy/crd/catalog.kube-bind.io_modules.yaml (1)
203-212: Redundant but harmless scope validation.The
allOfcontains two identical enum lists (just in different orders). This is redundant but doesn't affect functionality—it's likely generated by kubebuilder.Simplify to a single enum:
- allOf: - - enum: - - Cluster - - Namespaced - - enum: - - Namespaced - - Cluster + enum: + - Cluster + - Namespacedsdk/apis/catalog/v1alpha1/module_types.go (1)
60-64: Optional: remove omitempty on required field.scope is required; omitempty is misleading (marshalling only). Safe to drop.
- Scope kubebindv1alpha2.InformerScope `json:"scope,omitempty"` + Scope kubebindv1alpha2.InformerScope `json:"scope"`contrib/kcp/deploy/resources/apiresourceschema-modules.catalog.kube-bind.io.yaml (2)
201-209: Simplify duplicated scope enum.Two identical enums under allOf are redundant; keep one for clarity.
- scope: - allOf: - - enum: - - Cluster - - Namespaced - - enum: - - Namespaced - - Cluster + scope: + enum: + - Cluster + - Namespaced
84-90: Polish description grammar (non-functional).Use “cannot” and “a service”.
- Note: it is worth noting that you can not ask for permissions for resource provided by a CRD - not provided by an service binding export. + Note: you cannot ask for permissions for a resource provided by a CRD + not provided by a service binding export.Also applies to: 170-186
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (77)
.coderabbit.yaml(1 hunks)README.md(1 hunks)apiserviceexport.yaml(1 hunks)backend/config.go(2 hunks)backend/controllers/serviceexport/serviceexport_controller.go(2 hunks)backend/controllers/serviceexport/serviceexport_reconcile.go(3 hunks)backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go(2 hunks)backend/controllers/servicenamespace/servicenamespace_reconcile.go(1 hunks)backend/http/handler.go(6 hunks)backend/kubernetes/manager.go(2 hunks)backend/kubernetes/resources/resources.go(1 hunks)backend/server.go(1 hunks)backend/template/login.html(0 hunks)backend/template/resources.gohtml(1 hunks)cli/pkg/kubectl/bind-apiservice/plugin/servicebindings.go(2 hunks)contrib/kcp/README.md(3 hunks)contrib/kcp/deploy/examples/collection-wildwest.yaml(1 hunks)contrib/kcp/deploy/examples/module-cowboys.yaml(1 hunks)contrib/kcp/deploy/examples/module-sheriffs.yaml(1 hunks)contrib/kcp/deploy/resources/apiexport-catalog.kube-bind.io.yaml(1 hunks)contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml(2 hunks)contrib/kcp/deploy/resources/apiresourceschema-apiservicebindings.kube-bind.io.yaml(2 hunks)contrib/kcp/deploy/resources/apiresourceschema-apiserviceexportrequests.kube-bind.io.yaml(3 hunks)contrib/kcp/deploy/resources/apiresourceschema-apiserviceexports.kube-bind.io.yaml(2 hunks)contrib/kcp/deploy/resources/apiresourceschema-collections.catalog.kube-bind.io.yaml(1 hunks)contrib/kcp/deploy/resources/apiresourceschema-modules.catalog.kube-bind.io.yaml(1 hunks)contrib/kcp/hack/update-kcp-codegen.sh(1 hunks)deploy/crd/catalog.kube-bind.io_collections.yaml(1 hunks)deploy/crd/catalog.kube-bind.io_modules.yaml(1 hunks)deploy/crd/kube-bind.io_apiservicebindings.yaml(1 hunks)deploy/crd/kube-bind.io_apiserviceexportrequests.yaml(2 hunks)deploy/crd/kube-bind.io_apiserviceexports.yaml(1 hunks)docs/content/.pages(1 hunks)docs/content/index.md(1 hunks)docs/content/setup/index.md(1 hunks)docs/content/setup/quickstart.md(1 hunks)docs/content/usage/.pages(1 hunks)docs/content/usage/index.md(1 hunks)docs/content/usage/migration.md(1 hunks)pkg/konnector/controllers/cluster/claimedresources/claimedresources_controller.go(2 hunks)pkg/konnector/controllers/cluster/claimedresources/claimedresources_reconciler.go(4 hunks)pkg/konnector/controllers/cluster/cluster_controller.go(5 hunks)pkg/konnector/controllers/cluster/namespacelifecycle/namespacelifecycle_controller.go(5 hunks)pkg/resources/resources.go(2 hunks)pkg/resources/resources_test.go(9 hunks)sdk/apis/catalog/v1alpha1/collection_types.go(1 hunks)sdk/apis/catalog/v1alpha1/doc.go(1 hunks)sdk/apis/catalog/v1alpha1/module_types.go(1 hunks)sdk/apis/catalog/v1alpha1/register.go(1 hunks)sdk/apis/catalog/v1alpha1/zz_generated.deepcopy.go(1 hunks)sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go(1 hunks)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go(2 hunks)sdk/apis/kubebind/v1alpha2/helpers/namespaces.go(1 hunks)sdk/apis/kubebind/v1alpha2/zz_generated.deepcopy.go(3 hunks)sdk/client/clientset/versioned/clientset.go(4 hunks)sdk/client/clientset/versioned/fake/clientset_generated.go(2 hunks)sdk/client/clientset/versioned/fake/register.go(2 hunks)sdk/client/clientset/versioned/scheme/register.go(2 hunks)sdk/client/clientset/versioned/typed/catalog/v1alpha1/catalog_client.go(1 hunks)sdk/client/clientset/versioned/typed/catalog/v1alpha1/collection.go(1 hunks)sdk/client/clientset/versioned/typed/catalog/v1alpha1/doc.go(1 hunks)sdk/client/clientset/versioned/typed/catalog/v1alpha1/fake/doc.go(1 hunks)sdk/client/clientset/versioned/typed/catalog/v1alpha1/fake/fake_catalog_client.go(1 hunks)sdk/client/clientset/versioned/typed/catalog/v1alpha1/fake/fake_collection.go(1 hunks)sdk/client/clientset/versioned/typed/catalog/v1alpha1/fake/fake_module.go(1 hunks)sdk/client/clientset/versioned/typed/catalog/v1alpha1/generated_expansion.go(1 hunks)sdk/client/clientset/versioned/typed/catalog/v1alpha1/module.go(1 hunks)sdk/client/informers/externalversions/catalog/interface.go(1 hunks)sdk/client/informers/externalversions/catalog/v1alpha1/collection.go(1 hunks)sdk/client/informers/externalversions/catalog/v1alpha1/interface.go(1 hunks)sdk/client/informers/externalversions/catalog/v1alpha1/module.go(1 hunks)sdk/client/informers/externalversions/factory.go(2 hunks)sdk/client/informers/externalversions/generic.go(2 hunks)sdk/client/listers/catalog/v1alpha1/collection.go(1 hunks)sdk/client/listers/catalog/v1alpha1/expansion_generated.go(1 hunks)sdk/client/listers/catalog/v1alpha1/module.go(1 hunks)test/e2e/bind/happy-case_test.go(2 hunks)
💤 Files with no reviewable changes (1)
- backend/template/login.html
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-22T13:20:49.952Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#304
File: sdk/apis/kubebind/v1alpha2/boundchema_types.go:49-0
Timestamp: 2025-09-22T13:20:49.952Z
Learning: In kube-bind, BoundSchema.ResourceGroupName() is only used for CRDs (Custom Resource Definitions), and CRDs must always have non-empty API groups. Therefore, handling empty groups (core API group "") is not necessary in this context, unlike general Kubernetes GroupResource handling.
Applied to files:
sdk/apis/catalog/v1alpha1/register.go
📚 Learning: 2025-09-22T13:20:49.952Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#304
File: sdk/apis/kubebind/v1alpha2/boundchema_types.go:49-0
Timestamp: 2025-09-22T13:20:49.952Z
Learning: In kube-bind, there are two different ResourceGroupName() methods: BoundSchema.ResourceGroupName() for CRDs (always non-empty groups) uses simple fmt.Sprintf formatting, while APIServiceExportRequestResource.ResourceGroupName() for export requests handles empty groups by converting to "core". BoundSchema is exclusively for CRDs which cannot have empty API groups per Kubernetes validation.
Applied to files:
sdk/apis/catalog/v1alpha1/register.go
📚 Learning: 2025-09-12T08:40:15.290Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go:140-143
Timestamp: 2025-09-12T08:40:15.290Z
Learning: APIServiceExportRequest resources in kube-bind are short-lived and automatically deleted after 10 minutes, so they should not be used as owner references for longer-lived resources like BoundSchema. The proper lifecycle management for BoundSchema resources created by APIServiceExportRequest is tracked in issue #297.
Applied to files:
contrib/kcp/deploy/resources/apiresourceschema-apiserviceexportrequests.kube-bind.io.yamlapiserviceexport.yamlcontrib/kcp/deploy/resources/apiresourceschema-apiserviceexports.kube-bind.io.yaml
🧬 Code graph analysis (36)
sdk/apis/catalog/v1alpha1/module_types.go (4)
sdk/apis/third_party/conditions/apis/conditions/v1alpha1/types.go (1)
Conditions(92-92)sdk/apis/kubebind/v1alpha2/boundchema_types.go (1)
InformerScope(59-59)sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go (1)
APIServiceExportResource(83-83)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (2)
PermissionClaim(206-213)Namespaces(129-135)
sdk/client/informers/externalversions/catalog/v1alpha1/interface.go (5)
sdk/client/informers/externalversions/catalog/v1alpha1/collection.go (1)
CollectionInformer(37-40)sdk/client/informers/externalversions/catalog/v1alpha1/module.go (1)
ModuleInformer(37-40)sdk/client/informers/externalversions/factory.go (1)
SharedInformerFactory(227-260)sdk/client/informers/externalversions/internalinterfaces/factory_interfaces.go (1)
TweakListOptionsFunc(40-40)sdk/client/informers/externalversions/catalog/interface.go (2)
New(39-41)Interface(27-30)
backend/controllers/serviceexport/serviceexport_reconcile.go (2)
sdk/apis/kubebind/v1alpha2/boundchema_types.go (3)
InformerScope(59-59)BoundSchema(41-47)ClusterScope(62-62)sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go (1)
APIServiceExport(61-72)
pkg/resources/resources_test.go (1)
sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (1)
NamedResource(165-179)
sdk/client/informers/externalversions/factory.go (3)
sdk/client/clientset/versioned/clientset.go (2)
Interface(33-38)New(138-146)sdk/client/informers/externalversions/catalog/interface.go (2)
Interface(27-30)New(39-41)sdk/client/informers/externalversions/catalog/v1alpha1/interface.go (2)
Interface(26-31)New(40-42)
sdk/client/clientset/versioned/typed/catalog/v1alpha1/fake/fake_catalog_client.go (4)
sdk/client/clientset/versioned/typed/catalog/v1alpha1/collection.go (1)
CollectionInterface(39-51)sdk/client/clientset/versioned/typed/catalog/v1alpha1/module.go (1)
ModuleInterface(39-51)sdk/client/clientset/versioned/clientset.go (1)
Interface(33-38)sdk/client/informers/externalversions/catalog/v1alpha1/interface.go (1)
Interface(26-31)
sdk/apis/catalog/v1alpha1/collection_types.go (1)
sdk/apis/third_party/conditions/apis/conditions/v1alpha1/types.go (1)
Conditions(92-92)
backend/controllers/serviceexport/serviceexport_controller.go (1)
sdk/apis/kubebind/v1alpha2/boundchema_types.go (1)
InformerScope(59-59)
backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go (4)
sdk/apis/third_party/conditions/util/conditions/setter.go (1)
SetSummary(126-128)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (2)
APIServiceExportRequest(46-60)Namespaces(129-135)sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go (1)
APIServiceExport(61-72)sdk/apis/kubebind/v1alpha2/helpers/namespaces.go (1)
APIServiceNamespaceFromExport(25-39)
sdk/client/informers/externalversions/generic.go (2)
sdk/apis/kubebind/v1alpha1/register.go (1)
SchemeGroupVersion(39-39)sdk/apis/kubebind/v1alpha2/register.go (1)
SchemeGroupVersion(39-39)
sdk/client/clientset/versioned/typed/catalog/v1alpha1/catalog_client.go (4)
sdk/client/clientset/versioned/clientset.go (4)
Interface(33-38)NewForConfig(76-90)NewForConfigAndClient(96-125)New(138-146)sdk/client/clientset/versioned/typed/catalog/v1alpha1/collection.go (2)
CollectionsGetter(34-36)CollectionInterface(39-51)sdk/client/clientset/versioned/typed/catalog/v1alpha1/module.go (2)
ModulesGetter(34-36)ModuleInterface(39-51)sdk/client/clientset/versioned/scheme/register.go (2)
Scheme(32-32)Codecs(33-33)
sdk/apis/kubebind/v1alpha2/zz_generated.deepcopy.go (1)
sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (1)
Namespaces(129-135)
sdk/client/clientset/versioned/typed/catalog/v1alpha1/fake/fake_module.go (3)
sdk/apis/catalog/v1alpha1/module_types.go (2)
Module(36-47)ModuleList(94-99)sdk/client/clientset/versioned/typed/catalog/v1alpha1/fake/fake_catalog_client.go (1)
FakeCatalogV1alpha1(27-29)sdk/client/clientset/versioned/typed/catalog/v1alpha1/module.go (1)
ModuleInterface(39-51)
backend/kubernetes/manager.go (2)
sdk/apis/catalog/v1alpha1/collection_types.go (1)
CollectionList(89-94)sdk/apis/catalog/v1alpha1/module_types.go (1)
Module(36-47)
sdk/client/clientset/versioned/clientset.go (1)
sdk/client/clientset/versioned/typed/catalog/v1alpha1/catalog_client.go (4)
CatalogV1alpha1Interface(29-33)CatalogV1alpha1Client(36-38)NewForConfigAndClient(63-71)New(84-86)
sdk/apis/kubebind/v1alpha2/helpers/namespaces.go (3)
sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go (2)
APIServiceExport(61-72)ObjectOwnerLabel(30-30)sdk/apis/kubebind/v1alpha1/apiservicenamespace_types.go (2)
APIServiceNamespace(41-50)APIServiceNamespaceSpec(52-53)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (1)
OwnerProvider(220-220)
sdk/apis/catalog/v1alpha1/register.go (2)
sdk/apis/catalog/v1alpha1/module_types.go (2)
Module(36-47)ModuleList(94-99)sdk/apis/catalog/v1alpha1/collection_types.go (2)
Collection(35-46)CollectionList(89-94)
pkg/konnector/controllers/cluster/claimedresources/claimedresources_reconciler.go (2)
sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go (1)
ObjectOwnerLabel(30-30)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (2)
OwnerConsumer(222-222)OwnerProvider(220-220)
sdk/client/clientset/versioned/fake/register.go (3)
sdk/client/clientset/versioned/scheme/register.go (1)
AddToScheme(55-55)sdk/apis/kubebind/v1alpha1/register.go (1)
AddToScheme(27-27)sdk/apis/kubebind/v1alpha2/register.go (1)
AddToScheme(27-27)
sdk/client/informers/externalversions/catalog/v1alpha1/collection.go (5)
sdk/client/listers/catalog/v1alpha1/collection.go (2)
CollectionLister(30-37)NewCollectionLister(45-47)sdk/client/informers/externalversions/factory.go (1)
SharedInformerFactory(227-260)sdk/client/informers/externalversions/internalinterfaces/factory_interfaces.go (1)
TweakListOptionsFunc(40-40)sdk/client/clientset/versioned/clientset.go (1)
Interface(33-38)sdk/client/informers/externalversions/catalog/v1alpha1/interface.go (1)
Interface(26-31)
backend/server.go (1)
sdk/apis/kubebind/v1alpha2/boundchema_types.go (1)
InformerScope(59-59)
sdk/client/informers/externalversions/catalog/interface.go (3)
sdk/client/informers/externalversions/catalog/v1alpha1/interface.go (2)
Interface(26-31)New(40-42)sdk/client/informers/externalversions/factory.go (1)
SharedInformerFactory(227-260)sdk/client/informers/externalversions/internalinterfaces/factory_interfaces.go (1)
TweakListOptionsFunc(40-40)
sdk/apis/catalog/v1alpha1/zz_generated.deepcopy.go (4)
sdk/apis/catalog/v1alpha1/collection_types.go (5)
Collection(35-46)CollectionList(89-94)CollectionSpec(57-69)ModuleReference(72-78)CollectionStatus(81-84)sdk/apis/third_party/conditions/apis/conditions/v1alpha1/types.go (1)
Conditions(92-92)sdk/apis/catalog/v1alpha1/module_types.go (4)
Module(36-47)ModuleList(94-99)ModuleSpec(58-83)ModuleStatus(86-89)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (3)
APIServiceExportRequestResource(137-143)PermissionClaim(206-213)Namespaces(129-135)
sdk/client/clientset/versioned/scheme/register.go (3)
sdk/client/clientset/versioned/fake/register.go (1)
AddToScheme(55-55)sdk/apis/kubebind/v1alpha1/register.go (1)
AddToScheme(27-27)sdk/apis/kubebind/v1alpha2/register.go (1)
AddToScheme(27-27)
cli/pkg/kubectl/bind-apiservice/plugin/servicebindings.go (4)
cli/pkg/kubectl/base/options.go (1)
Options(26-40)sdk/apis/third_party/conditions/util/conditions/setter.go (1)
MarkFalse(120-122)sdk/apis/third_party/conditions/apis/conditions/v1alpha1/constants.go (1)
ReadyCondition(24-24)sdk/apis/third_party/conditions/apis/conditions/v1alpha1/types.go (1)
ConditionSeverityInfo(37-37)
sdk/client/clientset/versioned/typed/catalog/v1alpha1/collection.go (5)
sdk/apis/catalog/v1alpha1/collection_types.go (2)
Collection(35-46)CollectionList(89-94)sdk/client/clientset/versioned/clientset.go (1)
Interface(33-38)sdk/client/clientset/versioned/typed/catalog/v1alpha1/generated_expansion.go (1)
CollectionExpansion(21-21)sdk/client/clientset/versioned/typed/catalog/v1alpha1/catalog_client.go (1)
CatalogV1alpha1Client(36-38)sdk/client/clientset/versioned/scheme/register.go (1)
ParameterCodec(34-34)
sdk/client/clientset/versioned/fake/clientset_generated.go (3)
sdk/client/clientset/versioned/clientset.go (1)
Clientset(41-46)sdk/client/clientset/versioned/typed/catalog/v1alpha1/catalog_client.go (1)
CatalogV1alpha1Interface(29-33)sdk/client/clientset/versioned/typed/catalog/v1alpha1/fake/fake_catalog_client.go (1)
FakeCatalogV1alpha1(27-29)
test/e2e/bind/happy-case_test.go (1)
sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (1)
NamedResource(165-179)
sdk/client/clientset/versioned/typed/catalog/v1alpha1/fake/fake_collection.go (3)
sdk/apis/catalog/v1alpha1/collection_types.go (2)
Collection(35-46)CollectionList(89-94)sdk/client/clientset/versioned/typed/catalog/v1alpha1/fake/fake_catalog_client.go (1)
FakeCatalogV1alpha1(27-29)sdk/client/clientset/versioned/typed/catalog/v1alpha1/collection.go (1)
CollectionInterface(39-51)
sdk/client/listers/catalog/v1alpha1/module.go (2)
sdk/apis/catalog/v1alpha1/module_types.go (1)
Module(36-47)sdk/client/listers/catalog/v1alpha1/expansion_generated.go (2)
ModuleListerExpansion(31-31)ModuleNamespaceListerExpansion(35-35)
sdk/client/clientset/versioned/typed/catalog/v1alpha1/module.go (5)
sdk/apis/catalog/v1alpha1/module_types.go (2)
Module(36-47)ModuleList(94-99)sdk/client/clientset/versioned/clientset.go (1)
Interface(33-38)sdk/client/informers/externalversions/catalog/v1alpha1/interface.go (1)
Interface(26-31)sdk/client/clientset/versioned/typed/catalog/v1alpha1/generated_expansion.go (1)
ModuleExpansion(23-23)sdk/client/clientset/versioned/scheme/register.go (1)
ParameterCodec(34-34)
sdk/client/informers/externalversions/catalog/v1alpha1/module.go (8)
pkg/konnector/controllers/dynamic/eventhandler.go (2)
Informer(40-43)SharedIndexInformer(27-38)sdk/client/listers/catalog/v1alpha1/module.go (2)
ModuleLister(30-37)NewModuleLister(45-47)sdk/client/informers/externalversions/factory.go (1)
SharedInformerFactory(227-260)sdk/client/informers/externalversions/internalinterfaces/factory_interfaces.go (1)
TweakListOptionsFunc(40-40)sdk/client/clientset/versioned/clientset.go (1)
Interface(33-38)sdk/client/informers/externalversions/catalog/interface.go (1)
Interface(27-30)sdk/client/informers/externalversions/catalog/v1alpha1/interface.go (1)
Interface(26-31)sdk/apis/catalog/v1alpha1/module_types.go (1)
Module(36-47)
sdk/client/listers/catalog/v1alpha1/collection.go (2)
sdk/apis/catalog/v1alpha1/collection_types.go (1)
Collection(35-46)sdk/client/listers/catalog/v1alpha1/expansion_generated.go (2)
CollectionListerExpansion(23-23)CollectionNamespaceListerExpansion(27-27)
backend/http/handler.go (3)
sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go (1)
APIServiceExportResource(83-83)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (5)
PermissionClaim(206-213)Namespaces(129-135)APIServiceExportRequestResponse(66-79)NameObjectMeta(81-84)APIServiceExportRequestSpec(95-127)sdk/apis/catalog/v1alpha1/module_types.go (1)
ModuleList(94-99)
pkg/konnector/controllers/cluster/cluster_controller.go (2)
pkg/konnector/controllers/cluster/namespacelifecycle/namespacelifecycle_controller.go (1)
NewController(48-112)pkg/konnector/konnector_controller.go (1)
GenericController(175-177)
pkg/konnector/controllers/cluster/namespacelifecycle/namespacelifecycle_controller.go (3)
pkg/konnector/controllers/cluster/cluster_controller.go (1)
NewController(57-175)sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go (1)
ObjectOwnerLabel(30-30)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (2)
OwnerProvider(220-220)Namespaces(129-135)
🪛 LanguageTool
docs/content/usage/migration.md
[grammar] ~94-~94: Ensure spelling is correct
Context: ... ``` ## Step-by-Step Migration ### 1. Backup Existing Resources Before starting mig...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
docs/content/usage/index.md
[grammar] ~125-~125: Use a hyphen to join words.
Context: ...nd the primary CRDs. ### Label Selector Based Claims Access resources based on ...
(QB_NEW_EN_HYPHEN)
🪛 YAMLlint (1.37.1)
.coderabbit.yaml
[error] 34-34: syntax error: expected ',' or ']', but got ''
(syntax)
⏰ 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). (4)
- GitHub Check: verify
- GitHub Check: go-test-e2e
- GitHub Check: go-test
- GitHub Check: lint
mjudeikis
force-pushed
the
mjudeikis/catalog.api
branch
from
d1ae972 to
db0166e
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
contrib/kcp/deploy/resources/apiresourceschema-apiserviceexports.kube-bind.io.yaml (1)
425-427: Validation message/rule mismatch (copy/paste bug).Message says “informerScope is immutable” but the rule enforces metadata.name equals .. Fix the message to reflect the rule.
- - message: informerScope is immutable - rule: self.metadata.name == self.spec.names.plural+"."+self.spec.group + - message: metadata.name must be <plural>.<group> + rule: self.metadata.name == self.spec.names.plural+"."+self.spec.groupbackend/http/handler.go (2)
249-256: Fix base64 codec mismatch for OAuth state.You encode with
base64.URLEncodingin handleAuthorize but decode withbase64.StdEncodinghere; decoding can fail. UseURLEncodingfor both.Apply this diff:
- decoded, err := base64.StdEncoding.DecodeString(state) + decoded, err := base64.URLEncoding.DecodeString(state)
417-425: Validate required query param “template” in /bind.Return 400 if missing to avoid fetching an empty-name template.
Apply this diff:
func (h *handler) handleBind(w http.ResponseWriter, r *http.Request) { logger := getLogger(r) - templateName := r.URL.Query().Get("template") + templateName := r.URL.Query().Get("template") + if templateName == "" { + http.Error(w, "missing template query parameter", http.StatusBadRequest) + return + }
♻️ Duplicate comments (7)
.coderabbit.yaml (1)
33-34: Fix YAML syntax error: quote strings starting with!in flow sequences.The unquoted strings
!sdk/client/**and!'**/*.yaml'cause a YAML parse error. In flow sequences (bracket notation), strings starting with!must be quoted to be treated as literals rather than YAML tag indicators.Apply this diff to fix the syntax:
path_filters: [ - !sdk/client/**, - !'**/*.yaml', + '!sdk/client/**', + '!**/*.yaml', ]cli/pkg/kubectl/bind-apiservice/plugin/servicebindings.go (3)
79-80: Remove namespace field from cluster-scoped APIServiceBinding resource.APIServiceBinding is a cluster-scoped resource, but line 80 sets
Namespace: "kube-bind"in the ObjectMeta. For cluster-scoped resources, the namespace field must be omitted.Apply this diff:
ObjectMeta: metav1.ObjectMeta{ Name: bindingName, - Namespace: "kube-bind", },
57-57: Message incorrectly implies an update operation.The message says "Updating existing APIServiceBinding" but the code only validates and returns the existing binding without performing any update.
Apply this diff:
- fmt.Fprintf(b.Options.IOStreams.ErrOut, "✅ Updating existing APIServiceBinding %s.\n", existing.Name) + fmt.Fprintf(b.Options.IOStreams.ErrOut, "✅ Reusing existing APIServiceBinding %s.\n", existing.Name)
76-107: Replace polling wrapper with single Create call and explicit AlreadyExists handling.Using
wait.PollUntilContextCancelfor a single Create operation adds unnecessary complexity. The poll will fail immediately on error anyway. Instead, perform a single Create call and handle concurrent creation via the AlreadyExists error.Apply this diff:
- // Create new APIServiceBinding - var created *kubebindv1alpha2.APIServiceBinding - if err := wait.PollUntilContextCancel(ctx, 1*time.Second, false, func(ctx context.Context) (bool, error) { - created, err = bindClient.KubeBindV1alpha2().APIServiceBindings().Create(ctx, &kubebindv1alpha2.APIServiceBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: bindingName, - Namespace: "kube-bind", - }, - Spec: kubebindv1alpha2.APIServiceBindingSpec{ - KubeconfigSecretRef: kubebindv1alpha2.ClusterSecretKeyRef{ - LocalSecretKeyRef: kubebindv1alpha2.LocalSecretKeyRef{ - Name: secretName, - Key: "kubeconfig", - }, - Namespace: "kube-bind", - }, - }, - }, metav1.CreateOptions{}) - if err != nil { - return false, err - } - - // Best effort status update to have "Pending" in the Ready condition - conditions.MarkFalse(created, - conditionsapi.ReadyCondition, - "Pending", - conditionsapi.ConditionSeverityInfo, - "Pending", - ) - _, _ = bindClient.KubeBindV1alpha2().APIServiceBindings().UpdateStatus(ctx, created, metav1.UpdateOptions{}) - - return true, nil - }); err != nil { - return nil, err - } + // Create new APIServiceBinding + created, err := bindClient.KubeBindV1alpha2().APIServiceBindings().Create(ctx, &kubebindv1alpha2.APIServiceBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: bindingName, + }, + Spec: kubebindv1alpha2.APIServiceBindingSpec{ + KubeconfigSecretRef: kubebindv1alpha2.ClusterSecretKeyRef{ + LocalSecretKeyRef: kubebindv1alpha2.LocalSecretKeyRef{ + Name: secretName, + Key: "kubeconfig", + }, + Namespace: "kube-bind", + }, + }, + }, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + // Concurrent creation occurred; fetch and return existing binding + existing, err := bindClient.KubeBindV1alpha2().APIServiceBindings().Get(ctx, bindingName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + return []*kubebindv1alpha2.APIServiceBinding{existing}, nil + } + if err != nil { + return nil, err + } + + // Best-effort status update to mark Pending + conditions.MarkFalse(created, + conditionsapi.ReadyCondition, + "Pending", + conditionsapi.ConditionSeverityInfo, + "Pending", + ) + _, _ = bindClient.KubeBindV1alpha2().APIServiceBindings().UpdateStatus(ctx, created, metav1.UpdateOptions{})sdk/client/listers/kubebind/v1alpha2/collection.go (1)
45-47: Use plural resource for indexer key (“collections”) to fix lookups.Singular breaks lister queries. Switch to plural.
func NewCollectionLister(indexer cache.Indexer) CollectionLister { - return &collectionLister{listers.New[*kubebindv1alpha2.Collection](indexer, kubebindv1alpha2.Resource("collection"))} + return &collectionLister{listers.New[*kubebindv1alpha2.Collection](indexer, kubebindv1alpha2.Resource("collections"))} }#!/usr/bin/env bash # Find any listers using singular resource keys. rg -nP --glob 'sdk/**/listers/**/*.go' '\bResource\("collection"\)|\bResource\("[a-z]+"\)' -C2docs/content/usage/migration.md (2)
34-37: Fix “mangodb” → “mongodb” in groups/resources and example names.Use mongodb.com/mongodbs consistently across examples.
- - group: mangodb.com + - group: mongodb.com @@ - resource: mangodbs + resource: mongodbs @@ - name: mangodbs-mangodb-com + name: mongodbs-mongodb-com @@ - - group: mangodb.com + - group: mongodb.com @@ - resource: mangodbs + resource: mongodbs @@ - - group: mangodb.com + - group: mongodb.com @@ - resource: mangodbs + resource: mongodbsAlso applies to: 59-60, 87-90, 143-146, 180-183
96-101: Use “back up” (verb), not “backup.”Minor grammar fix.
-Before starting migration, backup your existing resources: +Before starting migration, back up your existing resources:
🧹 Nitpick comments (8)
backend/kubernetes/manager.go (2)
168-182: Accept a selector (and/or paging) to avoid unbounded cluster-wide lists.Listing all Collections without filters can be costly in large clusters. Consider adding a labels.Selector/ListOptions parameter, or filtering server-side.
-func (m *Manager) ListCollections(ctx context.Context, cluster string) (*kubebindv1alpha2.CollectionList, error) { +func (m *Manager) ListCollections(ctx context.Context, cluster string, selector labels.Selector) (*kubebindv1alpha2.CollectionList, error) { @@ - var collections kubebindv1alpha2.CollectionList - err = c.List(ctx, &collections) + var collections kubebindv1alpha2.CollectionList + listOpts := []client.ListOption{} + if selector != nil { + listOpts = append(listOpts, client.MatchingLabelsSelector{Selector: selector}) + } + err = c.List(ctx, &collections, listOpts...)
184-198: Rename GetTemplates → GetTemplate for semantic accuracy.The namespace concern is resolved:
APIServiceExportTemplateis cluster-scoped (scope=Cluster), so the currentGet()call without namespace is correct. However, the function name should match its return type—it returns a single template (*kubebindv1alpha2.APIServiceExportTemplate), so rename it toGetTemplate(singular).contrib/kcp/deploy/resources/apiresourceschema-apiserviceexports.kube-bind.io.yaml (1)
565-586: Pluralization nit in description for namedResources.The field is named namedResources (plural) but the description uses singular “NamedResource”. Align wording.
- namedResources: - description: NamedResource is a shorthand for selecting a - single resource by name and namespace. + namedResources: + description: NamedResources is a shorthand for selecting one or more + resources by name and namespace. Each item selects a single resource.contrib/kcp/deploy/resources/apiresourceschema-apiserviceexporttemplates.kube-bind.io.yaml (2)
141-162: Pluralization nit in description for namedResources.Same wording issue as elsewhere; use plural for field-level description.
- namedResources: - description: NamedResource is a shorthand for selecting a - single resource by name and namespace. + namedResources: + description: NamedResources is a shorthand for selecting one or more + resources by name and namespace. Each item selects a single resource.
200-209: Redundant enum duplication for scope.
allOfwith two identical enums is redundant. Prefer a single enum for clarity.- scope: - allOf: - - enum: - - Cluster - - Namespaced - - enum: - - Namespaced - - Cluster - description: scope defines the scope of the resources in this template. - type: string + scope: + description: scope defines the scope of the resources in this template. + type: string + enum: + - Cluster + - Namespacedsdk/apis/kubebind/v1alpha2/apiserviceexporttemplate_types.go (1)
58-63: Minor: required field tagged omitempty.Spec.Scope is required but uses
json:"scope,omitempty". Safe, but consider droppingomitemptyto always serialize it.deploy/crd/kube-bind.io_apiserviceexporttemplates.yaml (1)
202-211: CRD scope and schema look good.Minor nit:
scopehas duplicated enum entries viaallOf; harmless generator artefact. No action needed.backend/http/handler.go (1)
526-548: Nit: local var shadows imported package name; fix typo in comment.Rename local
templatetotmpland fix “consutruct” → “construct” for clarity.Apply this diff:
-// 2. Get modules from the backend cluster and consutruct shallow-bound schemas (no crd content). +// 2. Get modules from the backend cluster and construct shallow-bound schemas (no CRD content). @@ - for _, collection := range collections.Items { - for _, t := range collection.Spec.Templates { - template, err := h.kubeManager.GetTemplates(ctx, cluster, t.Name) + for _, collection := range collections.Items { + for _, t := range collection.Spec.Templates { + tmpl, err := h.kubeManager.GetTemplates(ctx, cluster, t.Name) if err != nil { return nil, fmt.Errorf("failed to get template %q: %w", t.Name, err) } - templates.Items = append(templates.Items, *template) + templates.Items = append(templates.Items, *tmpl) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (55)
.coderabbit.yaml(1 hunks)README.md(1 hunks)apiserviceexport.yaml(1 hunks)backend/http/handler.go(5 hunks)backend/kubernetes/manager.go(1 hunks)backend/kubernetes/resources/resources.go(1 hunks)backend/template/login.html(0 hunks)backend/template/resources.gohtml(1 hunks)cli/pkg/kubectl/bind-apiservice/plugin/servicebindings.go(2 hunks)contrib/kcp/README.md(1 hunks)contrib/kcp/deploy/examples/collection-wildwest.yaml(1 hunks)contrib/kcp/deploy/examples/template-cowboys.yaml(1 hunks)contrib/kcp/deploy/examples/template-sheriffs.yaml(1 hunks)contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml(3 hunks)contrib/kcp/deploy/resources/apiresourceschema-apiservicebindings.kube-bind.io.yaml(2 hunks)contrib/kcp/deploy/resources/apiresourceschema-apiserviceexportrequests.kube-bind.io.yaml(1 hunks)contrib/kcp/deploy/resources/apiresourceschema-apiserviceexports.kube-bind.io.yaml(2 hunks)contrib/kcp/deploy/resources/apiresourceschema-apiserviceexporttemplates.kube-bind.io.yaml(1 hunks)contrib/kcp/deploy/resources/apiresourceschema-collections.kube-bind.io.yaml(1 hunks)contrib/kcp/hack/update-kcp-codegen.sh(1 hunks)deploy/crd/kube-bind.io_apiservicebindings.yaml(1 hunks)deploy/crd/kube-bind.io_apiserviceexportrequests.yaml(1 hunks)deploy/crd/kube-bind.io_apiserviceexports.yaml(1 hunks)deploy/crd/kube-bind.io_apiserviceexporttemplates.yaml(1 hunks)deploy/crd/kube-bind.io_collections.yaml(1 hunks)docs/content/.pages(1 hunks)docs/content/index.md(1 hunks)docs/content/setup/index.md(1 hunks)docs/content/setup/quickstart.md(1 hunks)docs/content/usage/.pages(1 hunks)docs/content/usage/index.md(1 hunks)docs/content/usage/migration.md(1 hunks)pkg/konnector/controllers/cluster/claimedresources/claimedresources_controller.go(2 hunks)pkg/resources/resources.go(2 hunks)pkg/resources/resources_test.go(9 hunks)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go(1 hunks)sdk/apis/kubebind/v1alpha2/apiserviceexporttemplate_types.go(1 hunks)sdk/apis/kubebind/v1alpha2/collection_types.go(1 hunks)sdk/apis/kubebind/v1alpha2/register.go(1 hunks)sdk/apis/kubebind/v1alpha2/zz_generated.deepcopy.go(3 hunks)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/apiserviceexporttemplate.go(1 hunks)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/collection.go(1 hunks)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/fake/fake_apiserviceexporttemplate.go(1 hunks)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/fake/fake_collection.go(1 hunks)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/fake/fake_kubebind_client.go(2 hunks)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/generated_expansion.go(1 hunks)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/kubebind_client.go(3 hunks)sdk/client/informers/externalversions/generic.go(1 hunks)sdk/client/informers/externalversions/kubebind/v1alpha2/apiserviceexporttemplate.go(1 hunks)sdk/client/informers/externalversions/kubebind/v1alpha2/collection.go(1 hunks)sdk/client/informers/externalversions/kubebind/v1alpha2/interface.go(3 hunks)sdk/client/listers/kubebind/v1alpha2/apiserviceexporttemplate.go(1 hunks)sdk/client/listers/kubebind/v1alpha2/collection.go(1 hunks)sdk/client/listers/kubebind/v1alpha2/expansion_generated.go(2 hunks)test/e2e/bind/happy-case_test.go(2 hunks)
💤 Files with no reviewable changes (1)
- backend/template/login.html
✅ Files skipped from review due to trivial changes (1)
- contrib/kcp/deploy/resources/apiresourceschema-collections.kube-bind.io.yaml
🚧 Files skipped from review as they are similar to previous changes (16)
- sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go
- docs/content/setup/index.md
- docs/content/index.md
- docs/content/setup/quickstart.md
- test/e2e/bind/happy-case_test.go
- README.md
- deploy/crd/kube-bind.io_apiservicebindings.yaml
- docs/content/usage/.pages
- contrib/kcp/deploy/resources/apiresourceschema-apiservicebindings.kube-bind.io.yaml
- deploy/crd/kube-bind.io_apiserviceexportrequests.yaml
- docs/content/.pages
- apiserviceexport.yaml
- backend/kubernetes/resources/resources.go
- pkg/konnector/controllers/cluster/claimedresources/claimedresources_controller.go
- pkg/resources/resources_test.go
- contrib/kcp/README.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-22T13:20:49.952Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#304
File: sdk/apis/kubebind/v1alpha2/boundchema_types.go:49-0
Timestamp: 2025-09-22T13:20:49.952Z
Learning: In kube-bind, there are two different ResourceGroupName() methods: BoundSchema.ResourceGroupName() for CRDs (always non-empty groups) uses simple fmt.Sprintf formatting, while APIServiceExportRequestResource.ResourceGroupName() for export requests handles empty groups by converting to "core". BoundSchema is exclusively for CRDs which cannot have empty API groups per Kubernetes validation.
Applied to files:
contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
📚 Learning: 2025-09-12T08:40:15.290Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go:140-143
Timestamp: 2025-09-12T08:40:15.290Z
Learning: APIServiceExportRequest resources in kube-bind are short-lived and automatically deleted after 10 minutes, so they should not be used as owner references for longer-lived resources like BoundSchema. The proper lifecycle management for BoundSchema resources created by APIServiceExportRequest is tracked in issue #297.
Applied to files:
contrib/kcp/deploy/resources/apiresourceschema-apiserviceexports.kube-bind.io.yaml
🧬 Code graph analysis (18)
cli/pkg/kubectl/bind-apiservice/plugin/servicebindings.go (4)
cli/pkg/kubectl/base/options.go (1)
Options(26-40)sdk/apis/third_party/conditions/util/conditions/setter.go (1)
MarkFalse(120-122)sdk/apis/third_party/conditions/apis/conditions/v1alpha1/constants.go (1)
ReadyCondition(24-24)sdk/apis/third_party/conditions/apis/conditions/v1alpha1/types.go (1)
ConditionSeverityInfo(37-37)
sdk/apis/kubebind/v1alpha2/collection_types.go (1)
sdk/apis/third_party/conditions/apis/conditions/v1alpha1/types.go (1)
Conditions(92-92)
sdk/client/clientset/versioned/typed/kubebind/v1alpha2/fake/fake_kubebind_client.go (2)
sdk/client/clientset/versioned/typed/kubebind/v1alpha2/apiserviceexporttemplate.go (1)
APIServiceExportTemplateInterface(39-51)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/collection.go (1)
CollectionInterface(39-51)
sdk/client/clientset/versioned/typed/kubebind/v1alpha2/fake/fake_collection.go (4)
sdk/apis/kubebind/v1alpha2/collection_types.go (2)
Collection(35-46)CollectionList(89-94)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/fake/fake_kubebind_client.go (1)
FakeKubeBindV1alpha2(27-29)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/collection.go (1)
CollectionInterface(39-51)sdk/apis/kubebind/v1alpha2/register.go (1)
SchemeGroupVersion(39-39)
sdk/client/clientset/versioned/typed/kubebind/v1alpha2/kubebind_client.go (2)
sdk/client/clientset/versioned/typed/kubebind/v1alpha2/apiserviceexporttemplate.go (2)
APIServiceExportTemplatesGetter(34-36)APIServiceExportTemplateInterface(39-51)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/collection.go (2)
CollectionsGetter(34-36)CollectionInterface(39-51)
backend/kubernetes/manager.go (2)
sdk/apis/kubebind/v1alpha2/collection_types.go (1)
CollectionList(89-94)sdk/apis/kubebind/v1alpha2/apiserviceexporttemplate_types.go (1)
APIServiceExportTemplate(35-46)
sdk/apis/kubebind/v1alpha2/register.go (2)
sdk/apis/kubebind/v1alpha2/apiserviceexporttemplate_types.go (2)
APIServiceExportTemplate(35-46)APIServiceExportTemplateList(93-98)sdk/apis/kubebind/v1alpha2/collection_types.go (2)
Collection(35-46)CollectionList(89-94)
sdk/client/informers/externalversions/kubebind/v1alpha2/collection.go (5)
sdk/client/listers/kubebind/v1alpha2/collection.go (2)
CollectionLister(30-37)NewCollectionLister(45-47)sdk/client/informers/externalversions/factory.go (1)
SharedInformerFactory(226-258)sdk/client/informers/externalversions/internalinterfaces/factory_interfaces.go (1)
TweakListOptionsFunc(40-40)sdk/client/informers/externalversions/kubebind/v1alpha2/interface.go (1)
Interface(26-43)sdk/client/clientset/versioned/clientset.go (1)
Interface(32-36)
sdk/client/informers/externalversions/kubebind/v1alpha2/apiserviceexporttemplate.go (5)
sdk/client/listers/kubebind/v1alpha2/apiserviceexporttemplate.go (2)
APIServiceExportTemplateLister(30-37)NewAPIServiceExportTemplateLister(45-47)sdk/client/informers/externalversions/factory.go (1)
SharedInformerFactory(226-258)sdk/client/informers/externalversions/internalinterfaces/factory_interfaces.go (1)
TweakListOptionsFunc(40-40)sdk/client/informers/externalversions/kubebind/v1alpha2/interface.go (1)
Interface(26-43)sdk/client/clientset/versioned/clientset.go (1)
Interface(32-36)
sdk/client/clientset/versioned/typed/kubebind/v1alpha2/fake/fake_apiserviceexporttemplate.go (4)
sdk/apis/kubebind/v1alpha2/apiserviceexporttemplate_types.go (2)
APIServiceExportTemplate(35-46)APIServiceExportTemplateList(93-98)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/fake/fake_kubebind_client.go (1)
FakeKubeBindV1alpha2(27-29)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/apiserviceexporttemplate.go (1)
APIServiceExportTemplateInterface(39-51)sdk/apis/kubebind/v1alpha2/register.go (1)
SchemeGroupVersion(39-39)
sdk/client/informers/externalversions/kubebind/v1alpha2/interface.go (2)
sdk/client/informers/externalversions/kubebind/v1alpha2/apiserviceexporttemplate.go (1)
APIServiceExportTemplateInformer(37-40)sdk/client/informers/externalversions/kubebind/v1alpha2/collection.go (1)
CollectionInformer(37-40)
backend/http/handler.go (3)
sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go (1)
APIServiceExportResource(83-83)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (3)
PermissionClaim(206-213)Namespaces(129-135)APIServiceExportRequestSpec(95-127)sdk/apis/kubebind/v1alpha2/apiserviceexporttemplate_types.go (1)
APIServiceExportTemplateList(93-98)
sdk/apis/kubebind/v1alpha2/apiserviceexporttemplate_types.go (4)
sdk/apis/third_party/conditions/apis/conditions/v1alpha1/types.go (1)
Conditions(92-92)sdk/apis/kubebind/v1alpha2/boundchema_types.go (1)
InformerScope(59-59)sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go (1)
APIServiceExportResource(83-83)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (2)
PermissionClaim(206-213)Namespaces(129-135)
sdk/apis/kubebind/v1alpha2/zz_generated.deepcopy.go (4)
sdk/apis/kubebind/v1alpha2/apiserviceexporttemplate_types.go (4)
APIServiceExportTemplate(35-46)APIServiceExportTemplateList(93-98)APIServiceExportTemplateSpec(57-82)APIServiceExportTemplateStatus(85-88)sdk/apis/kubebind/v1alpha2/collection_types.go (5)
APIServiceExportTemplateReference(72-78)Collection(35-46)CollectionList(89-94)CollectionSpec(57-69)CollectionStatus(81-84)sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (3)
APIServiceExportRequestResource(137-143)PermissionClaim(206-213)Namespaces(129-135)sdk/apis/third_party/conditions/apis/conditions/v1alpha1/types.go (1)
Conditions(92-92)
sdk/client/listers/kubebind/v1alpha2/collection.go (3)
sdk/apis/kubebind/v1alpha2/collection_types.go (1)
Collection(35-46)sdk/client/listers/kubebind/v1alpha2/expansion_generated.go (2)
CollectionListerExpansion(75-75)CollectionNamespaceListerExpansion(79-79)sdk/apis/kubebind/v1alpha2/register.go (1)
Resource(42-44)
sdk/client/listers/kubebind/v1alpha2/apiserviceexporttemplate.go (3)
sdk/apis/kubebind/v1alpha2/apiserviceexporttemplate_types.go (1)
APIServiceExportTemplate(35-46)sdk/client/listers/kubebind/v1alpha2/expansion_generated.go (2)
APIServiceExportTemplateListerExpansion(43-43)APIServiceExportTemplateNamespaceListerExpansion(47-47)sdk/apis/kubebind/v1alpha2/register.go (1)
Resource(42-44)
sdk/client/clientset/versioned/typed/kubebind/v1alpha2/apiserviceexporttemplate.go (5)
sdk/apis/kubebind/v1alpha2/apiserviceexporttemplate_types.go (2)
APIServiceExportTemplate(35-46)APIServiceExportTemplateList(93-98)sdk/client/informers/externalversions/kubebind/v1alpha2/interface.go (1)
Interface(26-43)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/generated_expansion.go (1)
APIServiceExportTemplateExpansion(27-27)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/kubebind_client.go (1)
KubeBindV1alpha2Client(42-44)sdk/client/clientset/versioned/scheme/register.go (1)
ParameterCodec(33-33)
sdk/client/clientset/versioned/typed/kubebind/v1alpha2/collection.go (4)
sdk/apis/kubebind/v1alpha2/collection_types.go (2)
Collection(35-46)CollectionList(89-94)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/generated_expansion.go (1)
CollectionExpansion(35-35)sdk/client/clientset/versioned/typed/kubebind/v1alpha2/kubebind_client.go (1)
KubeBindV1alpha2Client(42-44)sdk/client/clientset/versioned/scheme/register.go (1)
ParameterCodec(33-33)
🪛 LanguageTool
docs/content/usage/index.md
[grammar] ~125-~125: Use a hyphen to join words.
Context: ...nd the primary CRDs. ### Label Selector Based Claims Access resources based on ...
(QB_NEW_EN_HYPHEN)
docs/content/usage/migration.md
[grammar] ~94-~94: Ensure spelling is correct
Context: ... ``` ## Step-by-Step Migration ### 1. Backup Existing Resources Before starting mig...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 YAMLlint (1.37.1)
.coderabbit.yaml
[error] 34-34: syntax error: expected ',' or ']', but got ''
(syntax)
⏰ 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). (4)
- GitHub Check: verify
- GitHub Check: lint
- GitHub Check: go-test
- GitHub Check: go-test-e2e
🔇 Additional comments (35)
contrib/kcp/hack/update-kcp-codegen.sh (1)
31-31: LGTM: Preserve resources during KCP codegen.The
--preserve-resourcesflag ensures that newly introduced APIResourceSchema definitions and CRD resources are retained during code generation, preventing loss of state.contrib/kcp/deploy/resources/apiresourceschema-apiserviceexportrequests.kube-bind.io.yaml (2)
313-313: LGTM: Field renamed to plural form.The rename from
namedResourcetonamedResourcesaligns with the broader API changes across the codebase, standardizing on plural naming for collection fields.
452-452: Verify v1alpha2 is the intended storage version.Storage version configuration is correct: v1alpha1 has
storage: false(line 172) and v1alpha2 hasstorage: true(line 452), ensuring only one version is designated as storage.However, the conversion strategy is set to
None, meaning no conversion rules exist between versions. If v1alpha1 was the previous storage version with existing objects in the cluster, verify that:
- A migration strategy exists outside this schema (e.g., kubectl patch, custom controllers)
- OR v1alpha1 was never the storage version in production clusters
Confirm this aligns with kube-bind v0.6.0 release strategy and that the cluster upgrade path is documented.
docs/content/usage/index.md (1)
1-281: LGTM: Comprehensive usage documentation for kube-bind v0.6.0.This documentation effectively covers the new Catalog API features, permission claims, provider-side namespace management, and migration guidance. The structure is clear, examples are helpful, and it aligns well with the API changes in this PR.
pkg/resources/resources.go (2)
33-33: LGTM: Field rename reflects API change.The update from
selector.NamedResourcetoselector.NamedResourcesis consistent with the plural naming convention adopted across the v1alpha2 API.
55-65: LGTM: Named resources iteration updated.The logic correctly iterates over the renamed
NamedResourcesfield, maintaining the existing matching behavior for name and namespace checks.deploy/crd/kube-bind.io_apiserviceexports.yaml (1)
567-567: LGTM: CRD schema updated to match API types.The field rename from
namedResourcetonamedResourcesin the v1alpha2 CRD schema aligns with the corresponding Go type changes, ensuring validation consistency.backend/template/resources.gohtml (1)
1-231: LGTM: UI refactored to support module-centric workflow.The template successfully transitions from a single-export view to a modular, grid-based layout that displays multiple modules with their resources, permissions, and namespaces. Key improvements:
- Responsive two-column grid for module cards
- Clear visualization of permissions with collapsible details for NamedResources and LabelSelector
- Per-module "Bind Module" action aligning with the template-based workflow
- Professional styling with proper hover states and visual hierarchy
This change aligns well with the Catalog API features (Collections, Modules) introduced in this PR.
contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (2)
52-62: Schema version updates look correct.The schema version bumps for
apiservicebindingsandapiserviceexportsfollow the established date-based versioning pattern and are consistent with the PR's changes.
75-99: Remove the orphaned catalog.kube-bind.io collections entry or verify if missing CRD definition is required.Investigation reveals the duplication appears unintentional. Only one collections CRD definition exists in the codebase (
./deploy/crd/kube-bind.io_collections.yamlfor thekube-bind.iogroup). The first entry referencingcatalog.kube-bind.iocollections has no corresponding CRD file and appears to be an orphaned or leftover entry. Either remove it from the export or confirm if a separatecatalog.kube-bind.iocollections CRD definition is required.contrib/kcp/deploy/examples/collection-wildwest.yaml (1)
1-10: Well-structured example manifest.The Collection example is clean and demonstrates the basic usage pattern correctly. The referenced templates (
cowboysandsheriffs) align with the broader catalog API changes introduced in this PR.sdk/client/informers/externalversions/kubebind/v1alpha2/interface.go (1)
33-34: Generated informer code follows correct patterns.The new informer methods for
APIServiceExportTemplatesandCollectionsare properly generated and follow the established patterns in this file. The implementations correctly wire factory, namespace, and tweakListOptions.Also applies to: 41-42, 71-74, 91-94
sdk/client/informers/externalversions/generic.go (1)
75-76: Generated ForResource switch cases look correct.The new cases for
apiserviceexporttemplatesandcollectionsproperly follow the existing pattern and correctly wire the resource metadata and informer references.Also applies to: 83-84
sdk/client/clientset/versioned/typed/kubebind/v1alpha2/generated_expansion.go (1)
27-27: Expansion interfaces correctly generated.The new
APIServiceExportTemplateExpansionandCollectionExpansioninterfaces follow the standard Kubernetes client-gen pattern for providing extension points without modifying generated code.Also applies to: 35-35
sdk/apis/kubebind/v1alpha2/register.go (1)
63-66: Type registration is correct.The four new types (
APIServiceExportTemplate,APIServiceExportTemplateList,Collection,CollectionList) are properly registered with the scheme following the established pattern.sdk/client/clientset/versioned/typed/kubebind/v1alpha2/fake/fake_apiserviceexporttemplate.go (1)
27-52: Fake client implementation follows Kubernetes patterns correctly.The generated fake client for
APIServiceExportTemplateproperly implements the standard Kubernetes fake client pattern usinggentype.FakeClientWithList. All metadata, constructors, and list conversion functions are correctly wired.sdk/apis/kubebind/v1alpha2/collection_types.go (3)
25-46: Collection type definition is well-structured.The
Collectiontype follows Kubernetes API conventions correctly with proper kubebuilder markers, cluster scope, and standard status subresource support. The design appropriately represents a logical grouping of templates for UI organization.
56-69: CollectionSpec design is reasonable with discussed validation.The spec design is clean with an optional
Descriptionand requiredTemplateslist. TheMinItems=1validation on Templates was discussed in past reviews and the decision to keep it is reasonable—a Collection should logically contain at least one template to be meaningful.Note: Based on past review comments, the team considered future extensibility with label selectors but opted for the simpler reference-by-name approach for now, which is appropriate for an initial implementation.
71-78: Template reference type is appropriately simple.The
APIServiceExportTemplateReferenceusing only aNamefield follows standard Kubernetes patterns for referencing resources within the same scope. This is sufficient for the current use case.contrib/kcp/deploy/examples/template-cowboys.yaml (1)
5-12: Confirm selector semantics and “versions” field name.
- Does APIServiceExportTemplate allow using both selector.namedResources and selector.labelSelector simultaneously (AND vs OR)? Please confirm expected evaluation.
- Verify the field is “versions” (array) for resources here; docs examples elsewhere use singular “version”. Aligning across docs/templates would prevent confusion.
Also applies to: 12-22, 23-32
contrib/kcp/deploy/examples/template-sheriffs.yaml (1)
5-12: Template scope and namespace expectations.
- This manifest omits metadata.namespace; confirm APIServiceExportTemplate is cluster-scoped as assumed.
- For namespaces provisioning, confirm controller behavior when they already exist (idempotency/ownership labels).
Also applies to: 12-32, 33-35
sdk/client/listers/kubebind/v1alpha2/expansion_generated.go (1)
41-48: LGTM — expansion points correctly added for new resources.Matches lister-gen conventions; no further action.
Also applies to: 73-79
sdk/client/clientset/versioned/typed/kubebind/v1alpha2/fake/fake_collection.go (1)
33-50: LGTM — fake client wiring uses correct “collections” resource and kind.Good use of NewFakeClientWithList and slice converters.
sdk/client/informers/externalversions/kubebind/v1alpha2/collection.go (1)
35-40: LGTM — standard generated informer.sdk/client/clientset/versioned/typed/kubebind/v1alpha2/collection.go (1)
32-36: LGTM — standard generated typed client for Collections.backend/http/handler.go (1)
386-394: Scope filter semantics — confirm intent.Condition shows all templates when
h.scope == ClusterScopeand filters otherwise. If the intention was “Cluster shows both; Namespaced shows only namespaced,” this is correct; otherwise, adjust.sdk/apis/kubebind/v1alpha2/zz_generated.deepcopy.go (9)
517-548: APIServiceExportTemplateList deepcopy/object are standard and correct.
550-564: APIServiceExportTemplateReference deepcopy is fine (value-only struct).
566-599: APIServiceExportTemplateSpec deep-copies complex slices; shallow-copies Namespaces appropriately.Resources and PermissionClaims use DeepCopyInto; Namespaces uses copy, which is correct given Namespaces is a simple value struct today.
601-623: APIServiceExportTemplateStatus conditions deepcopy is correct.
1090-1116: Collection runtime.Object deepcopy follows the standard pattern.
1118-1149: CollectionList deepcopy/object look good.
1151-1161: CollectionSpec shallow-copies Templates; OK since APIServiceExportTemplateReference is value-only.
1172-1194: CollectionStatus conditions deepcopy is correct.
489-516: Now I'll generate scripts to locate and inspect the relevant v1alpha2 files to verify the registration:Deepcopy code is correct; verify type registration if these are CRDs.
The DeepCopy methods are independent and do not require scheme registration since Kubernetes 1.9. The deepcopy patterns in your changes are correct: ObjectMeta uses DeepCopyInto, Spec/Status fields properly use DeepCopyInto, and runtime.Object returns handle nil checks appropriately.
However, if
APIServiceExportTemplateandCollectionare Custom Resources that need to be usable by controllers or the API server, they should also be registered with SchemeBuilder and included in OpenAPI specs. Verification shows these types are not currently registered. Confirm whether these types require scheme registration for your use case.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/http/handler.go (1)
417-453: Validate the template query parameter before use.The
templatequery parameter is retrieved on line 419 but never validated before being passed toGetTemplateson line 448. An empty or missing template name would likely cause errors downstream.Apply this diff to add validation:
func (h *handler) handleBind(w http.ResponseWriter, r *http.Request) { logger := getLogger(r) templateName := r.URL.Query().Get("template") + if templateName == "" { + logger.Error(errors.New("missing template parameter"), "template parameter is required") + http.Error(w, "missing template parameter", http.StatusBadRequest) + return + } providerCluster := mux.Vars(r)["cluster"]
🧹 Nitpick comments (2)
backend/template/resources.gohtml (1)
14-133: Consider extracting inline CSS to an external stylesheet.The 119 lines of inline CSS could be extracted to a separate stylesheet for better maintainability, reusability, and testability. However, since this is described as a skeleton implementation, this refactor can be deferred.
backend/http/handler.go (1)
535-548: Consider optimizing template fetching to reduce API calls.The current implementation makes N+1 API calls (one
ListCollectionsfollowed by oneGetTemplatesper template). While acceptable for a skeleton implementation, consider batch-fetching templates in the future to improve performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/http/handler.go(5 hunks)backend/template/resources.gohtml(1 hunks)contrib/kcp/README.md(2 hunks)contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-22T13:20:49.952Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#304
File: sdk/apis/kubebind/v1alpha2/boundchema_types.go:49-0
Timestamp: 2025-09-22T13:20:49.952Z
Learning: In kube-bind, there are two different ResourceGroupName() methods: BoundSchema.ResourceGroupName() for CRDs (always non-empty groups) uses simple fmt.Sprintf formatting, while APIServiceExportRequestResource.ResourceGroupName() for export requests handles empty groups by converting to "core". BoundSchema is exclusively for CRDs which cannot have empty API groups per Kubernetes validation.
Applied to files:
contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
🧬 Code graph analysis (1)
backend/http/handler.go (2)
sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go (5)
PermissionClaim(206-213)Namespaces(129-135)APIServiceExportRequestResponse(66-79)NameObjectMeta(81-84)APIServiceExportRequestSpec(95-127)sdk/apis/kubebind/v1alpha2/apiserviceexporttemplate_types.go (1)
APIServiceExportTemplateList(93-98)
⏰ 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). (4)
- GitHub Check: lint
- GitHub Check: verify
- GitHub Check: go-test-e2e
- GitHub Check: go-test
🔇 Additional comments (9)
contrib/kcp/README.md (2)
96-104: Preferkubectl applyfor reproducible setup documentation.The change from
kubectl create -ftokubectl apply -fis a best practice improvement. Theapplycommand is idempotent and more appropriate for documentation that may be executed multiple times or used as reference material. ✓
111-111: Cluster URL updates are consistent.The example cluster URLs have been updated consistently across both the LogicalCluster output (line 111) and the kubectl-bind consumer command (line 127), maintaining documentation coherence. ✓
Also applies to: 127-127
contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (2)
80-89: Verification complete—schema resources are properly defined.The APIResourceSchema files for both new resources exist:
apiresourceschema-apiserviceexporttemplates.kube-bind.io.yamlapiresourceschema-collections.kube-bind.io.yamlThe schema versions referenced in the APIExport are correctly configured.
52-52: All schema version updates are correctly aligned with corresponding APIResourceSchema definitions.Verification confirms:
- Lines 52 & 62: Updated schema identifiers (
v251015-f561c7c.apiservicebindings.kube-bind.ioandv251015-f561c7c.apiserviceexports.kube-bind.io) match their respective APIResourceSchema resources- Lines 80-89: New resources (
apiserviceexporttemplatesandcollections) reference valid schema identifiers (v251020-d1ae972.apiserviceexporttemplates.kube-bind.ioandv251020-d1ae972.collections.kube-bind.io) that match their corresponding APIResourceSchema filesbackend/template/resources.gohtml (1)
219-221: LGTM: Module bind flow correctly passes template name.The bind URL correctly passes the module name via the
templatequery parameter, which aligns with the backend's module-centric binding flow.backend/http/handler.go (4)
344-356: LGTM: UISchema correctly restructured for module-centric workflow.The UISchema struct has been appropriately refactored from per-resource fields to module-level aggregates (Resources, PermissionClaims, Namespaces arrays), which aligns with the template-based catalog API design.
384-398: LGTM: Template iteration and UISchema construction are correct.The loop correctly filters templates by scope and constructs UISchema entries with the appropriate module-level fields from the templates.
455-468: LGTM: APIServiceExportRequestResponse construction is correct.The request object is correctly constructed from the template specification, with proper field mappings for Resources, PermissionClaims, and Namespaces.
447-447: Fix typo in comment.Line 447 has a typo: "consutruct" should be "construct".
Apply this diff:
- // Module consist of many resources and permissionClaims. Read it and translate to + // Module consists of many resources and permissionClaims. Read it and translate to request.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
docs/content/usage/migration.md (2)
34-37: Fix typo: "mangodb" → "mongodb" (flagged in prior review).These examples still use the incorrect API group and resource names. This was previously flagged but remains unresolved.
Apply this diff to fix all instances:
--- migration.md (lines 34-36) - group: mangodb.com + group: mongodb.com version: v1alpha1 - resource: mangodbs + resource: mongodbs --- migration.md (line 59) - name: mangodbs-mangodb-com + name: mongodbs-mongodb-com --- migration.md (lines 87-89) - group: mangodb.com + group: mongodb.com version: v1alpha1 - resource: mangodbs + resource: mongodbs --- migration.md (lines 143-145) - group: mangodb.com + group: mongodb.com version: v1alpha1 - resource: mangodbs + resource: mongodbs --- migration.md (lines 180-182) - group: mangodb.com + group: mongodb.com version: v1alpha1 - resource: mangodbs + resource: mongodbsAlso applies to: 59-60, 87-90, 143-146, 180-183
94-102: Fix verb form: "backup" → "back up" (flagged in prior review).The verb form must be two words. This was previously flagged but remains unresolved.
Apply this diff:
- ### 1. Backup Existing Resources + ### 1. Back Up Existing Resources - Before starting migration, backup your existing resources: + Before starting migration, back up your existing resources: ```bash - # Backup APIServiceExports + # Back up APIServiceExports kubectl get apiserviceexports -o yaml > apiserviceexports-backup.yaml - # Backup APIServiceBindings + # Back up APIServiceBindings kubectl get apiservicebindings -o yaml > apiservicebindings-backup.yaml - # Backup bound CRDs + # Back up bound CRDs kubectl get crds -l kube-bind.io/bound-crd=true -o yaml > bound-crds-backup.yamlAlso applies to: 105-105
cli/pkg/kubectl/bind-apiservice/plugin/servicebindings.go (1)
74-107: PollUntilContextCancel is unnecessary and concurrent creates are unhandledA previous review comment suggested removing
wait.PollUntilContextCanceland handlingAlreadyExistsexplicitly (marked as "✅ Addressed"), but the current code still uses it. The Poll always returnstrueon first iteration (line 104), making it a pointless wrapper. More critically, if two concurrent callers both seeNotFoundat line 49, both will attempt Create, and the second will fail withAlreadyExists(not handled).Apply this diff to fix:
- // Create new APIServiceBinding - var created *kubebindv1alpha2.APIServiceBinding - if err := wait.PollUntilContextCancel(ctx, 1*time.Second, false, func(ctx context.Context) (bool, error) { - created, err = bindClient.KubeBindV1alpha2().APIServiceBindings().Create(ctx, &kubebindv1alpha2.APIServiceBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: bindingName, - }, - Spec: kubebindv1alpha2.APIServiceBindingSpec{ - KubeconfigSecretRef: kubebindv1alpha2.ClusterSecretKeyRef{ - LocalSecretKeyRef: kubebindv1alpha2.LocalSecretKeyRef{ - Name: secretName, - Key: "kubeconfig", - }, - Namespace: "kube-bind", - }, - }, - }, metav1.CreateOptions{}) - if err != nil { - return false, err - } - - // Best effort status update to have "Pending" in the Ready condition - conditions.MarkFalse(created, - conditionsapi.ReadyCondition, - "Pending", - conditionsapi.ConditionSeverityInfo, - "Pending", - ) - _, _ = bindClient.KubeBindV1alpha2().APIServiceBindings().UpdateStatus(ctx, created, metav1.UpdateOptions{}) - - return true, nil - }); err != nil { - return nil, err - } + // Create new APIServiceBinding + created, err := bindClient.KubeBindV1alpha2().APIServiceBindings().Create(ctx, &kubebindv1alpha2.APIServiceBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: bindingName, + }, + Spec: kubebindv1alpha2.APIServiceBindingSpec{ + KubeconfigSecretRef: kubebindv1alpha2.ClusterSecretKeyRef{ + LocalSecretKeyRef: kubebindv1alpha2.LocalSecretKeyRef{ + Name: secretName, + Key: "kubeconfig", + }, + Namespace: "kube-bind", + }, + }, + }, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + // Concurrent create; fetch existing + existing, err := bindClient.KubeBindV1alpha2().APIServiceBindings().Get(ctx, bindingName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + return []*kubebindv1alpha2.APIServiceBinding{existing}, nil + } + if err != nil { + return nil, err + } + + // Best-effort status update + conditions.MarkFalse(created, + conditionsapi.ReadyCondition, + "Pending", + conditionsapi.ConditionSeverityInfo, + "Pending", + ) + _, _ = bindClient.KubeBindV1alpha2().APIServiceBindings().UpdateStatus(ctx, created, metav1.UpdateOptions{})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/kubernetes/resources/resources.go(0 hunks)cli/pkg/kubectl/bind-apiservice/plugin/servicebindings.go(2 hunks)docs/content/setup/quickstart.md(1 hunks)docs/content/usage/index.md(1 hunks)docs/content/usage/migration.md(1 hunks)
💤 Files with no reviewable changes (1)
- backend/kubernetes/resources/resources.go
✅ Files skipped from review due to trivial changes (1)
- docs/content/setup/quickstart.md
🧰 Additional context used
🧬 Code graph analysis (1)
cli/pkg/kubectl/bind-apiservice/plugin/servicebindings.go (5)
sdk/apis/third_party/conditions/util/conditions/getter.go (1)
Get(39-51)cli/pkg/kubectl/base/options.go (1)
Options(26-40)sdk/apis/third_party/conditions/util/conditions/setter.go (1)
MarkFalse(120-122)sdk/apis/third_party/conditions/apis/conditions/v1alpha1/constants.go (1)
ReadyCondition(24-24)sdk/apis/third_party/conditions/apis/conditions/v1alpha1/types.go (1)
ConditionSeverityInfo(37-37)
🪛 LanguageTool
docs/content/usage/index.md
[grammar] ~125-~125: Use a hyphen to join words.
Context: ...nd the primary CRDs. ### Label Selector Based Claims Access resources based on ...
(QB_NEW_EN_HYPHEN)
docs/content/usage/migration.md
[grammar] ~94-~94: Ensure spelling is correct
Context: ... ``` ## Step-by-Step Migration ### 1. Backup Existing Resources Before starting mig...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ 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). (4)
- GitHub Check: lint
- GitHub Check: go-test
- GitHub Check: verify
- GitHub Check: go-test-e2e
🔇 Additional comments (3)
docs/content/usage/index.md (1)
61-61: Use hyphens for the compound adjective modifier.The section heading "Label Selector Based Claims" should follow grammatical rules for compound adjectives preceding nouns.
-### Label Selector Based Claims +### Label-Selector-Based ClaimsLikely an incorrect or invalid review comment.
cli/pkg/kubectl/bind-apiservice/plugin/servicebindings.go (2)
47-72: LGTM: Reuse logic correctly validates ownership and secret referencesThe reuse path properly validates that the existing binding's secret reference matches and that all CRDs are owned by kube-bind before proceeding. The refactoring to use a single binding name per request aligns with the PR's goal of consolidating per-resource bindings into a unified workflow.
109-110: LGTM: Success message accurately reflects unified binding workflowThe message clearly indicates a single binding was created for multiple resources, which aligns with the PR's consolidation of per-resource bindings.
mjudeikis
force-pushed
the
mjudeikis/catalog.api
branch
2 times, most recently
from
172a62f to
dc93a02
Compare
Signed-off-by: Mangirdas Judeikis <mangirdas@judeikis.lt> On-behalf-of: @SAP mangirdas.judeikis@sap.com
mjudeikis
force-pushed
the
mjudeikis/catalog.api
branch
from
dc93a02 to
c1dcfc4
Compare
Co-authored-by: Nelo-T. Wallus <10514301+ntnn@users.noreply.github.com>
2 participants
Summary
This adds machinery for catalog API. Its not used anywhere, just api and some kcp wiring.
What Type of PR Is This?
/kind feature
Related Issue(s)
Fixes #
Release Notes
Summary by CodeRabbit
New Features
Documentation
Changes