feat: handled dns test with skip validation support by Hardikl · Pull Request #148 · NetApp/ontap-mcp

Hardikl · 2026-06-12T10:06:42Z

No description provided.

Add create/update/delete tools for NFS and CIFS protocol services, plus create/delete DNS configuration tools. New tools: - create_nfs_service: enable NFS on an SVM with v3/v4.0/v4.1 config - update_nfs_service: toggle NFS protocol versions - delete_nfs_service: disable NFS on an SVM - create_cifs_service: enable CIFS via AD domain join - update_cifs_service: rename CIFS server - delete_cifs_service: disable CIFS and unjoin AD domain - create_dns: configure DNS on an SVM - delete_dns: remove DNS configuration from an SVM

- Replace custom parseBool with strconv.ParseBool - Remove unused bytes.Buffer in CreateNFSService - Fix error return pattern: return nil instead of err in error results - Move BodyJSON before Delete in CIFS service delete

- Remove start-ontap-mcp.bat and stop-ontap-mcp.bat from tracking - Add integration tests for NFS service (create/update/delete) - Add integration tests for DNS (create/delete)

- Extract shared getSVMUUID helper in rest/svm.go to eliminate duplicated SVM lookup logic across nfsService, cifsService, and dns - Fix integration tests: delete after create should expect success, not 'does not exist' error

- Fix gofmt formatting in rest/svm.go (getSVMUUID helper) - Add handleJob support to CreateDNS for async job responses

… pair - Fix .gitignore entry (was concatenated with previous line) - Remove start-ontap-mcp.bat from git tracking - Validate AD user and password are provided as a pair in DeleteCIFSService

- Add verifyDNSConfig that checks domains and nameservers match expected values after creation (not just record count)

- DNS DeleteDNS now uses handleJob for async job support - NFS integration test verifies v40_enabled after update

Copilot

Pull request overview

This PR expands the MCP server’s management capabilities for ONTAP SVM services by adding tools and REST client support for NFS service management, CIFS/SMB service management (AD join/unjoin), and DNS configuration (including skip validation support), along with integration tests for the new NFS/DNS behaviors.

Changes:

Add new MCP tools + server handlers for NFS service, CIFS service, and DNS create/delete operations.
Add REST client methods and ONTAP model types to call the corresponding ONTAP REST endpoints (including SVM UUID lookup helper).
Add integration tests covering NFS service lifecycle and DNS configuration with/without skip validation.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tool/tool.go	Adds new tool parameter structs for NFS/CIFS/DNS service operations.
server/server.go	Registers new MCP tools for NFS service, CIFS service, and DNS operations.
server/nfsService.go	Implements server handlers + request mapping for NFS service create/update/delete.
server/cifsService.go	Implements server handlers + request mapping for CIFS service create/update/delete.
server/dns.go	Implements server handlers + request mapping for DNS create/delete (incl. skip validation).
rest/svm.go	Adds getSVMUUID helper used by new REST operations.
rest/nfsService.go	Adds REST client methods for NFS service create/update/delete.
rest/cifsService.go	Adds REST client methods for CIFS service create/update/delete (job-handling).
rest/dns.go	Adds REST client methods for DNS create/delete.
ontap/ontap.go	Adds ONTAP request/response model structs for NFS/CIFS/DNS operations.
descriptions/descriptions.go	Adds tool descriptions for the newly registered MCP tools.
integration/test/nfs_service_test.go	Adds integration coverage for NFS service lifecycle.
integration/test/dns_test.go	Adds integration coverage for DNS config + skip validation behavior.
.gitignore	Adds Windows helper scripts to ignored files and fixes formatting of an entry.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	// NFS service (enable/disable/configure NFS on an SVM)
+	addTool(a, server, "create_nfs_service", descriptions.CreateNFSService, createAnnotation, a.CreateNFSService)
+	addTool(a, server, "update_nfs_service", descriptions.UpdateNFSService, updateAnnotation, a.UpdateNFSService)
+	addTool(a, server, "delete_nfs_service", descriptions.DeleteNFSService, deleteAnnotation, a.DeleteNFSService)
+
+	// CIFS service (enable/disable CIFS/SMB on an SVM via AD join)
+	addTool(a, server, "create_cifs_service", descriptions.CreateCIFSService, createAnnotation, a.CreateCIFSService)
+	addTool(a, server, "update_cifs_service", descriptions.UpdateCIFSService, updateAnnotation, a.UpdateCIFSService)
+	addTool(a, server, "delete_cifs_service", descriptions.DeleteCIFSService, deleteAnnotation, a.DeleteCIFSService)
+
+	// DNS configuration
+	addTool(a, server, "create_dns", descriptions.CreateDNS, createAnnotation, a.CreateDNS)
+	addTool(a, server, "delete_dns", descriptions.DeleteDNS, deleteAnnotation, a.DeleteDNS)


