Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 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
10 changes: 5 additions & 5 deletions .claude/skills/review-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ gh pr diff <number> --repo <owner/repo>
For each specific issue, post a comment on the exact file and line:

```bash
gh api -X POST -H "Accept: application/vnd.github+json" "repos/<owner/repo>/pulls/<number>/comments" -f body=$'Your comment\n\n— Claude Code' -f path="path/to/file" -F line=<line_number> -f side="RIGHT" -f commit_id="<headRefOid>"
gh api -X POST -H "Accept: application/vnd.github+json" "repos/<owner/repo>/pulls/<number>/comments" -f body="Your comment<br><br>— Claude Code" -f path="path/to/file" -F line=<line_number> -f side="RIGHT" -f commit_id="<headRefOid>"
```

**The command must stay on a single bash line.** Use `$'...'` quoting for the `-f body=` value, with `\n` for line breaks. Never use `<br>` — it renders as literal text inside code blocks and suggestion blocks.
**The command must stay on a single bash line.** Never use newlines in bash commands — use `<br>` for line breaks in comment bodies. Never put `<br>` inside code blocks or suggestion blocks — use `\n` within `$'...'` quoting only for content inside fenced code blocks.

Each inline comment must:
- Be short and direct — say what's wrong, why it's wrong, and how to fix it in 1-3 sentences
Expand All @@ -76,7 +76,7 @@ Each inline comment must:
Only suggest when you can show the exact replacement. For architectural or design issues, just describe the problem.
Example with a suggestion block:
```bash
gh api ... -f body=$'Missing the shared-guidelines update command.\n\n```suggestion\n/plugin update shared-guidelines@scality-agent-hub\n/plugin update scality-skills@scality-agent-hub\n```\n\n— Claude Code' ...
gh api ... -f body=$'Missing the shared-guidelines update command.<br><br>```suggestion\n/plugin update shared-guidelines@scality-agent-hub\n/plugin update scality-skills@scality-agent-hub\n```<br><br>— Claude Code' ...
```
- Escape single quotes inside `$'...'` as `\'` (e.g., `don\'t`)
- End with: `— Claude Code`
Expand All @@ -86,10 +86,10 @@ Use the line number from the **new version** of the file (the line number you'd
#### Part B: Summary comment

```bash
gh pr comment <number> --repo <owner/repo> --body $'LGTM\n\nReview by Claude Code'
gh pr comment <number> --repo <owner/repo> --body "LGTM<br><br>Review by Claude Code"
```

**The command must stay on a single bash line.** Use `$'...'` quoting with `\n` for line breaks.
**The command must stay on a single bash line.** Never use newlines in bash commands — use `<br>` for line breaks in comment bodies. Never put `<br>` inside code blocks or suggestion blocks.

Do not describe or summarize the PR. For each issue, state the problem on one line, then list one or more suggestions below it:

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
},
"homepage": "https://github.com/scality/s3utils#readme",
"dependencies": {
"@aws-sdk/client-iam": "^3.962.0",
"@aws-sdk/client-s3": "^3.873.0",
"@aws-sdk/node-http-handler": "^3.374.0",
"@scality/cloudserverclient": "^1.0.4",
Expand Down Expand Up @@ -54,7 +55,6 @@
"string-width": "4.2.3"
},
"devDependencies": {
"@aws-sdk/client-iam": "^3.962.0",
"@scality/eslint-config-scality": "scality/Guidelines#8.3.0",
"@sinonjs/fake-timers": "^14.0.0",
"eslint": "^9.14.0",
Expand Down
180 changes: 179 additions & 1 deletion replicationAudit/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,19 @@ ansible -i env/$ENV_DIR/inventory runners_s3[0] -m shell \
-a 'cat {{ env_host_logs}}/scality-vault{{ container_name_suffix | default("")}}/logs/missing.json' \
| grep -v CHANGED | tee /root/replicationAudit_missing.json

# Step 6: Clean up
# Step 6: Clean up remote files
ansible -i env/$ENV_DIR/inventory runners_s3[0] -m shell \
-a 'rm -f {{ env_host_logs}}/scality-vault{{ container_name_suffix | default("")}}/logs/missing.json \
{{ env_host_logs}}/scality-vault{{ container_name_suffix | default("")}}/logs/check-replication-permissions.js \
{{ env_host_logs}}/scality-vault{{ container_name_suffix | default("")}}/logs/buckets-with-replication.json \
/root/list-buckets-with-replication.sh'

# Step 7 (optional): Fix missing permissions
# Run from your local machine (requires vaultclient and @aws-sdk/client-iam)
node replicationAudit/fix-missing-replication-permissions.js \
/root/replicationAudit_missing.json <supervisor-ip> admin1.json

# Step 8: Re-run check to verify fixes (repeat steps 3-5)
```

# Scripts Documentation
Expand Down Expand Up @@ -285,6 +292,13 @@ node check-replication-permissions.js [input-file] [leader-ip] [output-file] [--

### Output Format

> **Breaking change (since 1.17.5):** The output now includes `ownerDisplayName`
> in each result entry. This field is required by
> `fix-missing-replication-permissions.js` to identify accounts without an
> extra API call. If you ran `check-replication-permissions.js` on version
> 1.17.4 or earlier, **re-run it** to produce an output that
> `fix-missing-replication-permissions.js` can consume.

The script produces a JSON file with metadata and results. The `results` array
contains **only buckets missing the `s3:ReplicateObject` permission**.

Expand All @@ -310,6 +324,7 @@ contains **only buckets missing the `s3:ReplicateObject` permission**.
"results": [
{
"bucket": "bucket-old-1",
"ownerDisplayName": "testaccount",
"sourceRole": "arn:aws:iam::267390090509:role/crr-role-outdated",
"policies": [
{
Expand Down Expand Up @@ -461,3 +476,166 @@ Output saved to: /tmp/missing.json
**Script timeout**

- For many buckets, run directly on the S3 connector node via interactive SSH

---

## fix-missing-replication-permissions.js

Reads the output of `check-replication-permissions.js` and creates IAM policies
with `s3:ReplicateObject` for roles that are missing it.

The script applies **minimal changes**: one policy per role (covering all affected
buckets), with an explicit Statement ID (`AllowReplicateObjectAuditFix`) so the
policies are easily identifiable later.

### Prerequisites

- Output from `check-replication-permissions.js` (missing.json)
- Vault admin credentials (`admin1.json` with `accessKey` and `secretKeyValue`).
Found on the supervisor at:
```
/srv/scality/s3/s3-offline/federation/env/<ENV_DIR>/vault/admin-clientprofile/admin1.json
```
- Network access to Vault admin/IAM API (port 8600) from the machine running the script
- `vaultclient` and `@aws-sdk/client-iam` installed (both in s3utils dependencies)

### Usage

```bash
node fix-missing-replication-permissions.js <input-file> <vault-host> <admin-config> [output-file] [options]
```

| Argument | Default | Description |
|----------|---------|-------------|
| `input-file` | (required) | Path to missing.json from check script |
| `vault-host` | (required) | Vault admin host (e.g., 13.50.166.21) |
| `admin-config` | (required) | Path to admin credentials JSON |
| `output-file` | replication-fix-results.json | Output file path |
| `--iam-port <port>` | 8600 | Vault admin and IAM API port |
| `--https` | (not set) | Use HTTPS to connect to Vault |
| `--dry-run` | (not set) | Show what would be done without making changes |

### How It Works

1. **Reads** the missing permissions file and groups buckets by account and role
2. **Maps** account IDs to names using `ownerDisplayName` from the input (no API call)
3. For each account:
- **Generates** a temporary access key via vault admin API (15-minute auto-expiry)
- **Creates** one IAM policy per role with `s3:ReplicateObject` for all affected buckets
- **Attaches** the policy to the role
- **Deletes** the temporary access key (falls back to auto-expiry on failure)
4. **Writes** results to the output file

### Policy Created

For each role, the script creates a single policy named
`s3-replication-audit-fix-<roleName>`:

```json
{
"Version": "2012-10-17",
"Statement": [{
"Sid": "AllowReplicateObjectAuditFix",
"Effect": "Allow",
"Action": "s3:ReplicateObject",
"Resource": [
"arn:aws:s3:::bucket-old-1/*",
"arn:aws:s3:::bucket-old-2/*"
]
}]
}
```

### Output Format

```json
{
"metadata": {
"timestamp": "2026-02-23T20:35:00.000Z",
"durationMs": 65,
"inputFile": "missing.json",
"dryRun": false,
"counts": {
"totalRolesProcessed": 1,
"totalBucketsFixed": 3,
"policiesCreated": 1,
"policiesAttached": 1,
"keysCreated": 1,
"keysDeleted": 1,
"errors": 0
}
},
"fixes": [
{
"accountId": "267390090509",
"accountName": "testaccount",
"roleName": "crr-role-outdated",
"roleArn": "arn:aws:iam::267390090509:role/crr-role-outdated",
"policyName": "s3-replication-audit-fix-crr-role-outdated",
"policyArn": "arn:aws:iam::267390090509:policy/s3-replication-audit-fix-crr-role-outdated",
"buckets": ["bucket-old-1", "bucket-old-2", "bucket-old-3"],
"status": "success"
}
],
"errors": []
}
```

### Example Run

```
=== Fix Missing Replication Permissions ===
Input: missing.json
Output: replication-fix-results.json
Vault/IAM: 13.50.166.21:8600

Processing 1 role(s)

[1/1] Role "crr-role-outdated" — account "testaccount" (3 bucket(s))
Created policy "s3-replication-audit-fix-crr-role-outdated"
Attached policy to role "crr-role-outdated"
Deleted temp key for account "testaccount" (267390090509)

=== Summary ===
Roles processed: 1
Buckets fixed: 3
Policies created: 1
Policies attached: 1
Keys created: 1
Keys deleted: 1
Errors: 0
Duration: 0.7s
Output saved to: replication-fix-results.json

Done.
```

### Idempotency

The script is safe to run multiple times:

- If the policy already exists, it is reused (not duplicated)
- Attaching an already-attached policy is a no-op in IAM
- Temporary access keys auto-expire after 15 minutes even if deletion fails

### Troubleshooting

**"No ownerDisplayName found for account"**

- The input file is missing `ownerDisplayName`. Re-run `check-replication-permissions.js`
to generate a fresh output that includes this field.

**"Failed to generate temp key"**

- Verify admin credentials in the config file
- Ensure vault admin API is reachable on the specified host and port

**IAM operation errors**

- Check that the IAM port is correct (default 8600, may differ per deployment)
- Verify the role still exists in vault

**"Failed to delete temp key"**

- Non-critical: the key auto-expires after 15 minutes
- The error is logged but does not prevent other operations
5 changes: 3 additions & 2 deletions replicationAudit/check-replication-permissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ async function main() {

// Process each bucket
for (let i = 0; i < buckets.length; i++) {
const { bucket, sourceRole } = buckets[i];
const { bucket, sourceRole, ownerDisplayName } = buckets[i];

if (!sourceRole) {
logProgress(i + 1, buckets.length, bucket, 'SKIP (no role)');
Expand All @@ -440,12 +440,13 @@ async function main() {
} else {
const reason = result.error || 's3:ReplicateObject';
logProgress(i + 1, buckets.length, bucket, `MISSING: ${reason}`);
result.ownerDisplayName = ownerDisplayName;
results.push(result);
stats.missing++;
}
} catch (e) {
logProgress(i + 1, buckets.length, bucket, `ERROR: ${e.message}`);
results.push({ bucket, sourceRole, error: e.message, policies: [] });
results.push({ bucket, sourceRole, ownerDisplayName, error: e.message, policies: [] });
stats.errors++;
}
}
Expand Down
Loading
Loading