Repository: kubernetes/cloud-provider-aws
Pull request #598 (2023-04-17 13:40:11)

fix: handle subnets with resource based naming more flexibly
**What type of PR is this?**
/kind bug

**What this PR does / why we need it**:
This adds support for RHEL based distributions that typically use a FQDN.

**Which issue(s) this PR fixes**:
<!-- 
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

**Special notes for your reviewer**:

See cloud-init for RHEL at https://github.com/canonical/cloud-init/blob/main/cloudinit/distros/rhel.py#L46
and https://github.com/canonical/cloud-init/blob/f146fe71733e72b94fad525b8cc9988b1405e760/cloudinit/distros/__init__.py#L343
**Does this PR introduce a user-facing change?**:
<!--  
If no, just write "NONE".
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
2. 
-->
```release-note
NONE
```


k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
**Once this PR has been reviewed and has the lgtm label**, please assign dims for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes%2Fcloud-provider-aws).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes/cloud-provider-aws/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["dims"]} -->
k8s-ci-robot: This issue is currently awaiting triage.

If cloud-provider-aws contributors determine this is a relevant issue, they will accept it by applying the `triage/accepted` label and provide further guidance.

The `triage/accepted` label can be added by org members by writing `/triage accepted` in a comment.


<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
tzneal: CC @nckturner 
olemarkus: Looks good. But perhaps update the prerequisite docs to reflect this?
tzneal: > Looks good. But perhaps update the prerequisite docs to reflect this?

Updated, I tried to clarify it a bit.  Unfortunately between DHCP Options, RBN and OS config, I think there may be some situations where the hostname override is still needed.
nckturner: Some nits, otherwise LGTM.  

Patch:
diff --git a/cmd/ecr-credential-provider/main.go b/cmd/ecr-credential-provider/main.go
index b4e1a4a..cbdf22d 100644
--- a/cmd/ecr-credential-provider/main.go
+++ b/cmd/ecr-credential-provider/main.go
@@ -33,7 +33,7 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog/v2"
-	"k8s.io/kubelet/pkg/apis/credentialprovider/v1"
+	"k8s.io/kubelet/pkg/apis/credentialprovider/v1alpha1"
 )
 
 var ecrPattern = regexp.MustCompile(`^(\d{12})\.dkr\.ecr(\-fips)?\.([a-zA-Z0-9][a-zA-Z0-9-_]*)\.(amazonaws\.com(\.cn)?|sc2s\.sgov\.gov|c2s\.ic\.gov)$`)
@@ -59,7 +59,7 @@ func defaultECRProvider(region string, registryID string) (*ecr.ECR, error) {
 	return ecr.New(sess), nil
 }
 
