diff --git a/.claude/skills/skill-linter/scripts/validate-skill.sh b/.claude/skills/skill-linter/scripts/validate-skill.sh index 34f014a4..1d3fe00e 100755 --- a/.claude/skills/skill-linter/scripts/validate-skill.sh +++ b/.claude/skills/skill-linter/scripts/validate-skill.sh @@ -158,9 +158,17 @@ if [ -n "$COMPAT" ]; then pass "Compatibility valid ($COMPAT_LEN chars)" fi -# Check allowed-tools (if present) +# Check allowed-tools (optional) TOOLS=$(echo "$FRONTMATTER" | grep '^allowed-tools:' | sed 's/^allowed-tools:[[:space:]]*//' || true) -if [ -n "$TOOLS" ]; then +if [ -z "$TOOLS" ]; then + # Check for YAML multi-line array (allowed-tools:\n - item) + if echo "$FRONTMATTER" | grep -q '^allowed-tools:$'; then + NEXT_LINE=$(echo "$FRONTMATTER" | grep -A1 '^allowed-tools:$' | sed -n '2p') + if echo "$NEXT_LINE" | grep -qE '^[[:space:]]*-[[:space:]]'; then + fail "allowed-tools must be space-delimited, not YAML array" + fi + fi +else # FAIL if commas found (must be space-delimited) if echo "$TOOLS" | grep -qE ','; then fail "allowed-tools must be space-delimited, not comma-separated: $TOOLS" @@ -171,13 +179,6 @@ if [ -n "$TOOLS" ]; then pass "Allowed-tools field valid" fi fi -# FAIL if YAML multi-line array detected (allowed-tools:\n - item) -if echo "$FRONTMATTER" | grep -q '^allowed-tools:$'; then - NEXT_LINE=$(echo "$FRONTMATTER" | grep -A1 '^allowed-tools:$' | sed -n '2p') - if echo "$NEXT_LINE" | grep -qE '^[[:space:]]*-[[:space:]]'; then - fail "allowed-tools must be space-delimited, not YAML array" - fi -fi # 8. Check line count (max 500) LINE_COUNT=$(wc -l < "$SKILL_FILE" | tr -d ' ') diff --git a/.github/workflows/skill-code-review.yml b/.github/workflows/skill-code-review.yml index 88f8d228..cc2a1367 100644 --- a/.github/workflows/skill-code-review.yml +++ b/.github/workflows/skill-code-review.yml @@ -297,9 +297,28 @@ jobs: set -euo pipefail MARKER="" WORKFLOW_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" + TIMESTAMP=$(date -u '+%Y-%m-%d %H:%M UTC') + + EXISTING_COMMENT_ID=$(gh api --paginate \ + "/repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \ + --jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \ + | head -1) + + EXISTING_BODY="" + RUN_NUMBER=1 + if [ -n "$EXISTING_COMMENT_ID" ]; then + EXISTING_BODY=$(gh api \ + "/repos/${{ github.repository }}/issues/comments/$EXISTING_COMMENT_ID" \ + --jq '.body') + RUN_NUMBER=$(( $(echo "$EXISTING_BODY" | grep -c '### 🔄 Run #' || true) + 1 )) + fi { - echo "$MARKER" + if [ "$RUN_NUMBER" -eq 1 ]; then + echo "$MARKER" + fi + echo "### 🔄 Run #${RUN_NUMBER} — ${TIMESTAMP}" + echo "" echo "## Gemini Code Review" echo "" @@ -317,21 +336,38 @@ jobs: fi echo "" - echo "---" echo "> [Workflow run]($WORKFLOW_URL)" - } > /tmp/comment.md - - EXISTING_COMMENT_ID=$(gh api --paginate \ - "/repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \ - --jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \ - | head -1) + } > /tmp/new_report.md if [ -n "$EXISTING_COMMENT_ID" ]; then - gh api --method PATCH \ + { + echo "$EXISTING_BODY" + echo "" + echo "---" + echo "" + cat /tmp/new_report.md + } > /tmp/comment.md + gh api \ + --method PATCH \ "/repos/${{ github.repository }}/issues/comments/$EXISTING_COMMENT_ID" \ -f body="$(cat /tmp/comment.md)" else gh pr comment "$PR_NUMBER" \ --repo "${{ github.repository }}" \ - --body-file /tmp/comment.md + --body-file /tmp/new_report.md fi + + - name: React to PR + if: always() + uses: actions/github-script@v7 + with: + script: | + const result = '${{ job.status }}'; + const prNumber = Number('${{ steps.inputs.outputs.pr_number }}'); + await github.rest.reactions.createForIssue({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + content: result === 'success' ? '+1' : '-1' + }); + diff --git a/.github/workflows/skill-security-scan.yml b/.github/workflows/skill-security-scan.yml index 81333e3d..64d2c265 100644 --- a/.github/workflows/skill-security-scan.yml +++ b/.github/workflows/skill-security-scan.yml @@ -220,6 +220,7 @@ jobs: --recursive \ --use-behavioral \ --use-llm \ + --llm-consensus-runs 3 \ --check-overlap \ --enable-meta \ --fail-on-severity medium \ @@ -237,6 +238,7 @@ jobs: fi - name: Upload security reports + id: upload if: steps.detect.outputs.pack_count != '0' && steps.creds.outputs.available == 'true' && always() uses: actions/upload-artifact@v4 with: @@ -257,6 +259,8 @@ jobs: set -euo pipefail MARKER="" SCAN_RESULT="${{ steps.scan.outputs.scan_result }}" + ARTIFACT_URL="${{ steps.upload.outputs.artifact-url }}" + TIMESTAMP=$(date -u '+%Y-%m-%d %H:%M UTC') if [ "$SCAN_RESULT" = "passed" ]; then ICON="✅" @@ -264,8 +268,26 @@ jobs: ICON="❌" fi + EXISTING_COMMENT_ID=$(gh api --paginate \ + "/repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \ + --jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \ + | head -1) + + EXISTING_BODY="" + RUN_NUMBER=1 + if [ -n "$EXISTING_COMMENT_ID" ]; then + EXISTING_BODY=$(gh api \ + "/repos/${{ github.repository }}/issues/comments/$EXISTING_COMMENT_ID" \ + --jq '.body') + RUN_NUMBER=$(( $(echo "$EXISTING_BODY" | grep -c '### 🔄 Run #' || true) + 1 )) + fi + { - echo "$MARKER" + if [ "$RUN_NUMBER" -eq 1 ]; then + echo "$MARKER" + fi + echo "### 🔄 Run #${RUN_NUMBER} — ${TIMESTAMP}" + echo "" echo "## $ICON Skill Security Scan" echo "" @@ -286,20 +308,51 @@ jobs: fi echo "" - echo "> [Workflow run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})" - } > /tmp/comment.md - - EXISTING_COMMENT_ID=$(gh api --paginate \ - "/repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \ - --jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \ - | head -1) + if [ -n "$ARTIFACT_URL" ]; then + echo "> đŸ“Ļ [Download security reports]($ARTIFACT_URL) | [Workflow run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})" + else + echo "> [Workflow run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})" + fi + } > /tmp/new_report.md if [ -n "$EXISTING_COMMENT_ID" ]; then - gh api --method PATCH \ + { + echo "$EXISTING_BODY" + echo "" + echo "---" + echo "" + cat /tmp/new_report.md + } > /tmp/comment.md + gh api \ + --method PATCH \ "/repos/${{ github.repository }}/issues/comments/$EXISTING_COMMENT_ID" \ -f body="$(cat /tmp/comment.md)" else gh pr comment "$PR_NUMBER" \ --repo "${{ github.repository }}" \ - --body-file /tmp/comment.md + --body-file /tmp/new_report.md + fi + + - name: Check scan results + if: steps.detect.outputs.pack_count != '0' && steps.creds.outputs.available == 'true' + run: | + if [ "${{ steps.scan.outputs.scan_result }}" = "failed" ]; then + echo "❌ Security scan found MEDIUM or higher severity issues — blocking merge" + exit 1 + else + echo "✅ Security scan passed — no MEDIUM or higher severity issues found" fi + + - name: React to PR + if: always() + uses: actions/github-script@v7 + with: + script: | + const result = '${{ job.status }}'; + const prNumber = Number('${{ steps.inputs.outputs.pr_number }}'); + await github.rest.reactions.createForIssue({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + content: result === 'success' ? '+1' : '-1' + }); diff --git a/rh-virt/mcps.json b/rh-virt/mcps.json index 22f6d970..74a95e99 100644 --- a/rh-virt/mcps.json +++ b/rh-virt/mcps.json @@ -9,8 +9,8 @@ "--network=host", "--userns=keep-id:uid=65532,gid=65532", "-v", "${KUBECONFIG}:/kubeconfig:ro,Z", - "--entrypoint", "/openshift-mcp-server", - "quay.io/redhat-user-workloads/crt-nshift-lightspeed-tenant/openshift-mcp-server@sha256:2f52c860f91ab3c8a5129b727bdef0d620e733013f073b10355866c45eafd053", + "--entrypoint", "/app/kubernetes-mcp-server", + "quay.io/ecosystem-appeng/openshift-mcp-server@sha256:3531cb78f51f8c7ebcdb21adc21358ab8924116994848a3ce1ff542b3fc23742", "--kubeconfig", "/kubeconfig", "--toolsets", "core,kubevirt" ], diff --git a/rh-virt/skills/vm-clone/SKILL.md b/rh-virt/skills/vm-clone/SKILL.md index 5bc3a990..f19f3719 100644 --- a/rh-virt/skills/vm-clone/SKILL.md +++ b/rh-virt/skills/vm-clone/SKILL.md @@ -151,7 +151,11 @@ Parse: Extract storage names, calculate size, determine DataSources vs container **Option 2 warning**: `âš ī¸ Shared Storage Dangerous - Both VMs share disk, data corruption risk. Only safe if source stopped. Use Option 1 instead. Proceed anyway? (yes/cancel)` Wait for explicit "yes". -### Step 3: Present Clone Configuration for Confirmation +### Step 3: Check Namespace Quota and Present Clone Configuration + +**3.0: Check ResourceQuota** (before presenting confirmation) + +Use `resources_list` (apiVersion="v1", kind="ResourceQuota", namespace=``) to check if quotas exist. If a quota is found, compare current usage against limits for CPU, memory, and storage. If the clone would push usage above 80% of any limit, include a warning in the confirmation summary. **Present configuration summary:** @@ -275,8 +279,8 @@ Would you like help troubleshooting this error? ### Batch Cloning **User request:** "Create 3 copies of template-vm named web-01, web-02, web-03" -**Workflow**: Validate source once, generate/check target names, present combined scope, ask storage strategy, confirm, execute sequentially -**Batch confirmation**: Show source, targets list, strategy, total impact (VMs, storage, vCPU, memory), estimated time +**Limit**: Maximum 5 clones per batch request. If user requests more, refuse and explain the limit exists to prevent resource exhaustion. +**Workflow**: Validate source once, generate/check target names, ask storage strategy once, then process each clone individually through Steps 3-5 with separate confirmation per VM. Stop on first failure. ### Cross-Namespace Cloning **User request:** "Clone production-vm from production to staging namespace" @@ -417,17 +421,22 @@ Agent: "✓ VM Cloned Successfully" ``` User: "Create 3 copies of template-vm named web-01, web-02, web-03 in production" -Agent: [Validates source, checks all names, presents batch review] - "Source: template-vm, Targets: 3 VMs (web-01, web-02, web-03)" - "Total Impact: 90Gi, 6 vCPU, 12Gi, ~20-30 min" - "Proceed? (yes/no)" +Agent: [Validates source, checks all names, asks storage strategy once] + "Storage strategy for all clones: Clone Storage (1)" + + ## Clone 1 of 3: web-01 + "Source: template-vm → Target: web-01 (production), 30Gi clone" + "Proceed with VM cloning? (yes/no)" +User: "yes" +Agent: "✓ web-01 cloned" + ## Clone 2 of 3: web-02 + "Source: template-vm → Target: web-02 (production), 30Gi clone" + "Proceed with VM cloning? (yes/no)" User: "yes" +Agent: "✓ web-02 cloned" -Agent: "đŸ“Ļ Cloning VM 1 of 3: web-01... ✓" - "đŸ“Ļ Cloning VM 2 of 3: web-02... ✓" - "đŸ“Ļ Cloning VM 3 of 3: web-03... ✓" - "✓ Batch Cloning Completed - 3 VMs, 90Gi storage, all Stopped" + [... repeats for each VM with individual confirmation ...] ``` ### Example 4: Shared Storage Warning diff --git a/rh-virt/skills/vm-create/SKILL.md b/rh-virt/skills/vm-create/SKILL.md index 2468ea52..8afe1430 100644 --- a/rh-virt/skills/vm-create/SKILL.md +++ b/rh-virt/skills/vm-create/SKILL.md @@ -83,8 +83,7 @@ Create virtual machines in OpenShift Virtualization using the `vm_create` tool f **Optional (use defaults):** OS (fedora), Size (medium), Storage (30Gi), Performance (u1), Autostart (false) **Gather cluster info:** -- Detect current namespace: `kubectl config view --minify -o jsonpath='{..namespace}' || echo "default"` -- List namespaces: `namespaces_list` (from openshift-virtualization) +- List namespaces: `namespaces_list` (from openshift-virtualization) — ask user to select if not provided - List StorageClasses: `resources_list` with apiVersion="storage.k8s.io/v1", kind="StorageClass" - Identify default SC: annotation `storageclass.kubernetes.io/is-default-class`="true" - Analyze SC: `.volumeBindingMode` (Immediate/WaitForFirstConsumer), provisioner (rbd/cephfs=RWX hint) @@ -166,15 +165,13 @@ Confirm: yes/no/modify **Status interpretation:** - Stopped/Halted → Success (VM created, not started) - Running → Success (if autostart=true) -- Provisioning → Wait 5s, check again +- Provisioning → Wait 5s, check again (max 3 retries, then report status as "still provisioning" and suggest the user check back later) - ErrorUnschedulable → Execute diagnostic workflow (Step 5a) - ErrorDataVolumeNotReady → Storage issue (see Common Issues) #### 5a. Diagnostic Workflow (ErrorUnschedulable) -**CRITICAL: Document Consultation FIRST:** -1. Read [scheduling-errors.md](../../docs/troubleshooting/scheduling-errors.md) using Read tool -2. Output: "I detected ErrorUnschedulable. I consulted [scheduling-errors.md] to understand diagnosis strategies." +**Reference**: See [scheduling-errors.md](../../docs/troubleshooting/scheduling-errors.md) for diagnosis strategies. **Gather diagnostics:** - List events: `events_list` (namespace=``) → Filter for VM/VMI @@ -194,7 +191,7 @@ Confirm: yes/no/modify ### Recommended Solution -**Command**: `oc patch vm -n ...` +**Action**: Update VM via `resources_create_or_update` **Impact**: **Options**: 1) Apply workaround, 2) Manual, 3) Cancel, 4) Ignore âš ī¸ MCP limitation: vm_create doesn't support tolerations @@ -203,7 +200,7 @@ Confirm: yes/no/modify **Wait for user decision.** **If user confirms:** -1. Apply patch: `resources_create_or_update` (fetch, add tolerations, update) OR `oc patch` +1. Apply patch: `resources_create_or_update` (fetch, add tolerations, update) 2. Verify: `resources_get` → Check `.spec.template.spec.tolerations` 3. **Restart VM**: `vm_lifecycle` (action="restart") to apply new spec 4. Wait 15-20s, check status → Stopped → Provisioning → Running @@ -241,7 +238,7 @@ Start: "Start VM " | View: "Show VM " **Error**: **Common Causes**: -- Namespace not exists → `oc create namespace ` +- Namespace not exists → Create via `resources_create_or_update` - RBAC denied → Check ServiceAccount permissions - Resource constraints → Try smaller size - Invalid parameters → Verify OS, size, storage format @@ -254,7 +251,7 @@ Troubleshooting: See Common Issues ### Issue 1: Namespace Not Found **Error**: "Namespace 'xyz' not found" -**Solution**: List with `namespaces_list`, create with `resources_create_or_update` or `oc create namespace ` +**Solution**: List with `namespaces_list`, create with `resources_create_or_update` ### Issue 2: Insufficient Permissions **Error**: "Forbidden: User cannot create VirtualMachines" @@ -331,7 +328,7 @@ Troubleshooting: See Common Issues - **RBAC**: Requires create VirtualMachines (kubevirt.io/v1) in namespace - **Namespace Isolation**: VMs in specified namespace only - **Storage Quotas**: Respects ResourceQuotas -- **Image Security**: Uses official images from trusted registries +- **Image Security**: Default OS options (fedora, ubuntu, rhel, centos-stream, debian, opensuse) use upstream container disk images. Custom images via the `workload` parameter are not validated — warn the user when a custom image URL is provided and confirm the source is trusted before proceeding - **KUBECONFIG**: Never exposed (presence only) - **Audit**: All ops logged via K8s audit @@ -392,6 +389,6 @@ Agent: [Creates] ## Advanced Features -**Custom Images**: `vm_create({"workload": "quay.io/containerdisks/fedora:latest", ...})` +**Custom Images**: `vm_create({"workload": "quay.io/containerdisks/fedora:latest", ...})` — âš ī¸ When a custom image URL is provided, warn the user that image provenance is not verified and confirm the source is trusted before creating the VM **Secondary Networks**: `vm_create({"networks": ["vlan-network"], ...})` or `{"networks": [{"name": "eth1", "networkName": "vlan"}], ...}` **Explicit Instance Type**: `vm_create({"instancetype": "u1.large", ...})` diff --git a/rh-virt/skills/vm-delete/SKILL.md b/rh-virt/skills/vm-delete/SKILL.md index ac079445..e6d2d5f2 100644 --- a/rh-virt/skills/vm-delete/SKILL.md +++ b/rh-virt/skills/vm-delete/SKILL.md @@ -394,7 +394,7 @@ This is preview only. No resources deleted. ## Advanced Features ### Batch Deletion -Delete multiple VMs with confirmation for each: `"Delete VMs test-01, test-02, test-03 in dev"` → Process each individually with full workflow. Use typed confirmation: `DELETE-3-VMS` for batch. +Delete multiple VMs: `"Delete VMs test-01, test-02, test-03 in dev"` → Process each VM individually through the full workflow (Steps 1-5), requiring typed confirmation of each VM name separately. Never use a generic batch confirmation — each VM must be confirmed by its exact name. ### Dry-Run Mode Show deletion scope without executing: Execute Step 1-2, skip Steps 3-4. User request: "Show what would be deleted if I delete VM xyz" diff --git a/rh-virt/skills/vm-inventory/SKILL.md b/rh-virt/skills/vm-inventory/SKILL.md index 3b9c78f9..8a292540 100644 --- a/rh-virt/skills/vm-inventory/SKILL.md +++ b/rh-virt/skills/vm-inventory/SKILL.md @@ -30,10 +30,6 @@ List and inspect virtual machines in OpenShift Virtualization clusters. This ski - `resources_list` (from openshift-virtualization) - List Kubernetes resources including VirtualMachines - `resources_get` (from openshift-virtualization) - Get specific Kubernetes resource details -**Fallback CLI Commands** (if MCP tools unavailable): -- `oc get virtualmachines` / `oc get vm` - List VirtualMachines -- `oc get vm -n -o yaml` - Get VM details - **Required Environment Variables**: - `KUBECONFIG` - Path to Kubernetes configuration file with cluster access @@ -51,12 +47,7 @@ List and inspect virtual machines in OpenShift Virtualization clusters. This ski **Human Notification Protocol:** `❌ Cannot execute vm-inventory: MCP server not available. Setup: Add to mcps.json, set KUBECONFIG, restart Claude Code. Docs: https://github.com/openshift/openshift-mcp-server` -âš ī¸ **SECURITY**: Never display KUBECONFIG path or credential values. - -**Note on Fallback Behavior**: -- If MCP server is unavailable but KUBECONFIG is set, the skill CAN proceed with CLI commands -- Always offer the user the choice between setup (MCP) or CLI fallback -- CLI fallback requires explicit user confirmation before executing any commands +âš ī¸ **SECURITY**: Never display KUBECONFIG path or credential values. Never fall back to CLI commands (`oc`, `kubectl`) — all operations must go through MCP tools exclusively. ## When to Use This Skill @@ -83,11 +74,9 @@ List and inspect virtual machines in OpenShift Virtualization clusters. This ski ## Workflow **CRITICAL EXECUTION PATTERN**: -1. **ALWAYS attempt MCP server tools FIRST** - Try `resources_list` or `resources_get` -2. **If MCP tools fail** - Propose CLI commands (`oc get vm`) with user confirmation -3. **Never skip MCP attempt** - Always try them first - -**Tool Execution Priority**: MCP tools (primary) → CLI commands (fallback with confirmation) +1. **Use MCP server tools exclusively** - `resources_list` or `resources_get` +2. **If MCP tools fail** - Report the error and guide the user to fix MCP setup +3. **Never use CLI commands** - No `oc` or `kubectl` execution under any circumstances ### Workflow A: List All VMs (Across All Namespaces) @@ -95,11 +84,13 @@ List and inspect virtual machines in OpenShift Virtualization clusters. This ski **MCP Tool**: `resources_list` (apiVersion="kubevirt.io/v1", kind="VirtualMachine", allNamespaces=true) -**Errors:** Tool not found/connection error → Report, offer CLI fallback: `oc get virtualmachines -A -o json` +**Errors:** Tool not found/connection error → Report error, guide user to fix MCP setup **Step 2: Get Resource Details for Running VMs** -**CRITICAL**: To display complete VM information, query VirtualMachineInstance (VMI) resources: +**Large cluster safeguard**: If Step 1 returned more than 20 VMs, skip VMI queries entirely. Display a summary table using only VM resource data (Name, Namespace, Status) and suggest the user narrow down by namespace: `âš ī¸ Found VMs across all namespaces. Showing summary view. Use "List VMs in namespace " for full details.` + +To display complete VM information (when 20 or fewer VMs), query VirtualMachineInstance (VMI) resources: **MCP Tool**: `resources_list` (apiVersion="kubevirt.io/v1", kind="VirtualMachineInstance") @@ -158,7 +149,7 @@ Ask user for namespace if not provided. **MCP Tool**: `resources_list` (apiVersion="kubevirt.io/v1", kind="VirtualMachine", namespace=``) -**Errors:** Tool fails → Report, offer CLI fallback: `oc get virtualmachines -n -o json` +**Errors:** Tool fails → Report error, guide user to fix MCP setup **Step 3: Get Resource Details and Display** @@ -186,14 +177,11 @@ Required: VM name, Namespace (ask if not provided) **MCP Tool**: `resources_get` (apiVersion="kubevirt.io/v1", kind="VirtualMachine", namespace=``, name=``) -**Errors:** Tool fails → Report, offer CLI fallback: `oc get vm -n -o yaml` +**Errors:** Tool fails → Report error, guide user to fix MCP setup -**Step 3: Interpret Status and Conditions (Optional)** +**Step 3: Interpret Status and Conditions** -**OPTIONAL**: If VM has error status (ErrorUnschedulable, ErrorDataVolumeNotReady, CrashLoopBackOff), consult [troubleshooting/INDEX.md](../../docs/troubleshooting/INDEX.md) using Read tool. Output: "Consulted INDEX.md to interpret status." - -**When to consult**: VM status is Error/Warning or stuck state (CrashLoopBackOff, Terminating) -**When NOT to consult**: VM status is normal (Running, Stopped, Provisioning) +Report the VM status as-is from the API response. Do NOT read external documentation files to interpret status — use the status indicators defined in the Output Formatting Guidelines section below. If the user needs troubleshooting guidance, suggest they use a dedicated troubleshooting skill instead. **Step 4: Display Detailed Information** @@ -253,7 +241,7 @@ Required: VM name, Namespace (ask if not provided) - By Status (post-processing): Filter results by `status.printableStatus` field - By Resource Size (post-processing): Parse instance type or VMI resource specs -**Errors:** Tool fails → Report, offer CLI fallback: `oc get virtualmachines -A -l -o json` +**Errors:** Tool fails → Report error, guide user to fix MCP setup **Step 2: Display Filtered Results** @@ -304,14 +292,7 @@ Display with explanation: `## 📋 VMs with label 'app=web'` + list/table using - `resources_list` - List resources (apiVersion, kind, namespace optional, allNamespaces optional, labelSelector optional) - `resources_get` - Get resource details (apiVersion, kind, namespace, name) -### CLI Fallback Commands (Use only if MCP tools fail) -- `oc get virtualmachines` / `oc get vm` - List VirtualMachines -- `oc get vm -n ` - Get specific VM -- `oc get vm -A` - List VMs across all namespaces -- `oc get vm -n ` - List VMs in specific namespace -- `oc get vm -l ` - Filter VMs by label selector - -**Important**: Always attempt MCP tools first. Only use CLI commands after MCP tool failure and with user confirmation. +**Important**: All operations must use MCP tools exclusively. CLI commands (`oc`, `kubectl`) are prohibited to prevent command injection risks. ### Related Skills - `vm-create` - Create VMs after checking inventory @@ -319,28 +300,14 @@ Display with explanation: `## 📋 VMs with label 'app=web'` + list/table using - `vm-troubleshooter` (planned) - Diagnose problematic VMs from inventory ### Reference Documentation -- [Troubleshooting INDEX](../../docs/troubleshooting/INDEX.md) - VM status interpretation (optionally consulted when displaying VM details with error states) - [OpenShift Virtualization Documentation](https://docs.redhat.com/en/documentation/openshift_container_platform/4.21/html-single/virtualization/index#virt/about_virt/about-virt.html) - [KubeVirt VirtualMachine API](https://kubevirt.io/api-reference/) - [Accessing VMs](https://docs.redhat.com/en/documentation/openshift_container_platform/4.21/html-single/virtualization/index#virt/virtual_machines/virt-accessing-vm-consoles.html) - [VM Status Conditions](https://kubevirt.io/user-guide/virtual_machines/vm_status_conditions/) -## Critical: Human-in-the-Loop Requirements - -**Not applicable** - This skill performs read-only operations and does not modify any cluster resources. No user confirmation required. - -**Read-only operations:** -- Listing VirtualMachines across namespaces or in specific namespaces -- Retrieving VM details, status, and resource configurations -- Displaying VM health conditions and resource usage -- Filtering VMs by labels or field selectors -- Viewing VM network, storage, and node placement information +## Human-in-the-Loop Requirements -**No modifications performed:** -- ✓ Does not change VM state (start/stop/restart) -- ✓ Does not modify VM configuration -- ✓ Does not delete VMs or resources -- ✓ Does not consume cluster resources +This skill is **read-only** — no user confirmation is required. It does not change VM state, modify configuration, delete resources, or consume cluster capacity. ## Security Considerations @@ -362,15 +329,14 @@ Agent: [MCP: resources_list(apiVersion="kubevirt.io/v1", kind="VirtualMachine", [Displays table format from Workflow A Step 3] ``` -### Example 2: CLI fallback when MCP unavailable +### Example 2: MCP unavailable ``` User: "List all VMs" Agent: [MCP tool fails] - âš ī¸ MCP tool 'resources_list' not available. Use CLI: `oc get virtualmachines -A`? -User: "yes" -Agent: [Executes: oc get virtualmachines -A -o json] - [Displays table format] + ❌ Cannot execute vm-inventory: MCP server not available. + Setup: Add openshift-virtualization to mcps.json, set KUBECONFIG, restart Claude Code. + Docs: https://github.com/openshift/openshift-mcp-server ``` ### Example 3: Get specific VM details diff --git a/rh-virt/skills/vm-lifecycle-manager/SKILL.md b/rh-virt/skills/vm-lifecycle-manager/SKILL.md index be1903f6..b7f8ed71 100644 --- a/rh-virt/skills/vm-lifecycle-manager/SKILL.md +++ b/rh-virt/skills/vm-lifecycle-manager/SKILL.md @@ -220,6 +220,8 @@ Confirm: yes/no If user says "no" or wants to reconsider, do not proceed. +**Batch operations**: When the user requests actions on multiple VMs, process each VM individually through the full workflow (Step 1-3) with separate confirmation for each. Never use a single bulk confirmation for multiple VMs. + **Why**: start (consumes resources), stop (interrupts services), restart (brief downtime). User should verify correct VM and understand impact. ## Security Considerations @@ -282,7 +284,7 @@ Impact: Running after stop+start. Brief interruption. Monitor app logs. ``` User: "Start web-server in namespace vms" Agent: [vm-lifecycle-manager skill] - [vm_lifecycle(action="start")] + [Checks VM status via resources_get — already Running] ## â„šī¸ VM Already Running VM: `web-server` | Namespace: `vms` | Status: Running Result: No action taken - VM already in desired state. @@ -293,16 +295,18 @@ To restart: "Restart VM web-server in namespace vms" ``` User: "Stop VMs web-01, web-02, web-03 in namespace production" -Agent: [vm-lifecycle-manager skill - batch mode] -## Batch Lifecycle Operation -Stopping 3 VMs in 'production': web-01, web-02, web-03 -Impact: All 3 VMs will shut down, services interrupted. +Agent: [vm-lifecycle-manager skill — processes each VM individually with full workflow] + +## VM Lifecycle Operation +| VM Name | `web-01` | Namespace | `production` | Action | `stop` | graceful shutdown | +Confirm: yes/no +User: "yes" +Agent: [vm_lifecycle(namespace="production", name="web-01", action="stop")] +✓ web-01: Stopped + +## VM Lifecycle Operation +| VM Name | `web-02` | Namespace | `production` | Action | `stop` | graceful shutdown | Confirm: yes/no User: "yes" -Agent: [Executes vm_lifecycle for each VM sequentially] -## ✓ Batch Stop Successful -- web-01: Stopped -- web-02: Stopped -- web-03: Stopped -All VMs stopped. Resources freed. +[...repeats for each VM with individual confirmation...] ``` diff --git a/rh-virt/skills/vm-rebalance/SKILL.md b/rh-virt/skills/vm-rebalance/SKILL.md index 8b77b5c0..af1e04ee 100644 --- a/rh-virt/skills/vm-rebalance/SKILL.md +++ b/rh-virt/skills/vm-rebalance/SKILL.md @@ -90,8 +90,8 @@ Orchestrate VM migrations across OpenShift cluster nodes for load balancing, mai **For Manual Mode:** **Document Consultation** (REQUIRED - Execute FIRST): -1. Read [REBALANCE_MANUAL.md](./REBALANCE_MANUAL.md) using Read tool -2. Output: "I consulted [REBALANCE_MANUAL.md](rh-virt/skills/vm-rebalance/REBALANCE_MANUAL.md) to understand the manual migration workflow." +1. **Action**: Read [REBALANCE_MANUAL.md](./REBALANCE_MANUAL.md) using Read tool +2. **Output to user**: "I consulted [REBALANCE_MANUAL.md](./REBALANCE_MANUAL.md) to understand the manual migration workflow." 3. **Then execute**: Follow workflow in REBALANCE_MANUAL.md --- @@ -99,8 +99,8 @@ Orchestrate VM migrations across OpenShift cluster nodes for load balancing, mai **For Automatic Mode:** **Document Consultation** (REQUIRED - Execute FIRST): -1. Read [REBALANCE_AUTOMATIC.md](./REBALANCE_AUTOMATIC.md) using Read tool -2. Output: "I consulted [REBALANCE_AUTOMATIC.md](rh-virt/skills/vm-rebalance/REBALANCE_AUTOMATIC.md) to understand the automatic rebalancing workflow." +1. **Action**: Read [REBALANCE_AUTOMATIC.md](./REBALANCE_AUTOMATIC.md) using Read tool +2. **Output to user**: "I consulted [REBALANCE_AUTOMATIC.md](./REBALANCE_AUTOMATIC.md) to understand the automatic rebalancing workflow." 3. **Then execute**: Follow workflow in REBALANCE_AUTOMATIC.md ## Common Validation Logic @@ -143,7 +143,7 @@ Orchestrate VM migrations across OpenShift cluster nodes for load balancing, mai **Errors**: Not found → "Node doesn't exist" | Not Ready → "Choose different target" | Cordoned → "Uncordon or choose different target" -**Reference**: [../../docs/troubleshooting/scheduling-errors.md](../../docs/troubleshooting/scheduling-errors.md) +**Reference**: [scheduling-errors.md](../../docs/troubleshooting/scheduling-errors.md) ## Node Selection for Automatic Rebalancing @@ -272,12 +272,12 @@ If any node's CPU or Memory percentage **exceeds 100%** after rebalancing: ### Error 1: Live Migration Fails - Storage Not RWX **Symptom**: "Cannot live migrate: PVC access mode is ReadWriteOnce" **Solution**: Use cold migration OR convert PVC to RWX -**Reference**: [../../docs/troubleshooting/storage-errors.md](../../docs/troubleshooting/storage-errors.md) +**Reference**: [storage-errors.md](../../docs/troubleshooting/storage-errors.md) ### Error 2: VM Stuck ErrorUnschedulable After Cold Migration **Symptom**: "VM cannot be scheduled: ErrorUnschedulable" **Solution**: Check node capacity (`nodes_top`), verify no blocking taints (`resources_get` Node), add tolerations, choose different target, remove nodeSelector -**Reference**: [../../docs/troubleshooting/scheduling-errors.md](../../docs/troubleshooting/scheduling-errors.md) +**Reference**: [scheduling-errors.md](../../docs/troubleshooting/scheduling-errors.md) ### Error 3: Live Migration Times Out **Symptom**: "Migration exceeded timeout: 150s per GiB" @@ -331,10 +331,10 @@ If any node's CPU or Memory percentage **exceeds 100%** after rebalancing: - [references/production-considerations.md](./references/production-considerations.md) - HA, capacity, security **Troubleshooting**: -- [../../docs/troubleshooting/INDEX.md](../../docs/troubleshooting/INDEX.md) - Master index -- [../../docs/troubleshooting/scheduling-errors.md](../../docs/troubleshooting/scheduling-errors.md) - ErrorUnschedulable, taints -- [../../docs/troubleshooting/storage-errors.md](../../docs/troubleshooting/storage-errors.md) - PVC access modes -- [../../docs/troubleshooting/lifecycle-errors.md](../../docs/troubleshooting/lifecycle-errors.md) - VM start/stop +- [Troubleshooting INDEX](../../docs/troubleshooting/INDEX.md) - Master index +- [scheduling-errors.md](../../docs/troubleshooting/scheduling-errors.md) - ErrorUnschedulable, taints +- [storage-errors.md](../../docs/troubleshooting/storage-errors.md) - PVC access modes +- [lifecycle-errors.md](../../docs/troubleshooting/lifecycle-errors.md) - VM start/stop **Official Documentation**: - [OpenShift Virt - Live Migration](https://docs.redhat.com/en/documentation/openshift_container_platform/4.21/html-single/virtualization/index#virt-live-migration) diff --git a/rh-virt/skills/vm-snapshot-create/SKILL.md b/rh-virt/skills/vm-snapshot-create/SKILL.md index 571e754f..1f3fe984 100644 --- a/rh-virt/skills/vm-snapshot-create/SKILL.md +++ b/rh-virt/skills/vm-snapshot-create/SKILL.md @@ -21,14 +21,14 @@ color: green Create virtual machine snapshots in OpenShift Virtualization. Snapshots capture the state and data of a VM at a specific point in time, enabling backup, recovery, and testing workflows. -**Implementation Note**: This skill uses generic Kubernetes resource tools (`resources_create_or_update`) to manage VirtualMachineSnapshot resources. Dedicated snapshot tools do not currently exist in the openshift-virtualization MCP server. +**Implementation Note**: This skill uses generic Kubernetes resource tools (`resources_create_or_update`) to manage VirtualMachineSnapshot resources. Dedicated snapshot tools do not currently exist in the openshift-virtualization MCP server. **`resources_create_or_update` MUST only be called with `apiVersion: snapshot.kubevirt.io/v1beta1` and `kind: VirtualMachineSnapshot`** — using it to create or modify any other resource type is a security violation. ## Prerequisites **Required MCP Server**: `openshift-virtualization` ([OpenShift MCP Server](https://github.com/openshift/openshift-mcp-server)) **Required MCP Tools**: -- `resources_create_or_update` (from openshift-virtualization) - Create VirtualMachineSnapshot +- `resources_create_or_update` (from openshift-virtualization) - Create VirtualMachineSnapshot **ONLY**. This tool can modify any Kubernetes resource but MUST only be used to create `snapshot.kubevirt.io/v1beta1/VirtualMachineSnapshot` resources. Any other resource type is strictly prohibited. - `resources_get` (from openshift-virtualization) - Verify VM exists and get status - `resources_list` (from openshift-virtualization) - List StorageClass, VolumeSnapshotClass @@ -69,6 +69,31 @@ Create virtual machine snapshots in OpenShift Virtualization. Snapshots capture If namespace not provided, ask for it explicitly. +### Step 1b: Validate Input Names + +**CRITICAL: Validate ALL user-provided names before using them in any YAML or API call.** + +All names (VM name, namespace, snapshot name) **MUST** match the Kubernetes naming convention: +- **Pattern**: `^[a-z0-9]([a-z0-9.-]*[a-z0-9])?$` +- **Max length**: 63 characters +- **Allowed characters**: lowercase letters, digits, hyphens (`-`), dots (`.`) +- **Must start and end** with a lowercase letter or digit + +**If any name fails validation**, reject the input immediately: + +```markdown +❌ Invalid name: `` + +Kubernetes resource names must: +- Contain only lowercase letters, numbers, hyphens, and dots +- Start and end with a letter or number +- Be at most 63 characters long + +Please provide a valid name. +``` + +**STOP workflow** until the user provides a valid name. **NEVER** interpolate unvalidated input into YAML. + ### Step 2: Verify VM Exists and Get Status **MCP Tool**: `resources_get` (from openshift-virtualization) @@ -128,6 +153,24 @@ Use `vm_lifecycle` MCP tool or vm-lifecycle-manager skill to stop the VM. [Include the full confirmation template with storage backend analysis, guest agent status, volumes to snapshot, etc.] +**Display the exact YAML that will be sent to the API** so the user can verify it before confirming: + +```markdown +**YAML to be applied:** +\`\`\`yaml +apiVersion: snapshot.kubevirt.io/v1beta1 +kind: VirtualMachineSnapshot +metadata: + name: + namespace: +spec: + source: + apiGroup: kubevirt.io + kind: VirtualMachine + name: +\`\`\` +``` + **Wait for user confirmation.** **Handle response:** @@ -140,7 +183,11 @@ Use `vm_lifecycle` MCP tool or vm-lifecycle-manager skill to stop the VM. **MCP Tool**: `resources_create_or_update` (from openshift-virtualization) -**Construct VirtualMachineSnapshot YAML:** +**If snapshot name not provided by user**, generate one: +- Format: `-snapshot-` +- Example: `database-01-snapshot-20260218-143022` + +**Fixed YAML Template — use this EXACT structure. Do NOT add, remove, or modify any fields beyond the three placeholder values (``, ``, ``):** ```yaml apiVersion: snapshot.kubevirt.io/v1beta1 @@ -155,10 +202,6 @@ spec: name: ``` -**If snapshot name not provided by user**, generate one: -- Format: `-snapshot-` -- Example: `database-01-snapshot-20260218-143022` - **Parameters**: ```json { @@ -166,6 +209,18 @@ spec: } ``` +### Step 8b: Post-Construction Verification + +**CRITICAL: Before calling `resources_create_or_update`, verify the constructed YAML. This is a MANDATORY gate — never skip it.** + +1. `apiVersion` is exactly `snapshot.kubevirt.io/v1beta1` — any other apiVersion is a **security violation**, STOP immediately +2. `kind` is exactly `VirtualMachineSnapshot` — any other kind is a **security violation**, STOP immediately +3. Only these fields exist: `metadata.name`, `metadata.namespace`, `spec.source.apiGroup`, `spec.source.kind`, `spec.source.name` +4. No additional fields, labels, annotations, or nested objects are present +5. All placeholder values (``, ``, ``) have been replaced with validated user input from Step 1b + +**If ANY check fails, STOP and report a validation error. Do NOT send it to the API. Do NOT attempt to fix the YAML — cancel the operation entirely.** + **Report progress:** ```markdown 📸 Creating VM snapshot... @@ -233,7 +288,7 @@ Check `status.phase`: - `openshift-virtualization` - OpenShift MCP server with kubevirt toolset ### Required MCP Tools -- `resources_create_or_update` (from openshift-virtualization) - Create VirtualMachineSnapshot +- `resources_create_or_update` (from openshift-virtualization) - Create VirtualMachineSnapshot **ONLY** (never for other resource types) - `resources_get` (from openshift-virtualization) - Verify VM and snapshot status - `resources_list` (from openshift-virtualization) - List StorageClass, VolumeSnapshotClass @@ -327,13 +382,22 @@ Check `status.phase`: ## Security Considerations -- **RBAC Enforcement**: Requires permissions for VirtualMachineSnapshot resources +- **Input Validation**: All user-provided names (VM, namespace, snapshot) are validated against Kubernetes naming rules (`^[a-z0-9]([a-z0-9.-]*[a-z0-9])?$`, max 63 chars) before use +- **Fixed YAML Template**: Resource YAML uses a fixed structure with only three replaceable values — no additional fields may be added +- **Post-Construction Verification**: The constructed YAML is verified against expected structure before being sent to the API +- **Human Review of YAML**: The exact YAML to be applied is shown to the user in the confirmation step +- **RBAC Enforcement**: Requires permissions for VirtualMachineSnapshot resources — the ServiceAccount should be scoped to only VirtualMachineSnapshot create/get in target namespaces - **Storage Quotas**: Respects namespace storage quotas - **Hot-Plugged Volume Detection**: Prevents snapshots when hot-plugged volumes present - **KUBECONFIG Security**: Credentials never exposed in output - **Namespace Isolation**: Snapshots scoped to namespace boundaries - **Audit Trail**: All snapshot operations logged in Kubernetes API audit logs +**Defense-in-depth note**: This skill uses `resources_create_or_update`, a generic MCP tool capable of creating or modifying any Kubernetes resource. The controls in this skill (input validation, fixed YAML template, post-construction verification, allowed-tools declaration) are instruction-level — they guide the LLM but are not hard security boundaries. The real enforcement layers are: +- **Kubernetes RBAC**: The KUBECONFIG ServiceAccount should follow least-privilege — grant only `create` and `get` for `VirtualMachineSnapshot` in target namespaces, not broad resource permissions +- **Admission webhooks**: Can enforce server-side policies on resource creation +- **Future migration**: If the openshift-virtualization MCP server adds dedicated snapshot tools, this skill should migrate to them and drop `resources_create_or_update` + ## Example Usage ### Example 1: Create Snapshot Before Upgrade diff --git a/rh-virt/skills/vm-snapshot-delete/SKILL.md b/rh-virt/skills/vm-snapshot-delete/SKILL.md index 561cabf2..f7c292ad 100644 --- a/rh-virt/skills/vm-snapshot-delete/SKILL.md +++ b/rh-virt/skills/vm-snapshot-delete/SKILL.md @@ -53,15 +53,11 @@ Permanently delete virtual machine snapshots in OpenShift Virtualization. Deleti ## When to Use This Skill **Trigger this skill when:** -- User wants to free storage by removing old snapshots -- User wants to delete a specific snapshot -- User wants to implement snapshot retention policies +- User wants to delete a specific snapshot by name **User phrases that trigger this skill:** - "Delete snapshot pre-upgrade-backup" -- "Remove old snapshots for VM database-01" -- "Delete all snapshots older than 7 days" -- "Free up snapshot storage" +- "Remove snapshot pre-upgrade-backup in production" **Do NOT use this skill when:** - User wants to create snapshots → Use `vm-snapshot-create` skill @@ -171,14 +167,14 @@ Delete operation cancelled. --- -**Proceed with snapshot deletion? This action cannot be undone. (yes/no)** +**Type the snapshot name `` to confirm deletion (cannot be undone):** _____ ``` -**Wait for user confirmation.** +**Wait for user typed confirmation.** **Handle response:** -- If "yes" → Proceed to Step 5 (execute deletion) -- If "no", "cancel", or anything else → Cancel operation +- If input matches snapshot name exactly (case-sensitive) → Proceed to Step 5 +- If mismatch → Report: `❌ Confirmation failed. You typed: . Expected: . Cancelled.` **STOP workflow.** **On cancellation:** ```markdown @@ -368,11 +364,12 @@ Would you like help troubleshooting this error? - Show snapshot details (VM, age, size) - Confirm snapshot won't be needed for recovery - List other available snapshots for the VM - - Ask: "Proceed with snapshot deletion? (yes/no)" - - Wait for explicit "yes" + - **Require typed confirmation**: user must type the exact snapshot name to confirm + - Accept only exact match (case-sensitive) — mismatch cancels the operation 2. **Never Auto-Execute** - - **NEVER delete without user confirmation** + - **NEVER delete without typed confirmation** + - **NEVER accept a simple "yes" — require the snapshot name** - **ALWAYS show what will be lost before deletion** **Why This Matters:** @@ -383,7 +380,7 @@ Would you like help troubleshooting this error? ## Security Considerations - **RBAC Enforcement**: Requires delete permissions for VirtualMachineSnapshot resources -- **User Confirmation**: Always requires explicit "yes" before deletion +- **Typed Confirmation**: Requires exact snapshot name to confirm deletion — prevents accidental "yes" - **Last Snapshot Warning**: Warns users when deleting the only snapshot for a VM - **Namespace Isolation**: Snapshots scoped to namespace boundaries - **Audit Trail**: Deletions logged in Kubernetes API audit logs @@ -421,9 +418,9 @@ Impact of Deletion: Available snapshots for VM `database-01`: - `database-01-pre-upgrade` (created 2024-01-15 10:30) -Proceed with snapshot deletion? This action cannot be undone. (yes/no) +Type the snapshot name `database-01-daily-backup` to confirm deletion (cannot be undone): _____ -User: "yes" +User: "database-01-daily-backup" Agent: [Step 5: Deletes snapshot] diff --git a/rh-virt/skills/vm-snapshot-restore/SKILL.md b/rh-virt/skills/vm-snapshot-restore/SKILL.md index 05f70803..cd3f395f 100644 --- a/rh-virt/skills/vm-snapshot-restore/SKILL.md +++ b/rh-virt/skills/vm-snapshot-restore/SKILL.md @@ -43,14 +43,11 @@ Restore virtual machines from snapshots in OpenShift Virtualization. **CRITICAL* ## When to Use This Skill **Trigger this skill when:** -- User wants to restore a VM to a previous state -- User wants to recover from failed changes/upgrades -- User explicitly requests snapshot restore +- User explicitly requests restoring a VM from a named snapshot **User phrases that trigger this skill:** - "Restore VM api-server from snapshot snapshot-20240115" -- "Roll back database-01 to pre-upgrade snapshot" -- "Recover VM web-server from backup" +- "Roll back database-01 to snapshot pre-upgrade" **Do NOT use this skill when:** - User wants to create snapshots → Use `vm-snapshot-create` skill