Add guards against using distinct subscriptions between owner and chi…

…ld resources (#3546)

* Add check to match subscription with owner

* Update test

* Update guids to be in a proper format

* Update guids to be in a proper format

* Fix linter error

* Refactor on requested changes

* Minor test fix

v2/internal/reconcilers/arm/azure_generic_arm_reconciler_instance.go

@@ -293,7 +293,7 @@ func (r *azureDeploymentReconcilerInstance) BeginCreateOrUpdateResource(
 
 	resourceID := genruntime.GetResourceIDOrDefault(r.Obj)
 	if resourceID != "" {
-		err = checkSubscription(resourceID, r.ARMConnection.SubscriptionID())
+		err = r.checkSubscription(resourceID)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
@@ -303,7 +303,6 @@ func (r *azureDeploymentReconcilerInstance) BeginCreateOrUpdateResource(
 	if err != nil {
 		return ctrl.Result{}, err
 	}
-
 	// Use conditions.SetConditionReasonAware here to override any Warning conditions set earlier in the reconciliation process.
 	// Note that this call should be done after all validation has passed and all that is left to do is send the payload to ARM.
 	conditions.SetConditionReasonAware(r.Obj, r.PositiveConditions.Ready.Reconciling(r.Obj.GetGeneration()))
@@ -369,13 +368,15 @@ func (r *azureDeploymentReconcilerInstance) preReconciliationCheck(ctx context.C
 	return check, nil
 }
 
-func checkSubscription(resourceID string, clientSubID string) error {
+// checkSubscription checks if subscription on resource matches with credentials used while creating a resource.
+// Which prevents users to modify subscription in their credential.
+func (r *azureDeploymentReconcilerInstance) checkSubscription(resourceID string) error {
 	parsedRID, err := arm.ParseResourceID(resourceID)
 	// Some resources like '/providers/Microsoft.Subscription/aliases' do not have subscriptionID, so we need to make sure subscriptionID exists before we check.
 	// TODO: we need a better way?
 	if err == nil {
-		if parsedRID.ResourceGroupName != "" && parsedRID.SubscriptionID != clientSubID {
-			err = errors.Errorf("SubscriptionID %q for %q resource does not match with Client Credential: %q", parsedRID.SubscriptionID, resourceID, clientSubID)
+		if parsedRID.ResourceGroupName != "" && parsedRID.SubscriptionID != r.ARMConnection.SubscriptionID() {
+			err = errors.Errorf("SubscriptionID %q for %q resource does not match with Client Credential: %q", parsedRID.SubscriptionID, resourceID, r.ARMConnection.SubscriptionID())
 			return conditions.NewReadyConditionImpactingError(err, conditions.ConditionSeverityError, conditions.ReasonSubscriptionMismatch)
 		}
 	}
@@ -826,7 +827,7 @@ func (r *azureDeploymentReconcilerInstance) deleteResource(
 		return ctrl.Result{}, nil
 	}
 
-	err := checkSubscription(resourceID, r.ARMConnection.SubscriptionID()) // TODO: Possibly we should pass this in as a parameter?
+	err := r.checkSubscription(resourceID)
 	if err != nil {
 		return ctrl.Result{}, err
 	}

v2/internal/resolver/resolver_test.go

@@ -7,6 +7,7 @@ package resolver_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -89,6 +90,9 @@ func createResourceGroup(name string) *resources.ResourceGroup {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
 			Namespace: testNamespace,
+			Annotations: map[string]string{
+				genruntime.ResourceIDAnnotation: fmt.Sprintf("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/%s", name),
+			},
 		},
 		Spec: resources.ResourceGroup_Spec{
 			Location:  to.Ptr("West US"),

v2/internal/resolver/resource_hierarchy.go

@@ -142,6 +142,24 @@ func (h ResourceHierarchy) fullyQualifiedARMIDImpl(subscriptionID string, origin
 			return "", err
 		}
 
+		root := h[0]
+
+		err = genruntime.VerifyResourceOwnerARMID(root)
+		if err != nil {
+			return "", err
+		}
+
+		// Safe to do it this way, Claimer makes sure the owner exists and is Ready and will always have an armId annotation before we reach here.
+		ownerARMID, err := genruntime.GetAndParseResourceID(root)
+		if err != nil {
+			return "", err
+		}
+
+		// Confirm that the subscription ID the user specified matches the subscription ID we're using from our credential
+		if ok := genruntime.CheckARMIDMatchesSubscription(subscriptionID, ownerARMID); !ok {
+			return "", core.NewSubscriptionMismatchError(ownerARMID.SubscriptionID, subscriptionID)
+		}
+
 		// Ensure that we have the same number of names and types
 		if len(remainingNames) != len(resourceTypes) {
 			return "", errors.Errorf(
@@ -198,17 +216,14 @@ func (h ResourceHierarchy) fullyQualifiedARMIDImpl(subscriptionID string, origin
 			return "", err
 		}
 
-		armID, err := arm.ParseResourceID(armIDStr)
+		ownerARMID, err := arm.ParseResourceID(armIDStr)
 		if err != nil {
 			return "", err
 		}
-		// armIDSub may be empty if there is no subscription in the user specified ARM ID (for example because the resource roots
-		// at the tenant level)
-		if armID.SubscriptionID != "" {
-			// Confirm that the subscription ID the user specified matches the subscription ID we're using from our credential
-			if !strings.EqualFold(armID.SubscriptionID, subscriptionID) {
-				return "", core.NewSubscriptionMismatchError(armID.SubscriptionID, subscriptionID)
-			}
+
+		// Confirm that the subscription ID the user specified matches the subscription ID we're using from our credential
+		if ok := genruntime.CheckARMIDMatchesSubscription(subscriptionID, ownerARMID); !ok {
+			return "", core.NewSubscriptionMismatchError(ownerARMID.SubscriptionID, subscriptionID)
 		}
 
 		// Rooting to an ARM ID means that some of the resourceTypes may not actually be included explicitly in our