Skip to content

Commit

Permalink
validate ProviderID equality by comparing entire string
Browse files Browse the repository at this point in the history
  • Loading branch information
jackfrancis committed Jul 22, 2022
1 parent 7e82b56 commit 1a2e324
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 24 deletions.
13 changes: 5 additions & 8 deletions controllers/noderefutil/providerid.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package noderefutil

import (
"errors"
"fmt"
"regexp"
"strings"
)
Expand Down Expand Up @@ -87,9 +86,9 @@ func (p *ProviderID) ID() string {
return p.id
}

// Equals returns true if both the CloudProvider and ID match.
// Equals returns true if this ProviderID string matches another ProviderID string.
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.
Expand All @@ -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()
}
38 changes: 27 additions & 11 deletions controllers/noderefutil/providerid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
)

const aws = "aws"
const azure = "azure"

func TestNewProviderID(t *testing.T) {
tests := []struct {
Expand Down Expand Up @@ -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())
}
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions internal/controllers/machine/machine_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestWatches(t *testing.T) {
Namespace: ns.Name,
},
Spec: corev1.NodeSpec{
ProviderID: "test:///id-1",
ProviderID: "test://id-1",
},
}

Expand Down Expand Up @@ -1675,7 +1675,7 @@ func TestNodeToMachine(t *testing.T) {
Name: "test-node-to-machine-1",
},
Spec: corev1.NodeSpec{
ProviderID: "test:///id-1",
ProviderID: "test://id-1",
},
}

Expand All @@ -1684,7 +1684,7 @@ func TestNodeToMachine(t *testing.T) {
Name: "test-node-to-machine-node-2",
},
Spec: corev1.NodeSpec{
ProviderID: "test:///id-2",
ProviderID: "test://id-2",
},
}

Expand Down

0 comments on commit 1a2e324

Please sign in to comment.