From 500c3c2d876c7afc58e975fc7c5ae575add2bcb3 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Thu, 26 Mar 2020 11:13:21 -0400 Subject: [PATCH 1/4] tests: Add tests for EC2 Metadata immitation cases Test that nomad doesn't set empty/bad network configuration when in an environment that does incomplete immitation of EC2 Metadata API. --- client/fingerprint/env_aws_test.go | 238 +++++++++++++++++++++-------- 1 file changed, 175 insertions(+), 63 deletions(-) diff --git a/client/fingerprint/env_aws_test.go b/client/fingerprint/env_aws_test.go index 93198978fcfe..379e4ae5d2ae 100644 --- a/client/fingerprint/env_aws_test.go +++ b/client/fingerprint/env_aws_test.go @@ -1,7 +1,6 @@ package fingerprint import ( - "encoding/json" "fmt" "net/http" "net/http/httptest" @@ -29,7 +28,7 @@ func TestEnvAWSFingerprint_nonAws(t *testing.T) { } func TestEnvAWSFingerprint_aws(t *testing.T) { - endpoint, cleanup := startFakeEC2Metadata(t) + endpoint, cleanup := startFakeEC2Metadata(t, awsStubs) defer cleanup() f := NewEnvAWSFingerprint(testlog.HCLogger(t)) @@ -70,7 +69,7 @@ func TestEnvAWSFingerprint_aws(t *testing.T) { } func TestNetworkFingerprint_AWS(t *testing.T) { - endpoint, cleanup := startFakeEC2Metadata(t) + endpoint, cleanup := startFakeEC2Metadata(t, awsStubs) defer cleanup() f := NewEnvAWSFingerprint(testlog.HCLogger(t)) @@ -98,7 +97,7 @@ func TestNetworkFingerprint_AWS(t *testing.T) { } func TestNetworkFingerprint_AWS_network(t *testing.T) { - endpoint, cleanup := startFakeEC2Metadata(t) + endpoint, cleanup := startFakeEC2Metadata(t, awsStubs) defer cleanup() f := NewEnvAWSFingerprint(testlog.HCLogger(t)) @@ -158,16 +157,56 @@ func TestNetworkFingerprint_AWS_network(t *testing.T) { } } -/// Utility functions for tests +func TestNetworkFingerprint_AWS_NoNetwork(t *testing.T) { + endpoint, cleanup := startFakeEC2Metadata(t, noNetworkAWSStubs) + defer cleanup() + + f := NewEnvAWSFingerprint(testlog.HCLogger(t)) + f.(*EnvAWSFingerprint).endpoint = endpoint -func startFakeEC2Metadata(t *testing.T) (endpoint string, cleanup func()) { - routes := routes{} - if err := json.Unmarshal([]byte(aws_routes), &routes); err != nil { - t.Fatalf("Failed to unmarshal JSON in AWS ENV test: %s", err) + node := &structs.Node{ + Attributes: make(map[string]string), } + request := &FingerprintRequest{Config: &config.Config{}, Node: node} + var response FingerprintResponse + err := f.Fingerprint(request, &response) + require.NoError(t, err) + + require.True(t, response.Detected, "expected response to be applicable") + + require.Equal(t, "ami-1234", response.Attributes["platform.aws.ami-id"]) + + require.Nil(t, response.NodeResources) +} + +func TestNetworkFingerprint_AWS_IncompleteImmitation(t *testing.T) { + endpoint, cleanup := startFakeEC2Metadata(t, incompleteAWSImmitationStubs) + defer cleanup() + + f := NewEnvAWSFingerprint(testlog.HCLogger(t)) + f.(*EnvAWSFingerprint).endpoint = endpoint + + node := &structs.Node{ + Attributes: make(map[string]string), + } + + request := &FingerprintRequest{Config: &config.Config{}, Node: node} + var response FingerprintResponse + err := f.Fingerprint(request, &response) + require.NoError(t, err) + + require.False(t, response.Detected, "expected response not to be applicable") + + require.NotContains(t, response.Attributes, "platform.aws.ami-id") + require.Nil(t, response.NodeResources) +} + +/// Utility functions for tests + +func startFakeEC2Metadata(t *testing.T, endpoints []endpoint) (endpoint string, cleanup func()) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - for _, e := range routes.Endpoints { + for _, e := range endpoints { if r.RequestURI == e.Uri { w.Header().Set("Content-Type", e.ContentType) fmt.Fprintln(w, e.Body) @@ -181,60 +220,133 @@ func startFakeEC2Metadata(t *testing.T) (endpoint string, cleanup func()) { type routes struct { Endpoints []*endpoint `json:"endpoints"` } + type endpoint struct { - Uri string `json:"uri"` - ContentType string `json:"content-type"` - Body string `json:"body"` + Uri string + ContentType string + Body string +} + +// awsStubs mimics normal EC2 instance metadata +var awsStubs = []endpoint{ + { + Uri: "/latest/meta-data/ami-id", + ContentType: "text/plain", + Body: "ami-1234", + }, + { + Uri: "/latest/meta-data/hostname", + ContentType: "text/plain", + Body: "ip-10-0-0-207.us-west-2.compute.internal", + }, + { + Uri: "/latest/meta-data/placement/availability-zone", + ContentType: "text/plain", + Body: "us-west-2a", + }, + { + Uri: "/latest/meta-data/instance-id", + ContentType: "text/plain", + Body: "i-b3ba3875", + }, + { + Uri: "/latest/meta-data/instance-type", + ContentType: "text/plain", + Body: "m3.2xlarge", + }, + { + Uri: "/latest/meta-data/local-hostname", + ContentType: "text/plain", + Body: "ip-10-0-0-207.us-west-2.compute.internal", + }, + { + Uri: "/latest/meta-data/local-ipv4", + ContentType: "text/plain", + Body: "10.0.0.207", + }, + { + Uri: "/latest/meta-data/public-hostname", + ContentType: "text/plain", + Body: "ec2-54-191-117-175.us-west-2.compute.amazonaws.com", + }, + { + Uri: "/latest/meta-data/public-ipv4", + ContentType: "text/plain", + Body: "54.191.117.175", + }, } -const aws_routes = ` -{ - "endpoints": [ - { - "uri": "/latest/meta-data/ami-id", - "content-type": "text/plain", - "body": "ami-1234" - }, - { - "uri": "/latest/meta-data/hostname", - "content-type": "text/plain", - "body": "ip-10-0-0-207.us-west-2.compute.internal" - }, - { - "uri": "/latest/meta-data/placement/availability-zone", - "content-type": "text/plain", - "body": "us-west-2a" - }, - { - "uri": "/latest/meta-data/instance-id", - "content-type": "text/plain", - "body": "i-b3ba3875" - }, - { - "uri": "/latest/meta-data/instance-type", - "content-type": "text/plain", - "body": "m3.2xlarge" - }, - { - "uri": "/latest/meta-data/local-hostname", - "content-type": "text/plain", - "body": "ip-10-0-0-207.us-west-2.compute.internal" - }, - { - "uri": "/latest/meta-data/local-ipv4", - "content-type": "text/plain", - "body": "10.0.0.207" - }, - { - "uri": "/latest/meta-data/public-hostname", - "content-type": "text/plain", - "body": "ec2-54-191-117-175.us-west-2.compute.amazonaws.com" - }, - { - "uri": "/latest/meta-data/public-ipv4", - "content-type": "text/plain", - "body": "54.191.117.175" - } - ] +// noNetworkAWSStubs mimics an EC2 instance but without local ip address +// may happen in environments with odd EC2 Metadata emulation +var noNetworkAWSStubs = []endpoint{ + { + Uri: "/latest/meta-data/ami-id", + ContentType: "text/plain", + Body: "ami-1234", + }, + { + Uri: "/latest/meta-data/hostname", + ContentType: "text/plain", + Body: "ip-10-0-0-207.us-west-2.compute.internal", + }, + { + Uri: "/latest/meta-data/placement/availability-zone", + ContentType: "text/plain", + Body: "us-west-2a", + }, + { + Uri: "/latest/meta-data/instance-id", + ContentType: "text/plain", + Body: "i-b3ba3875", + }, + { + Uri: "/latest/meta-data/instance-type", + ContentType: "text/plain", + Body: "m3.2xlarge", + }, + { + Uri: "/latest/meta-data/local-hostname", + ContentType: "text/plain", + Body: "ip-10-0-0-207.us-west-2.compute.internal", + }, + { + Uri: "/latest/meta-data/local-ipv4", + ContentType: "text/plain", + Body: "", + }, + { + Uri: "/latest/meta-data/public-hostname", + ContentType: "text/plain", + Body: "ec2-54-191-117-175.us-west-2.compute.amazonaws.com", + }, + { + Uri: "/latest/meta-data/public-ipv4", + ContentType: "text/plain", + Body: "54.191.117.175", + }, +} + +// incompleteAWSImmitationsStub mimics environments where some AWS endpoints +// return empty, namely Hetzner +var incompleteAWSImmitationStubs = []endpoint{ + { + Uri: "/latest/meta-data/hostname", + ContentType: "text/plain", + Body: "ip-10-0-0-207.us-west-2.compute.internal", + }, + { + Uri: "/latest/meta-data/instance-id", + ContentType: "text/plain", + Body: "i-b3ba3875", + }, + { + Uri: "/latest/meta-data/local-ipv4", + ContentType: "text/plain", + Body: "", + }, + { + Uri: "/latest/meta-data/public-ipv4", + ContentType: "text/plain", + Body: "54.191.117.175", + }, } -` From e37f7af811239593e3cbe8c65d0a122cf673024f Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Thu, 26 Mar 2020 11:16:16 -0400 Subject: [PATCH 2/4] fingerprint: handle incomplete AWS immitation APIs Fix a regression where we accidentally started treating non-AWS environments as AWS environments, resulting in bad networking settings. Two factors some at play: First, in [1], we accidentally switched the ultimate AWS test from checking `ami-id` to `instance-id`. This means that nomad started treating more environments as AWS; e.g. Hetzner implements `instance-id` but not `ami-id`. Second, some of these environments return empty values instead of errors! Hetzner returns empty 200 response for `local-ipv4`, resulting into bad networking configuration. This change fix the situation by restoring the check to `ami-id` and ensuring that we only set network configuration when the ip address is not-empty. Also, be more defensive around response whitespace input. [1] https://github.com/hashicorp/nomad/pull/6779 --- client/fingerprint/env_aws.go | 80 ++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/client/fingerprint/env_aws.go b/client/fingerprint/env_aws.go index fdd46f5841b5..eeb204c08688 100644 --- a/client/fingerprint/env_aws.go +++ b/client/fingerprint/env_aws.go @@ -80,15 +80,10 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F return fmt.Errorf("failed to setup ec2Metadata client: %v", err) } - if !ec2meta.Available() { + if !isAWS(ec2meta) { return nil } - // newNetwork is populated and added to the Nodes resources - newNetwork := &structs.NetworkResource{ - Device: "eth0", - } - // Keys and whether they should be namespaced as unique. Any key whose value // uniquely identifies a node, such as ip, should be marked as unique. When // marked as unique, the key isn't included in the computed node class. @@ -103,9 +98,14 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F "public-ipv4": true, "placement/availability-zone": false, } + for k, unique := range keys { resp, err := ec2meta.GetMetadata(k) - if awsErr, ok := err.(awserr.RequestFailure); ok { + v := strings.TrimSpace(strings.Trim(resp, "\n")) + if v == "" { + f.logger.Debug("read an empty value", "attribute", k) + continue + } else if awsErr, ok := err.(awserr.RequestFailure); ok { f.logger.Debug("could not read attribute value", "attribute", k, "error", awsErr) continue } else if awsErr, ok := err.(awserr.Error); ok { @@ -125,45 +125,28 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F key = structs.UniqueNamespace(key) } - response.AddAttribute(key, strings.Trim(resp, "\n")) + response.AddAttribute(key, v) } + // newNetwork is populated and added to the Nodes resources + var newNetwork *structs.NetworkResource + // copy over network specific information if val, ok := response.Attributes["unique.platform.aws.local-ipv4"]; ok && val != "" { response.AddAttribute("unique.network.ip-address", val) - newNetwork.IP = val - newNetwork.CIDR = newNetwork.IP + "/32" - } - // find LinkSpeed from lookup - throughput := cfg.NetworkSpeed - if throughput == 0 { - throughput = f.linkSpeed(ec2meta) - } - if throughput == 0 { - // Failed to determine speed. Check if the network fingerprint got it - found := false - if request.Node.Resources != nil && len(request.Node.Resources.Networks) > 0 { - for _, n := range request.Node.Resources.Networks { - if n.IP == newNetwork.IP { - throughput = n.MBits - found = true - break - } - } + newNetwork = &structs.NetworkResource{ + Device: "eth0", + IP: val, + CIDR: val + "/32", + MBits: f.throughput(request, ec2meta, val), } - // Nothing detected so default - if !found { - throughput = defaultNetworkSpeed + response.NodeResources = &structs.NodeResources{ + Networks: []*structs.NetworkResource{newNetwork}, } } - newNetwork.MBits = throughput - response.NodeResources = &structs.NodeResources{ - Networks: []*structs.NetworkResource{newNetwork}, - } - // populate Links response.AddLink("aws.ec2", fmt.Sprintf("%s.%s", response.Attributes["platform.aws.placement.availability-zone"], @@ -173,6 +156,28 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F return nil } +func (f *EnvAWSFingerprint) throughput(request *FingerprintRequest, ec2meta *ec2metadata.EC2Metadata, ip string) int { + throughput := request.Config.NetworkSpeed + if throughput != 0 { + return throughput + } + + throughput = f.linkSpeed(ec2meta) + if throughput != 0 { + return throughput + } + + if request.Node.Resources != nil && len(request.Node.Resources.Networks) > 0 { + for _, n := range request.Node.Resources.Networks { + if n.IP == ip { + return n.MBits + } + } + } + + return defaultNetworkSpeed +} + // EnvAWSFingerprint uses lookup table to approximate network speeds func (f *EnvAWSFingerprint) linkSpeed(ec2meta *ec2metadata.EC2Metadata) int { @@ -211,3 +216,8 @@ func ec2MetaClient(endpoint string, timeout time.Duration) (*ec2metadata.EC2Meta } return ec2metadata.New(session, c), nil } + +func isAWS(ec2meta *ec2metadata.EC2Metadata) bool { + v, err := ec2meta.GetMetadata("ami-id") + return err == nil && v != "" +} From 0c1dd0e75bd3d33567c40b2cdf3b5af07792498b Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Thu, 26 Mar 2020 11:33:44 -0400 Subject: [PATCH 3/4] fixup! tests: Add tests for EC2 Metadata immitation cases --- client/fingerprint/env_aws_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/client/fingerprint/env_aws_test.go b/client/fingerprint/env_aws_test.go index 379e4ae5d2ae..8ab53af05f90 100644 --- a/client/fingerprint/env_aws_test.go +++ b/client/fingerprint/env_aws_test.go @@ -180,8 +180,8 @@ func TestNetworkFingerprint_AWS_NoNetwork(t *testing.T) { require.Nil(t, response.NodeResources) } -func TestNetworkFingerprint_AWS_IncompleteImmitation(t *testing.T) { - endpoint, cleanup := startFakeEC2Metadata(t, incompleteAWSImmitationStubs) +func TestNetworkFingerprint_AWS_IncompleteImitation(t *testing.T) { + endpoint, cleanup := startFakeEC2Metadata(t, incompleteAWSImitationStubs) defer cleanup() f := NewEnvAWSFingerprint(testlog.HCLogger(t)) @@ -222,9 +222,9 @@ type routes struct { } type endpoint struct { - Uri string - ContentType string - Body string + Uri string `json:"uri"` + ContentType string `json:"content-type"` + Body string `json:"body"` } // awsStubs mimics normal EC2 instance metadata @@ -326,9 +326,9 @@ var noNetworkAWSStubs = []endpoint{ }, } -// incompleteAWSImmitationsStub mimics environments where some AWS endpoints +// incompleteAWSImitationsStub mimics environments where some AWS endpoints // return empty, namely Hetzner -var incompleteAWSImmitationStubs = []endpoint{ +var incompleteAWSImitationStubs = []endpoint{ { Uri: "/latest/meta-data/hostname", ContentType: "text/plain", From 6199e9697293f52f67d3b9437cb010d44b6b22c4 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Thu, 26 Mar 2020 11:37:54 -0400 Subject: [PATCH 4/4] fixup! tests: Add tests for EC2 Metadata immitation cases --- client/fingerprint/env_aws.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/fingerprint/env_aws.go b/client/fingerprint/env_aws.go index eeb204c08688..1e1d47127c02 100644 --- a/client/fingerprint/env_aws.go +++ b/client/fingerprint/env_aws.go @@ -101,7 +101,7 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F for k, unique := range keys { resp, err := ec2meta.GetMetadata(k) - v := strings.TrimSpace(strings.Trim(resp, "\n")) + v := strings.TrimSpace(resp) if v == "" { f.logger.Debug("read an empty value", "attribute", k) continue @@ -219,5 +219,6 @@ func ec2MetaClient(endpoint string, timeout time.Duration) (*ec2metadata.EC2Meta func isAWS(ec2meta *ec2metadata.EC2Metadata) bool { v, err := ec2meta.GetMetadata("ami-id") + v = strings.TrimSpace(v) return err == nil && v != "" }