Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions .claude/skills/review-pr/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
---
name: review-pr
description: Review a PR on workbench (Go CLI for S3C Docker Compose dev environments)
argument-hint: <pr-number-or-url>
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 <number> --repo <owner/repo> --json title,body,headRefOid,author,files
gh pr diff <number> --repo <owner/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, `<br>` for line breaks:
```bash
gh api -X POST -H "Accept: application/vnd.github+json" "repos/<owner/repo>/pulls/<number>/comments" -f body="Issue description.<br><br>— Claude Code" -f path="file" -F line=42 -f side="RIGHT" -f commit_id="<headRefOid>"
```

**With suggestion block** — use a heredoc (`-F body=@-`) so code renders correctly:
```bash
gh api -X POST -H "Accept: application/vnd.github+json" "repos/<owner/repo>/pulls/<number>/comments" -F body=@- -f path="file" -F line=42 -f side="RIGHT" -f commit_id="<headRefOid>" <<'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, `<br>` for line breaks. No markdown headings — they render as giant bold text. Flat bullet list only:

```bash
gh pr comment <number> --repo <owner/repo> --body "- file:line — issue<br>- file:line — issue<br><br>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:

```
**<file_path>:<line_number>** — <what's wrong and how to fix it>
```

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)
14 changes: 14 additions & 0 deletions .github/workflows/review.yml
Original file line number Diff line number Diff line change
@@ -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 }}
188 changes: 188 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
@@ -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-name>] [--env-dir <dir>]

# Start an environment (regenerates configs and runs docker compose)
workbench up [-d] [--name <env-name>] [--env-dir <dir>]

# Stop an environment
workbench down [--name <env-name>] [--env-dir <dir>]

# Destroy an environment (removes containers and volumes)
workbench destroy [--name <env-name>] [--env-dir <dir>]

# View logs
workbench logs [--name <env-name>] [--env-dir <dir>]

# Regenerate configuration files without starting
workbench configure [--name <env-name>] [--env-dir <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/<name>/values.yaml`
2. `configure` command loads config (with defaults merged in via `mergo`)
3. Templates are rendered with config data → `env/<name>/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/<name>/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 <env-name>

# 2. COMPLETE clean - cache, binary, and environment
go clean -cache -modcache -testcache
rm -f workbench
sudo rm -rf env/<env-name>

# 3. Build fresh
make build

# 4. Create completely new environment
./workbench create-env -n <env-name>

# 5. Configure with new templates
./workbench configure -n <env-name>

# 6. Verify your changes are present
grep "your-change" env/<env-name>/docker-compose.yaml

# 7. Start
./workbench up -n <env-name>
```

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.
Loading