Azure / bicep-registry-modules

Bicep registry modules
MIT License
460 stars 305 forks source link

[AVM Module Issue]: key-vault/key issue #2758

Closed timdhaeyer closed 3 weeks ago

timdhaeyer commented 1 month ago

Check for previous/existing GitHub issues

Issue Type?

Bug

Module Name

avm/res/key-vault/vault

(Optional) Module Version

0.6.2

Description

There seems to be a discrepancy between the "keysType" type and the actual usage of this in the main.bicep of key-vault. More specifically, look at the "attributes" section. defined like below.

type keysType = {
  @description('Required. The name of the key.')
  name: string

  @description('Optional. Resource tags.')
  tags: object?

  @description('Optional. Contains attributes of the key.')
  attributes: {
    @description('Optional. Defines whether the key is enabled or disabled.')
    enabled: bool?

    @description('Optional. Defines when the key will become invalid. Defined in seconds since 1970-01-01T00:00:00Z.')
    exp: int?

    @description('Optional. If set, defines the date from which onwards the key becomes valid. Defined in seconds since 1970-01-01T00:00:00Z.')
    nbf: int?
  }?
  @description('Optional. The elliptic curve name. Only works if "keySize" equals "EC" or "EC-HSM". Default is "P-256".')
  curveName: ('P-256' | 'P-256K' | 'P-384' | 'P-521')?

  @description('Optional. The allowed operations on this key.')
  keyOps: ('decrypt' | 'encrypt' | 'import' | 'release' | 'sign' | 'unwrapKey' | 'verify' | 'wrapKey')[]?

  @description('Optional. The key size in bits. Only works if "keySize" equals "RSA" or "RSA-HSM". Default is "4096".')
  keySize: (2048 | 3072 | 4096)?

  @description('Optional. The type of the key. Default is "EC".')
  kty: ('EC' | 'EC-HSM' | 'RSA' | 'RSA-HSM')?

  @description('Optional. Key release policy.')
  releasePolicy: {
    @description('Optional. Content type and version of key release policy.')
    contentType: string?

    @description('Optional. Blob encoding the policy rules under which the key can be released.')
    data: string?
  }?

  @description('Optional. Key rotation policy.')
  rotationPolicy: rotationPoliciesType?

  @description('Optional. Array of role assignments to create.')
  roleAssignments: roleAssignmentType?
}[]?

But when calling the key module I see the following.

module keyVault_keys 'key/main.bicep' = [
  for (key, index) in (keys ?? []): {
    name: '${uniqueString(deployment().name, location)}-KeyVault-Key-${index}'
    params: {
      name: key.name
      keyVaultName: keyVault.name
      attributesEnabled: key.?attributesEnabled
      attributesExp: key.?attributesExp
      attributesNbf: key.?attributesNbf
      curveName: (key.?kty != 'RSA' && key.?kty != 'RSA-HSM') ? (key.?curveName ?? 'P-256') : null
      keyOps: key.?keyOps
      keySize: (key.?kty == 'RSA' || key.?kty == 'RSA-HSM') ? (key.?keySize ?? 4096) : null
      releasePolicy: key.?releasePolicy ?? {}
      kty: key.?kty ?? 'EC'
      tags: key.?tags ?? tags
      roleAssignments: key.?roleAssignments
      rotationPolicy: key.?rotationPolicy
    }
  }
]

I would expect to see key.?attributes.enabled for example, and not key.?attributesEnabled.

(Optional) Correlation Id

No response

microsoft-github-policy-service[bot] commented 1 month ago

[!IMPORTANT] The "Needs: Triage :mag:" label must be removed once the triage process is complete!

[!TIP] For additional guidance on how to triage this issue/PR, see the BRM Issue Triage documentation.

avm-team-linter[bot] commented 1 month ago

@timdhaeyer, thanks for submitting this issue for the avm/res/key-vault/vault module!

[!IMPORTANT] A member of the @Azure/avm-res-keyvault-vault-module-owners-bicep or @Azure/avm-res-keyvault-vault-module-contributors-bicep team will review it soon!

AlexanderSehr commented 1 month ago

Hey @timdhaeyer, seems incorrect indeed. The reference to the key child-module expects the information like its provided in the test. Yet the UDT does not reflect that. It's a bit weird that it does not show any errors when passing the information. For example attributesEnabled: key.?attributesEnabled should be attributesEnabled: key.?attributes.?enabled. But it doesn't mind it - maybe due to the try get assertions (?). Then again, if you just type key. I'd expect Bicep to propose properties of the UDT which it doesn't do either.