-func (e *ecrPlugin) GetCredentials(ctx context.Context, image string, args []string) (*v1.CredentialProviderResponse, error) {
+func (e *ecrPlugin) GetCredentials(ctx context.Context, image string, args []string) (*v1alpha1.CredentialProviderResponse, error) {
 	registryID, region, registry, err := parseRepoURL(image)
 	if err != nil {
 		return nil, err
@@ -104,10 +104,10 @@ func (e *ecrPlugin) GetCredentials(ctx context.Context, image string, args []str
 
 	cacheDuration := getCacheDuration(data.ExpiresAt)
 
-	return &v1.CredentialProviderResponse{
-		CacheKeyType:  v1.RegistryPluginCacheKeyType,
+	return &v1alpha1.CredentialProviderResponse{
+		CacheKeyType:  v1alpha1.RegistryPluginCacheKeyType,
 		CacheDuration: cacheDuration,
-		Auth: map[string]v1.AuthConfig{
+		Auth: map[string]v1alpha1.AuthConfig{
 			registry: {
 				Username: parts[0],
 				Password: parts[1],
diff --git a/cmd/ecr-credential-provider/plugin.go b/cmd/ecr-credential-provider/plugin.go
index ec68d73..2a43706 100644
--- a/cmd/ecr-credential-provider/plugin.go
+++ b/cmd/ecr-credential-provider/plugin.go
@@ -22,13 +22,14 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"os"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/runtime/serializer/json"
 	"k8s.io/kubelet/pkg/apis/credentialprovider/install"
-	"k8s.io/kubelet/pkg/apis/credentialprovider/v1"
+	"k8s.io/kubelet/pkg/apis/credentialprovider/v1alpha1"
 )
 
 var (
@@ -43,7 +44,7 @@ func init() {
 // CredentialProvider is an interface implemented by the kubelet credential provider plugin to fetch
 // the username/password based on the provided image name.
 type CredentialProvider interface {
-	GetCredentials(ctx context.Context, image string, args []string) (response *v1.CredentialProviderResponse, err error)
+	GetCredentials(ctx context.Context, image string, args []string) (response *v1alpha1.CredentialProviderResponse, err error)
 }
 
 // ExecPlugin implements the exec-based plugin for fetching credentials that is invoked by the kubelet.
@@ -58,7 +59,7 @@ func NewCredentialProvider(plugin CredentialProvider) *ExecPlugin {
 }
 
 // Run executes the credential provider plugin. Required information for the plugin request (in
-// the form of v1.CredentialProviderRequest) is provided via stdin from the kubelet.
+// the form of v1alpha1.CredentialProviderRequest) is provided via stdin from the kubelet.
 // The CredentialProviderResponse, containing the username/password required for pulling
 // the provided image, will be sent back to the kubelet via stdout.
 func (e *ExecPlugin) Run(ctx context.Context) error {
@@ -66,7 +67,7 @@ func (e *ExecPlugin) Run(ctx context.Context) error {
 }
 
 func (e *ExecPlugin) runPlugin(ctx context.Context, r io.Reader, w io.Writer, args []string) error {
-	data, err := io.ReadAll(r)
+	data, err := ioutil.ReadAll(r)
 	if err != nil {
 		return err
 	}
@@ -76,7 +77,7 @@ func (e *ExecPlugin) runPlugin(ctx context.Context, r io.Reader, w io.Writer, ar
 		return err
 	}
 
-	if gvk.GroupVersion() != v1.SchemeGroupVersion {
+	if gvk.GroupVersion() != v1alpha1.SchemeGroupVersion {
 		return fmt.Errorf("group version %s is not supported", gvk.GroupVersion())
 	}
 
@@ -112,8 +113,8 @@ func (e *ExecPlugin) runPlugin(ctx context.Context, r io.Reader, w io.Writer, ar
 	return nil
 }
 
-func decodeRequest(data []byte) (*v1.CredentialProviderRequest, error) {
-	obj, gvk, err := codecs.UniversalDecoder(v1.SchemeGroupVersion).Decode(data, nil, nil)
+func decodeRequest(data []byte) (*v1alpha1.CredentialProviderRequest, error) {
+	obj, gvk, err := codecs.UniversalDecoder(v1alpha1.SchemeGroupVersion).Decode(data, nil, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -122,11 +123,11 @@ func decodeRequest(data []byte) (*v1.CredentialProviderRequest, error) {
 		return nil, fmt.Errorf("kind was %q, expected CredentialProviderRequest", gvk.Kind)
 	}
 
-	if gvk.Group != v1.GroupName {
-		return nil, fmt.Errorf("group was %q, expected %s", gvk.Group, v1.GroupName)
+	if gvk.Group != v1alpha1.GroupName {
+		return nil, fmt.Errorf("group was %q, expected %s", gvk.Group, v1alpha1.GroupName)
 	}
 
-	request, ok := obj.(*v1.CredentialProviderRequest)
+	request, ok := obj.(*v1alpha1.CredentialProviderRequest)
 	if !ok {
 		return nil, fmt.Errorf("unable to convert %T to *CredentialProviderRequest", obj)
 	}
@@ -134,14 +135,14 @@ func decodeRequest(data []byte) (*v1.CredentialProviderRequest, error) {
 	return request, nil
 }
 
-func encodeResponse(response *v1.CredentialProviderResponse) ([]byte, error) {
+func encodeResponse(response *v1alpha1.CredentialProviderResponse) ([]byte, error) {
 	mediaType := "application/json"
 	info, ok := runtime.SerializerInfoForMediaType(codecs.SupportedMediaTypes(), mediaType)
 	if !ok {
 		return nil, fmt.Errorf("unsupported media type %q", mediaType)
 	}
 
-	encoder := codecs.EncoderForVersion(info.Serializer, v1.SchemeGroupVersion)
+	encoder := codecs.EncoderForVersion(info.Serializer, v1alpha1.SchemeGroupVersion)
 	data, err := runtime.Encode(encoder, response)
 	if err != nil {
 		return nil, fmt.Errorf("failed to encode response: %v", err)
diff --git a/docs/credential_provider.md b/docs/credential_provider.md
index 9774e64..bb36abb 100644
--- a/docs/credential_provider.md
+++ b/docs/credential_provider.md
@@ -22,7 +22,7 @@ Example config:
                 "*.dkr.ecr.*.amazonaws.com",
                 "*.dkr.ecr.*.amazonaws.com.cn",
             ],
-            "apiVersion": "credentialprovider.kubelet.k8s.io/v1",
+            "apiVersion": "credentialprovider.kubelet.k8s.io/v1alpha1",
             "defaultCacheDuration": "0"
         }
     ]
diff --git a/docs/prerequisites.md b/docs/prerequisites.md
index 6f9bb19..d7f0422 100644
--- a/docs/prerequisites.md
+++ b/docs/prerequisites.md
@@ -6,7 +6,7 @@ AWS supports two naming conventions: [IP-based or resource-based naming](https:/
 
 When _IP-based naming_ is used, the nodes must be named after the instance followed by the regional domain name (`ip-xxx-xxx-xxx-xxx.ec2.<region>.internal`). If you have custom domain name set in the DHCP options, you must set `--hostname-override` on kube-proxy and kubelet to match the above-mentioned naming convention.
 
-When _resource based naming_ is used, the node must be named after the instance without any domain name (`i-1234567890abcdefg`). Custom domain name may be used as long as the output of `hostname` does not include the domain name. `--hostname-override` should not be set on any components when using resource-based naming.
+When _resource based naming_ is used, the node must be named after the instance either with or without a domain name (`i-1234567890abcdefg` or `i-1234567890abcdefg.<region>.compute.internal`). A custom domain name, configured through DHCP options, may also be used. 
 
 ## IAM Policies
 For the `aws-cloud-controller-manager` to be able to communicate to AWS APIs, you will need to create a few IAM policies for your EC2 instances. The control plane (formerly master) policy is a bit open and can be scaled back depending on the use case. Adjust these based on your needs.
diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go
index 63a31dd..e707882 100644
--- a/pkg/providers/v1/aws.go
+++ b/pkg/providers/v1/aws.go
@@ -5124,6 +5124,12 @@ func nodeNameToIPAddress(nodeName string) string {
 
 func (c *Cloud) nodeNameToInstanceID(nodeName types.NodeName) (InstanceID, error) {
 	if strings.HasPrefix(string(nodeName), rbnNamePrefix) {
+		// depending on if you use a RHEL (e.g. AL2) or Debian (e.g. standard Ubuntu) based distribution, the
+		// hostname on the machine may be either i-00000000000000001 or i-00000000000000001.region.compute.internal.
+		// This handles both scenarios by returning anything before the first '.' in the node name if it has an RBN prefix.
+		if idx := strings.IndexByte(string(nodeName), '.'); idx != -1 {
+			return InstanceID(nodeName[0:idx]), nil
+		}
 		return InstanceID(nodeName), nil
 	}
 	if len(nodeName) == 0 {
diff --git a/version.txt b/version.txt
index 08002f8..5db08bf 100644
--- a/version.txt
+++ b/version.txt
@@ -1,1 +1,1 @@
-1.27.1
+1.27.0


Test patch:
diff --git a/cmd/ecr-credential-provider/main_test.go b/cmd/ecr-credential-provider/main_test.go
index 6d16c5f..6738a81 100644
--- a/cmd/ecr-credential-provider/main_test.go
+++ b/cmd/ecr-credential-provider/main_test.go
@@ -29,7 +29,7 @@ import (
 	"github.com/golang/mock/gomock"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/cloud-provider-aws/pkg/providers/v2/mocks"
-	"k8s.io/kubelet/pkg/apis/credentialprovider/v1"
+	"k8s.io/kubelet/pkg/apis/credentialprovider/v1alpha1"
 )
 
 func generateGetAuthorizationTokenOutput(user string, password string, proxy string, expiration *time.Time) *ecr.GetAuthorizationTokenOutput {
@@ -45,11 +45,11 @@ func generateGetAuthorizationTokenOutput(user string, password string, proxy str
 	return output
 }
 
-func generateResponse(registry string, username string, password string) *v1.CredentialProviderResponse {
-	return &v1.CredentialProviderResponse{
-		CacheKeyType:  v1.RegistryPluginCacheKeyType,
+func generateResponse(registry string, username string, password string) *v1alpha1.CredentialProviderResponse {
+	return &v1alpha1.CredentialProviderResponse{
+		CacheKeyType:  v1alpha1.RegistryPluginCacheKeyType,
 		CacheDuration: &metav1.Duration{Duration: 0},
-		Auth: map[string]v1.AuthConfig{
+		Auth: map[string]v1alpha1.AuthConfig{
 			registry: {
 				Username: username,
 				Password: password,
@@ -70,7 +70,7 @@ func Test_GetCredentials(t *testing.T) {
 		args                        []string
 		getAuthorizationTokenOutput *ecr.GetAuthorizationTokenOutput
 		getAuthorizationTokenError  error
-		response                    *v1.CredentialProviderResponse
+		response                    *v1alpha1.CredentialProviderResponse
 		expectedError               error
 	}{
 		{
diff --git a/cmd/ecr-credential-provider/plugin_test.go b/cmd/ecr-credential-provider/plugin_test.go
index 0e1cade..456602d 100644
--- a/cmd/ecr-credential-provider/plugin_test.go
+++ b/cmd/ecr-credential-provider/plugin_test.go
@@ -24,17 +24,17 @@ import (
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/kubelet/pkg/apis/credentialprovider/v1"
+	"k8s.io/kubelet/pkg/apis/credentialprovider/v1alpha1"
 )
 
 type fakePlugin struct {
 }
 
-func (f *fakePlugin) GetCredentials(ctx context.Context, image string, args []string) (*v1.CredentialProviderResponse, error) {
-	return &v1.CredentialProviderResponse{
-		CacheKeyType:  v1.RegistryPluginCacheKeyType,
+func (f *fakePlugin) GetCredentials(ctx context.Context, image string, args []string) (*v1alpha1.CredentialProviderResponse, error) {
+	return &v1alpha1.CredentialProviderResponse{
+		CacheKeyType:  v1alpha1.RegistryPluginCacheKeyType,
 		CacheDuration: &metav1.Duration{Duration: 10 * time.Minute},
-		Auth: map[string]v1.AuthConfig{
+		Auth: map[string]v1alpha1.AuthConfig{
 			"*.registry.io": {
 				Username: "user",
 				Password: "password",
@@ -52,26 +52,26 @@ func Test_runPlugin(t *testing.T) {
 	}{
 		{
 			name: "successful test case",
-			in:   bytes.NewBufferString(`{"kind":"CredentialProviderRequest","apiVersion":"credentialprovider.kubelet.k8s.io/v1","image":"test.registry.io/foobar"}`),
-			expectedOut: []byte(`{"kind":"CredentialProviderResponse","apiVersion":"credentialprovider.kubelet.k8s.io/v1","cacheKeyType":"Registry","cacheDuration":"10m0s","auth":{"*.registry.io":{"username":"user","password":"password"}}}
+			in:   bytes.NewBufferString(`{"kind":"CredentialProviderRequest","apiVersion":"credentialprovider.kubelet.k8s.io/v1alpha1","image":"test.registry.io/foobar"}`),
+			expectedOut: []byte(`{"kind":"CredentialProviderResponse","apiVersion":"credentialprovider.kubelet.k8s.io/v1alpha1","cacheKeyType":"Registry","cacheDuration":"10m0s","auth":{"*.registry.io":{"username":"user","password":"password"}}}
 `),
 			expectErr: false,
 		},
 		{
 			name:        "invalid kind",
-			in:          bytes.NewBufferString(`{"kind":"CredentialProviderFoo","apiVersion":"credentialprovider.kubelet.k8s.io/v1","image":"test.registry.io/foobar"}`),
+			in:          bytes.NewBufferString(`{"kind":"CredentialProviderFoo","apiVersion":"credentialprovider.kubelet.k8s.io/v1alpha1","image":"test.registry.io/foobar"}`),
 			expectedOut: nil,
 			expectErr:   true,
 		},
 		{
 			name:        "invalid apiVersion",
-			in:          bytes.NewBufferString(`{"kind":"CredentialProviderRequest","apiVersion":"foo.k8s.io/v1","image":"test.registry.io/foobar"}`),
+			in:          bytes.NewBufferString(`{"kind":"CredentialProviderRequest","apiVersion":"foo.k8s.io/v1alpha1","image":"test.registry.io/foobar"}`),
 			expectedOut: nil,
 			expectErr:   true,
 		},
 		{
 			name:        "empty image",
-			in:          bytes.NewBufferString(`{"kind":"CredentialProviderRequest","apiVersion":"credentialprovider.kubelet.k8s.io/v1","image":""}`),
+			in:          bytes.NewBufferString(`{"kind":"CredentialProviderRequest","apiVersion":"credentialprovider.kubelet.k8s.io/v1alpha1","image":""}`),
 			expectedOut: nil,
 			expectErr:   true,
 		},
diff --git a/pkg/providers/v1/aws_test.go b/pkg/providers/v1/aws_test.go
index 528a413..afe586c 100644
--- a/pkg/providers/v1/aws_test.go
+++ b/pkg/providers/v1/aws_test.go
@@ -560,14 +560,14 @@ func testHasNodeAddress(t *testing.T, addrs []v1.NodeAddress, addressType v1.Nod
 	t.Errorf("Did not find expected address: %s:%s in %v", addressType, address, addrs)
 }
 
-func makeInstance(num int, privateIP, publicIP, privateDNSName, publicDNSName string, ipv6s []string, setNetInterface bool) ec2.Instance {
+func makeInstance(instanceID string, privateIP, publicIP, privateDNSName, publicDNSName string, ipv6s []string, setNetInterface bool) ec2.Instance {
 	var tag ec2.Tag
 	tag.Key = aws.String(TagNameKubernetesClusterLegacy)
 	tag.Value = aws.String(TestClusterID)
 	tags := []*ec2.Tag{&tag}
 
 	instance := ec2.Instance{
-		InstanceId:       aws.String(fmt.Sprintf("i-%d", num)),
+		InstanceId:       &instanceID,
 		PrivateDnsName:   aws.String(privateDNSName),
 		PrivateIpAddress: aws.String(privateIP),
 		PublicDnsName:    aws.String(publicDNSName),
@@ -602,124 +602,202 @@ func makeInstance(num int, privateIP, publicIP, privateDNSName, publicDNSName st
 }
 
 func TestNodeAddressesByProviderID(t *testing.T) {
-	// Note instance0 and instance1 have the same name
-	// (we test that this produces an error)
-	instance0 := makeInstance(0, "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true)
-	instance1 := makeInstance(1, "192.168.0.2", "", "instance-same.ec2.internal", "", nil, false)
-	instance2 := makeInstance(2, "192.168.0.1", "1.2.3.4", "instance-other.ec2.internal", "", nil, false)
-	instance3 := makeInstance(3, "192.168.0.3", "", "instance-ipv6.ec2.internal", "", []string{"2a05:d014:aa7:911:fc7e:1600:fc4d:ab2", "2a05:d014:aa7:911:9f44:e737:1aa0:6489"}, true)
-	instances := []*ec2.Instance{&instance0, &instance1, &instance2, &instance3}
-
-	aws1, _ := mockInstancesResp(&instance0, []*ec2.Instance{&instance0})
-	_, err1 := aws1.NodeAddressesByProviderID(context.TODO(), "i-xxx")
-	if err1 == nil {
-		t.Errorf("Should error when no instance found")
-	}
-
-	aws2, _ := mockInstancesResp(&instance0, instances[0:1])
-	// change node name so it uses the instance instead of metadata
-	aws2.selfAWSInstance.nodeName = "foo"
-	addrs2, err2 := aws2.NodeAddressesByProviderID(context.TODO(), "i-0")
-	if err2 != nil {
-		t.Errorf("Should not error when instance found")
-	}
-	if len(addrs2) != 5 {
-		t.Errorf("Should return exactly 5 NodeAddresses")
-	}
-	testHasNodeAddress(t, addrs2, v1.NodeInternalIP, "192.168.0.1")
-	testHasNodeAddress(t, addrs2, v1.NodeExternalIP, "1.2.3.4")
-	testHasNodeAddress(t, addrs2, v1.NodeExternalDNS, "instance-same.ec2.external")
-	testHasNodeAddress(t, addrs2, v1.NodeInternalDNS, "instance-same.ec2.internal")
-	testHasNodeAddress(t, addrs2, v1.NodeHostName, "instance-same.ec2.internal")
-
-	aws3, _ := mockInstancesResp(&instance3, instances)
-	aws3.cfg.Global.NodeIPFamilies = []string{"ipv4", "ipv6"}
-	// change node name so it uses the instance instead of metadata
-	aws3.selfAWSInstance.nodeName = "foo"
-	addrs3, err3 := aws3.NodeAddressesByProviderID(context.TODO(), "i-3")
-	if err3 != nil {
-		t.Errorf("Should not error when instance found")
-	}
-	if len(addrs3) != 4 {
-		t.Errorf("Should return exactly 4 NodeAddresses")
-	}
-	testHasNodeAddress(t, addrs3, v1.NodeInternalIP, "192.168.0.3")
-	testHasNodeAddress(t, addrs3, v1.NodeInternalIP, "2a05:d014:aa7:911:fc7e:1600:fc4d:ab2")
-	testHasNodeAddress(t, addrs3, v1.NodeInternalDNS, "instance-ipv6.ec2.internal")
-	testHasNodeAddress(t, addrs3, v1.NodeHostName, "instance-ipv6.ec2.internal")
-
-	aws4, _ := mockInstancesResp(&instance3, instances)
-	aws4.cfg.Global.NodeIPFamilies = []string{"ipv6"}
-	// change node name so it uses the instance instead of metadata
-	aws4.selfAWSInstance.nodeName = "foo"
-	addrs4, err4 := aws4.NodeAddressesByProviderID(context.TODO(), "i-3")
-	if err4 != nil {
-		t.Errorf("Should not error when instance found")
-	}
-	if len(addrs4) != 1 {
-		t.Errorf("Should return exactly 1 NodeAddresses")
-	}
-	testHasNodeAddress(t, addrs4, v1.NodeInternalIP, "2a05:d014:aa7:911:fc7e:1600:fc4d:ab2")
+	for _, tc := range []struct {
+		Name            string
+		InstanceID      string
+		PrivateIP       string
+		PublicIP        string
+		PrivateDNSName  string
+		PublicDNSName   string
+		Ipv6s           []string
+		SetNetInterface bool
+		NodeName        string
+		Ipv6Only        bool
+
+		ExpectedNumAddresses int
+	}{
+		{
+			Name:                 "ipv4 w/public IP",
+			InstanceID:           "i-00000000000000000",
+			PrivateIP:            "192.168.0.1",
+			PublicIP:             "1.2.3.4",
+			PrivateDNSName:       "instance-same.ec2.internal",
+			PublicDNSName:        "instance-same.ec2.external",
+			SetNetInterface:      true,
+			ExpectedNumAddresses: 5,
+		},
+		{
+			Name:                 "ipv4 w/private IP only",
+			InstanceID:           "i-00000000000000001",
+			PrivateIP:            "192.168.0.2",
+			PrivateDNSName:       "instance-same.ec2.internal",
+			ExpectedNumAddresses: 2,
+		},
+		{
+			Name:                 "ipv4 w/public IP and no public DNS",
+			InstanceID:           "i-00000000000000002",
+			PrivateIP:            "192.168.0.1",
+			PublicIP:             "1.2.3.4",
+			PrivateDNSName:       "instance-other.ec2.internal",
+			ExpectedNumAddresses: 3,
+		},
+		{
+			Name:                 "ipv6 only",
+			InstanceID:           "i-00000000000000003",
+			PrivateIP:            "192.168.0.3",
+			PrivateDNSName:       "instance-ipv6.ec2.internal",
+			Ipv6s:                []string{"2a05:d014:aa7:911:fc7e:1600:fc4d:ab2", "2a05:d014:aa7:911:9f44:e737:1aa0:6489"},
+			SetNetInterface:      true,
+			ExpectedNumAddresses: 1,
+			NodeName:             "foo",
+			Ipv6Only:             true,
+		},
+	} {
+		t.Run(tc.Name, func(t *testing.T) {
+			instance := makeInstance(tc.InstanceID, tc.PrivateIP, tc.PublicIP, tc.PrivateDNSName, tc.PublicDNSName, tc.Ipv6s, tc.SetNetInterface)
+			aws1, _ := mockInstancesResp(&instance, []*ec2.Instance{&instance})
+			_, err := aws1.NodeAddressesByProviderID(context.TODO(), "i-xxx")
+			if err == nil {
+				t.Errorf("Should error when no instance found")
+			}
+			if tc.Ipv6Only {
+				aws1.cfg.Global.NodeIPFamilies = []string{"ipv6"}
+			}
+			if tc.NodeName != "" {
+				aws1.selfAWSInstance.nodeName = types.NodeName(tc.NodeName)
+			}
+			addrs, err := aws1.NodeAddressesByProviderID(context.TODO(), tc.InstanceID)
+			if err != nil {
+				t.Errorf("Should not error when instance found, %s", err)
+			}
+			if len(addrs) != tc.ExpectedNumAddresses {
+				t.Errorf("Should return exactly %d NodeAddresses, got %d (%v)", tc.ExpectedNumAddresses, len(addrs), addrs)
+			}
 
+			if tc.SetNetInterface && !tc.Ipv6Only {
+				testHasNodeAddress(t, addrs, v1.NodeInternalIP, tc.PrivateIP)
+			}
+			if tc.PublicIP != "" {
+				testHasNodeAddress(t, addrs, v1.NodeExternalIP, tc.PublicIP)
+			}
+			if tc.PublicDNSName != "" {
+				testHasNodeAddress(t, addrs, v1.NodeExternalDNS, tc.PublicDNSName)
+			}
+			if tc.PrivateDNSName != "" && !tc.Ipv6Only {
+				testHasNodeAddress(t, addrs, v1.NodeInternalDNS, tc.PrivateDNSName)
+				testHasNodeAddress(t, addrs, v1.NodeHostName, tc.PrivateDNSName)
+			}
+			if tc.Ipv6Only {
+				testHasNodeAddress(t, addrs, v1.NodeInternalIP, tc.Ipv6s[0])
+			}
+		})
+	}
 }
 
 func TestNodeAddresses(t *testing.T) {
-	instance0 := makeInstance(0, "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true)
-	instance2 := makeInstance(2, "192.168.0.1", "1.2.3.4", "instance-other.ec2.internal", "", nil, false)
-	instance3 := makeInstance(3, "192.168.0.3", "", "instance-ipv6.ec2.internal", "", []string{"2a05:d014:aa7:911:fc7e:1600:fc4d:ab2", "2a05:d014:aa7:911:9f44:e737:1aa0:6489"}, true)
-	instances := []*ec2.Instance{&instance0, &instance2, &instance3}
-
-	aws1, _ := mockInstancesResp(&instance0, []*ec2.Instance{&instance0})
-	_, err1 := aws1.NodeAddresses(context.TODO(), "instance-mismatch.ec2.internal")
-	if err1 == nil {
-		t.Errorf("Should error when no instance found")
-	}
-
-	aws3, _ := mockInstancesResp(&instance0, instances[0:1])
-	// change node name so it uses the instance instead of metadata
-	aws3.selfAWSInstance.nodeName = "foo"
-	addrs3, err3 := aws3.NodeAddresses(context.TODO(), "instance-same.ec2.internal")
-	if err3 != nil {
-		t.Errorf("Should not error when instance found")
-	}
-	if len(addrs3) != 5 {
-		t.Errorf("Should return exactly 5 NodeAddresses")
-	}
-	testHasNodeAddress(t, addrs3, v1.NodeInternalIP, "192.168.0.1")
-	testHasNodeAddress(t, addrs3, v1.NodeExternalIP, "1.2.3.4")
-	testHasNodeAddress(t, addrs3, v1.NodeExternalDNS, "instance-same.ec2.external")
-	testHasNodeAddress(t, addrs3, v1.NodeInternalDNS, "instance-same.ec2.internal")
-	testHasNodeAddress(t, addrs3, v1.NodeHostName, "instance-same.ec2.internal")
-
-	aws4, _ := mockInstancesResp(&instance3, instances)
-	aws4.cfg.Global.NodeIPFamilies = []string{"ipv4", "ipv6"}
-	// change node name so it uses the instance instead of metadata
-	aws4.selfAWSInstance.nodeName = "foo"
-	addrs4, err4 := aws4.NodeAddresses(context.TODO(), "instance-ipv6.ec2.internal")
-	if err4 != nil {
-		t.Errorf("Should not error when instance found")
-	}
-	if len(addrs4) != 4 {
-		t.Errorf("Should return exactly 4 NodeAddresses")
-	}
-	testHasNodeAddress(t, addrs4, v1.NodeInternalIP, "192.168.0.3")
-	testHasNodeAddress(t, addrs4, v1.NodeInternalIP, "2a05:d014:aa7:911:fc7e:1600:fc4d:ab2")
-	testHasNodeAddress(t, addrs4, v1.NodeInternalDNS, "instance-ipv6.ec2.internal")
-	testHasNodeAddress(t, addrs4, v1.NodeHostName, "instance-ipv6.ec2.internal")
-
-	aws5, _ := mockInstancesResp(&instance3, instances)
-	aws5.cfg.Global.NodeIPFamilies = []string{"ipv6"}
-	// change node name so it uses the instance instead of metadata
-	aws5.selfAWSInstance.nodeName = "foo"
-	addrs5, err5 := aws5.NodeAddresses(context.TODO(), "instance-ipv6.ec2.internal")
-	if err5 != nil {
-		t.Errorf("Should not error when instance found")
-	}
-	if len(addrs5) != 1 {
-		t.Errorf("Should return exactly 1 NodeAddresses")
-	}
-	testHasNodeAddress(t, addrs5, v1.NodeInternalIP, "2a05:d014:aa7:911:fc7e:1600:fc4d:ab2")
+	for _, tc := range []struct {
+		Name            string
+		InstanceID      string
+		PrivateIP       string
+		PublicIP        string
+		PrivateDNSName  string
+		PublicDNSName   string
+		Ipv6s           []string
+		SetNetInterface bool
+		NodeName        string
+		Ipv6Only        bool
+
+		ExpectedNumAddresses int
+	}{
+		{
+			Name:                 "ipv4 w/public IP",
+			InstanceID:           "i-00000000000000000",
+			PrivateIP:            "192.168.0.1",
+			PublicIP:             "1.2.3.4",
+			PrivateDNSName:       "instance-same.ec2.internal",
+			PublicDNSName:        "instance-same.ec2.external",
+			SetNetInterface:      true,
+			NodeName:             "foo",
+			ExpectedNumAddresses: 5,
+		},
+		{
+			Name:                 "ipv4 w/private IP only",
+			InstanceID:           "i-00000000000000002",
+			PrivateIP:            "192.168.0.1",
+			PublicIP:             "1.2.3.4",
+			PrivateDNSName:       "instance-other.ec2.internal",
+			ExpectedNumAddresses: 3,
+		},
+		{
+			Name:                 "ipv6 only",
+			InstanceID:           "i-00000000000000003",
+			PrivateIP:            "192.168.0.3",
+			PrivateDNSName:       "instance-ipv6.ec2.internal",
+			PublicDNSName:        "instance-same.ec2.external",
+			Ipv6s:                []string{"2a05:d014:aa7:911:fc7e:1600:fc4d:ab2", "2a05:d014:aa7:911:9f44:e737:1aa0:6489"},
+			SetNetInterface:      true,
+			Ipv6Only:             true,
+			NodeName:             "foo",
+			ExpectedNumAddresses: 1,
+		},
+		{
+			Name:                 "resource based naming using FQDN",
+			InstanceID:           "i-00000000000000004",
+			PrivateIP:            "192.168.0.4",
+			PublicIP:             "1.2.3.4",
+			PrivateDNSName:       "i-00000000000000004.ec2.internal",
+			SetNetInterface:      true,
+			ExpectedNumAddresses: 4,
+		},
+		{
+			Name:                 "resource based naming using hostname only",
+			InstanceID:           "i-00000000000000005",
+			PrivateIP:            "192.168.0.5",
+			PublicIP:             "1.2.3.4",
+			PrivateDNSName:       "i-00000000000000005",
+			SetNetInterface:      true,
+			ExpectedNumAddresses: 4,
+		},
+	} {
+		t.Run(tc.Name, func(t *testing.T) {
+			instance := makeInstance(tc.InstanceID, tc.PrivateIP, tc.PublicIP, tc.PrivateDNSName, tc.PublicDNSName, tc.Ipv6s, tc.SetNetInterface)
+			aws1, _ := mockInstancesResp(&instance, []*ec2.Instance{&instance})
+			_, err := aws1.NodeAddresses(context.TODO(), "instance-mismatch.ec2.internal")
+			if err == nil {
+				t.Errorf("Should error when no instance found")
+			}
+			if tc.Ipv6Only {
+				aws1.cfg.Global.NodeIPFamilies = []string{"ipv6"}
+			}
+			if tc.NodeName != "" {
+				aws1.selfAWSInstance.nodeName = types.NodeName(tc.NodeName)
+			}
+			addrs, err := aws1.NodeAddresses(context.TODO(), types.NodeName(tc.PrivateDNSName))
+			if err != nil {
+				t.Errorf("Should not error when instance found, %s", err)
+			}
+			if len(addrs) != tc.ExpectedNumAddresses {
+				t.Errorf("Should return exactly %d NodeAddresses, got %d (%v)", tc.ExpectedNumAddresses, len(addrs), addrs)
+			}
 
+			if tc.SetNetInterface && !tc.Ipv6Only {
+				testHasNodeAddress(t, addrs, v1.NodeInternalIP, tc.PrivateIP)
+			}
+			if tc.PublicIP != "" && !tc.Ipv6Only {
+				testHasNodeAddress(t, addrs, v1.NodeExternalIP, tc.PublicIP)
+			}
+			if tc.PublicDNSName != "" && !tc.Ipv6Only {
+				testHasNodeAddress(t, addrs, v1.NodeExternalDNS, tc.PublicDNSName)
+			}
+			if tc.PrivateDNSName != "" && !tc.Ipv6Only {
+				testHasNodeAddress(t, addrs, v1.NodeInternalDNS, tc.PrivateDNSName)
+				testHasNodeAddress(t, addrs, v1.NodeHostName, tc.PrivateDNSName)
+			}
+			if tc.Ipv6Only {
+				testHasNodeAddress(t, addrs, v1.NodeInternalIP, tc.Ipv6s[0])
+			}
+		})
+	}
 }
 
 func TestGetRegion(t *testing.T) {