diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md new file mode 100644 index 0000000..29f064b --- /dev/null +++ b/.claude/skills/review-pr/SKILL.md @@ -0,0 +1,114 @@ +--- +name: review-pr +description: Review a PR on workbench (Go CLI for S3C Docker Compose dev environments) +argument-hint: +disable-model-invocation: true +allowed-tools: Read, Bash(gh repo view *), Bash(gh pr view *), Bash(gh pr diff *), Bash(gh pr comment *), Bash(gh api *), Bash(git diff *), Bash(git log *), Bash(git show *) +--- + +# Review GitHub PR + +You are an expert code reviewer. Review this PR: $ARGUMENTS + +## Determine PR target + +Parse `$ARGUMENTS` to extract the repo and PR number: + +- If arguments contain `REPO:` and `PR_NUMBER:` (CI mode), use those values directly. +- If the argument is a GitHub URL (starts with `https://github.com/`), extract `owner/repo` and the PR number from it. +- If the argument is just a number, use the current repo from `gh repo view --json nameWithOwner -q .nameWithOwner`. + +## Output mode + +- **CI mode** (arguments contain `REPO:` and `PR_NUMBER:`): post inline comments and summary to GitHub. +- **Local mode** (all other cases): output the review as text directly. Do NOT post anything to GitHub. + +## Steps + +1. **Fetch PR details:** + +```bash +gh pr view --repo --json title,body,headRefOid,author,files +gh pr diff --repo +``` + +2. **Read changed files** to understand the full context around each change (not just the diff hunks). + +3. **Analyze the changes** against these criteria: + +| Area | What to check | +|------|---------------| +| Error wrapping | Use `fmt.Errorf("...: %w", err)`, not `%v` — ensure errors are properly wrapped for `errors.Is`/`errors.As` | +| Context propagation | Pass `context.Context` through call chains, respect cancellation | +| Goroutine leaks | Ensure goroutines have exit conditions, use `errgroup` where appropriate | +| Template correctness | Go text templates in `templates/` must produce valid JSON, YAML, shell scripts, or config files | +| Config defaults | Changes to `EnvironmentConfig` or defaults must not break existing environments | +| Docker Compose validity | Changes to `docker-compose.yaml` template must produce valid Compose v2 syntax | +| Embed consistency | New or renamed template files must be covered by the `embed.go` directives | +| Port conflicts | New services or port changes must not conflict with existing host-network ports | +| Feature flag gating | Optional components must be gated behind the correct Docker Compose profile | +| Security | No credentials or secrets in templates, no command injection in shell script templates | +| Breaking changes | Changes to CLI flags, values.yaml schema, or environment structure that break existing usage | + +4. **Deliver your review:** + +### If CI mode: post to GitHub + +#### Part A: Inline file comments + +For each issue, post a comment on the exact file and line. Keep comments short (1-3 sentences), end with `— Claude Code`. Use line numbers from the **new version** of the file. + +**Without suggestion block** — single-line command, `
` for line breaks: +```bash +gh api -X POST -H "Accept: application/vnd.github+json" "repos//pulls//comments" -f body="Issue description.

— Claude Code" -f path="file" -F line=42 -f side="RIGHT" -f commit_id="" +``` + +**With suggestion block** — use a heredoc (`-F body=@-`) so code renders correctly: +```bash +gh api -X POST -H "Accept: application/vnd.github+json" "repos//pulls//comments" -F body=@- -f path="file" -F line=42 -f side="RIGHT" -f commit_id="" <<'COMMENT_BODY' +Issue description. + +```suggestion +first line of suggested code +second line of suggested code +``` + +— Claude Code +COMMENT_BODY +``` + +Only suggest when you can show the exact replacement. For architectural or design issues, just describe the problem. + +#### Part B: Summary comment + +Single-line command, `
` for line breaks. No markdown headings — they render as giant bold text. Flat bullet list only: + +```bash +gh pr comment --repo --body "- file:line — issue
- file:line — issue