In any case - it doesn't fit and should be fixed. @fblix can you look into this / at least triage the issue? This must be fixed either by you, or literally anybody else šŸ˜„

cc: @ChrisSidebotham (as you supported @fblix on this last time :) )

microsoft-github-policy-service[bot] commented 1 month ago

[!WARNING] Tagging the AVM Core Team (@Azure/avm-core-team-technical-bicep) due to a module owner or contributor having not responded to this issue within 3 business days. The AVM Core Team will attempt to contact the module owners/contributors directly.

[!TIP]

  • To prevent further actions to take effect, the "Status: Response Overdue šŸš©" label must be removed, once this issue has been responded to.
  • To avoid this rule being (re)triggered, the ""Needs: Triage :mag:" label must be removed as part of the triage process (when the issue is first responded to)!
AlexanderSehr commented 1 month ago

@ChrisSidebotham / @fblix ?

microsoft-github-policy-service[bot] commented 1 month ago

[!WARNING] Tagging the AVM Core Team (@Azure/avm-core-team-technical-bicep) due to a module owner or contributor having not responded to this issue within 3 business days. The AVM Core Team will attempt to contact the module owners/contributors directly.

[!TIP]

  • To prevent further actions to take effect, the "Status: Response Overdue šŸš©" label must be removed, once this issue has been responded to.
  • To avoid this rule being (re)triggered, the ""Needs: Triage :mag:" label must be removed as part of the triage process (when the issue is first responded to)!
microsoft-github-policy-service[bot] commented 1 month ago

[!CAUTION] This issue requires the AVM Core Team's (@Azure/avm-core-team-technical-bicep) immediate attention as it hasn't been responded to within 6 business days.

[!TIP]

  • To avoid this rule being (re)triggered, the "Needs: Triage :mag:" and "Status: Response Overdue :triangular_flag_on_post:" labels must be removed when the issue is first responded to!
  • Remove the "Needs: Immediate Attention :bangbang:" label once the issue has been responded to.
microsoft-github-policy-service[bot] commented 1 month ago

[!WARNING] Tagging the AVM Core Team (@Azure/avm-core-team-technical-bicep) due to a module owner or contributor having not responded to this issue within 3 business days. The AVM Core Team will attempt to contact the module owners/contributors directly.

[!TIP]

  • To prevent further actions to take effect, the "Status: Response Overdue šŸš©" label must be removed, once this issue has been responded to.
  • To avoid this rule being (re)triggered, the ""Needs: Triage :mag:" label must be removed as part of the triage process (when the issue is first responded to)!
microsoft-github-policy-service[bot] commented 1 month ago

[!CAUTION] This issue requires the AVM Core Team's (@Azure/avm-core-team-technical-bicep) immediate attention as it hasn't been responded to within 6 business days.

[!TIP]

  • To avoid this rule being (re)triggered, the "Needs: Triage :mag:" and "Status: Response Overdue :triangular_flag_on_post:" labels must be removed when the issue is first responded to!
  • Remove the "Needs: Immediate Attention :bangbang:" label once the issue has been responded to.
microsoft-github-policy-service[bot] commented 3 weeks ago

[!WARNING] Tagging the AVM Core Team (@Azure/avm-core-team-technical-bicep) due to a module owner or contributor having not responded to this issue within 3 business days. The AVM Core Team will attempt to contact the module owners/contributors directly.

[!TIP]

  • To prevent further actions to take effect, the "Status: Response Overdue šŸš©" label must be removed, once this issue has been responded to.
  • To avoid this rule being (re)triggered, the ""Needs: Triage :mag:" label must be removed as part of the triage process (when the issue is first responded to)!
microsoft-github-policy-service[bot] commented 3 weeks ago

[!CAUTION] This issue requires the AVM Core Team's (@Azure/avm-core-team-technical-bicep) immediate attention as it hasn't been responded to within 6 business days.

[!TIP]

  • To avoid this rule being (re)triggered, the "Needs: Triage :mag:" and "Status: Response Overdue :triangular_flag_on_post:" labels must be removed when the issue is first responded to!
  • Remove the "Needs: Immediate Attention :bangbang:" label once the issue has been responded to.