Skip to content

Update docs for v0.5#324

Merged
mjudeikis merged 1 commit into
kbind-dev:mainfrom
mjudeikis:mjudeikis/prep.for.v0.5
Sep 26, 2025
Merged

Update docs for v0.5#324
mjudeikis merged 1 commit into
kbind-dev:mainfrom
mjudeikis:mjudeikis/prep.for.v0.5

Conversation

@mjudeikis

@mjudeikis mjudeikis commented Sep 24, 2025

Copy link
Copy Markdown
Contributor

Summary

In preparation to cut v0.5 release, cleaning some docs and updating refrences to represent whats changed.

What Type of PR Is This?

/kind cleanup
/kind documentation

Related Issue(s)

Fixes #

Release Notes

Update documentation 

Summary by CodeRabbit

  • New Features

    • v1alpha2 resource-based API exports: multi-resource support and BoundSchema for schema/status tracking; multi-cluster provider integration (KCP support).
  • Documentation

    • Added backend architecture, BoundSchema and controller docs, KCP setup guide, updated Quickstart and setup guidance; clarified ClusterBinding description.
  • Chores

    • Updated provider/runtime replacements and KCP version; changed docs generator default container engine to Docker.
  • Style

    • Minor wording and formatting refinements.

@coderabbitai

coderabbitai Bot commented Sep 24, 2025

Copy link
Copy Markdown

Walkthrough

Updates docs, manifests, and backend wiring to adopt a v1alpha2 resource-based APIServiceExport model with BoundSchema, bump clusterbindings schema names, add KCP/multicluster-runtime provider docs and KCP setup, change a docs generator runtime, introduce a provider MultiClusterProvider interface and adapt Config, adjust go.mod replacements, and apply minor formatting edits.

Changes

Cohort / File(s) Summary of Changes
Docs: Backend & Controllers
docs/content/developers/backend/index.md, docs/content/developers/konnector/controllers/cluster/apiserviceexport.md, docs/content/developers/konnector/controllers/cluster/boundschema.md
New/updated documentation describing backend architecture, multicluster-runtime and KCP provider support, APIServiceExport v1alpha2 resource-based spec (Resources []APIServiceExportResource), BoundSchema resource controller, lifecycle and responsibilities.
Setup & Quickstart Docs
docs/content/setup/index.md, docs/content/setup/kcp-setup.md, docs/content/setup/quickstart.md
Add KCP setup guide and KCP-specific quickstart steps; introduce multi-provider model and guidance for standard vs KCP deployments.
KCP Deployment Manifests
contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml, contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml
Bump APIResourceSchema name/version for clusterbindings and update schema reference in APIExport; adjust ClusterBinding v1alpha1 description text.
CRD Spec
deploy/crd/kube-bind.io_clusterbindings.yaml
Minor OpenAPI description wording change for v1alpha1 ClusterBinding (removed "per namespace").
Top-level README
README.md
Heading and content edits to reflect v0.5.0 resource-based exports, BoundSchema, backend/provider architecture, and revised limitations.
Docs Script
docs/generators/crd-ref/run-crd-ref-gen.sh
Change default container engine from podman to docker.
Backend Provider Interface & Config
backend/provider/provider.go, backend/config.go
Add provider.MultiClusterProvider interface (embeds multicluster.Provider and ProviderRunnable) and change backend.Config.Provider type to provider.MultiClusterProvider; update imports.
Go Module Replacements
go.mod
Update replace directives to forked/updated versions of multicluster-provider and multicluster-runtime.
Makefile
Makefile
Downgrade referenced KCP version from v0.28.1 to v0.28.0.
SDK Types / Formatting
sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go, sdk/apis/kubebind/v1alpha1/clusterbinding_types.go
Minor formatting and comment edits; ClusterBinding comment wording adjusted; no API signature changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant APIExport as APIServiceExport (v1alpha2)
  participant Provider as MultiClusterProvider
  participant Controller as Controller Manager
  participant Consumer as Consumer Cluster
  participant Bound as BoundSchema (v1alpha2)

  note right of APIExport #e6f7ff: Resource-based export model\nResources[] per APIServiceExport
  APIExport->>Provider: Register referenced resources (group/version/kind, names)
  Provider->>Controller: Notify runtime / resource availability
  Controller->>Consumer: Ensure BoundSchema created/updated for each referenced resource
  Consumer->>Bound: Create/Update BoundSchema (schema, storedVersions)
  Bound->>Controller: Report status (Synced / Drift / Ready)
  Controller-->>APIExport: Aggregate resource statuses into APIServiceExport status
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Update Dex dependency #279 — Related: updates to go.mod replacement entries for multicluster-runtime/multicluster-provider (dependency fork/version changes).
  • Move to schemaless backend #295 — Strongly related: same migration to resource-based APIServiceExport/BoundSchema and controller/provider wiring changes.

Suggested reviewers

  • xrstf
  • moadqassem

Poem

I thump with joy on schema ground,
New Bounds are set and exports found.
I hop through clusters, map each core,
One export points to many more.
v1alpha2 hums — my whiskers soar. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly conveys that documentation is being updated for the upcoming v0.5 release and aligns with the PR’s primary objective of refreshing docs. It is clear and concise, though it does not mention the accompanying code and API changes included in the same changeset. Overall, it accurately reflects a significant aspect of the PR.
Description Check ✅ Passed The description follows the repository’s template by including the Summary, What Type of PR Is This?, Related Issue(s), and Release Notes sections, each populated with appropriate content. The release-note block specifies the user-facing change and the placeholder for “Fixes #” remains unchanged, which is acceptable when no specific issue is referenced. All required sections are present and correctly formatted.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@mjudeikis mjudeikis force-pushed the mjudeikis/prep.for.v0.5 branch from 5045fcf to eb9500b Compare September 24, 2025 07:23
@mjudeikis mjudeikis marked this pull request as ready for review September 24, 2025 08:48
@mjudeikis mjudeikis requested a review from a team as a code owner September 24, 2025 08:48

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/content/developers/konnector/controllers/cluster/apiserviceexport.md (1)

32-39: Diagram still CRD‑centric; update to reflect “referenced resources” and BoundSchema.

Nodes and edges name CRDs explicitly; v1alpha2 watches referenced resources and integrates BoundSchema. Update labels to avoid misleading readers.

Apply:

