Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
yks0000 opened this issue Jun 24, 2022 · 8 comments

Comments

@yks0000
Copy link

yks0000 commented Jun 24, 2022

Bicep version
Bicep CLI version 0.7.4 (5afc312)

Describe the bug

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

Seeing some old 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
Copy link
Collaborator

brwilkinson commented Jun 29, 2022

@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
Copy link
Collaborator

@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.

  • 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?
    • Unless you specifically had a reason to manually provide SANs? I am not sure which scenario that would be for?
  1. The resource itself will automatically determine the SANS subjectAlternativeNames from the certificate, without providing those values at deployment time?
  2. 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
Copy link
Author

yks0000 commented Jun 30, 2022

@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
Copy link
Collaborator

brwilkinson commented Jun 30, 2022

@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
Copy link
Collaborator

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
Copy link
Collaborator

Action items:

  • reach out to keyvault team to get them to publish swagger for Microsoft.KeyVault/vaults/certificates
  • open what-if noise issue for microsoft.cdn (2 what-if examples in comments above)

@bkane-msft
Copy link

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

@JBalhatchet
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants