From fa8f827726b902f6e5ffc5d7f8f10ed60e91a578 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 27 Mar 2023 12:06:58 -0700 Subject: [PATCH 1/6] Make FQDN lookup function more testable --- providers/shared/fqdn.go | 4 ++ providers/shared/fqdn_test.go | 71 +++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 providers/shared/fqdn_test.go diff --git a/providers/shared/fqdn.go b/providers/shared/fqdn.go index 387fda70..2872906e 100644 --- a/providers/shared/fqdn.go +++ b/providers/shared/fqdn.go @@ -32,6 +32,10 @@ func FQDN() (string, error) { return "", fmt.Errorf("could not get hostname to look for FQDN: %w", err) } + return fqdn(hostname) +} + +func fqdn(hostname string) (string, error) { var errs error cname, err := net.LookupCNAME(hostname) if err != nil { diff --git a/providers/shared/fqdn_test.go b/providers/shared/fqdn_test.go new file mode 100644 index 00000000..fde654e8 --- /dev/null +++ b/providers/shared/fqdn_test.go @@ -0,0 +1,71 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package shared + +import ( + "fmt" + "github.com/stretchr/testify/require" + "testing" +) + +func TestFQDN(t *testing.T) { + tests := map[string]struct { + osHostname string + expectedFQDN string + expectedErr error + }{ + "long_real_hostname": { + osHostname: "elastic.co", + expectedFQDN: "elastic.co", + expectedErr: nil, + }, + "long_nonexistent_hostname": { + osHostname: "foo.bar.elastic.co", + expectedFQDN: "", + expectedErr: makeError("foo.bar.elastic.co"), + }, + "short_nonexistent_hostname": { + osHostname: "foobarbaz", + expectedFQDN: "", + expectedErr: makeError("foobarbaz"), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + actualFQDN, err := fqdn(test.osHostname) + require.Equal(t, test.expectedFQDN, actualFQDN) + + if test.expectedErr == nil { + require.Nil(t, err) + } else { + require.Equal(t, test.expectedErr.Error(), err.Error()) + } + }) + } +} + +func makeError(osHostname string) error { + return fmt.Errorf( + "could not get FQDN, all methods failed: "+ + "failed looking up CNAME: lookup %s: no such host: "+ + "failed looking up IP: lookup %s: no such host", + osHostname, + osHostname, + ) +} From 3915f6babf9bc3762c3e99536b3ed0123679cb7c Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 27 Mar 2023 12:23:41 -0700 Subject: [PATCH 2/6] Run goimports --- providers/shared/fqdn_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/providers/shared/fqdn_test.go b/providers/shared/fqdn_test.go index fde654e8..a5fa2868 100644 --- a/providers/shared/fqdn_test.go +++ b/providers/shared/fqdn_test.go @@ -19,8 +19,9 @@ package shared import ( "fmt" - "github.com/stretchr/testify/require" "testing" + + "github.com/stretchr/testify/require" ) func TestFQDN(t *testing.T) { From 64305badfda5ab5183c1f04168a8e8cbada75d51 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 27 Mar 2023 12:54:34 -0700 Subject: [PATCH 3/6] Add comment about potential flakiness --- providers/shared/fqdn_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/providers/shared/fqdn_test.go b/providers/shared/fqdn_test.go index a5fa2868..280c064b 100644 --- a/providers/shared/fqdn_test.go +++ b/providers/shared/fqdn_test.go @@ -30,6 +30,10 @@ func TestFQDN(t *testing.T) { expectedFQDN string expectedErr error }{ + // This test case depends on network, particularly DNS, + // being available. If it starts to fail often enough + // due to occasional network/DNS unavailability, we should + // probably just delete this test case. "long_real_hostname": { osHostname: "elastic.co", expectedFQDN: "elastic.co", From 2ceb27fa66b811f8fc04080391de36d4b026b7bb Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 27 Mar 2023 13:12:57 -0700 Subject: [PATCH 4/6] Make error assertion bit more flexible --- providers/shared/fqdn_test.go | 36 +++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/providers/shared/fqdn_test.go b/providers/shared/fqdn_test.go index 280c064b..a5761f7a 100644 --- a/providers/shared/fqdn_test.go +++ b/providers/shared/fqdn_test.go @@ -26,28 +26,28 @@ import ( func TestFQDN(t *testing.T) { tests := map[string]struct { - osHostname string - expectedFQDN string - expectedErr error + osHostname string + expectedFQDN string + expectedErrRegex string }{ // This test case depends on network, particularly DNS, // being available. If it starts to fail often enough // due to occasional network/DNS unavailability, we should // probably just delete this test case. "long_real_hostname": { - osHostname: "elastic.co", - expectedFQDN: "elastic.co", - expectedErr: nil, + osHostname: "elastic.co", + expectedFQDN: "elastic.co", + expectedErrRegex: "", }, "long_nonexistent_hostname": { - osHostname: "foo.bar.elastic.co", - expectedFQDN: "", - expectedErr: makeError("foo.bar.elastic.co"), + osHostname: "foo.bar.elastic.co", + expectedFQDN: "", + expectedErrRegex: makeErrorRegex("foo.bar.elastic.co"), }, "short_nonexistent_hostname": { - osHostname: "foobarbaz", - expectedFQDN: "", - expectedErr: makeError("foobarbaz"), + osHostname: "foobarbaz", + expectedFQDN: "", + expectedErrRegex: makeErrorRegex("foobarbaz"), }, } @@ -56,20 +56,20 @@ func TestFQDN(t *testing.T) { actualFQDN, err := fqdn(test.osHostname) require.Equal(t, test.expectedFQDN, actualFQDN) - if test.expectedErr == nil { + if test.expectedErrRegex == "" { require.Nil(t, err) } else { - require.Equal(t, test.expectedErr.Error(), err.Error()) + require.Regexp(t, test.expectedErrRegex, err.Error()) } }) } } -func makeError(osHostname string) error { - return fmt.Errorf( +func makeErrorRegex(osHostname string) string { + return fmt.Sprintf( "could not get FQDN, all methods failed: "+ - "failed looking up CNAME: lookup %s: no such host: "+ - "failed looking up IP: lookup %s: no such host", + "failed looking up CNAME: lookup %s.*: no such host: "+ + "failed looking up IP: lookup %s.*: no such host", osHostname, osHostname, ) From 774c234f8bca4953cdc5737264255c93208f1a6a Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 27 Mar 2023 13:22:15 -0700 Subject: [PATCH 5/6] More flexibility --- providers/shared/fqdn_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/providers/shared/fqdn_test.go b/providers/shared/fqdn_test.go index a5761f7a..1eece96c 100644 --- a/providers/shared/fqdn_test.go +++ b/providers/shared/fqdn_test.go @@ -68,8 +68,8 @@ func TestFQDN(t *testing.T) { func makeErrorRegex(osHostname string) string { return fmt.Sprintf( "could not get FQDN, all methods failed: "+ - "failed looking up CNAME: lookup %s.*: no such host: "+ - "failed looking up IP: lookup %s.*: no such host", + "failed looking up CNAME: lookup %s.*: "+ + "failed looking up IP: lookup %s.*", osHostname, osHostname, ) From 86045b22c0ce496d1b12bd842bdeedfafab771de Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 27 Mar 2023 13:32:40 -0700 Subject: [PATCH 6/6] Forgot the build tags on the test file --- providers/shared/fqdn_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/providers/shared/fqdn_test.go b/providers/shared/fqdn_test.go index 1eece96c..23a94501 100644 --- a/providers/shared/fqdn_test.go +++ b/providers/shared/fqdn_test.go @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +//go:build linux || darwin + package shared import (