docs(clp): Rewrite clp methodology docs for better clarity; Bump clp asset version to 0.5.1.
#10
Conversation
WalkthroughUpdated documentation in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-09-24T18:22:45.461ZApplied to files:
🪛 Checkov (3.2.334)assets/clp/Dockerfile[low] 1-18: Ensure that HEALTHCHECK instructions have been added to container images (CKV_DOCKER_2) [low] 1-18: Ensure that a user for the container has been created (CKV_DOCKER_3) [low] 1-18: Ensure that HEALTHCHECK instructions have been added to container images (CKV_DOCKER_2) [low] 1-18: Ensure that a user for the container has been created (CKV_DOCKER_3) 🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (3)
assets/clp/Dockerfile (3)
6-14: Slim the image and harden apt usage.Use
--no-install-recommendsand cleanaptlists to reduce size.RUN apt-get update \ - && DEBIAN_FRONTEND=noninteractive apt-get install -y \ + && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ htop \ jq \ python3-venv \ rsync \ sqlite3 \ tmux \ - vim + vim \ + && rm -rf /var/lib/apt/lists/*
16-16: Use the named stage and place the binary on PATH.Copying by numeric index is brittle; install to a standard bin dir.
-COPY --from=0 /clp/clp-s /clp/clp-s +COPY --from=core /clp/clp-s /usr/local/bin/clp-sPlease verify the path
/clp/clp-sin thecoreimage is stable on:main; otherwise this may break unexpectedly.
1-16: Run as non‑root and add a basic healthcheck.Security posture: containers should avoid running as root; also add a simple healthcheck (can be adjusted if this isn’t a long‑running container).
Suggested additions (append near the end of the Dockerfile):
# Create non-root user RUN useradd -m -u 10001 app && chown app:app /usr/local/bin/clp-s USER 10001 # Optional: basic healthcheck HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD ["clp-s", "--version"]
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
assets/clp/Dockerfile(1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
assets/clp/Dockerfile
[low] 1-16: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-16: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🔇 Additional comments (1)
assets/clp/Dockerfile (1)
1-2: Confirm scope: Dockerfile change in a docs‑rename PR.This alters base image pinning (now floating
:main). Confirm this change is intentional for this PR; otherwise, consider a separate PR to avoid coupling doc renames with runtime base image changes.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
assets/clp/methodology.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/log-archival-bench#10
File: assets/clp/methodology.md:27-27
Timestamp: 2025-09-24T18:22:45.449Z
Learning: In CLP documentation, `dev-guide` URLs remain valid and unchanged, while only `user-guide` paths were renamed to `user-docs`. The dev-guide and user-guide/user-docs are separate documentation sections.
📚 Learning: 2025-09-24T18:22:45.449Z
Learnt from: Bill-hbrhbr
PR: y-scope/log-archival-bench#10
File: assets/clp/methodology.md:27-27
Timestamp: 2025-09-24T18:22:45.449Z
Learning: In CLP documentation, `dev-guide` URLs remain valid and unchanged, while only `user-guide` paths were renamed to `user-docs`. The dev-guide and user-guide/user-docs are separate documentation sections.
Applied to files:
assets/clp/methodology.md
🔇 Additional comments (4)
assets/clp/methodology.md (4)
14-15: Resolved: Anchor text now matches “user docs”.Good catch aligning visible text with the updated path.
16-17: Nice clarity fix.The rewritten sentence is clear and grammatically sound.
22-25: Link hygiene fixes look good.HTTPS for GHCR and repo‑relative Config File path are correct. User‑docs link targets v0.5.1 as intended.
25-25: Verify upstream docs rename Upstream PR #1155 must be merged before this change; no remaininguser-guidereferences detected.
Bill-hbrhbr
changed the title
user-guide to user-docs and dev-guide to dev-docs, as per CLP docs PRclp methodology docs for better clarity; Bump clp version to 0.5.1.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
assets/clp/Dockerfile(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/log-archival-bench#10
File: assets/clp/methodology.md:27-27
Timestamp: 2025-09-24T18:22:45.449Z
Learning: In CLP documentation, `dev-guide` URLs remain valid and unchanged, while only `user-guide` paths were renamed to `user-docs`. The dev-guide and user-guide/user-docs are separate documentation sections.
🪛 Checkov (3.2.334)
assets/clp/Dockerfile
[low] 1-16: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-16: Ensure that a user for the container has been created
(CKV_DOCKER_3)
[low] 1-16: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-16: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🔇 Additional comments (2)
assets/clp/Dockerfile (2)
1-2: Pin the base image digest; avoid floating :main.Good call naming the stage. Please replace
:mainwith an immutable digest for reproducibility and supply‑chain hygiene.-# TODO: use the latest version until the clp core/deps images are properly tagged with versions. -FROM ghcr.io/y-scope/clp/clp-core-x86-ubuntu-jammy:main AS clp-core +# TODO(quinntaylormitchell): Replace with pinned digest as soon as upstream publishes tags. +FROM ghcr.io/y-scope/clp/clp-core-x86-ubuntu-jammy@sha256:<digest> AS clp-corePlease confirm the planned date (or upstream release) when you’ll switch from
:mainto a pinned digest/tag so we can track it. If you want, I can open a follow‑up issue.
4-4: Also pin the dependencies image; avoid:main.Stage name is fine; same reproducibility concern applies here.
-FROM ghcr.io/y-scope/clp/clp-core-dependencies-x86-ubuntu-jammy:main AS clp-deps-core +FROM ghcr.io/y-scope/clp/clp-core-dependencies-x86-ubuntu-jammy@sha256:<digest> AS clp-deps-coreConfirm when this will be updated to a static digest (or versioned tag).
Bill-hbrhbr
changed the title
clp methodology docs for better clarity; Bump clp version to 0.5.1.clp methodology docs for better clarity; Bump clp asset version to 0.5.1.
Co-authored-by: Quinn Taylor Mitchell <q.mitchell@mail.utoronto.ca>
2 participants
Description
PR #1155 in
y-scope/clpchangesuser-guidetouser-docs. This PR modifies the single instance ofuser-guidein this repo touser-docsto comply.Do not merge this PR until PR #1155 is merged.
Checklist
breaking change.
Validation performed
N/A
Summary by CodeRabbit
Documentation
Chores