Merge pull request #644 from foostan/add_validation_of_deregistration

Validate ServiceID/CheckID when deregistering.

command/agent/agent.go

@@ -637,6 +637,11 @@ func (a *Agent) RemoveService(serviceID string, persist bool) error {
 			consul.ConsulServiceID)
 	}
 
+	// Validate ServiceID
+	if serviceID == "" {
+		return fmt.Errorf("ServiceID missing")
+	}
+
 	// Remove service immeidately
 	a.state.RemoveService(serviceID)
 
@@ -757,6 +762,11 @@ func (a *Agent) AddCheck(check *structs.HealthCheck, chkType *CheckType, persist
 // RemoveCheck is used to remove a health check.
 // The agent will make a best effort to ensure it is deregistered
 func (a *Agent) RemoveCheck(checkID string, persist bool) error {
+	// Validate CheckID
+	if checkID == "" {
+		return fmt.Errorf("CheckID missing")
+	}
+
 	// Add to the local state for anti-entropy
 	a.state.RemoveCheck(checkID)
 

command/agent/agent_test.go

@@ -247,6 +247,11 @@ func TestAgent_RemoveService(t *testing.T) {
 		t.Fatalf("should have errored")
 	}
 
+	// Remove without an ID
+	if err := agent.RemoveService("", false); err == nil {
+		t.Fatalf("should have errored")
+	}
+
 	// Removing a service with a single check works
 	{
 		srv := &structs.NodeService{
@@ -406,6 +411,11 @@ func TestAgent_RemoveCheck(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
+	// Remove without an ID
+	if err := agent.RemoveCheck("", false); err == nil {
+		t.Fatalf("should have errored")
+	}
+
 	health := &structs.HealthCheck{
 		Node:    "foo",
 		CheckID: "mem",

command/agent/local.go

@@ -1,6 +1,7 @@
 package agent
 
 import (
+	"fmt"
 	"log"
 	"reflect"
 	"strings"
@@ -408,6 +409,10 @@ func (l *localState) syncChanges() error {
 
 // deleteService is used to delete a service from the server
 func (l *localState) deleteService(id string) error {
+	if id == "" {
+		return fmt.Errorf("ServiceID missing")
+	}
+
 	req := structs.DeregisterRequest{
 		Datacenter:   l.config.Datacenter,
 		Node:         l.config.NodeName,
@@ -425,6 +430,10 @@ func (l *localState) deleteService(id string) error {
 
 // deleteCheck is used to delete a service from the server
 func (l *localState) deleteCheck(id string) error {
+	if id == "" {
+		return fmt.Errorf("CheckID missing")
+	}
+
 	req := structs.DeregisterRequest{
 		Datacenter:   l.config.Datacenter,
 		Node:         l.config.NodeName,