Review by Claude Code" +``` + +If no issues: just say "LGTM". End with: `Review by Claude Code` + +### If local mode: output the review as text + +Do NOT post anything to GitHub. Instead, output the review directly as text. + +For each issue found, output: + +``` +**:** — +``` + +When the fix is a concrete line change, include a fenced code block showing the suggested replacement. + +At the end, output a summary section listing all issues. If no issues: just say "LGTM". + +End with: `Review by Claude Code` + +## What NOT to do + +- Do not comment on markdown formatting preferences +- Do not suggest refactors unrelated to the PR's purpose +- Do not praise code — only flag problems or stay silent +- If no issues are found, post only a summary saying "LGTM" +- Do not flag style issues already covered by the project's linter (eslint, biome, pylint, golangci-lint) diff --git a/.github/workflows/review.yml b/.github/workflows/review.yml new file mode 100644 index 0000000..60dc140 --- /dev/null +++ b/.github/workflows/review.yml @@ -0,0 +1,14 @@ +name: Code Review + +on: + pull_request: + types: [opened, synchronize, labeled, unlabeled] + +jobs: + review: + uses: scality/workflows/.github/workflows/claude-code-review.yml@v2 + secrets: + GCP_WORKLOAD_IDENTITY_PROVIDER: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }} + GCP_SERVICE_ACCOUNT: ${{ secrets.GCP_SERVICE_ACCOUNT }} + ANTHROPIC_VERTEX_PROJECT_ID: ${{ secrets.ANTHROPIC_VERTEX_PROJECT_ID }} + CLOUD_ML_REGION: ${{ secrets.CLOUD_ML_REGION }} diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..d5c0302 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,188 @@ +# workbench + +This is a **Go CLI tool** for creating Docker Compose-based development environments for Scality S3C (S3 Connector). It contains: + +- CLI commands for environment lifecycle (`cmd/`) +- Go text templates rendered into Docker Compose configs, shell scripts, and service configurations (`templates/`) +- Template embedding via `embed.go` +- Kong-based CLI parsing, zerolog structured logging, mergo config merging + +## Overview + +Workbench uses a feature-based approach allowing users to run only the components needed for their use case. + +**Key Concept**: Workbench manages isolated "environments" in the `env/` directory. Each environment contains its own configuration, docker-compose setup, and runtime state. + +## Building and Testing + +### Build +```bash +make build +# Creates executable named 'workbench' +``` + +### Lint +```bash +make lint +# Requires golangci-lint to be installed +``` + +### Clean +```bash +make clean +``` + +## Core Commands + +Workbench requires Docker Compose v2 and uses Go 1.24.3+. + +### Environment Management +```bash +# Create a new environment (defaults to 'default' if no name provided) +workbench create-env [--name ] [--env-dir ] + +# Start an environment (regenerates configs and runs docker compose) +workbench up [-d] [--name ] [--env-dir ] + +# Stop an environment +workbench down [--name ] [--env-dir ] + +# Destroy an environment (removes containers and volumes) +workbench destroy [--name ] [--env-dir ] + +# View logs +workbench logs [--name ] [--env-dir ] + +# Regenerate configuration files without starting +workbench configure [--name ] [--env-dir ] +``` + +### Environment Variables +- `WORKBENCH_ENV_DIR`: Override default environment directory (default: `./env`) +- `WORKBENCH_ENV_NAME`: Override default environment name (default: `default`) + +## Architecture + +### Configuration System + +The configuration system uses a three-level hierarchy: + +1. **Runtime Config** (`cmd/config.go:RuntimeConfig`): Determines which environment to operate on + - Environment directory location + - Environment name + +2. **Environment Config** (`cmd/config.go:EnvironmentConfig`): Per-environment settings loaded from `values.yaml` + - Global settings (log level) + - Feature flags (scuba, bucket notifications, utapi, migration) + - Component-specific settings (images, ports, log levels) + +3. **Component Templates** (`templates/`): Go text templates rendered into configuration files + - Each component has templates for configs, scripts, Dockerfiles + - Templates are embedded into the binary via `embed.go` + +### Config Flow + +1. User creates/modifies `env//values.yaml` +2. `configure` command loads config (with defaults merged in via `mergo`) +3. Templates are rendered with config data → `env//config/` +4. Docker Compose profiles are selected based on enabled features +5. Docker Compose starts containers using generated configs + +### Component Architecture + +**Main components** (all use host networking): +- **cloudserver**: S3 API server (two containers: API server + data server) +- **metadata-s3**: Metadata storage for S3 (standalone mode, Raft-based) +- **vault**: Authentication and authorization service +- **backbeat**: Replication and lifecycle management +- **scuba**: Utilization metrics reporting (requires `features.scuba.enabled: true`) +- **metadata-scuba**: Separate metadata instance for Scuba +- **utapi**: Usage metrics API (requires `features.utapi.enabled: true`) +- **zookeeper**: Distributed coordination and leader election +- **kafka**: Message queue for notifications/replication +- **redis**: Caching layer +- **migration-tools**: Metadata migration utilities (requires `features.migration.enabled: true`) + +**Docker Compose Profiles**: +- `base`: Core S3 functionality (always active) +- `feature-scuba`: Scuba utilization metrics +- `feature-notifications`: Bucket notifications via Backbeat +- `feature-utapi`: Usage metrics collection +- `feature-migration`: Metadata migration support + +### Key Files + +- `cmd/main.go`: CLI entry point using Kong for command parsing +- `cmd/config.go`: Configuration structs and loading logic with defaults +- `cmd/configure.go`: Template rendering orchestration +- `cmd/util.go`: Template rendering helpers, profile selection, docker compose command building +- `cmd/up.go`, `cmd/down.go`, etc.: Individual command implementations +- `embed.go`: Embeds `templates/` directory into binary +- `templates/global/docker-compose.yaml`: Master Docker Compose file template +- `templates/global/defaults.env`: Environment variables for Docker Compose +- `templates/global/values.yaml`: Example configuration with all available options + +### Metadata Configuration + +Metadata instances support two version formats: +- `v0` +- `v1` + +Metadata instances use configurable ports via `base_ports`: +- `bucketd`: Bucket service API +- `repd`: Replication service +- `repdAdmin`: Replication admin API + +Each metadata instance can have multiple Raft sessions (controlled by `raft_sessions`). + +### Migration Support + +When `features.migration.enabled: true`: +- Deploys a second set of metadata bucketd instances with separate ports +- Configured via `s3_metadata.migration.base_ports` + +## Development Notes + +- All container logs go to `env//logs/` +- Container names are prefixed with `workbench-` +- Host networking is used for all containers (no port mapping needed) +- Configuration generation is idempotent (safe to run `configure` multiple times) +- The `up` command always regenerates configs before starting (ensures config matches values.yaml) +- Templates can be overridden via `--templates-dir` flag for testing + +### Template Update Gotcha + +**IMPORTANT**: When modifying template files in `templates/`, Go's build cache can cause stale templates to be embedded in the binary. Running `configure` after a simple rebuild may NOT pick up your changes. + +**Correct sequence to force template updates:** + +```bash +# 1. Stop everything +./workbench down -n + +# 2. COMPLETE clean - cache, binary, and environment +go clean -cache -modcache -testcache +rm -f workbench +sudo rm -rf env/ + +# 3. Build fresh +make build + +# 4. Create completely new environment +./workbench create-env -n + +# 5. Configure with new templates +./workbench configure -n + +# 6. Verify your changes are present +grep "your-change" env//docker-compose.yaml + +# 7. Start +./workbench up -n +``` + +The key is: `go clean -cache` + `rm binary` + `rm env` BEFORE rebuilding. This forces Go to rebuild everything from scratch without using any cached embed data. + +### Cloudserver Version Support + +Cloudserver v7 and v9 are both supported. The version is auto-detected from the `cloudserver.image` tag in values.yaml: semver tags starting with `7.` use v7 config, everything else (including `latest`, git SHAs) defaults to v9.