Skip to content

Clarify COSI access control requirements and cleanup#444

Merged
fanzy618 merged 1 commit intomasterfrom
replace-access-control-ceph-cosi
Dec 23, 2025
Merged

Clarify COSI access control requirements and cleanup#444
fanzy618 merged 1 commit intomasterfrom
replace-access-control-ceph-cosi

Conversation

@fanzy618
Copy link
Copy Markdown
Contributor

@fanzy618 fanzy618 commented Dec 22, 2025

Summary

  • note required protocols: S3 in BucketClaim spec
  • add troubleshooting tip for not-ready buckets
  • clarify bucket deletion behavior when not empty

Testing

  • not run (docs change)

Summary by CodeRabbit

  • New Features

    • BucketClaim now supports an optional protocols field to specify data API protocols such as S3.
  • Documentation

    • Enhanced troubleshooting guidance recommending log checks when buckets remain unavailable.
    • Improved cleanup instructions clarifying that objects must be deleted before removing buckets when backends reject empty operations.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 22, 2025

Walkthrough

Documentation updates to COSI access control guide adding a new optional spec.protocols field to BucketClaim, along with enhanced troubleshooting guidance for non-ready buckets and improved cleanup instructions for non-empty backends.

Changes

Cohort / File(s) Summary
COSI Documentation Updates
docs/en/configure/storage/cosi/how_to/access_control_ceph.mdx
Added optional spec.protocols field (array of strings) to BucketClaim API schema with S3 example; added troubleshooting guidance to check controller/driver logs when bucket remains not-ready; expanded cleanup instructions for scenarios where backend refuses deletion due to non-empty state

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Single documentation file with straightforward additions (API field documentation, troubleshooting tips, and cleanup guidance)
  • No code logic or complex interactions to validate
  • Changes follow standard documentation patterns

Possibly related PRs

  • Add cosi docs #71: Touches COSI documentation and BucketClaim protocol usage, complementary to the spec.protocols API field introduction in this PR.

Poem

🐰 A protocol field takes its place,
In BucketClaim's namespace,
With S3 shining bright and clear,
And troubleshooting wisdom near—
COSI storage, now complete! 📦✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Clarify COSI access control requirements and cleanup' accurately reflects the main changes in the pull request, which include documenting protocol requirements, adding troubleshooting guidance, and clarifying cleanup instructions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch replace-access-control-ceph-cosi

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
docs/en/configure/storage/cosi/how_to/access_control_ceph.mdx (1)

175-175: Consider making the namespace reference more generic or document it clearly.

The troubleshooting command references cpaas-system namespace, which may be specific to your environment. Users with different installations might have the Ceph COSI driver deployed in other namespaces (e.g., rook-ceph, objectstorage-system, or custom namespaces).

🔎 Suggested improvements

Option 1: Use a placeholder

-* **Bucket stays not-ready**: check controller/driver logs (e.g. `kubectl -n cpaas-system logs deploy/ceph-cosi-driver -c ceph-cosi-driver`).
+* **Bucket stays not-ready**: check controller/driver logs (e.g. `kubectl -n <driver-namespace> logs deploy/ceph-cosi-driver -c ceph-cosi-driver`).

Option 2: Provide discovery guidance

-* **Bucket stays not-ready**: check controller/driver logs (e.g. `kubectl -n cpaas-system logs deploy/ceph-cosi-driver -c ceph-cosi-driver`).
+* **Bucket stays not-ready**: check controller/driver logs. First locate the driver: `kubectl get deploy -A | grep ceph-cosi-driver`, then check logs: `kubectl -n <namespace> logs deploy/ceph-cosi-driver -c ceph-cosi-driver`.
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 86d6801 and fcdbc1d.

📒 Files selected for processing (1)
  • docs/en/configure/storage/cosi/how_to/access_control_ceph.mdx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Pipelines as Code CI / doc-pr-build-container-platform
🔇 Additional comments (2)
docs/en/configure/storage/cosi/how_to/access_control_ceph.mdx (2)

182-182: Helpful operational guidance.

The added clarification about deleting objects first when the backend refuses deletion is valuable for users troubleshooting cleanup issues. This improves the documentation quality.


86-88: The inline comment contradicts the AI-generated summary regarding the protocols field requirement status.

The documentation states "Required by the COSI API" but the AI summary describes this field as "optional." This inconsistency needs clarification—verify whether the protocols field is actually required or optional in the COSI BucketClaim specification, and update the comment accordingly.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying alauda-container-platform with  Cloudflare Pages  Cloudflare Pages

Latest commit: fcdbc1d
Status: ✅  Deploy successful!
Preview URL: https://2c66c2ae.alauda-container-platform.pages.dev
Branch Preview URL: https://replace-access-control-ceph.alauda-container-platform.pages.dev

View logs

@fanzy618 fanzy618 merged commit 6e89c85 into master Dec 23, 2025
3 checks passed
@fanzy618 fanzy618 deleted the replace-access-control-ceph-cosi branch December 23, 2025 02:02
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.

1 participant