Merge pull request #3362 from hashicorp/b-3326-malformed-consul-resp

Don't panic on unexpeced Consul response

client/fingerprint/consul.go

@@ -65,15 +65,39 @@ func (f *ConsulFingerprint) Fingerprint(config *client.Config, node *structs.Nod
 		return false, nil
 	}
 
-	node.Attributes["consul.server"] = strconv.FormatBool(info["Config"]["Server"].(bool))
-	node.Attributes["consul.version"] = info["Config"]["Version"].(string)
-	node.Attributes["consul.revision"] = info["Config"]["Revision"].(string)
-	node.Attributes["unique.consul.name"] = info["Config"]["NodeName"].(string)
-	node.Attributes["consul.datacenter"] = info["Config"]["Datacenter"].(string)
+	if s, ok := info["Config"]["Server"].(bool); ok {
+		node.Attributes["consul.server"] = strconv.FormatBool(s)
+	} else {
+		f.logger.Printf("[WARN] fingerprint.consul: unable to fingerprint consul.server")
+	}
+	if v, ok := info["Config"]["Version"].(string); ok {
+		node.Attributes["consul.version"] = v
+	} else {
+		f.logger.Printf("[WARN] fingerprint.consul: unable to fingerprint consul.version")
+	}
+	if r, ok := info["Config"]["Revision"].(string); ok {
+		node.Attributes["consul.revision"] = r
+	} else {
+		f.logger.Printf("[WARN] fingerprint.consul: unable to fingerprint consul.revision")
+	}
+	if n, ok := info["Config"]["NodeName"].(string); ok {
+		node.Attributes["unique.consul.name"] = n
+	} else {
+		f.logger.Printf("[WARN] fingerprint.consul: unable to fingerprint unique.consul.name")
+	}
+	if d, ok := info["Config"]["Datacenter"].(string); ok {
+		node.Attributes["consul.datacenter"] = d
+	} else {
+		f.logger.Printf("[WARN] fingerprint.consul: unable to fingerprint consul.datacenter")
+	}
 
+	if node.Attributes["consul.datacenter"] != "" || node.Attributes["unique.consul.name"] != "" {
 		node.Links["consul"] = fmt.Sprintf("%s.%s",
 			node.Attributes["consul.datacenter"],
 			node.Attributes["unique.consul.name"])
+	} else {
+		f.logger.Printf("[WARN] fingerprint.consul: malformed Consul response prevented linking")
+	}
 
 	// If the Consul Agent was previously unavailable print a message to
 	// indicate the Agent is available now

client/fingerprint/consul_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/hashicorp/nomad/client/config"
 	"github.com/hashicorp/nomad/nomad/structs"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestConsulFingerprint(t *testing.T) {
@@ -159,3 +160,44 @@ const mockConsulResponse = `
   }
 }
 `
+
+// TestConsulFingerprint_UnexpectedResponse asserts that the Consul
+// fingerprinter does not panic when it encounters an unexpected response.
+// See https://github.com/hashicorp/nomad/issues/3326
+func TestConsulFingerprint_UnexpectedResponse(t *testing.T) {
+	assert := assert.New(t)
+	fp := NewConsulFingerprint(testLogger())
+	node := &structs.Node{
+		Attributes: make(map[string]string),
+	}
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		fmt.Fprintln(w, "{}")
+	}))
+	defer ts.Close()
+
+	config := config.DefaultConfig()
+	config.ConsulConfig.Addr = strings.TrimPrefix(ts.URL, "http://")
+
+	ok, err := fp.Fingerprint(config, node)
+	assert.Nil(err)
+	assert.True(ok)
+
+	attrs := []string{
+		"consul.server",
+		"consul.version",
+		"consul.revision",
+		"unique.consul.name",
+		"consul.datacenter",
+	}
+	for _, attr := range attrs {
+		if v, ok := node.Attributes[attr]; ok {
+			t.Errorf("unexpected node attribute %q with vlaue %q", attr, v)
+		}
+	}
+
+	if v, ok := node.Links["consul"]; ok {
+		t.Errorf("Unexpected link to consul: %v", v)
+	}
+}