+		if len(data.Records) > 0 {
+			rec := data.Records[0]
+			sort.Strings(rec.Domains)
+			if len(rec.Domains) != len(expectedDomains) || !slices.Equal(rec.Domains, expectedDomains) {
+				t.Errorf("verifyDNSConfig: expected domains %v but got %v", expectedDomains, rec.Domains)
+				return false
+			}
+			sort.Strings(rec.Servers)
+			if len(rec.Servers) != len(expectedServers) || !slices.Equal(rec.Servers, expectedServers) {
+				t.Errorf("verifyDNSConfig: expected servers %v but got %v", expectedServers, rec.Servers)
+				return false
+			}
+		}


+// getSVMUUID looks up the UUID of an SVM by name.
+func (c *Client) getSVMUUID(ctx context.Context, svmName string) (string, error) {
+	var (
+		statusCode int
+		svmData    ontap.GetData
+	)
+	responseHeaders := http.Header{}
+
+	params := url.Values{}
+	params.Set("name", svmName)
+	params.Set("fields", "uuid")
+
+	builder := c.baseRequestBuilder(`/api/svm/svms`, &statusCode, responseHeaders).
+		Params(params).
+		ToJSON(&svmData)
+
+	if err := c.buildAndExecuteRequest(ctx, builder); err != nil {
+		return "", err
+	}
+
+	if svmData.NumRecords == 0 {
+		return "", fmt.Errorf("failed to get details of SVM %s because it does not exist", svmName)
+	}
+	if svmData.NumRecords != 1 {
+		return "", fmt.Errorf("failed to get details of SVM %s because there are %d matching records", svmName, svmData.NumRecords)
+	}
+
+	return svmData.Records[0].UUID, nil
+}


Hardikl · 2026-06-12T14:48:14Z

Closing this PR as #149 would handle the same.

tasosnetapp and others added 9 commits June 2, 2026 17:47

fix: address PR review comments

708f59b

- Replace custom parseBool with strconv.ParseBool - Remove unused bytes.Buffer in CreateNFSService - Fix error return pattern: return nil instead of err in error results - Move BodyJSON before Delete in CIFS service delete

chore: remove convenience scripts from repo, add integration tests

7505745

- Remove start-ontap-mcp.bat and stop-ontap-mcp.bat from tracking - Add integration tests for NFS service (create/update/delete) - Add integration tests for DNS (create/delete)

refactor: extract getSVMUUID helper, fix integration tests

0f54214

- Extract shared getSVMUUID helper in rest/svm.go to eliminate duplicated SVM lookup logic across nfsService, cifsService, and dns - Fix integration tests: delete after create should expect success, not 'does not exist' error

fix: format rest/svm.go, add job link support to DNS create

a517da1

- Fix gofmt formatting in rest/svm.go (getSVMUUID helper) - Add handleJob support to CreateDNS for async job responses

fix: remove start-ontap-mcp.bat from tracking, validate AD credential…

eebdf1b

… pair - Fix .gitignore entry (was concatenated with previous line) - Remove start-ontap-mcp.bat from git tracking - Validate AD user and password are provided as a pair in DeleteCIFSService

test: add DNS field validation in integration test

942fbda

- Add verifyDNSConfig that checks domains and nameservers match expected values after creation (not just record count)

fix: add handleJob to DNS delete, add NFS update verification

7c2d562

- DNS DeleteDNS now uses handleJob for async job support - NFS integration test verifies v40_enabled after update

feat: handled dns test with skip validation support

9b5b135

Copilot AI review requested due to automatic review settings June 12, 2026 10:06

Hardikl requested review from cgrinds and rahulguptajss as code owners June 12, 2026 10:06

cla-bot Bot added the cla-signed label Jun 12, 2026

Copilot started reviewing on behalf of Hardikl June 12, 2026 10:07 View session

Copilot AI reviewed Jun 12, 2026

View reviewed changes

Hardikl closed this Jun 12, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: handled dns test with skip validation support#148

feat: handled dns test with skip validation support#148
Hardikl wants to merge 9 commits into
mainfrom
hl_dns_tes

Hardikl commented Jun 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Hardikl commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Hardikl commented Jun 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Hardikl commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants