validate ProviderID equality by comparing entire string

kubernetes-sigs · Apr 13, 2022 · 364a7bf · 364a7bf
1 parent fe7656c
commit 364a7bf
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 23 deletions.
diff --git a/controllers/noderefutil/providerid.go b/controllers/noderefutil/providerid.go
@@ -19,7 +19,6 @@ package noderefutil
 
 import (
 	"errors"
-	"fmt"
 	"regexp"
 	"strings"
 )
@@ -89,7 +88,7 @@ func (p *ProviderID) ID() string {
 
 // Equals returns true if both the CloudProvider and ID match.
 func (p *ProviderID) Equals(o *ProviderID) bool {
-	return p.CloudProvider() == o.CloudProvider() && p.ID() == o.ID()
+	return p.String() == o.String()
 }
 
 // String returns the string representation of this object.
@@ -102,10 +101,8 @@ func (p *ProviderID) Validate() bool {
 	return p.CloudProvider() != "" && p.ID() != ""
 }
 
-// IndexKey returns a string concatenating the cloudProvider and the ID parts of the providerID.
-// E.g Format: cloudProvider://optional/segments/etc/id. IndexKey: cloudProvider/id
-// This is useful to use the providerID as a reliable index between nodes and machines
-// as it guarantees the infra Providers contract.
+// IndexKey returns the required level of uniqueness
+// to represent and index machines uniquely from their node providerID.
 func (p *ProviderID) IndexKey() string {
-	return fmt.Sprintf("%s/%s", p.CloudProvider(), p.ID())
+	return p.String()
 }
diff --git a/controllers/noderefutil/providerid_test.go b/controllers/noderefutil/providerid_test.go
@@ -23,6 +23,7 @@ import (
 )
 
 const aws = "aws"
+const azure = "azure"
 
 func TestNewProviderID(t *testing.T) {
 	tests := []struct {
@@ -115,19 +116,34 @@ func TestInvalidProviderID(t *testing.T) {
 func TestProviderIDEquals(t *testing.T) {
 	g := NewWithT(t)
 
-	input1 := "aws:////instance-id1"
-	parsed1, err := NewProviderID(input1)
+	inputAWS1 := "aws:////instance-id1"
+	parsedAWS1, err := NewProviderID(inputAWS1)
 	g.Expect(err).NotTo(HaveOccurred())
-	g.Expect(parsed1.String()).To(Equal(input1))
-	g.Expect(parsed1.ID()).To(Equal("instance-id1"))
-	g.Expect(parsed1.CloudProvider()).To(Equal(aws))
+	g.Expect(parsedAWS1.String()).To(Equal(inputAWS1))
+	g.Expect(parsedAWS1.ID()).To(Equal("instance-id1"))
+	g.Expect(parsedAWS1.CloudProvider()).To(Equal(aws))
 
-	input2 := "aws:///us-west-1/instance-id1"
-	parsed2, err := NewProviderID(input2)
+	inputAWS2 := "aws:///us-west-1/instance-id1"
+	parsedAWS2, err := NewProviderID(inputAWS2)
 	g.Expect(err).NotTo(HaveOccurred())
-	g.Expect(parsed2.String()).To(Equal(input2))
-	g.Expect(parsed2.ID()).To(Equal("instance-id1"))
-	g.Expect(parsed2.CloudProvider()).To(Equal(aws))
+	g.Expect(parsedAWS2.String()).To(Equal(inputAWS2))
+	g.Expect(parsedAWS2.ID()).To(Equal("instance-id1"))
+	g.Expect(parsedAWS2.CloudProvider()).To(Equal(aws))
 
-	g.Expect(parsed1.Equals(parsed2)).To(BeTrue())
+	// Test for inequality
+	g.Expect(parsedAWS1.Equals(parsedAWS2)).To(BeFalse())
+
+	inputAzure1 := "azure:///subscriptions/4920076a-ba9f-11ec-8422-0242ac120002/resourceGroups/default-template/providers/Microsoft.Compute/virtualMachines/default-template-control-plane-fhrvh"
+	parsedAzure1, err := NewProviderID(inputAzure1)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(parsedAzure1.String()).To(Equal(inputAzure1))
+	g.Expect(parsedAzure1.ID()).To(Equal("default-template-control-plane-fhrvh"))
+	g.Expect(parsedAzure1.CloudProvider()).To(Equal(azure))
+
+	inputAzure2 := "azure:///subscriptions/4920076a-ba9f-11ec-8422-0242ac120002/resourceGroups/default-template/providers/Microsoft.Compute/virtualMachines/default-template-control-plane-fhrvh"
+	parsedAzure2, err := NewProviderID(inputAzure2)
+	g.Expect(err).NotTo(HaveOccurred())
+
+	// Test for equalitey
+	g.Expect(parsedAzure1.Equals(parsedAzure2)).To(BeTrue())
 }
diff --git a/internal/controllers/machine/machine_controller_noderef_test.go b/internal/controllers/machine/machine_controller_noderef_test.go
@@ -81,7 +81,7 @@ func TestGetNode(t *testing.T) {
 					ProviderID: "aws://us-west-2/test-get-node-2",
 				},
 			},
-			providerIDInput: "aws:///test-get-node-2",
+			providerIDInput: "aws://us-west-2/test-get-node-2",
 		},
 		{
 			name: "gce prefix, cloudProvider and ID matches",
@@ -93,7 +93,7 @@ func TestGetNode(t *testing.T) {
 					ProviderID: "gce://us-central1/test-get-node-2",
 				},
 			},
-			providerIDInput: "gce:///test-get-node-2",
+			providerIDInput: "gce://us-central1/test-get-node-2",
 		},
 		{
 			name: "Node is not found",

diff --git a/internal/controllers/machine/machine_controller_test.go b/internal/controllers/machine/machine_controller_test.go
@@ -96,7 +96,7 @@ func TestWatches(t *testing.T) {
 			Namespace: ns.Name,
 		},
 		Spec: corev1.NodeSpec{
-			ProviderID: "test:///id-1",
+			ProviderID: "test://id-1",
 		},
 	}
 
@@ -1671,7 +1671,7 @@ func TestNodeToMachine(t *testing.T) {
 			Name: "test-node-to-machine-1",
 		},
 		Spec: corev1.NodeSpec{
-			ProviderID: "test:///id-1",
+			ProviderID: "test://id-1",
 		},
 	}
 
@@ -1680,7 +1680,7 @@ func TestNodeToMachine(t *testing.T) {
 			Name: "test-node-to-machine-node-2",
 		},
 		Spec: corev1.NodeSpec{
-			ProviderID: "test:///id-2",
+			ProviderID: "test://id-2",
 		},
 	}