Azure / bicep

Bicep is a declarative language for describing and deploying Azure resources
MIT License
3.25k stars 753 forks source link

Resource Type to get existing certificates from Microsoft.KeyVault is missing #7354

Open yks0000 opened 2 years ago

yks0000 commented 2 years ago

Bicep version Bicep CLI version 0.7.4 (5afc312467)

Describe the bug

There is no resource type available to query existing certificate from Microsoft.KeyVault/vaults.

Seeing some old issues https://github.com/Azure/bicep/issues/5630, it seems Microsoft.KeyVault/vaults/certificates@2019-09-01 was present. Is it replaced by something else?

May not be related but I do not see any alias here in PSRule.Rules.Azure to get Certificate information such as SAN etc, so most likely it never exists or removed.

Basically, we are using existing Custom Certificate in Azure FD CDN from KeyVault. We want to get secretVersion and subjectAlternativeNames from it and pass to Microsoft.Cdn/profiles/secrets@2021-06-01, or otherwise we need to hard code these values to avoid deployment drifts. Updating these two properties manually would be additional step to update Azure CDN Bicep templates whenever there is latest version of Certificate available. As we set useLatestVersion: true, not getting this value in runtime or not updating bicep with these value, will result in drift between Bicep and deployed config whenever we try to run deployment.

Though this drift which say one resource to modify does not do anything, but it looks confusing and not desirable.

Example:

  ~ Microsoft.Cdn/profiles/static-stg/secrets/cdn-byoc-certificate [2021-06-01]
    - properties.parameters.secretVersion: "528c4xxxxxxxxxxxxxxx5b2f6a820"
    - properties.parameters.subjectAlternativeNames: [
        0: "static.example.com"
        1: "static-azur.example.com"
        2: "static-azur-stg.example.com"
        3: "static-exp1.example.com"
        4: "static-exp2.example.com"
        5: "static-exp3.example.com"
        6: "static-afd.example.com"
        7: "static-afd-stg.example.com"
        8: "static-afd-test.example.com"
      ]

Resource changes: 1 to modify, 23 no change, 3 to ignore.

OR

if useLatestVersion is set to true, deployment should not show changes available for modify wrt secretVersion and subjectAlternativeNames

To Reproduce

Create a file cdn.bicep:


@sys.description('KeyVault name for Custom certificate for custom domains')
param keyVaultName string

@sys.description('KeyVault name Subscription Id')
param keyVaultSubscriptionId string

@sys.description('KeyVault name resource group name')
param keyVaultResourceGroup string

@sys.description('KeyVault secret name for custom domain')
param keyVaultSecretName string

var secretSource = '/subscriptions/${keyVaultSubscriptionId}/resourceGroups/${keyVaultResourceGroup}/providers/Microsoft.KeyVault/vaults/${keyVaultName}/secrets/${keyVaultSecretName}'

@sys.description('Attach Custom Certificate from Key Vault')
resource cdn_custom_cert_secret 'Microsoft.Cdn/profiles/secrets@2021-06-01' = {
  parent: cdn_profile
  name: 'cdn-byoc-certificate'
  properties: {
    parameters: {
      type: 'CustomerCertificate'
      secretSource: {
        id: secretSource
      }
      useLatestVersion: true
    }
  }
}

If we run this, it will always show Resource changes: 1 to modify which is change in properties.parameters.secretVersion and properties.parameters.subjectAlternativeNames

brwilkinson commented 2 years ago

@yks0000 are you defining Microsoft.Cdn/profiles/customDomains in your deployment?

https://github.com/brwilkinson/AzureDeploymentFramework/blob/main/ADF/bicep/FD.CDN-Profiles-AFDEP-EP.bicep#L53

In the above I am using ManagedCertificates, however If I replace the secret id, with a keyvault secret reference as part of the CustomDomain, I believe that should work? CDN will create the secret resource for you, so you never have to define it in your template?

      // secret: {
      //   id: 
      // }

here is the doc reference:

https://docs.microsoft.com/en-us/azure/templates/microsoft.cdn/profiles/customdomains?tabs=bicep#afddomainhttpsparameters https://docs.microsoft.com/en-us/azure/templates/microsoft.cdn/profiles/customdomains?tabs=bicep#resourcereference

Update Okay, I checked again and the Id is the secret reference id. not the kv secret id.

image

I will have to test this scenario some more.

brwilkinson commented 2 years ago

@yks0000 okay I completed testing with the whatif etc.

// What the resource has for the definition.

