[LFXV2-1284] docs: improve README, fix CLAUDE.md, and add branch build workflow#16
Merged
[LFXV2-1284] docs: improve README, fix CLAUDE.md, and add branch build workflow#16
Conversation
- Add values.local.yaml to .gitignore for local overrides - Add helm-install-local and helm-templates-local make targets - Add values.local.example.yaml as a template for local development - Document local values workflow in README - Set default replicaCount to 1 for local development Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
- Add ko-build-branch.yaml to publish images on PRs, tagged with commit SHA and sanitized branch name - Pin all action references to full commit SHAs matching voting-service - Upgrade actions/checkout to v6.0.2, actions/setup-go to v6.2.0, ko-build/setup-ko to v0.9, cosign-installer to v4.0.0, docker/login-action to v3.7.0 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
README: - Fix Docker image path to use GHCR - Fix typo "A access" -> "An access" - Update architecture diagrams to include fga-sync - Add API reference section with correct request/response format - Add OpenAPI spec endpoints - Add Testing section with integration test instructions - Add image tag table with GHCR package link - Replace raw helm command with make helm-install - Remove redundant env vars from docker run command - Remove emoji from License header - Remove unverified branch-name image tag row CLAUDE.md: - Fix title from "LFX v2" to "LFX Access Check Service" - Fix architecture diagram to include fga-sync - Fix API request format (version is query param, not body field) - Fix stale Helm make targets (remove helm-upgrade, add helm-install-local/helm-templates-local) - Fix integration test prerequisites (uses mocks, no external services needed) - Fix Docker image path to use GHCR - Fix "GOA" -> "Goa" capitalization throughout - Fix broken project structure tree (add missing charts entry) - Add fga-sync to prerequisites Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
WalkthroughIntroduces a new GitHub Actions workflow for PR container builds, pins several action versions to specific commit SHAs across workflows, adds local Helm values support and new Makefile targets, updates documentation to include fga-sync and other wording changes, reduces default replicas from 3 to 1, and adds a .gitignore entry for the local Helm values file. Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant GH as GitHub Actions Runner
participant Ko as ko-build
participant Reg as Container Registry
PR->>GH: push / open PR (to main)
GH->>GH: checkout repo\nsetup-go\nsetup-ko
GH->>GH: sanitize PR head ref → tag\nprepare env (SHA, platforms)
GH->>Ko: ko build (multi-arch, tag, SBOM)
Ko->>Reg: push container images + SBOM
Reg-->>GH: push result / image refs
GH-->>PR: workflow status (success/failure)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Pull request overview
Documentation and CI improvements for the LFX Access Check Service: updates README and CLAUDE.md with accurate architecture info (fga-sync, correct API format, Goa capitalization), adds a branch build CI workflow, and pins GitHub Actions to commit SHAs.
Changes:
- Rewrite README and CLAUDE.md with corrected architecture diagrams, API reference, deployment instructions, and terminology
- Add
ko-build-branch.yamlworkflow to publish container images on PRs, and pin all GitHub Actions to full commit SHAs - Add local Helm development workflow (
values.local.example.yaml,helm-install-local,helm-templates-localtargets)
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Major rewrite with fga-sync in diagrams, API reference, image tag table, testing/deployment sections |
| CLAUDE.md | Fix inaccuracies: GOA→Goa, correct API format, remove stale targets, add fga-sync references |
| .github/workflows/ko-build-branch.yaml | New CI workflow to publish PR images tagged by SHA and branch name |
| .github/workflows/ko-build-main.yaml | Pin actions to commit SHAs |
| .github/workflows/ko-build-tag.yaml | Pin actions to commit SHAs |
| Makefile | Add helm-install-local and helm-templates-local targets |
| charts/lfx-v2-access-check/values.yaml | Change default replicaCount from 3 to 1 |
| charts/lfx-v2-access-check/values.local.example.yaml | New example local values file |
| .gitignore | Ignore values.local.yaml |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
136-175:⚠️ Potential issue | 🟠 MajorRe-add the required
helm-upgradetarget to keep Makefile contract intact.
helm-upgradeis currently missing, which breaks the documented/required target set.🛠️ Proposed fix
.PHONY: helm-install helm-install: ## Install Helm chart `@echo` "==> Installing Helm chart..." helm upgrade --install $(HELM_RELEASE_NAME) $(HELM_CHART_PATH) --namespace $(HELM_NAMESPACE) --create-namespace --set image.tag=$(DOCKER_TAG) `@echo` "==> Helm chart installed: $(HELM_RELEASE_NAME)" + +.PHONY: helm-upgrade +helm-upgrade: ## Upgrade Helm chart + `@echo` "==> Upgrading Helm chart..." + helm upgrade $(HELM_RELEASE_NAME) $(HELM_CHART_PATH) --namespace $(HELM_NAMESPACE) --create-namespace --set image.tag=$(DOCKER_TAG) + `@echo` "==> Helm chart upgraded: $(HELM_RELEASE_NAME)"As per coding guidelines, "Makefile must include targets for development setup (setup-dev, deps, fmt, vet, lint, clean), building (build, build-linux, run), testing (test, test-coverage), Docker operations (docker-build, docker-push, docker-run), and Helm/Kubernetes operations (helm-install, helm-upgrade, helm-templates, helm-uninstall, helm-lint)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 136 - 175, Add a new .PHONY target named helm-upgrade that restores the expected Makefile contract: implement a target called helm-upgrade which runs the appropriate helm upgrade command against $(HELM_RELEASE_NAME) and $(HELM_CHART_PATH) using $(HELM_NAMESPACE) and the image tag $(DOCKER_TAG) (and/or --values $(HELM_VALUES_FILE) for local overrides as needed), include any desired flags such as --install/--force/--reuse-values to match project behavior, and add a brief echo before/after the command; ensure the target name and referenced variables (HELM_RELEASE_NAME, HELM_CHART_PATH, HELM_NAMESPACE, DOCKER_TAG, optionally HELM_VALUES_FILE) are used so tooling expecting helm-upgrade finds it.
🧹 Nitpick comments (1)
charts/lfx-v2-access-check/values.local.example.yaml (1)
4-8: Add commented local placeholders for NATS/JWKS/namespace in this example.This example currently documents only image overrides. Adding optional placeholders for
nats.url,heimdall.jwks_url, and namespace helps keep local overrides aligned with chart configuration expectations.♻️ Proposed update
# Image configuration image: repository: ghcr.io/linuxfoundation/lfx-v2-access-check/lfx-access-check tag: "latest" # Overrides appVersion from Chart.yaml pullPolicy: Never + +# Optional local overrides for external integrations +# nats: +# url: nats://localhost:4222 +# +# heimdall: +# jwks_url: http://localhost:4457/.well-known/jwks +# +# traefik: +# gateway: +# namespace: lfxAs per coding guidelines, "
charts/**/*.{yaml,yml}: Use Helm charts for Kubernetes deployment with configuration parameters for image tag, JWKS URL, NATS URL, and namespace".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/lfx-v2-access-check/values.local.example.yaml` around lines 4 - 8, Add commented local placeholder entries to the example values file so users can override NATS, JWKS and namespace like other image settings; specifically add commented keys for nats.url, heimdall.jwks_url, and namespace alongside the existing image.repository, image.tag and image.pullPolicy (e.g., include lines for nats.url: "<local-nats-url>", heimdall.jwks_url: "<local-jwks-url>", and namespace: "<local-namespace>" as commented examples) so local overrides align with the chart configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/lfx-v2-access-check/values.yaml`:
- Line 4: The shared chart default sets replicaCount: 1 which forces
single-replica installs; change the default replicaCount back to the HA-safe
value used previously (e.g., 2 or the project's standard HA replica number) in
this chart's values.yaml and move any developer/local single-replica override
into values.local.yaml so local testing can scale to 1 without affecting
shared/production defaults; update the replicaCount entry in values.yaml (and
ensure values.local.yaml contains replicaCount: 1 for local overrides).
In `@CLAUDE.md`:
- Around line 13-15: Add language identifiers to the two fenced code blocks in
CLAUDE.md: change the architecture flow block from ``` to ```text and change the
HTTP example block from ``` to ```http so markdown linters (MD040) recognize
them; also apply the same change for the additional fenced block occurrence
around lines 126-134. Locate the two blocks by their contents ("Client → Traefik
→ Heimdall → Access Check Service → NATS → fga-sync" and the "POST
/access-check?v=1" HTTP example) and update the opening backticks accordingly.
In `@README.md`:
- Around line 147-151: The fenced code block containing the HTTP request example
(the block starting with "POST /access-check?v=1" and headers "Authorization:
Bearer <JWT_TOKEN>" / "Content-Type: application/json") is missing a language
tag and triggers markdownlint MD040; update the opening triple-backtick fence to
include a language identifier such as http (or bash) so it reads ```http to
resolve the lint error.
---
Outside diff comments:
In `@Makefile`:
- Around line 136-175: Add a new .PHONY target named helm-upgrade that restores
the expected Makefile contract: implement a target called helm-upgrade which
runs the appropriate helm upgrade command against $(HELM_RELEASE_NAME) and
$(HELM_CHART_PATH) using $(HELM_NAMESPACE) and the image tag $(DOCKER_TAG)
(and/or --values $(HELM_VALUES_FILE) for local overrides as needed), include any
desired flags such as --install/--force/--reuse-values to match project
behavior, and add a brief echo before/after the command; ensure the target name
and referenced variables (HELM_RELEASE_NAME, HELM_CHART_PATH, HELM_NAMESPACE,
DOCKER_TAG, optionally HELM_VALUES_FILE) are used so tooling expecting
helm-upgrade finds it.
---
Nitpick comments:
In `@charts/lfx-v2-access-check/values.local.example.yaml`:
- Around line 4-8: Add commented local placeholder entries to the example values
file so users can override NATS, JWKS and namespace like other image settings;
specifically add commented keys for nats.url, heimdall.jwks_url, and namespace
alongside the existing image.repository, image.tag and image.pullPolicy (e.g.,
include lines for nats.url: "<local-nats-url>", heimdall.jwks_url:
"<local-jwks-url>", and namespace: "<local-namespace>" as commented examples) so
local overrides align with the chart configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 004b3ced-9527-443d-9fc1-884471debe5b
📒 Files selected for processing (9)
.github/workflows/ko-build-branch.yaml.github/workflows/ko-build-main.yaml.github/workflows/ko-build-tag.yaml.gitignoreCLAUDE.mdMakefileREADME.mdcharts/lfx-v2-access-check/values.local.example.yamlcharts/lfx-v2-access-check/values.yaml
…rkflow The .ko.yaml build config requires these env vars via ldflags templates. VERSION is set to the sanitized branch tag name. Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
… CLAUDE.md - Add branch-name tag row to image tag table in README - Add language identifiers to unlabeled fenced code blocks in CLAUDE.md Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.md (1)
147-151:⚠️ Potential issue | 🟡 MinorAdd language identifier to the fenced code block.
The code fence at line 147 is missing a language identifier, triggering markdownlint MD040. This was previously addressed but appears to have regressed. Add
httpafter the opening backticks.📝 Proposed fix
-``` +```http POST /access-check?v=1 Authorization: Bearer <JWT_TOKEN> Content-Type: application/json</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@README.mdaround lines 147 - 151, The fenced code block that shows the HTTP
request (the block containing "POST /access-check?v=1" and headers) is missing a
language identifier; update the opening triple backticks to include "http"
(i.e., changetohttp) so the block is recognized as HTTP and
markdownlint MD040 is satisfied.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In@README.md:
- Around line 147-151: The fenced code block that shows the HTTP request (the
block containing "POST /access-check?v=1" and headers) is missing a language
identifier; update the opening triple backticks to include "http" (i.e., change
tohttp) so the block is recognized as HTTP and markdownlint MD040 is
satisfied.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `b1c6275c-4733-40e9-8b6b-8e183e70e7bf` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between d8ed250f0b1617a3e336a9d0ba4511a06ca59e74 and 519d84ab3fe5602ab1d881ea24e9bdc7d800db8e. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `CLAUDE.md` * `README.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
jordane
approved these changes
Mar 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ko-build-branch.yamlCI workflow to publish images on PRs (tagged by commit SHA and branch name), matching the pattern used in voting-serviceTicket
LFXV2-1284
🤖 Generated with Claude Code