Remove ErrNotUnique (#90)

Instead of returning ErrNotUnique if a search for an id returns multiple entities we always search the returned entities for an exact match.
rubrikinc · May 12, 2022 · f992e05 · f992e05
1 parent 81e575b
commit f992e05
Show file tree

Hide file tree

Showing 9 changed files with 101 additions and 66 deletions.
diff --git a/pkg/polaris/aws/aws.go b/pkg/polaris/aws/aws.go
@@ -116,14 +116,15 @@ func (a API) toCloudAccountID(ctx context.Context, id IdentityFunc) (uuid.UUID,
 	if err != nil {
 		return uuid.Nil, fmt.Errorf("failed to get account: %v", err)
 	}
-	if len(accountsWithFeatures) < 1 {
-		return uuid.Nil, fmt.Errorf("account %w", graphql.ErrNotFound)
-	}
-	if len(accountsWithFeatures) > 1 {
-		return uuid.Nil, fmt.Errorf("account %w", graphql.ErrNotUnique)
+
+	// Find the exact match.
+	for _, accountWithFeatures := range accountsWithFeatures {
+		if accountWithFeatures.Account.NativeID == identity.id {
+			return accountWithFeatures.Account.ID, nil
+		}
 	}
 
-	return accountsWithFeatures[0].Account.ID, nil
+	return uuid.Nil, fmt.Errorf("account %w", graphql.ErrNotFound)
 }
 
 // toNativeID returns the AWS account id for the specified identity. If the
@@ -201,6 +202,7 @@ func (a API) Account(ctx context.Context, id IdentityFunc, feature core.Feature)
 			return CloudAccount{}, fmt.Errorf("failed to get account: %v", err)
 		}
 
+		// Find the exact match.
 		for _, accountWithFeatures := range accountsWithFeatures {
 			if accountWithFeatures.Account.ID == cloudAccountID {
 				return toCloudAccount(accountWithFeatures), nil
@@ -211,11 +213,12 @@ func (a API) Account(ctx context.Context, id IdentityFunc, feature core.Feature)
 		if err != nil {
 			return CloudAccount{}, fmt.Errorf("failed to get account: %v", err)
 		}
-		if len(accountsWithFeatures) == 1 {
-			return toCloudAccount(accountsWithFeatures[0]), nil
-		}
-		if len(accountsWithFeatures) > 1 {
-			return CloudAccount{}, fmt.Errorf("account %w", graphql.ErrNotUnique)
+
+		// Find the exact match.
+		for _, accountWithFeatures := range accountsWithFeatures {
+			if accountWithFeatures.Account.NativeID == identity.id {
+				return toCloudAccount(accountWithFeatures), nil
+			}
 		}
 	}
 

diff --git a/pkg/polaris/azure/azure.go b/pkg/polaris/azure/azure.go
@@ -109,13 +109,12 @@ func (a API) toCloudAccountID(ctx context.Context, id IdentityFunc) (uuid.UUID,
 		return uuid.Nil, fmt.Errorf("failed to lookup identity: %v", err)
 	}
 
+	uid, err := uuid.Parse(identity.id)
+	if err != nil {
+		return uuid.Nil, fmt.Errorf("failed to parse identity: %v", err)
+	}
 	if identity.internal {
-		id, err := uuid.Parse(identity.id)
-		if err != nil {
-			return uuid.Nil, fmt.Errorf("failed to parse identity: %v", err)
-		}
-
-		return id, nil
+		return uid, nil
 	}
 
 	// Note that the same tenant can show up for multiple features.
@@ -136,14 +135,13 @@ func (a API) toCloudAccountID(ctx context.Context, id IdentityFunc) (uuid.UUID,
 			if err != nil {
 				return uuid.Nil, fmt.Errorf("failed to get tenant: %v", err)
 			}
-			if len(tenantWithAccounts.Accounts) == 0 {
-				continue
-			}
-			if len(tenantWithAccounts.Accounts) > 1 {
-				return uuid.Nil, fmt.Errorf("subscription %w", graphql.ErrNotUnique)
-			}
 
-			return tenantWithAccounts.Accounts[0].ID, nil
+			// Find the exact match.
+			for _, account := range tenantWithAccounts.Accounts {
+				if account.NativeID == uid {
+					return account.ID, nil
+				}
+			}
 		}
 	}
 
@@ -281,19 +279,20 @@ func (a API) Subscription(ctx context.Context, id IdentityFunc, feature core.Fea
 		return CloudAccount{}, fmt.Errorf("failed to lookup identity: %v", err)
 	}
 
-	if identity.internal {
-		id, err := uuid.Parse(identity.id)
-		if err != nil {
-			return CloudAccount{}, fmt.Errorf("failed to parse identity: %v", err)
-		}
+	uid, err := uuid.Parse(identity.id)
+	if err != nil {
+		return CloudAccount{}, fmt.Errorf("failed to parse identity: %v", err)
+	}
 
+	if identity.internal {
 		accounts, err := a.Subscriptions(ctx, feature, "")
 		if err != nil {
 			return CloudAccount{}, fmt.Errorf("failed to get subscriptions: %v", err)
 		}
 
+		// Find the exact match.
 		for _, account := range accounts {
-			if account.ID == id {
+			if account.ID == uid {
 				return account, nil
 			}
 		}
@@ -302,11 +301,12 @@ func (a API) Subscription(ctx context.Context, id IdentityFunc, feature core.Fea
 		if err != nil {
 			return CloudAccount{}, fmt.Errorf("failed to get subscriptions: %v", err)
 		}
-		if len(accounts) > 1 {
-			return CloudAccount{}, fmt.Errorf("subscription %w", graphql.ErrNotUnique)
-		}
-		if len(accounts) == 1 {
-			return accounts[0], nil
+
+		// Find the exact match.
+		for _, account := range accounts {
+			if account.NativeID == uid {
+				return account, nil
+			}
 		}
 	}
 

diff --git a/pkg/polaris/gcp/gcp.go b/pkg/polaris/gcp/gcp.go
@@ -159,7 +159,7 @@ func (a API) Project(ctx context.Context, id IdentityFunc, feature core.Feature)
 		return CloudAccount{}, fmt.Errorf("failed to lookup identity: %v", err)
 	}
 
-	if identity.internal {
+	if identity.kind == internalID {
 		id, err := uuid.Parse(identity.id)
 		if err != nil {
 			return CloudAccount{}, fmt.Errorf("failed to parse identity: %v", err)
@@ -170,6 +170,7 @@ func (a API) Project(ctx context.Context, id IdentityFunc, feature core.Feature)
 			return CloudAccount{}, fmt.Errorf("failed to get projects: %v", err)
 		}
 
+		// Find the exact match.
 		for _, account := range accounts {
 			if account.ID == id {
 				return account, nil
@@ -180,11 +181,20 @@ func (a API) Project(ctx context.Context, id IdentityFunc, feature core.Feature)
 		if err != nil {
 			return CloudAccount{}, fmt.Errorf("failed to get projects: %v", err)
 		}
-		if len(accounts) > 1 {
-			return CloudAccount{}, fmt.Errorf("project %w", graphql.ErrNotUnique)
-		}
-		if len(accounts) == 1 {
-			return accounts[0], nil
+
+		// Find the exact match.
+		if identity.kind == externalID {
+			for _, account := range accounts {
+				if account.NativeID == identity.id {
+					return account, nil
+				}
+			}
+		} else {
+			for _, account := range accounts {
+				if strconv.FormatInt(account.ProjectNumber, 10) == identity.id {
+					return account, nil
+				}
+			}
 		}
 	}
 
@@ -217,15 +227,15 @@ func (a API) Projects(ctx context.Context, feature core.Feature, filter string)
 		if err != nil {
 			return nil, fmt.Errorf("failed to get native projects: %v", err)
 		}
-		if len(natives) < 1 {
-			accounts[i].OrganizationName = "<Native Disabled>"
-			continue
-		}
-		if len(natives) > 1 {
-			return nil, fmt.Errorf("native project %w", graphql.ErrNotUnique)
-		}
 
-		accounts[i].OrganizationName = natives[0].OrganizationName
+		// Find the exact match.
+		accounts[i].OrganizationName = "<Native Disabled>"
+		for _, native := range natives {
+			if native.NativeID == accounts[i].NativeID {
+				accounts[i].OrganizationName = native.OrganizationName
+				break
+			}
+		}
 	}
 
 	return accounts, nil
@@ -312,14 +322,20 @@ func (a API) RemoveProject(ctx context.Context, id IdentityFunc, feature core.Fe
 		if err != nil {
 			return fmt.Errorf("failed to get native projects: %v", err)
 		}
-		if len(natives) < 1 {
-			return fmt.Errorf("native project %w", graphql.ErrNotFound)
+
+		// Find the exact match.
+		var nativeID uuid.UUID
+		for _, native := range natives {
+			if native.NativeID == account.NativeID {
+				nativeID = native.ID
+				break
+			}
 		}
-		if len(natives) > 1 {
-			return fmt.Errorf("native project %w", graphql.ErrNotUnique)
+		if nativeID == uuid.Nil {
+			return fmt.Errorf("native project %w", graphql.ErrNotFound)
 		}
 
-		jobID, err := gcp.Wrap(a.gql).NativeDisableProject(ctx, natives[0].ID, deleteSnapshots)
+		jobID, err := gcp.Wrap(a.gql).NativeDisableProject(ctx, nativeID, deleteSnapshots)
 		if err != nil {
 			return fmt.Errorf("failed to disable native project: %v", err)
 		}

diff --git a/pkg/polaris/gcp/identity.go b/pkg/polaris/gcp/identity.go
@@ -29,9 +29,17 @@ import (
 	"github.com/google/uuid"
 )
 
+type identityKind int
+
+const (
+	internalID identityKind = iota
+	externalID
+	externalNumber
+)
+
 type identity struct {
-	id       string
-	internal bool
+	id   string
+	kind identityKind
 }
 
 // IdentityFunc returns a project identity initialized from the values passed
@@ -42,7 +50,7 @@ type IdentityFunc func(ctx context.Context) (identity, error)
 // the specified Polaris cloud account id.
 func CloudAccountID(id uuid.UUID) IdentityFunc {
 	return func(ctx context.Context) (identity, error) {
-		return identity{id: id.String(), internal: true}, nil
+		return identity{id: id.String(), kind: internalID}, nil
 	}
 }
 
@@ -55,7 +63,7 @@ func ID(project ProjectFunc) IdentityFunc {
 			return identity{}, fmt.Errorf("failed to lookup project: %v", err)
 		}
 
-		return identity{id: config.id, internal: false}, nil
+		return identity{id: config.id, kind: externalID}, nil
 	}
 }
 
@@ -67,14 +75,14 @@ func ProjectID(id string) IdentityFunc {
 			return identity{}, errors.New("invalid GCP project id")
 		}
 
-		return identity{id: id, internal: false}, nil
+		return identity{id: id, kind: externalID}, nil
 	}
 }
 
 // ProjectNumber returns an IdentityFunc that initializes the identity with the
 // specified project number.
 func ProjectNumber(number int64) IdentityFunc {
 	return func(ctx context.Context) (identity, error) {
-		return identity{id: strconv.FormatInt(number, 10), internal: false}, nil
+		return identity{id: strconv.FormatInt(number, 10), kind: externalNumber}, nil
 	}
 }
diff --git a/pkg/polaris/graphql/aws/native.go b/pkg/polaris/graphql/aws/native.go
@@ -132,8 +132,8 @@ func (a API) NativeAccounts(ctx context.Context, feature ProtectionFeature, filt
 	return accounts, nil
 }
 
-// StartNativeAccountDisableJob starts a task chain job to disables the native
-// account with the specified Polaris native account id. Returns the Polaris
+// StartNativeAccountDisableJob starts a task chain job to disable the native
+// account with the specified Polaris cloud account id. Returns the Polaris
 // task chain id.
 func (a API) StartNativeAccountDisableJob(ctx context.Context, id uuid.UUID, feature ProtectionFeature, deleteSnapshots bool) (uuid.UUID, error) {
 	a.GQL.Log().Print(log.Trace)

diff --git a/pkg/polaris/graphql/azure/native.go b/pkg/polaris/graphql/azure/native.go
@@ -32,6 +32,8 @@ import (
 )
 
 // NativeSubscription represents a Polaris native subscription.
+// NativeSubscriptions are connected to CloudAccounts through the NativeID
+// field.
 type NativeSubscription struct {
 	ID            uuid.UUID          `json:"id"`
 	Name          string             `json:"name"`

diff --git a/pkg/polaris/graphql/errors.go b/pkg/polaris/graphql/errors.go
@@ -25,7 +25,4 @@ import "errors"
 var (
 	// ErrNotFound signals that the specified entity could not be found.
 	ErrNotFound = errors.New("not found")
-
-	// ErrNotUnique signals that a request did not result in a unique entity.
-	ErrNotUnique = errors.New("not unique")
 )
diff --git a/pkg/polaris/graphql/gcp/native.go b/pkg/polaris/graphql/gcp/native.go
@@ -33,7 +33,8 @@ import (
 	"github.com/rubrikinc/rubrik-polaris-sdk-for-go/pkg/polaris/log"
 )
 
-// NativeProject represents a Polaris native project.
+// NativeProject represents a Polaris native project. NativeProjects are
+// connected to CloudAccounts through the NativeID field.
 type NativeProject struct {
 	ID               uuid.UUID          `json:"id"`
 	Name             string             `json:"name"`

diff --git a/pkg/polaris/polaris_test.go b/pkg/polaris/polaris_test.go
@@ -867,6 +867,14 @@ func TestGcpProjectAddAndRemove(t *testing.T) {
 		t.Errorf("invalid number of features: %v", n)
 	}
 
+	// Verify that the Project function does not return a project given a prefix
+	// of the project id.
+	prefix := testProject.ProjectID[:len(testProject.ProjectID)/2]
+	account, err = client.GCP().Project(ctx, gcp.ProjectID(prefix), core.FeatureCloudNativeProtection)
+	if !errors.Is(err, graphql.ErrNotFound) {
+		t.Fatalf("invalid error: %v", err)
+	}
+
 	// Remove GCP project from Polaris keeping the snapshots.
 	err = client.GCP().RemoveProject(ctx, gcp.ID(gcp.Default()), core.FeatureCloudNativeProtection, false)
 	if err != nil {