Azure / azure-service-operator

Azure Service Operator allows you to create Azure resources using kubectl
https://azure.github.io/azure-service-operator/
MIT License
777 stars 202 forks source link

Bug: remove subscription check between resource/owner for DNS records #3754

Open t3mi opened 9 months ago

t3mi commented 9 months ago

Version of Azure Service Operator 2.4.0

Describe the bug It's possible for clients to segregate management and location of the DNS zones to separate subscriptions. New alias records could be easily created to point to the azure resources in subscriptions other then the one containing DNS zone using Azure Portal with least privilege access. But current case is not working with ASO.

To Reproduce Try to create any alias DNS record in different subscription from where ASO is running using armId value to point to the owner (DNS Zone) in different subscription and dummy traffic manager profile, e.g.

# traffic manager profile create in Subscription-1
# ASO is running in AKS in Subscription-1
# DNS Zone is located in Subscription-2
apiVersion: network.azure.com/v1api20180501storage
kind: DnsZonesCNAMERecord
metadata:
  name: dummy-record
spec:
  owner:
    armId: >-
      /subscriptions/<Subscription-2>/resourceGroups/resourceGroup/providers/Microsoft.Network/dnszones/example.com
  targetResource:
    reference:
      group: network.azure.com
      kind: TrafficManagerProfile
      name: dummy-traffic-manager-profile
  TTL: 3600

Expected behavior Record should be created successfully.

Current behavior

resource subscription "<guid>" does not match parent subscription "<<guid>>"
matthchr commented 9 months ago

Do you by chance have a link to any documentation (or example ARM templates) that do the above?

I wasn't aware that DNSZones supported this pattern of creating a record in a different subscription than the DNSZone subscription.

t3mi commented 9 months ago

It seems such case isn't covered in quickstart templates but it's pretty simple - just use full ARM ID for target resource

{
    "type": "Microsoft.Network/dnszones/A",
    "apiVersion": "2018-05-01",
    "name": "example.com/example",
    "dependsOn": [
        "[resourceId('Microsoft.Network/dnszones', 'example.com')]"
    ],
    "properties": {
        "TTL": 3600,
        "targetResource": {
            "id": "/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/Development/providers/Microsoft.Network/publicIPAddresses/demogwpip"
        }
    }
}

Here are step by step docs how to do that in UI:

t3mi commented 9 months ago

Just re-read and it seems I've confused you with the wording - it's not that dns record could be created in separate subscription from the dns zone but it could target a resource in different subscription using alias type. Along with that, ASO could be running in resource subscription but provided an access to create dns records in dns zone hosted in different subscription. So it seems for ASO - instead of inheriting subscription of the owner, it uses credentials of the identity.. Hope I didn't confused you more :)

matthchr commented 9 months ago

I think I understand your problem now.

Along with that, ASO could be running in resource subscription but provided an access to create dns records in dns zone hosted in different subscription. So it seems for ASO - instead of inheriting subscription of the owner, it uses credentials of the identity..

Have you read through our documentation on credential scope? You're right that there's a credential <-> subscription mapping, and ASO enforces that mapping with the error you're seeing code here.

That's expected though: The solution to the problem is to either:

  1. Define a namespace scope credential for namespace2, which maps to subscription2. Create the DNSZone A record in namespace2. This will use the subscripiton2 credential that matches subscription2 in the ARM ID of the A record and all is good. If having a whole namespace dedicated to subscription2 resources makes sense in your model great, although from what you've said above it may not - possibly your namespaces map onto logical applications or environments that cross subscription boundaries. If so, use the next option.
  2. Define a resource-scoped credential for subscription2 , and use the credential-from annotation on the DNSZone A record to point at the subscription2 credential.

Note that, the actual credential (ClientID, etc) can be the same in both secrets, just with a different subscription if the same credential has access to both subscriptions, though we recommend that you use different credentials for different subscriptions just from a security posture perspective.

This error exists for two reasons:

  1. Legacy - initially ASO didn't support using armId for parent references. That wasn't added until ~2.4.0. Prior to that support, there had to be something that said "what subscription are you deploying this resource into" and that always came from the secret. So if you wanted to manage 2 subs you have to have 2 secrets. We maintain this experience even with resources owned by an armId.
  2. To encourage security best-practices by ensuring users are explicit about their credentials and their access. The hope is that this helps avoid uber-credentials with high levels of access to multiple subscriptions and instead encourages either namespace or resource scoped credentials that are more tightly scoped.
matthchr commented 9 months ago

Based on your thumbs up I'm assuming the above helps you resolve your issue. I'm going to close this issue on that assumption. Feel free to reopen this if I'm incorrect.

t3mi commented 5 months ago

@matthchr I'd like this to be reopened as it complicates automation process from the platform team/end user perspective for our use case. We're using single ASO identity per cluster. ASO bootstrap with role assignments are done by platform team during bootstrap time limiting identity access to selected resource groups in subscription controlled by platform team with some custom permissions for shared resources like DNS zone residing in another subscription controlled by different team. While your suggestion works but to have it fully automated there is a need in some admission controller which will generate multiple secrets per each subscription for each existing/new namespace for the same ASO client ID. Instead, it would be much simpler if a check for subscription match could be made as opt-out feature. WDYT?

matthchr commented 4 months ago

Reopened this, we will discuss.

theunrepentantgeek commented 4 months ago

We'll put design of this into the v2.9.0 release, as we think there are a bunch of fiddly details to ensure we get right.

matthchr commented 4 months ago

Thoughts that we discussed:

Not committing to doing any of the above yet but we'll discuss it.