Add ONTAP mode support with preflight validation and audit logging#28
Open
gnaveen-netapp wants to merge 1 commit into
Open
Add ONTAP mode support with preflight validation and audit logging#28gnaveen-netapp wants to merge 1 commit into
gnaveen-netapp wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds ONTAP expert-mode tooling and safety documentation to the GCNV MCP server, including discovery/execute workflows, dedicated ONTAP convenience tools, preflight scope-denial handling, audit logging, and string timestamp normalization.
Changes:
- Adds ONTAP HTTP client, discovery/execute handlers, dedicated SVM/volume/snapshot/LUN/job tools, and audit log support.
- Updates storage pool mode support and ONTAP guidance in tool descriptions/GEMINI.md.
- Converts multiple timestamp outputs from
Dateobjects to ISO strings and updates tests/schemas.
Reviewed changes
Copilot reviewed 66 out of 69 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/sleep.ts | Adds mockable delay helper for retries. |
| src/utils/scope-denied-envelope.ts | Adds canonical scope_denied envelope and Private-CLI path helpers. |
| src/utils/scope-denied-envelope.test.ts | Tests denial envelope and Private-CLI matching. |
| src/utils/ontap-response-utils.ts | Adds ONTAP success/error formatting and schema sanitization helpers. |
| src/utils/ontap-preflight-validator.ts | Adds index-based ONTAP preflight validation. |
| src/utils/ontap-preflight-validator.test.ts | Tests preflight validation paths and denial handling. |
| src/utils/ontap-index-loader.ts | Adds bundled ONTAP API index loader with decision normalization. |
| src/utils/ontap-index-loader.test.ts | Tests index loading and cache behavior. |
| src/utils/ontap-http-client.ts | Adds authenticated ONTAP proxy client with retries and response unwrapping. |
| src/utils/ontap-http-client.test.ts | Tests proxy URL construction, response unwrapping, and retry behavior. |
| src/utils/ontap-audit-logger.ts | Adds markdown audit logging wrapper and session summary generation. |
| src/utils/ontap-audit-logger.test.ts | Tests audit log enable/disable, grouping, summaries, and wrapper behavior. |
| src/types/tool.ts | Adds ONTAP guidance suffix and isError to handler return type. |
| src/tools/* | Adds ONTAP tools and updates existing tool schemas/descriptions for mode and timestamps. |
| src/tools/handlers/* | Adds ONTAP handlers and updates existing handlers to return ISO timestamps/mode fields. |
| src/registry/register-tools.ts | Registers ONTAP tools and audit wrappers. |
| src/registry/register-tools.test.ts | Updates registered tool count. |
| package.json | Includes ONTAP resources in package and bumps NetApp dependency. |
| package-lock.json | Updates NetApp dependency lockfile entries. |
| GEMINI.md | Adds ONTAP audit logging, safety, scope-denial, and workflow guidance. |
| eslint.config.mjs | Disables unbound-method rule for tests. |
| .prettierignore | Ignores audit logs. |
| .gitignore | Ignores audit logs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+133
to
+138
| export const ontapVolumeDeleteHandler: ToolHandler = async (args) => { | ||
| try { | ||
| const { projectId, locationId, storagePoolId, volumeUuid } = args; | ||
| const client = OntapHttpClient.create(projectId, locationId, storagePoolId); | ||
| const result = await client.delete(`/api/storage/volumes/${volumeUuid}`); | ||
| return asyncSuccessResponse(result); |
Comment on lines
+195
to
+202
| export const ontapSnapshotDeleteHandler: ToolHandler = async (args) => { | ||
| try { | ||
| const { projectId, locationId, storagePoolId, volumeUuid, snapshotUuid } = args; | ||
| const client = OntapHttpClient.create(projectId, locationId, storagePoolId); | ||
| const result = await client.delete( | ||
| `/api/storage/volumes/${volumeUuid}/snapshots/${snapshotUuid}` | ||
| ); | ||
| return asyncSuccessResponse(result); |
Comment on lines
+264
to
+269
| export const ontapLunDeleteHandler: ToolHandler = async (args) => { | ||
| try { | ||
| const { projectId, locationId, storagePoolId, lunUuid } = args; | ||
| const client = OntapHttpClient.create(projectId, locationId, storagePoolId); | ||
| const result = await client.delete(`/api/storage/luns/${lunUuid}`); | ||
| return asyncSuccessResponse(result); |
Comment on lines
+45
to
+49
| const ONTAP_TOOL_GUIDANCE = | ||
| '⚠️ ONTAP Expert Mode Pool Detected: This pool uses ONTAP expert mode. ' + | ||
| 'To manage resources (volumes, LUNs, snapshots, SVMs, etc.) on this pool, ' + | ||
| 'use the ontap_* tools (e.g. ontap_list_volumes, ontap_create_volume, ontap_discover, ontap_execute). ' + | ||
| 'Do NOT use the standard gcnv_* volume/snapshot tools — they are for DEFAULT-mode pools only.'; |
Comment on lines
+38
to
+57
| const SANITIZED_KEYS = new Set(['projectId']); | ||
|
|
||
| function toStr(value: unknown): string { | ||
| if (value === null || value === undefined) return ''; | ||
| if (typeof value === 'string') return value; | ||
| if (typeof value === 'number' || typeof value === 'boolean') return value.toString(); | ||
| return JSON.stringify(value); | ||
| } | ||
|
|
||
| function sanitizeArgs(args: Record<string, unknown>): Record<string, unknown> { | ||
| const sanitized: Record<string, unknown> = {}; | ||
| for (const [key, value] of Object.entries(args)) { | ||
| if (SANITIZED_KEYS.has(key)) continue; | ||
| if (typeof value === 'string' && value.length > 200) { | ||
| sanitized[key] = value.slice(0, 200) + '...'; | ||
| } else { | ||
| sanitized[key] = value; | ||
| } | ||
| } | ||
| return sanitized; |
Comment on lines
+184
to
+191
| filter: z | ||
| .string() | ||
| .optional() | ||
| .describe( | ||
| 'Optional AIP-160 filter. Format: field=value with string values in double quotes. ' + | ||
| 'ONTAP pools: mode then ONTAP in quotes. Standard pools: mode then DEFAULT in quotes. ' + | ||
| 'By state: state then READY in quotes. Combine multiple conditions with AND.' | ||
| ), |
Comment on lines
163
to
+166
| .union([z.string(), z.number()]) | ||
| .optional() | ||
| .describe('Storage pool type (StoragePoolType)'), | ||
| mode: z.string().optional().describe('Mode of the storage pool (DEFAULT or ONTAP)'), |
Comment on lines
236
to
+240
| storagePoolType: z | ||
| .union([z.string(), z.number()]) | ||
| .optional() | ||
| .describe('Storage pool type (StoragePoolType)'), | ||
| mode: z.string().optional().describe('Mode of the storage pool (DEFAULT or ONTAP)'), |
Comment on lines
+166
to
+172
| // 4. Pagination defaults for GET | ||
| let effectiveQueryParams = parsedQueryParams ? { ...parsedQueryParams } : undefined; | ||
| if (method === 'GET') { | ||
| effectiveQueryParams = effectiveQueryParams || {}; | ||
| if (!effectiveQueryParams['max_records']) { | ||
| effectiveQueryParams['max_records'] = '20'; | ||
| } |
Comment on lines
+381
to
+386
| const startMs = Date.now(); | ||
| const result = await handler(args); | ||
| const durationMs = Date.now() - startMs; | ||
|
|
||
| logOperation(toolName, args, result, durationMs); | ||
| return result; |
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.
This pull request makes several important updates to the
GEMINI.mddocumentation to clarify and enhance operational guidelines for managing Google Cloud NetApp Volumes (GCNV) and ONTAP resources. The changes focus on mandatory audit logging, improved error handling, stricter user consent requirements for disruptive actions, enhanced list formatting, and explicit handling of ONTAP expert mode.Key documentation and operational policy updates:
Audit Logging & Session Management
userIntentparameter for all ONTAP tool calls when logging is active.Error Handling & Safety Rules
scope_denied), error recovery, and operation safety. This includes strict guidelines for delete confirmations, parameter change approvals, and multi-step workflows to ensure no destructive or billable actions are taken without explicit user consent.List Response Formatting
ONTAP Expert Mode Handling
modeparameter (DEFAULTorONTAP) when creating storage pools if not provided, and clarified the operational impact of each mode.These changes significantly strengthen operational safety, auditability, and user clarity for GCNV and ONTAP management.