resource TestABC 'Microsoft.Cdn/profiles/secrets@2021-06-01' = {
  name: 'acu1brwaoat5cdnshared01/TestABC'
  properties: {
    parameters: {
      type: 'CustomerCertificate'
      secretSource: {
        id: '/subscriptions/b8f402aa-20f7-4888-b45c-3cf086dad9c3/resourcegroups/ACU1-BRW-AOA-RG-P0/providers/Microsoft.KeyVault/vaults/acu1-brw-aoa-p0-kvvlt01/secrets/TestABC'
      }
      secretVersion: 'f1defaaba183415b993f969a3dfb4da1'
      useLatestVersion: true
      subjectAlternativeNames: [
        'TestABC.contoso.com'
        'TestABC2.contoso.com'
        'TestABC3.contoso.com'
      ]
    }
  }
}

// The minimum that you actually want to deploy and define in your template.

resource TestABC 'Microsoft.Cdn/profiles/secrets@2021-06-01' = {
  name: 'acu1brwaoat5cdnshared01/TestABC'
  properties: {
    parameters: {
      type: 'CustomerCertificate'
      useLatestVersion: true  
      secretSource: {
        id: '/subscriptions/b8f402aa-20f7-4888-b45c-3cf086dad9c3/resourcegroups/ACU1-BRW-AOA-RG-P0/providers/Microsoft.KeyVault/vaults/acu1-brw-aoa-p0-kvvlt01/secrets/TestABC'
      }
    }
  }
}

the difference that whatif gives you, which should not be listed.

  ~ Microsoft.Cdn/profiles/acu1brwaoat5cdnshared01/secrets/TestABC [2021-06-01]
    - properties.parameters.secretVersion:              "f1defaaba183415b996f969a3dfb4da1"
    x properties.parameters.subjectAlternativeNames[0]: "TestABC.contoso.com"
    x properties.parameters.subjectAlternativeNames[1]: "TestABC2.contoso.com"
    x properties.parameters.subjectAlternativeNames[2]: "TestABC3.contoso.com"

Will add triage back on this to review.

This appears to fit more as a provider bug... which is reflected in the usage of Whatif.

1) The resource itself will automatically determine the SANS subjectAlternativeNames from the certificate, without providing those values at deployment time? 1) When you are using the useLatestVersion then the secretVersion should not be provided?

--

Certificate resource type is missing all together, so I think really there are two distinct issues to follow up on.

Some other reasons for adding the Certificate capability would be:

yks0000 commented 2 years ago

@brwilkinson Thank you for checking this. Agreed that whatIf and managing KeyVault Certificate is two different issue.

Do you want me to open another issue for KeyVault Certificate resource provider not being available?

My view on question raised on whatIf:

I am not sure that creating a workaround for this by looking up the values for SANs from the Certificate or the latest version of the certificate/secret would be a good idea?

If useLatestVersion is set to True, we should ideally not being checking this.

Unless you specifically had a reason to manually provide SANs? I am not sure which scenario that would be for?

This is actually not required as we The resource itself will automatically determine the SANS subjectAlternativeNames from the certificate, without providing those values at deployment time?

True, providing a list of SANs is not useful as resource Microsoft.Cdn/profiles/secrets is only for reference and attaching existing certificate and does not really modify or update certificate.

When you are using the useLatestVersion then the secretVersion should not be provided?

I tested this, we can not provide secretVersion when using useLatestVersion. The deployment provider pre-flight check error out if both are present in ARM.

brwilkinson commented 2 years ago

@yks0000 I think we have enough to go on here, thank you.

These 2 types of changes will take some time, since it's up to each of the 2 teams to implement or update the API specifications Etc.

In this case the: 1) provider bug/API specification issue is the CDN team 2) the Certificates resource type is owned by the KeyVault team.

Thanks again for bringing these gaps to our attention.

brwilkinson commented 2 years ago

Adding another related whatif scenario from:

Minimum input for using a platform Managed Certificate.

resource customDomain 'Microsoft.Cdn/profiles/customDomains@2021-06-01' = {
  name: 'acu1brwaoat5cdnshared01/acu1brwaoat5sadata1-psthing-com'
  properties: {
    hostName: 'acu1brwaoat5sadata1.psthing.com'
    tlsSettings: {
      certificateType: 'ManagedCertificate'
      minimumTlsVersion: 'TLS12'
    }
  }
}

whatif difference - secret id shows as a change however is an optional property.

  ~ Microsoft.Cdn/profiles/acu1brwaoat5cdnshared01/customDomains/acu1brwaoat5sadata1-psthing-com [2021-06-01]
    - properties.tlsSettings.secret:

        id: "/subscriptions/b8f402aa-20f7-4888-b45c-3cf086dad9c3/resourceGroups/ACU1-BRW-AOA-RG-T5/providers/Microsoft.Cdn/Profiles/acu1brwaoat5cdnshared01/secrets/5e7ee10a-2f85-45b4-827f-df249be674b7-acu1brwaoat5sadata1-psthing-com"
alex-frankel commented 2 years ago

Action items:

bkane-msft commented 1 year ago

@alex-frankel , is there any progress on this?

JBalhatchet commented 3 months ago

Has there been any progress on this? Is any work for it planned?