Update docs for v0.5#324
Conversation
WalkthroughUpdates 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
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
5045fcf to
eb9500b
Compare
There was a problem hiding this comment.
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_conditionOptionally 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 installedSwitching 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 reproducibilityTag-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 casesGuard 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" doneREADME.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
📒 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.mddocs/content/developers/konnector/controllers/cluster/apiserviceexport.mdcontrib/kcp/deploy/resources/apiexport-kube-bind.io.yamlcontrib/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.mdcontrib/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.
4a0f9ba to
d2dbed3
Compare
There was a problem hiding this comment.
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 panicIndex 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 placeholdersPrevent 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=clustercontrib/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 regeneratecontrib/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
⛔ Files ignored due to path filters (1)
go.sumis 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 toolingSwitching the default container engine to Docker matches the rest of the repository’s docs & tooling, while still letting Podman users override
CONTAINER_ENGINEwhen needed. LGTM.Makefile (1)
80-80: Confirm KCP binary compatibility and asset availabilityDowngrading 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 toolchainEnsure 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 othersThe 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 previouslyThis is the same issue flagged above regarding v250809-5ed76a1 references.
backend/config.go (2)
39-44: Provider type refactor is sensibleSwitching to a local MultiClusterProvider interface decouples backend from specific provider impls.
49-55: Nil provider on default path — confirm mcmanager.New behaviorWhen 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 RunnableThis local interface composes the required multicluster traits cleanly.
docs/content/setup/index.md (1)
3-31: Docs structure looks good; verify cross-links existCheck 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 placeholdersAvoid 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 kubectlUse 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 quotingEnsure 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 modelLine 73-75 succinctly capture the shift to
Resources[]and introduceBoundSchemawithout ambiguity. This should help readers understand the new export semantics at a glance. Nicely done.
79-81: No change:github.com/kcp-dev/multicluster-provideris valid
go.mod declares this module (with a replace) and backend/config.go imports it; README reference is accurate.
63242d4 to
56669b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis 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.mdcontrib/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 solidNice 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 crdgo.mod (1)
16-28: Confirm forked replacements before releaseWe now rely on personal forks for both
multicluster-providerandmulticluster-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.
6efe4cb to
ed104b4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis 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.mdcontrib/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 pinsmake run-kcptov0.28.0instead. 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.MultiClusterProvidermakes 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.
ed104b4 to
22a3a64
Compare
There was a problem hiding this comment.
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 = kcpProviderdocs/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
Bothsigs.k8s.io/multicluster-runtimeandgithub.com/kcp-dev/multicluster-providerare 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
⛔ Files ignored due to path filters (1)
go.sumis 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.yamldocs/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.yamldocs/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.yamldocs/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.yamldocs/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 eachschema:entry incontrib/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 ametadata.namein thecontrib/kcp/deploy/resources/apiresourceschema-*.yamlfiles.
Signed-off-by: Mangirdas Judeikis <mangirdas@judeikis.lt> On-behalf-of: @SAP mangirdas.judeikis@sap.com
22a3a64 to
9e57e79
Compare
There was a problem hiding this comment.
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 backendsREADME.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
⛔ Files ignored due to path filters (1)
go.sumis 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.yamldocs/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).
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
Summary by CodeRabbit
New Features
Documentation
Chores
Style