-    get_crd(["Get referenced
-    CustomResourceDefinition"])
+    get_resource(["Get referenced
+    resource (e.g., CRD)"])
-    is_crd_present{"CRD<br>exists?"}
+    is_resource_present{"Resource<br>exists?"}
-    get_crd2(["Get referenced
-    CustomResourceDefinition"])
+    get_resource2(["Get referenced resource"])
-    is_crd_present2{"CRD<br>exists?"}
+    is_resource_present2{"Resource<br>exists?"}
-    get_crd --> is_crd_present
+    get_resource --> is_resource_present
-    is_crd_present --> |yes| get_binding
-    is_crd_present --> |no| stop_apiserviceexport_sync
+    is_resource_present --> |yes| get_binding
+    is_resource_present --> |no| stop_apiserviceexport_sync
-    get_crd2 --> is_crd_present2
-    is_crd_present2 --> |no| stop
-    is_crd_present2 --> |yes| copy_crd_conditions
-    copy_crd_conditions --> set_summary_condition
+    get_resource2 --> is_resource_present2
+    is_resource_present2 --> |no| stop
+    is_resource_present2 --> |yes| set_summary_condition

Optionally add a step showing “BoundSchema status informs summary condition” if applicable.

Also applies to: 62-101

🧹 Nitpick comments (15)
docs/generators/crd-ref/run-crd-ref-gen.sh (4)

24-24: Add auto‑detection/fallback if Docker isn’t installed

Switching the default to docker is fine, but dev environments may only have podman. Add a simple fallback and a clear error message.

-CONTAINER_ENGINE=${CONTAINER_ENGINE:-docker}
+CONTAINER_ENGINE=${CONTAINER_ENGINE:-}
+if [[ -z "${CONTAINER_ENGINE}" ]]; then
+  if command -v docker >/dev/null 2>&1; then
+    CONTAINER_ENGINE=docker
+  elif command -v podman >/dev/null 2>&1; then
+    CONTAINER_ENGINE=podman
+  else
+    echo "Error: neither docker nor podman found. Install one or set CONTAINER_ENGINE." >&2
+    exit 1
+  fi
+fi

37-41: Avoid root‑owned files from the container (map user)

With docker on Linux, generated files can end up owned by root. Map the user to keep host ownership.

-$CONTAINER_ENGINE run --rm \
+$CONTAINER_ENGINE run --rm \
+    --user "$(id -u):$(id -g)" \
     -v "${DESTINATION}":/opt/crd-docs-generator/output"${BIND_MOUNT_OPTS}" \
     -v "${REPO_ROOT}"/docs/generators/crd-ref:/opt/crd-docs-generator/config"${BIND_MOUNT_OPTS}" \
     quay.io/giantswarm/crd-docs-generator:${CRD_DOCS_GENERATOR_VERSION} \
     --config /opt/crd-docs-generator/config/config.yaml

40-41: Pin image by digest for reproducibility

Tag-only pinning can drift if the publisher retags. Consider pinning by digest.

-    quay.io/giantswarm/crd-docs-generator:${CRD_DOCS_GENERATOR_VERSION} \
+    quay.io/giantswarm/crd-docs-generator@sha256:<digest-for-0.10.0> \

You can keep the version in a comment for clarity:

# crd-docs-generator 0.10.0

43-63: Harden globbing and quoting to handle edge cases

Guard against no-match globs and paths with spaces; quote expansions and enable nullglob.

+shopt -s nullglob
-for file in ${DESTINATION}/*.md; do
-    filename=$(basename $file)
-    apigroup=$(basename $filename .md | cut -d. -f2-)
-    crdname=$(basename $filename .md | cut -d. -f1)
-    echo "${filename} | ${apigroup}"
+for file in "${DESTINATION}"/*.md; do
+    filename="$(basename "$file")"
+    apigroup="${filename%.md}"; apigroup="${apigroup#*.}"
+    crdname="${filename%%.*}"
+    echo "${filename} | ${apigroup}"
 
-    mkdir -p "${DESTINATION}/${apigroup}"
-    mv "${file}" "${DESTINATION}/${apigroup}/${crdname}.md"
+    mkdir -p "${DESTINATION}/${apigroup}"
+    mv "$file" "${DESTINATION}/${apigroup}/${crdname}.md"
 done
 
-echo "nav:" > ${DESTINATION}/.pages
-for dir in ${DESTINATION}/*/; do
+echo "nav:" > "${DESTINATION}/.pages"
+for dir in "${DESTINATION}"/*/; do
     if [ -d "${dir}" ]; then
-        echo ${dir}
+        echo "${dir}"
     fi
-    apigroup=$(basename $dir)
-    echo "  - ${apigroup}: ${apigroup}" >> ${DESTINATION}/.pages
+    apigroup="$(basename "$dir")"
+    echo "  - ${apigroup}: ${apigroup}" >> "${DESTINATION}/.pages"
 done
README.md (4)

71-76: Clarify and cross-link new API constructs.

Consider adding links to the v1alpha2 API docs for APIServiceExport.Resources and BoundSchema, and explicitly state BoundSchema scope (namespaced vs cluster-scoped) to reduce ambiguity.

Apply:

-  - `APIServiceExportSpec` now uses `Resources []APIServiceExportResource` instead of embedded CRD specs
-  - `BoundSchema`: New resource type in `v1alpha2` that represents bound schemas in consumer clusters and tracks the status of synced resources
+  - `APIServiceExportSpec` now uses `Resources []APIServiceExportResource` instead of embedded CRD specs (see docs)
+  - `BoundSchema` (namespaced): New resource type in `v1alpha2` that represents bound schemas in consumer clusters and tracks the status of synced resources (see docs)

77-81: Add references to external components.

Link to multicluster-runtime and kcp provider docs to help readers.

Apply:

-- **MultiCluster Runtime Integration**: The backend now leverages `sigs.k8s.io/multicluster-runtime` for enhanced cluster management capabilities
-- **Provider Support**: Built-in support for multiple backend providers including KCP through `github.com/kcp-dev/multicluster-provider`
+- **MultiCluster Runtime Integration**: The backend now leverages `sigs.k8s.io/multicluster-runtime` for enhanced cluster management capabilities (repo)
+- **Provider Support**: Built-in support for multiple backend providers including KCP through `github.com/kcp-dev/multicluster-provider` (repo)

87-89: Reference the BoundSchema lifecycle limitation with a tracking issue.

Tie the limitation to the known lifecycle gap.

Apply:

-* We don't support lifecycle of `BoundSchema` resources, like schema changes.
+* We don't support lifecycle/upgrade flows of `BoundSchema` resources (e.g., schema changes). See issue #297.

28-30: Fix typo: “brower” → “browser”.

Minor reader-facing typo.

Apply:

-Redirect to the brower to authenticate via OIDC.
+Redirect to the browser to authenticate via OIDC.
docs/content/developers/konnector/controllers/cluster/boundschema.md (1)

42-49: Clarify mapping semantics and owner references.

Explicitly note that APIServiceExportRequest objects are short‑lived and must not be owners of BoundSchema; link to the lifecycle gap.

Apply:

  BoundSchema works closely with the APIServiceExport resource-based model:
 
  1. APIServiceExport references multiple resources via `Resources []APIServiceExportResource`
  2. Each resource can map to a BoundSchema in the consumer cluster
  3. BoundSchema tracks the actual application and status of these schemas
  4. This enables better multi-resource management and status tracking
+  5. Do not use APIServiceExportRequest as an ownerReference for BoundSchema (requests are auto-deleted after ~10 minutes); see issue #297.
docs/content/developers/backend/index.md (2)

24-29: Use plural resource in kubectl example; singular may not resolve for CRDs.

Kubernetes commonly expects the plural “apiexportendpointslices”.

Apply:

-  --server-url=$(kubectl get apiexportendpointslice kube-bind.io -o jsonpath="{.status.endpoints[0].url}") \
+  --server-url=$(kubectl get apiexportendpointslices kube-bind.io -o jsonpath="{.status.endpoints[0].url}") \

11-19: Preempt RBAC “*” concerns with a short note.

Add a sentence explaining why wide verbs are claimed (scoped to provider namespaces, required for bidirectional flow), per project stance.

deploy/crd/kube-bind.io_clusterbindings.yaml (1)

37-39: Clarify singleton scope (“per namespace”) for v1alpha1 to match v1alpha2.

Avoid ambiguity since the CRD scope is Namespaced.

Apply:

-          ClusterBinding represents a bound consumer class. It lives in a service provider cluster
-          and is a singleton named "cluster".
+          ClusterBinding represents a bound consumer class. It lives in a service provider cluster
+          and is a singleton named "cluster" per namespace.
docs/content/setup/quickstart.md (1)

16-22: Good addition: provider context and clear KCP handoff.

Nice clarity that Quickstart uses the Standard provider and points to KCP guide. Consider capitalizing “KCP” consistently (Line 21: “kcp integration” → “KCP integration”).

-This quickstart uses the default provider. For kcp integration, see the [KCP Setup Guide](kcp-setup.md).
+This quickstart uses the default provider. For KCP integration, see the [KCP Setup Guide](kcp-setup.md).
docs/content/setup/kcp-setup.md (2)

132-134: “Enable recursive binding” note doesn’t match the command.

If recursion is required, add the appropriate flag or clarify the text.


45-51: OIDC client secret in examples is fine, but add a cautionary note.

Even though this is an example value (“example-app-secret”), add a sentence reminding users to replace it in real setups.

   name: 'Kube Bind'
-  secret: ZXhhbXBsZS1hcHAtc2VjcmV0
+  secret: ZXhhbXBsZS1hcHAtc2VjcmV0 # Example only; replace with your own secret in non-demo environments
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ab03f00 and eb9500b.

📒 Files selected for processing (13)
  • README.md (1 hunks)
  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (1 hunks)
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml (2 hunks)
  • deploy/crd/kube-bind.io_clusterbindings.yaml (1 hunks)
  • docs/content/developers/backend/index.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/boundschema.md (1 hunks)
  • docs/content/setup/index.md (1 hunks)
  • docs/content/setup/kcp-setup.md (1 hunks)
  • docs/content/setup/quickstart.md (1 hunks)
  • docs/generators/crd-ref/run-crd-ref-gen.sh (1 hunks)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go (0 hunks)
  • sdk/apis/kubebind/v1alpha1/clusterbinding_types.go (1 hunks)
💤 Files with no reviewable changes (1)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go
🧰 Additional context used
🧠 Learnings (8)
📚 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:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md
  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml
📚 Learning: 2025-09-22T13:20:49.933Z
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.933Z
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:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-22T13:20:49.933Z
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.933Z
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:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
📚 Learning: 2025-09-12T08:55:41.860Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: sdk/apis/kubebind/v1alpha2/helpers/boundschema.go:115-123
Timestamp: 2025-09-12T08:55:41.860Z
Learning: In BoundSchemasSpecHash function in sdk/apis/kubebind/v1alpha2/helpers/boundschema.go, silent error handling during JSON encoding (continuing on encoding errors) is acceptable to mjudeikis for the current implementation, even though it could potentially lead to incorrect hash values.

Applied to files:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-12T09:05:29.762Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: sdk/client/listers/kubebind/v1alpha2/boundschema.go:46-48
Timestamp: 2025-09-12T09:05:29.762Z
Learning: In the kube-bind project, lister-gen is generating BoundSchema listers with singular resource names ("boundschema") instead of plural ("boundschemas"), which breaks client-go conventions and can cause cache lookup issues. This is identified as a generator issue that needs upstream investigation rather than manual code fixes.

Applied to files:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-22T13:32:29.471Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#304
File: pkg/konnector/controllers/cluster/claimedresources/claimedresources_controller.go:255-263
Timestamp: 2025-09-22T13:32:29.471Z
Learning: In kube-bind's claimedresources controller (pkg/konnector/controllers/cluster/claimedresources/claimedresources_controller.go), the controller does not run if apiServiceExport is not set. This means nil-checks for c.apiServiceExport are unnecessary since the controller lifecycle ensures it's always non-nil when active.

Applied to files:

  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md
📚 Learning: 2025-09-19T05:56:35.969Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#304
File: backend/controllers/servicenamespace/servicenamespace_reconcile.go:81-98
Timestamp: 2025-09-19T05:56:35.969Z
Learning: In kube-bind, RBAC permissions for PermissionClaims use "*" verbs intentionally. This is a design decision based on: 1) permissions are scoped to consumer-owned provider namespaces, limiting blast radius, 2) bidirectional resource flow requires broad permissions for operations like initial resource creation from consumer side, 3) kube-bind's architecture prioritizes operational simplicity over granular RBAC within the namespace security boundary.

Applied to files:

  • docs/content/developers/backend/index.md
📚 Learning: 2025-09-19T06:28:44.853Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#304
File: pkg/konnector/controllers/cluster/serviceexport/serviceexport_reconcile.go:148-150
Timestamp: 2025-09-19T06:28:44.853Z
Learning: In kube-bind, permission claims must all have the same scope (either all cluster-scoped or all namespace-scoped). Mixed scopes are not allowed, which means the isClusterScoped flag can be safely determined from any processed schema in the export.

Applied to files:

  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml
🪛 Gitleaks (8.28.0)
docs/content/setup/kcp-setup.md

[high] 89-89: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 90-90: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🔇 Additional comments (10)
docs/content/developers/konnector/controllers/cluster/apiserviceexport.md (1)

9-12: Solid summary of v1alpha2 shift.

Accurately states the resource-based model and BoundSchema linkage.

Please confirm BoundSchema docs use the correct group (kube-bind.io) to avoid cross-page inconsistency.

docs/content/developers/backend/index.md (1)

50-57: Good API change summary.

Concise and consistent with the rest of docs.

sdk/apis/kubebind/v1alpha1/clusterbinding_types.go (1)

37-38: Docstring wording LGTM.

Terminology aligned to “bound consumer class”; no functional change.

Ensure CRD descriptions for v1alpha1/v1alpha2 consistently reflect intended wording (“per namespace” where applicable) across generated manifests.

contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml (2)

5-5: APIResourceSchema name bumped correctly.

Aligned with APIExport reference.


170-173: Storage version flip to v1alpha2 is safe — no structural diffs detected.
OpenAPIV3Schema property sets for v1alpha1 and v1alpha2 in contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml are identical; no conversion required.

docs/content/setup/index.md (3)

22-24: Cross-check terminology consistency for “v1alpha2”, “resource-based exports”, and “BoundSchema”.

Ensure these terms match the API docs and CRD descriptions to prevent confusion.


32-32: Confirm docs engine supports the include directive.

“{% include "partials/section-overview.html" %}” requires a compatible templating pipeline. If unsupported, render will fail.


9-12: Verified — referenced setup pages exist
All referenced files (quickstart.md, helm.md, local-setup-with-kind.md, kubectl-plugin.md) were found; no broken links detected.

docs/content/setup/kcp-setup.md (2)

141-144: Verify resource name for logical clusters.

Confirm whether the correct type is “logicalclusters” and group (e.g., tenancy.kcp.io). Adjust command accordingly.


82-82: Harden quoting and use the fully-qualified, plural resource name.

File: docs/content/setup/kcp-setup.md:82

-  --server-url=$(kubectl get apiexportendpointslice kube-bind.io -o jsonpath="{.status.endpoints[0].url}") \
+  --server-url="$(kubectl get apiexportendpointslices.apis.kcp.io kube-bind.io -o jsonpath='{.status.endpoints[0].url}')" \

Confirm the exact GVK (group/version/kind) and the jsonpath field for APIExportEndpointSlice in your KCP version.

Comment thread contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
Comment thread docs/content/developers/konnector/controllers/cluster/boundschema.md Outdated
Comment thread docs/content/setup/kcp-setup.md
Comment thread docs/content/setup/kcp-setup.md Outdated
Comment thread docs/content/setup/kcp-setup.md
Comment thread docs/content/setup/kcp-setup.md Outdated
Comment thread docs/content/setup/kcp-setup.md Outdated
cnvergence
cnvergence previously approved these changes Sep 24, 2025
@mjudeikis mjudeikis force-pushed the mjudeikis/prep.for.v0.5 branch 2 times, most recently from 4a0f9ba to d2dbed3 Compare September 25, 2025 05:23

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
backend/config.go (1)

136-158: Off-by-one: pathParts length check can panic

Index 5 is accessed but only len<5 is guarded. Use len<6 to avoid out-of-bounds.

Apply this diff:

-    if len(pathParts) < 5 || pathParts[4] != "clusters" {
+    if len(pathParts) < 6 || pathParts[4] != "clusters" {
         return "", fmt.Errorf("invalid apiexport URL format")
     }
 
     clusterID := pathParts[5]
docs/content/setup/quickstart.md (1)

82-91: Replace example cookie keys with placeholders

Prevent accidental reuse; keep the generation section below.

-   bin/backend \
+   bin/backend \
      --oidc-issuer-client-secret=ZXhhbXBsZS1hcHAtc2VjcmV0 \
      --oidc-issuer-client-id=kube-bind \
      --oidc-issuer-url=http://127.0.0.1:5556/dex \
      --oidc-callback-url=http://127.0.0.1:8080/callback \
      --pretty-name="BigCorp.com" \
      --namespace-prefix="kube-bind-" \
-     --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \
-     --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \
+     --cookie-signing-key=<BASE64_32_OR_64_BYTE_KEY> \
+     --cookie-encryption-key=<BASE64_16_24_OR_32_BYTE_KEY> \
      --consumer-scope=cluster
contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (1)

47-68: Sync KCP APIExport schema IDs
apiconversions, apiservicebindings, apiserviceexportrequests and apiservicenamespaces still reference v250809-5ed76a1; bump them to the new version (e.g. v250902-878858c) to match apiserviceexports or regenerate contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml.

🧹 Nitpick comments (1)
go.mod (1)

12-28: Add rationale and revert comment for forked modules
Insert above the forked replacements in go.mod (lines 12–28):

// TEMP: use forked modules while upstream PRs land; revert to upstream tags before release.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eb9500b and 4a0f9ba.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (17)
  • Makefile (1 hunks)
  • README.md (1 hunks)
  • backend/config.go (1 hunks)
  • backend/provider/provider.go (1 hunks)
  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (1 hunks)
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml (2 hunks)
  • deploy/crd/kube-bind.io_clusterbindings.yaml (1 hunks)
  • docs/content/developers/backend/index.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/boundschema.md (1 hunks)
  • docs/content/setup/index.md (1 hunks)
  • docs/content/setup/kcp-setup.md (1 hunks)
  • docs/content/setup/quickstart.md (1 hunks)
  • docs/generators/crd-ref/run-crd-ref-gen.sh (1 hunks)
  • go.mod (2 hunks)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go (0 hunks)
  • sdk/apis/kubebind/v1alpha1/clusterbinding_types.go (1 hunks)
💤 Files with no reviewable changes (1)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go
🚧 Files skipped from review as they are similar to previous changes (6)
  • docs/content/developers/konnector/controllers/cluster/boundschema.md
  • sdk/apis/kubebind/v1alpha1/clusterbinding_types.go
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml
  • deploy/crd/kube-bind.io_clusterbindings.yaml
  • docs/content/developers/backend/index.md
  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md
🧰 Additional context used
🧠 Learnings (4)
📚 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/apiexport-kube-bind.io.yaml
📚 Learning: 2025-09-12T09:05:29.762Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: sdk/client/listers/kubebind/v1alpha2/boundschema.go:46-48
Timestamp: 2025-09-12T09:05:29.762Z
Learning: In the kube-bind project, lister-gen is generating BoundSchema listers with singular resource names ("boundschema") instead of plural ("boundschemas"), which breaks client-go conventions and can cause cache lookup issues. This is identified as a generator issue that needs upstream investigation rather than manual code fixes.

Applied to files:

  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
📚 Learning: 2025-09-22T13:20:49.933Z
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.933Z
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:

  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
📚 Learning: 2025-09-12T08:43:56.250Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: kcp/README.md:47-49
Timestamp: 2025-09-12T08:43:56.250Z
Learning: Demo/example values for secrets in documentation and README files are acceptable when they're clearly for demonstration purposes, unlike real secrets which should never be committed to documentation.

Applied to files:

  • docs/content/setup/kcp-setup.md
🧬 Code graph analysis (1)
backend/config.go (1)
backend/provider/provider.go (1)
  • MultiClusterProvider (22-25)
🪛 Gitleaks (8.28.0)
docs/content/setup/kcp-setup.md

[high] 89-89: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 90-90: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

⏰ 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). (5)
  • GitHub Check: go-test
  • GitHub Check: Generate and push docs
  • GitHub Check: verify
  • GitHub Check: lint
  • GitHub Check: go-test-e2e
🔇 Additional comments (15)
docs/generators/crd-ref/run-crd-ref-gen.sh (1)

24-24: Defaulting to Docker aligns with the repo’s tooling

Switching the default container engine to Docker matches the rest of the repository’s docs & tooling, while still letting Podman users override CONTAINER_ENGINE when needed. LGTM.

Makefile (1)

80-80: Confirm KCP binary compatibility and asset availability

Downgrading to v0.28.0 aligns with go.mod (kcp/sdk v0.28.0). Please verify the release asset exists for all supported OS/ARCH and that e2e still passes with this version.

go.mod (1)

3-3: Go version directive 1.24.0 — verify CI toolchain

Ensure CI/build images use Go 1.24.x; otherwise, bump down to the actual version used or upgrade the toolchain.

contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (2)

76-79: ClusterBinding schema ID bump looks good; align others

The clusterbindings entry is updated to v250924-ab03f00. Ensure all other listed resources reference their latest schema IDs for consistency.


52-57: Duplicate: stale schema IDs noted previously

This is the same issue flagged above regarding v250809-5ed76a1 references.

backend/config.go (2)

39-44: Provider type refactor is sensible

Switching to a local MultiClusterProvider interface decouples backend from specific provider impls.


49-55: Nil provider on default path — confirm mcmanager.New behavior

When not using kcp, Provider is nil. Verify mcmanager.New tolerates nil and runs in single-cluster mode; otherwise, construct a no-op/single-cluster provider.

backend/provider/provider.go (1)

21-25: Good abstraction: unify Provider and Runnable

This local interface composes the required multicluster traits cleanly.

docs/content/setup/index.md (1)

3-31: Docs structure looks good; verify cross-links exist

Check that quickstart.md, helm.md, local-setup-with-kind.md, kubectl-plugin.md, and kcp-setup.md are present and build with your mkdocs include plugin.

docs/content/setup/kcp-setup.md (4)

89-90: Replace example cookie keys with placeholders

Avoid publishing concrete keys; keep the generation instructions below.

-  --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \
-  --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \
+  --cookie-signing-key=<BASE64_32_OR_64_BYTE_KEY> \
+  --cookie-encryption-key=<BASE64_16_24_OR_32_BYTE_KEY> \

80-92: Fix APIExportEndpointSlice resource and JSONPath quoting; prefer explicit kubectl

Use the plural, fully-qualified resource and single-quote JSONPath for robustness.

-  --server-url=$(kubectl get apiexportendpointslice kube-bind.io -o jsonpath="{.status.endpoints[0].url}") \
+  --server-url=$(kubectl get apiexportendpointslices.apis.kcp.io kube-bind.io -o jsonpath='{.status.endpoints[0].url}') \

218-229: Replace alias ‘k’ with ‘kubectl’; fix resource name and quoting

Ensure examples work without aliases and use the fully-qualified plural resource.

-k ws use :root:kube-bind
+kubectl ws use :root:kube-bind
@@
-# Check available resources
-k -s "$(kubectl get apiexportendpointslice kube-bind.io -o jsonpath="{.status.endpoints[0].url}")/clusters/*" api-resources
+# Check available resources
+kubectl -s "$(kubectl get apiexportendpointslices.apis.kcp.io kube-bind.io -o jsonpath='{.status.endpoints[0].url}')/clusters/*" api-resources
@@
-# List CRDs
-k -s "$(kubectl get apiexportendpointslice kube-bind.io -o jsonpath="{.status.endpoints[0].url}")/clusters/*" get crd
+# List CRDs
+kubectl -s "$(kubectl get apiexportendpointslices.apis.kcp.io kube-bind.io -o jsonpath='{.status.endpoints[0].url}')/clusters/*" get crd

162-179: Generalize environment-specific values (cluster ID, secret name, namespace)

Use placeholders to prevent copy‑paste errors and add a short note on how to discover the values.

-./bin/kubectl-bind http://127.0.0.1:8080/clusters/1st3wdgmux1oirqa/exports --dry-run -o yaml > apiserviceexport.yaml
+./bin/kubectl-bind "http://127.0.0.1:8080/clusters/<LOGICAL_CLUSTER_ID>/exports" --dry-run -o yaml > apiserviceexport.yaml
@@
-kubectl get secret kubeconfig-npxrd -n kube-bind -o jsonpath='{.data.kubeconfig}' | base64 -d > remote.kubeconfig
+kubectl get secret <KUBECONFIG_SECRET_NAME> -n kube-bind -o jsonpath='{.data.kubeconfig}' | base64 -d > remote.kubeconfig
@@
-  --remote-namespace kube-bind-flsd8
+  --remote-namespace <REMOTE_NAMESPACE>

Tip: list secrets with prefix “kubeconfig-” and use the namespace printed by kubectl-bind output.

README.md (2)

71-75: Clear articulation of the v1alpha2 resource model

Line 73-75 succinctly capture the shift to Resources[] and introduce BoundSchema without ambiguity. This should help readers understand the new export semantics at a glance. Nicely done.


79-81: No change: github.com/kcp-dev/multicluster-provider is valid
go.mod declares this module (with a replace) and backend/config.go imports it; README reference is accurate.

@mjudeikis mjudeikis force-pushed the mjudeikis/prep.for.v0.5 branch 2 times, most recently from 63242d4 to 56669b8 Compare September 25, 2025 05:31

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4a0f9ba and 56669b8.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (17)
  • Makefile (1 hunks)
  • README.md (1 hunks)
  • backend/config.go (1 hunks)
  • backend/provider/provider.go (1 hunks)
  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (1 hunks)
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml (2 hunks)
  • deploy/crd/kube-bind.io_clusterbindings.yaml (1 hunks)
  • docs/content/developers/backend/index.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/boundschema.md (1 hunks)
  • docs/content/setup/index.md (1 hunks)
  • docs/content/setup/kcp-setup.md (1 hunks)
  • docs/content/setup/quickstart.md (1 hunks)
  • docs/generators/crd-ref/run-crd-ref-gen.sh (1 hunks)
  • go.mod (2 hunks)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go (0 hunks)
  • sdk/apis/kubebind/v1alpha1/clusterbinding_types.go (1 hunks)
💤 Files with no reviewable changes (1)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go
🚧 Files skipped from review as they are similar to previous changes (10)
  • backend/provider/provider.go
  • docs/content/setup/index.md
  • Makefile
  • backend/config.go
  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
  • docs/content/developers/backend/index.md
  • docs/generators/crd-ref/run-crd-ref-gen.sh
  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md
  • docs/content/setup/quickstart.md
  • deploy/crd/kube-bind.io_clusterbindings.yaml
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-09-12T09:05:29.762Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: sdk/client/listers/kubebind/v1alpha2/boundschema.go:46-48
Timestamp: 2025-09-12T09:05:29.762Z
Learning: In the kube-bind project, lister-gen is generating BoundSchema listers with singular resource names ("boundschema") instead of plural ("boundschemas"), which breaks client-go conventions and can cause cache lookup issues. This is identified as a generator issue that needs upstream investigation rather than manual code fixes.

Applied to files:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-22T13:20:49.933Z
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.933Z
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:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 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:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml
📚 Learning: 2025-09-22T13:20:49.933Z
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.933Z
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:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-12T08:55:41.860Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: sdk/apis/kubebind/v1alpha2/helpers/boundschema.go:115-123
Timestamp: 2025-09-12T08:55:41.860Z
Learning: In BoundSchemasSpecHash function in sdk/apis/kubebind/v1alpha2/helpers/boundschema.go, silent error handling during JSON encoding (continuing on encoding errors) is acceptable to mjudeikis for the current implementation, even though it could potentially lead to incorrect hash values.

Applied to files:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-19T06:28:44.853Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#304
File: pkg/konnector/controllers/cluster/serviceexport/serviceexport_reconcile.go:148-150
Timestamp: 2025-09-19T06:28:44.853Z
Learning: In kube-bind, permission claims must all have the same scope (either all cluster-scoped or all namespace-scoped). Mixed scopes are not allowed, which means the isClusterScoped flag can be safely determined from any processed schema in the export.

Applied to files:

  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml
📚 Learning: 2025-09-12T08:43:56.250Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: kcp/README.md:47-49
Timestamp: 2025-09-12T08:43:56.250Z
Learning: Demo/example values for secrets in documentation and README files are acceptable when they're clearly for demonstration purposes, unlike real secrets which should never be committed to documentation.

Applied to files:

  • docs/content/setup/kcp-setup.md
🪛 Gitleaks (8.28.0)
docs/content/setup/kcp-setup.md

[high] 89-89: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 90-90: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

⏰ 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 (5)
docs/content/developers/konnector/controllers/cluster/boundschema.md (1)

3-42: Documented integration path looks solid

Nice concise overview of the BoundSchema controller and how it ties into the APIServiceExport flow; the structure example reads well and lines up with the v1alpha2 model.

docs/content/setup/kcp-setup.md (2)

88-92: Replace the sample cookie keys with clear placeholders.

We’re publishing realistic-looking base64 keys here; readers may copy them verbatim, which is an avoidable security smell. Please swap them for explicit placeholders and keep the guidance below about generating fresh values.

   --namespace-prefix="kube-bind-" \
-  --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \
-  --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \
+  --cookie-signing-key=<BASE64_64_BYTE_SIGNING_KEY> \
+  --cookie-encryption-key=<BASE64_32_BYTE_ENCRYPTION_KEY> \

222-229: Fix spacing and resource references in the debug commands.

kubectlws, kubectl-s, and the malformed comment make this section unusable. While touching it, please also switch to the fully qualified plural resource and safe JSONPath quoting so the commands run as-is.

-kubectlws use :root:kube-bind
+# Switch to debug workspace
+kubectl ws use :root:kube-bind
-
-# Checkubectlavailable resources
-kubectl-s "$(kubectl get apiexportendpointslice kube-bind.io -o jsonpath="{.status.endpoints[0].url}")/clusters/*" api-resources
+# Check available resources
+kubectl -s "$(kubectl get apiexportendpointslices.apis.kcp.io kube-bind.io -o jsonpath='{.status.endpoints[0].url}')/clusters/*" api-resources
@@
-# List CRDs
-kubectl-s "$(kubectl get apiexportendpointslice kube-bind.io -o jsonpath="{.status.endpoints[0].url}")/clusters/*" get crd
+# List CRDs
+kubectl -s "$(kubectl get apiexportendpointslices.apis.kcp.io kube-bind.io -o jsonpath='{.status.endpoints[0].url}')/clusters/*" get crd
go.mod (1)

16-28: Confirm forked replacements before release

We now rely on personal forks for both multicluster-provider and multicluster-runtime. Before cutting v0.5, please double-check that shipping with these replacements is intentional, that the forks will remain available, and that any license/compliance requirements are satisfied. If they’re temporary stopgaps, let’s plan the upstream move before the release tag.

README.md (1)

71-81: Docs accurately capture the new API and backend posture.

Thanks for clearly outlining the v1alpha2 introduction, resource-based exports, and multicluster-runtime provider integration—this gives users the right expectations heading into v0.5.

Comment thread contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml Outdated
Comment thread docs/content/setup/kcp-setup.md
Comment thread sdk/apis/kubebind/v1alpha1/clusterbinding_types.go Outdated
@mjudeikis mjudeikis force-pushed the mjudeikis/prep.for.v0.5 branch 2 times, most recently from 6efe4cb to ed104b4 Compare September 25, 2025 06:18

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 56669b8 and ed104b4.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (17)
  • Makefile (1 hunks)
  • README.md (1 hunks)
  • backend/config.go (1 hunks)
  • backend/provider/provider.go (1 hunks)
  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (1 hunks)
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml (2 hunks)
  • deploy/crd/kube-bind.io_clusterbindings.yaml (1 hunks)
  • docs/content/developers/backend/index.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/boundschema.md (1 hunks)
  • docs/content/setup/index.md (1 hunks)
  • docs/content/setup/kcp-setup.md (1 hunks)
  • docs/content/setup/quickstart.md (1 hunks)
  • docs/generators/crd-ref/run-crd-ref-gen.sh (1 hunks)
  • go.mod (2 hunks)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go (0 hunks)
  • sdk/apis/kubebind/v1alpha1/clusterbinding_types.go (1 hunks)
💤 Files with no reviewable changes (1)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go
🚧 Files skipped from review as they are similar to previous changes (10)
  • docs/content/setup/kcp-setup.md
  • docs/generators/crd-ref/run-crd-ref-gen.sh
  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
  • sdk/apis/kubebind/v1alpha1/clusterbinding_types.go
  • backend/provider/provider.go
  • docs/content/developers/backend/index.md
  • docs/content/setup/index.md
  • docs/content/developers/konnector/controllers/cluster/boundschema.md
  • deploy/crd/kube-bind.io_clusterbindings.yaml
  • go.mod
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-22T13:32:29.471Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#304
File: pkg/konnector/controllers/cluster/claimedresources/claimedresources_controller.go:255-263
Timestamp: 2025-09-22T13:32:29.471Z
Learning: In kube-bind's claimedresources controller (pkg/konnector/controllers/cluster/claimedresources/claimedresources_controller.go), the controller does not run if apiServiceExport is not set. This means nil-checks for c.apiServiceExport are unnecessary since the controller lifecycle ensures it's always non-nil when active.

Applied to files:

  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md
📚 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:

  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml
📚 Learning: 2025-09-12T09:05:29.762Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: sdk/client/listers/kubebind/v1alpha2/boundschema.go:46-48
Timestamp: 2025-09-12T09:05:29.762Z
Learning: In the kube-bind project, lister-gen is generating BoundSchema listers with singular resource names ("boundschema") instead of plural ("boundschemas"), which breaks client-go conventions and can cause cache lookup issues. This is identified as a generator issue that needs upstream investigation rather than manual code fixes.

Applied to files:

  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml
📚 Learning: 2025-09-19T06:28:44.853Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#304
File: pkg/konnector/controllers/cluster/serviceexport/serviceexport_reconcile.go:148-150
Timestamp: 2025-09-19T06:28:44.853Z
Learning: In kube-bind, permission claims must all have the same scope (either all cluster-scoped or all namespace-scoped). Mixed scopes are not allowed, which means the isClusterScoped flag can be safely determined from any processed schema in the export.

Applied to files:

  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml
🧬 Code graph analysis (1)
backend/config.go (1)
backend/provider/provider.go (1)
  • MultiClusterProvider (22-25)
⏰ 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). (2)
  • GitHub Check: lint
  • GitHub Check: go-test-e2e
🔇 Additional comments (4)
Makefile (1)

80-83: Confirm the KCP version downgrade.

We used to ship v0.28.1; this change pins make run-kcp to v0.28.0 instead. If that regression is intentional (for example to match the new multicluster-provider fork), can we capture the rationale in release notes or a comment? Otherwise, let's keep the latest patch level.

backend/config.go (1)

49-51: Good call on narrowing the provider interface.

Embedding the new provider.MultiClusterProvider makes the expectations on anything wiring a provider into the backend config explicit.

README.md (1)

71-82: Doc update reads clearly.

Nice summary of the v1alpha2 resource model and how BoundSchema ties it together.

docs/content/developers/konnector/controllers/cluster/apiserviceexport.md (1)

9-12: Nice highlight of the spec changes.

Calling out the resource-based model and BoundSchema integration here will save folks hunting through the code.

Comment thread docs/content/setup/quickstart.md
Comment thread Makefile
Comment thread docs/content/setup/kcp-setup.md Outdated
@mjudeikis mjudeikis force-pushed the mjudeikis/prep.for.v0.5 branch from ed104b4 to 22a3a64 Compare September 25, 2025 13:21

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/config.go (1)

147-153: Fix off-by-one: pathParts index can panic.

When len(pathParts)==5, pathParts[5] panics. Require >=6 before accessing index 5.

- if len(pathParts) < 5 || pathParts[4] != "clusters" {
+ if len(pathParts) < 6 || pathParts[4] != "clusters" {
   return "", fmt.Errorf("invalid apiexport URL format")
 }
 
-clusterID := pathParts[5]
+clusterID := pathParts[5]
🧹 Nitpick comments (10)
docs/content/developers/konnector/controllers/cluster/boundschema.md (2)

24-31: Clarify group non-emptiness (CRDs never use core group "").

CRDs require a non-empty API group; BoundSchema should mirror that.

 spec:
-  group: wildwest.dev
+  group: wildwest.dev
+  # NOTE: CRDs must have a non-empty API group; the core API group ("") is not applicable here.

35-42: Add lifecycle caveat: don’t ownerRef BoundSchema to APIServiceExportRequest.

APIServiceExportRequest objects are short‑lived (~10m) and must not own longer‑lived BoundSchemas.

 4. This enables better multi-resource management and status tracking
+
+> Note: APIServiceExportRequest resources are short-lived and auto-deleted. Do not set them
+> as ownerReferences for BoundSchema; manage BoundSchema lifecycle independently.
backend/config.go (1)

102-111: Avoid shadowing ‘provider’ identifier for readability.

Rename the local var to reduce confusion with the imported provider package/type.

- provider, err := apiexport.New(config.ClientConfig, apiexport.Options{
+ kcpProvider, err := apiexport.New(config.ClientConfig, apiexport.Options{
     Scheme: scheme,
   })
   if err != nil {
     return nil, fmt.Errorf("error setting up kcp provider: %w", err)
   }
 
   config.ExternalAddressGenerator = parseKCPServerURL
-  config.Provider = provider
+  config.Provider = kcpProvider
docs/content/setup/kcp-setup.md (3)

162-162: Avoid “<...>” placeholders in shell commands; they break via redirection.

Use environment variables and quote the URL.

-./bin/kubectl-bind http://127.0.0.1:8080/clusters/<logical-cluster-id>/exports --dry-run -o yaml > apiserviceexport.yaml
+LOGICAL_CLUSTER_ID=<your_cluster_id_here>
+./bin/kubectl-bind "http://127.0.0.1:8080/clusters/${LOGICAL_CLUSTER_ID}/exports" --dry-run -o yaml > apiserviceexport.yaml

168-168: Avoid “<...>” placeholders; use a variable for secret name.

-kubectl get secret <secret-name> -n kube-bind -o jsonpath='{.data.kubeconfig}' | base64 -d > remote.kubeconfig
+KUBECONFIG_SECRET_NAME=<your_secret_name_here>
+kubectl get secret "${KUBECONFIG_SECRET_NAME}" -n kube-bind -o jsonpath='{.data.kubeconfig}' | base64 -d > remote.kubeconfig

178-179: Avoid “<...>” placeholders; use a variable for remote namespace.

-  --remote-namespace kube-bind-<random-suffix>
+  --remote-namespace "${REMOTE_NAMESPACE}"
+# e.g., export REMOTE_NAMESPACE="kube-bind-$(LC_ALL=C tr -dc a-z0-9 </dev/urandom | head -c 5)"
docs/content/setup/index.md (1)

20-24: Align version naming with the release (“v0.5” vs “v0.5.0”).

Elsewhere this PR refers to “v0.5”. Pick one consistently.

-Starting with v0.5.0, kube-bind uses a multicluster-runtime architecture that supports:
+Starting with v0.5, kube-bind uses a multicluster-runtime architecture that supports:
docs/content/setup/quickstart.md (1)

122-123: Avoid angle-bracket placeholders in shell commands.

Use an env var for safer copy/paste.

-./bin/kubectl-bind apiservice --remote-kubeconfig .kcp/provider.kubeconfig -f apiserviceexport.yaml  --skip-konnector --remote-namespace <namespace>
+REMOTE_NAMESPACE=<your_namespace_here>
+./bin/kubectl-bind apiservice --remote-kubeconfig .kcp/provider.kubeconfig -f apiserviceexport.yaml --skip-konnector --remote-namespace "${REMOTE_NAMESPACE}"
README.md (1)

77-82: Add fork note for multicluster-provider
Both sigs.k8s.io/multicluster-runtime and github.com/kcp-dev/multicluster-provider are replaced by your forks in go.mod; mirror this in the Backend Architecture section (or link the existing tracking issue) as you did under Limitations.

go.mod (1)

16-28: Document and track upstream PRs for forked replaces
Add comments above the replace entries in go.mod referencing the upstream PRs/issues (runtime PR #62; link/create PR for provider) and note to revert after release. [go.mod:16,27]

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed104b4 and 22a3a64.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (17)
  • Makefile (1 hunks)
  • README.md (1 hunks)
  • backend/config.go (1 hunks)
  • backend/provider/provider.go (1 hunks)
  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (1 hunks)
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml (2 hunks)
  • deploy/crd/kube-bind.io_clusterbindings.yaml (1 hunks)
  • docs/content/developers/backend/index.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/boundschema.md (1 hunks)
  • docs/content/setup/index.md (1 hunks)
  • docs/content/setup/kcp-setup.md (1 hunks)
  • docs/content/setup/quickstart.md (1 hunks)
  • docs/generators/crd-ref/run-crd-ref-gen.sh (1 hunks)
  • go.mod (2 hunks)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go (0 hunks)
  • sdk/apis/kubebind/v1alpha1/clusterbinding_types.go (1 hunks)
💤 Files with no reviewable changes (1)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go
🚧 Files skipped from review as they are similar to previous changes (8)
  • sdk/apis/kubebind/v1alpha1/clusterbinding_types.go
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml
  • docs/generators/crd-ref/run-crd-ref-gen.sh
  • Makefile
  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md
  • docs/content/developers/backend/index.md
  • backend/provider/provider.go
  • deploy/crd/kube-bind.io_clusterbindings.yaml
🧰 Additional context used
🧠 Learnings (7)
📚 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/apiexport-kube-bind.io.yaml
  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-12T09:05:29.762Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: sdk/client/listers/kubebind/v1alpha2/boundschema.go:46-48
Timestamp: 2025-09-12T09:05:29.762Z
Learning: In the kube-bind project, lister-gen is generating BoundSchema listers with singular resource names ("boundschema") instead of plural ("boundschemas"), which breaks client-go conventions and can cause cache lookup issues. This is identified as a generator issue that needs upstream investigation rather than manual code fixes.

Applied to files:

  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-12T08:55:41.860Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: sdk/apis/kubebind/v1alpha2/helpers/boundschema.go:115-123
Timestamp: 2025-09-12T08:55:41.860Z
Learning: In BoundSchemasSpecHash function in sdk/apis/kubebind/v1alpha2/helpers/boundschema.go, silent error handling during JSON encoding (continuing on encoding errors) is acceptable to mjudeikis for the current implementation, even though it could potentially lead to incorrect hash values.

Applied to files:

  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-22T13:20:49.933Z
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.933Z
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:

  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-22T13:20:49.933Z
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.933Z
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:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-19T06:28:44.853Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#304
File: pkg/konnector/controllers/cluster/serviceexport/serviceexport_reconcile.go:148-150
Timestamp: 2025-09-19T06:28:44.853Z
Learning: In kube-bind, permission claims must all have the same scope (either all cluster-scoped or all namespace-scoped). Mixed scopes are not allowed, which means the isClusterScoped flag can be safely determined from any processed schema in the export.

Applied to files:

  • docs/content/developers/konnector/controllers/cluster/boundschema.md
📚 Learning: 2025-09-12T08:43:56.250Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: kcp/README.md:47-49
Timestamp: 2025-09-12T08:43:56.250Z
Learning: Demo/example values for secrets in documentation and README files are acceptable when they're clearly for demonstration purposes, unlike real secrets which should never be committed to documentation.

Applied to files:

  • docs/content/setup/kcp-setup.md
🧬 Code graph analysis (1)
backend/config.go (1)
backend/provider/provider.go (1)
  • MultiClusterProvider (22-25)
🪛 Gitleaks (8.28.0)
docs/content/setup/kcp-setup.md

[high] 89-89: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 90-90: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

⏰ 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: go-test
  • GitHub Check: verify
  • GitHub Check: go-test-e2e
  • GitHub Check: lint
🔇 Additional comments (5)
docs/content/setup/quickstart.md (2)

89-91: Replace example cookie keys with placeholders.

Avoid publishing realistic-looking secrets; keep the “openssl rand” guidance.

-     --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \
-     --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \
+     --cookie-signing-key=REPLACE_WITH_BASE64_32_OR_64_BYTE_KEY \
+     --cookie-encryption-key=REPLACE_WITH_BASE64_16_24_OR_32_BYTE_KEY \

16-26: Quickstart scope is conflicting; KCP-specific steps appear in “standard provider” flow.

Either keep this page strictly standard-provider, or clearly split KCP steps and point to KCP guide.

Suggested approach:

  • Keep Quickstart limited to standard provider.
  • Move/trim KCP-specific sections (kcp binary, workspaces, ws create, etc.) to kcp-setup.md and link to it here.
docs/content/setup/kcp-setup.md (1)

89-91: Replace example cookie keys with placeholders.

Avoid copy-pasting realistic-looking secrets; keep generation guidance below.

-  --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \
-  --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \
+  --cookie-signing-key=REPLACE_WITH_BASE64_32_OR_64_BYTE_KEY \
+  --cookie-encryption-key=REPLACE_WITH_BASE64_16_24_OR_32_BYTE_KEY \
backend/config.go (1)

111-114: Confirm mcmanager.New supports a nil Provider
mcmanager.New is invoked with config.Provider==nil in the default case—verify it tolerates a nil provider or supply a default provider to avoid runtime failures.

contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (1)

47-68: Verify matching APIResourceSchema definitions for all schema IDs in APIExport. Confirm each schema: entry in contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (e.g. v250809-5ed76a1.apiconversions.kube-bind.io, v250902-878858c.apiserviceexports.kube-bind.io, etc.) corresponds to a metadata.name in the contrib/kcp/deploy/resources/apiresourceschema-*.yaml files.

Comment thread docs/content/setup/kcp-setup.md
Comment thread docs/content/setup/kcp-setup.md
Signed-off-by: Mangirdas Judeikis <mangirdas@judeikis.lt>
On-behalf-of: @SAP mangirdas.judeikis@sap.com
@mjudeikis mjudeikis force-pushed the mjudeikis/prep.for.v0.5 branch from 22a3a64 to 9e57e79 Compare September 25, 2025 14:59

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/content/setup/quickstart.md (1)

82-91: Replace hardcoded cookie keys with placeholders.

Prevents copy-pasting insecure values.

-     --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \
-     --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \
+     --cookie-signing-key=REPLACE_WITH_BASE64_32_OR_64_BYTE_KEY \
+     --cookie-encryption-key=REPLACE_WITH_BASE64_16_24_OR_32_BYTE_KEY \
🧹 Nitpick comments (3)
docs/content/setup/index.md (2)

16-16: Style: use “kcp” (lowercase) consistently.

Project style prefers lowercase “kcp”.

Apply this diff:

-- **[KCP Integration](kcp-setup.md)**: Advanced multi-tenant setup with kcp workspaces and APIExports
+- **[kcp integration](kcp-setup.md)**: Advanced multi-tenant setup with kcp workspaces and APIExports

22-24: Style: lowercase “kcp”.

-- **Multiple Providers**: Choose between standard Kubernetes or KCP backends
+- **Multiple Providers**: Choose between standard Kubernetes or kcp backends
README.md (1)

79-81: Style: lowercase “kcp” and consider linking to the setup guide.

Keep naming consistent and help readers discover the provider setup.

-- **Provider Support**: Built-in support for multiple backend providers including KCP through `github.com/kcp-dev/multicluster-provider`
+- **Provider Support**: Built-in support for multiple backend providers including kcp through `github.com/kcp-dev/multicluster-provider` (see setup/kcp-setup.md)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 22a3a64 and 9e57e79.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (17)
  • Makefile (1 hunks)
  • README.md (1 hunks)
  • backend/config.go (1 hunks)
  • backend/provider/provider.go (1 hunks)
  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (1 hunks)
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml (2 hunks)
  • deploy/crd/kube-bind.io_clusterbindings.yaml (1 hunks)
  • docs/content/developers/backend/index.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md (1 hunks)
  • docs/content/developers/konnector/controllers/cluster/boundschema.md (1 hunks)
  • docs/content/setup/index.md (1 hunks)
  • docs/content/setup/kcp-setup.md (1 hunks)
  • docs/content/setup/quickstart.md (1 hunks)
  • docs/generators/crd-ref/run-crd-ref-gen.sh (1 hunks)
  • go.mod (2 hunks)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go (0 hunks)
  • sdk/apis/kubebind/v1alpha1/clusterbinding_types.go (1 hunks)
💤 Files with no reviewable changes (1)
  • sdk/apis/kubebind/v1alpha1/apiserviceexport_types.go
🚧 Files skipped from review as they are similar to previous changes (10)
  • docs/generators/crd-ref/run-crd-ref-gen.sh
  • Makefile
  • backend/provider/provider.go
  • docs/content/developers/konnector/controllers/cluster/apiserviceexport.md
  • contrib/kcp/deploy/resources/apiresourceschema-clusterbindings.kube-bind.io.yaml
  • backend/config.go
  • docs/content/developers/konnector/controllers/cluster/boundschema.md
  • deploy/crd/kube-bind.io_clusterbindings.yaml
  • docs/content/developers/backend/index.md
  • go.mod
🧰 Additional context used
🧠 Learnings (5)
📚 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/apiexport-kube-bind.io.yaml
📚 Learning: 2025-09-12T09:05:29.762Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: sdk/client/listers/kubebind/v1alpha2/boundschema.go:46-48
Timestamp: 2025-09-12T09:05:29.762Z
Learning: In the kube-bind project, lister-gen is generating BoundSchema listers with singular resource names ("boundschema") instead of plural ("boundschemas"), which breaks client-go conventions and can cause cache lookup issues. This is identified as a generator issue that needs upstream investigation rather than manual code fixes.

Applied to files:

  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
  • docs/content/setup/kcp-setup.md
📚 Learning: 2025-09-12T08:55:41.860Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: sdk/apis/kubebind/v1alpha2/helpers/boundschema.go:115-123
Timestamp: 2025-09-12T08:55:41.860Z
Learning: In BoundSchemasSpecHash function in sdk/apis/kubebind/v1alpha2/helpers/boundschema.go, silent error handling during JSON encoding (continuing on encoding errors) is acceptable to mjudeikis for the current implementation, even though it could potentially lead to incorrect hash values.

Applied to files:

  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
📚 Learning: 2025-09-22T13:20:49.933Z
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.933Z
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:

  • contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml
📚 Learning: 2025-09-12T08:43:56.250Z
Learnt from: mjudeikis
PR: kube-bind/kube-bind#295
File: kcp/README.md:47-49
Timestamp: 2025-09-12T08:43:56.250Z
Learning: Demo/example values for secrets in documentation and README files are acceptable when they're clearly for demonstration purposes, unlike real secrets which should never be committed to documentation.

Applied to files:

  • docs/content/setup/kcp-setup.md
🪛 Gitleaks (8.28.0)
docs/content/setup/kcp-setup.md

[high] 89-89: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 90-90: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

⏰ 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-e2e
  • GitHub Check: go-test
🔇 Additional comments (4)
docs/content/setup/kcp-setup.md (2)

224-229: Fix typos and use fully-qualified resource with proper quoting.

Current commands won’t run (“kubectl-s”) and use fragile quoting/resource names.

-# Check available resources
-kubectl-s "$(kubectl get apiexportendpointslice kube-bind.io -o jsonpath="{.status.endpoints[0].url}")/clusters/*" api-resources
+# Check available resources
+kubectl -s "$(kubectl get apiexportendpointslices.apis.kcp.io kube-bind.io -o jsonpath='{.status.endpoints[0].url}')/clusters/*" api-resources
@@
-# List CRDs
-kubectl-s "$(kubectl get apiexportendpointslice kube-bind.io -o jsonpath="{.status.endpoints[0].url}")/clusters/*" get crd
+# List CRDs
+kubectl -s "$(kubectl get apiexportendpointslices.apis.kcp.io kube-bind.io -o jsonpath='{.status.endpoints[0].url}')/clusters/*" get crd

80-92: Fix server URL lookup, JSONPath quoting, and replace hardcoded cookie keys.

Commands as written may break due to quoting; publishing concrete keys is unsafe in docs.

-  --server-url=$(kubectl get apiexportendpointslice kube-bind.io -o jsonpath="{.status.endpoints[0].url}") \
+  --server-url="$(kubectl get apiexportendpointslices.apis.kcp.io kube-bind.io -o jsonpath='{.status.endpoints[0].url}')" \
@@
-  --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \
-  --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \
+  --cookie-signing-key=REPLACE_WITH_BASE64_32_OR_64_BYTE_KEY \
+  --cookie-encryption-key=REPLACE_WITH_BASE64_16_24_OR_32_BYTE_KEY \
contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml (1)

76-79: All schema IDs in APIExport match defined APIResourceSchema names
No mismatches detected.

docs/content/setup/quickstart.md (1)

14-22: Resolve provider scope inconsistency (says “standard provider” but uses kcp workflow).

Either scope this page to kcp or remove kcp-specific steps. Minimal fix: state it’s the kcp path.

-This section allows you to run local kube-bind backend and konnector with the standard Kubernetes provider.
+This section shows how to run the kube-bind backend and konnector with the kcp provider for local development.
@@
-This quickstart uses the default provider. For kcp integration, see the [KCP Setup Guide](kcp-setup.md).
+This quickstart uses the kcp provider. For the standard provider, see Helm/local setup in the Setup index; for more details on kcp, see the [kcp Setup Guide](kcp-setup.md).

Comment thread sdk/apis/kubebind/v1alpha1/clusterbinding_types.go

@xrstf xrstf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve
/worksforme

@moadqassem moadqassem left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/LGTM

@mjudeikis mjudeikis merged commit c2c7f47 into kbind-dev:main Sep 26, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants