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

Union() issues with 0.25.3 #13247

Closed
cedricbraekevelt opened this issue Feb 7, 2024 · 8 comments · Fixed by #13272
Closed

Union() issues with 0.25.3 #13247

cedricbraekevelt opened this issue Feb 7, 2024 · 8 comments · Fixed by #13272
Assignees
Labels
bug Something isn't working type system

Comments

@cedricbraekevelt
Copy link

Bicep version
0.25.3

Describe the bug
We use the Union() function a lot to merge a resource specific naming object with a general naming for all resources. Since 0.25.3 this gives us an error for all properties that aren't optional as you can see below.

To Reproduce

@export()
type naming = {
  @description('Override the abbreviation of this resource with this parameter')
  abbreviation: string?
  @description('The resource environment (for example: dev, tst, acc, prd)')
  environment: string?
  @description('The resource location (for example: weu, we, westeurope)')
  location: string?
  @description('The name of the customer')
  customer: string?
  @description('The delimiter between resources (default: -)')
  delimiter: string?
  @description('The order of the array defines the order of elements in the naming scheme')
  nameFormat: ('abbreviation' | 'function' | 'environment' | 'location' | 'customer' | 'param1' | 'param2' | 'param3')[]?
  @description('Extra parameter self defined')
  param1: string?
  @description('Extra parameter self defined')
  param2: string?
  @description('Extra parameter self defined')
  param3: string?
  @description('Full name of the resource overwrites the combinated name')
  overrideName: string?
  @description('Function of the resource [can be app, db, security,...]')
  function: string
  @description('Suffix for the resource, if empty non will be appended, otherwise will be added to the end [can be index, ...]')
  suffix: string?
}

param defaultNaming naming

param resourceNaming naming

param test naming = union(defaultNaming, resourceNaming)

The error that we receive in this case is:

The specified "object" declaration is missing the following required properties: "function".bicep(BCP035)

As you can see function is the only required property in this user defined object, and is the one causing trouble. However, if I require other properties, they also add to the list.

@jeskew jeskew self-assigned this Feb 7, 2024
@jeskew jeskew added bug Something isn't working type system and removed Needs: Triage 🔍 labels Feb 7, 2024
@jeskew
Copy link
Contributor

jeskew commented Feb 7, 2024

As a quick workaround on this if you need to unblock a build, the any() function will get you past the BCP035 diagnostic:

param test naming = any(union(defaultNaming, resourceNaming))

Working on fixing type inference for the union function, but it's a bit more complex than anticipated.

@pie-r
Copy link

pie-r commented Feb 8, 2024

Same here, we fixed it using the previous version 0.24.x to avoid this not backward compatible change.

@vlahane Can you share the other issue that we have? If is not related we should open a different github issue

@vlahane
Copy link
Contributor

vlahane commented Feb 8, 2024

seems like similar issue but without union as well as with union function.
resouces.bicep(7,5) : Error BCP035: The specified "object" declaration is missing the following required properties: "objectId".

Below are the simplified tests for existing bicep our code.

Test-1

bicep build result

0.25.3 (8627085) : Passed ✅
0.24.24 (5646341) : Passed ✅

//resource.bicep
param location string
param accessPolicies object[]

resource akv 'Microsoft.KeyVault/vaults@2023-07-01' = {
  location: location
  name: 'myakv'
  properties: {
    sku: {
      family: 'A'
      name: 'standard'
    }
    tenantId: 'xxxxxxxx-86xx-xxxx-91xx-xx7cd011dxxx'
    accessPolicies: accessPolicies
  }
}

Test-2

bicep build result

0.25.3 (8627085) : Failed ❌
0.24.24 (5646341) : Passed ✅

//dependencies.bicep
type accessPoliciesObj = {
  objectId: string
  permissions: object?
}

param location string
param accessPolicies accessPoliciesObj[]

resource akv 'Microsoft.KeyVault/vaults@2023-07-01' = {
  location: location
  name: 'myakv'
  properties: {
    sku: {
      family: 'A'
      name: 'standard'
    }
    tenantId: 'xxxxxxxx-86xx-xxxx-91xx-xx7cd011dxxx'
    accessPolicies: accessPolicies
  }
}

//resource.bicep
//hint: looks like not able to typecast to accessPoliciesObj type in new bicep version i.e. 0.25.3
param accessPolicies object[] 


module test 'dependencies.bicep' = {
  name: 'test-module'
  params: {
    accessPolicies: accessPolicies
    location: 'eastus'
  }
}

error in Test-2 and we have been using the same code in our project

resouces.bicep(7,5) : Error BCP035: The specified "object" declaration is missing the following required properties: "objectId".

@slavizh
Copy link
Contributor

slavizh commented Feb 8, 2024

+1 this is a big bug.

@miqm
Copy link
Collaborator

miqm commented Feb 8, 2024

please make sure it'll work with map:

map(items(_object), x=> union({ id: x.key }, x.value))

anthony-c-martin pushed a commit that referenced this issue Feb 9, 2024
…s an additional properties type (#13272)

Resolves #13247 

The type narrowing on objects introduced in #12798 will erroneously
raise a missing property diagnostic if the assigned value does not have
an explicit property defined for each of the target's required
properties, even if the assigned value allows additional properties.
This PR updates the type narrowing to consider required properties not
to be missing if the assigned value allows arbitrary additional
properties.
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/13272)
@vlahane
Copy link
Contributor

vlahane commented Feb 9, 2024

shouldn't we add support for explicit type casting as well in future? something like below?

import { accessPoliciesObjType } from './types.bicep'
param myPolicies object[]

module test 'dependencies.bicep' = {
  name: 'test-module'
  params: {
    accessPolicies: accessPoliciesObjType(myPolicies) //explicit type casting
    location: 'eastus'
  }
}

@timkatje
Copy link

Our Azure DevOps Pipeline started popping this error today after no changes to the bicep files. We figured there'd been an update that broke existing behavior.

For us, we simply read in a list of tags on the containing resource group and union them with a locally defined list of tags. Previously, this code worked:
tags: union(resourceGroup().tags, tags)

All the mention of "types" in this issue clued me in on what to look for. Knowing that there's a ToObject function available to us, I decided to test appending that to the existing code, and now our main.bicep lints passes linting:
tags: union(resourceGroup().tags, tags).ToObject

Interestingly, we use this exact tag union code three different times in our main.bicep, but only the first two uses errored out. The two erroring modules are two version newer than the non-failing module, so I expect that upon review, we'll find some defect in those modules that this error has brought to our attention.

TLDR: Using the ToObject() function may resolve this while awaiting a new version of bicep.

@anthony-c-martin
Copy link
Member

@timkatje we've just released v0.23.53 which should have the fix. Could you verify which version of Bicep you are using?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working type system
Projects
Archived in project
8 participants