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

Do *something* to make it easier to deal with resource types & api versions #622

Open
alex-frankel opened this issue Oct 9, 2020 · 32 comments
Labels
enhancement New feature or request

Comments

@alex-frankel
Copy link
Collaborator

alex-frankel commented Oct 9, 2020

The "type" of an resource is a relatively complicated and long string that nobody likes. E.g.:

'microsoft.compute/virtualMachines@2020-06-01'

Most new ARM Template/Bicep users don't understand the concept of resource providers and they don't know which apiVersion is the "right" one to use. It doesn't mean these things are not important, but we don't want to confuse them with information they may not need at this point.

We've already received a bunch of issues with a variety of proposals to make this type string easier to manage. I'm going to collect them all here and if/when we have a proposal for this, we will post it here first to get feedback.

Goals:

  • offer a way to declare "simple" types that don't require the apiVersion to be explicitly declared (possibly aspects of the resource type as well via an alias (i.e. 'microsoft.compute/virtualMachines' => virtualMachines or vm)
    • it should still be easy to figure out which apiVersion and full rp info if I need that level of detail
  • continue to expose apiVersion when it's important. If a type comes out with a new apiVersion I should not be blocked from consuming it.
  • offer safe upgrades. I shouldn't be surprised that I am consuming a new apiVersion

Non-goals:

  • Offer a "latest"/"auto upgrade" semantic. I think as a team we are philosophically against this. ARM templates are very stable today because of the explicit apiVersion. As long as the relevant resource type follows the rules of the ARM RPC, a working template should continue to work for as long as those APIs continue to exist.

Why this is tricky to get right

Taken from #1002 (comment);

Some things to point out about API versioning in ARM:

  • API version is not a property of a resource - it defines the contract for interacting with a resource. Generally if you haven't changed any of the properties you supply, the effect on the underlying resource state will be identical between the older and newer version, unless the newer API contract has a breaking change (next point). Although it can feel odd to be using a very old API version, there is usually no great need to update unless for example you need to access some new properties added to a newer version.
  • Teams are permitted to make breaking changes to the structure of the API contract between API versions. They are not permitted to make breaking changes to the structure of the API contract for an existing API version. This may mean that if you upgrade versions without changing the structure of your request, you run the risk of being unexpectedly broken at deploy time.
  • Without having a known API version at authoring time, we cannot provide a great editor experience (type validation, intellisense etc), as we cannot know what properties are available.

So generally, our feeling on this is that we want to discourage people from picking up the API version at runtime and instead encourage pinning to a version and making the explicit decision to upgrade. That being said, we definitely see the need to simplify the syntax, know whether you're on the latest, and have an easy path to upgrade if you're not and want to be - currently we're thinking of addressing this with #622.

Current issues:

@alex-frankel alex-frankel added the enhancement New feature or request label Oct 9, 2020
@ghost ghost added the Needs: Triage 🔍 label Oct 9, 2020
@SenthuranSivananthan
Copy link

+1 for explicit versioning. Auto upgrading to a newer versions could break the template or worse introduce different service behavior (i.e. default values) that wasn't anticipated. I think a way to use What-If to compare 2 api versions would be helpful and then DevOps teams can decide whether to bump their version or not.

@DDSoftDennie
Copy link

+1. It is complicated for beginners.

@MarcusFelling
Copy link
Collaborator

Ready to start on PM portion. UX research needed.

@rynowak
Copy link
Contributor

rynowak commented Feb 1, 2021

One of the ideas that appeals to me would be to support aliases for a combined type/version via an import.

This would functionally look-like a set of library features, and potentially could be generated or extensible via proper libraries in the future.

Example:

import microsoft.compute@2020-06-01 as compute

resource vm compute.virtualMachines = {
  ...
}

The key point here would be that compute.virtualMachines has to map to some constant value ('microsoft.compute/virtualMachines@2020-06-01') that can be evaluated inside the compiler for type-checking purposes.

This could grow up to support (for example) definition of types in native bicep as a redistributable library.

@miqm
Copy link
Collaborator

miqm commented Mar 16, 2021

@rynowak like your idea, but I've a question - is there possibility for a child resource type not to have api version that a parent have?

@majastrz
Copy link
Member

Yup, that is definitely possible.

@alex-frankel
Copy link
Collaborator Author

my concern with requiring imports is it just moves the complexity to the top of the file. I think in 99% of cases, user's don't need/want to worry about this level of detail, they simply "want a VM", so I think we need to have a set of defaults or something like that. What about something like:

resource myVm virtualMachine = { ... }

This would map to some static mapping that user's could check by hovering on the type alias or "Go to definition". If user's need to specify a different version they could still do that:

resource myVm virtualMachine@2020-01-01 = { ... }

Because I am not specifying the full type, I think there are possible ambiguities if there are two RPs that have the same resource type name. I know @majastrz has mentioned there are multiple cases of that. I wonder if we should resolve those on a case-by-case basis and choose the more popular one to have the shorter name.

@SenthuranSivananthan
Copy link

my concern with requiring imports is it just moves the complexity to the top of the file. I think in 99% of cases, user's don't need/want to worry about this level of detail, they simply "want a VM", so I think we need to have a set of defaults or something like that. What about something like:

resource myVm virtualMachine = { ... }

This would map to some static mapping that user's could check by hovering on the type alias or "Go to definition". If user's need to specify a different version they could still do that:

resource myVm virtualMachine@2020-01-01 = { ... }

Because I am not specifying the full type, I think there are possible ambiguities if there are two RPs that have the same resource type name. I know @majastrz has mentioned there are multiple cases of that. I wonder if we should resolve those on a case-by-case basis and choose the more popular one to have the shorter name.

+1 on this approach since it's inline with the current experience.

Questions:

  • How would the static version be selected (there are preview and non preview API versions)?
  • How often will the versions be changed?
  • There could be breaking changes when moving up a version that can break existing scripts and explicit versioning protects against this.

@miqm
Copy link
Collaborator

miqm commented Mar 16, 2021

We could solve it as it's in C# - the top-of-the-file usings are often added by IDE you just type virtualMachines and the using you add with help of quick fixes. You can also use FQDN if you like.

Name clashes happens often, so in C# you can either use FQDN or provide an alias for that type to avoid it:
This would not work:

using Microsoft.ServiceBus/namespaces@version
using Microsoft.Storage/storageAccounts/queueServices@version

resource queue queues { //error here on queues, as it's ambiguous
}

but these would:

using Microsoft.ServiceBus/namespaces@version
using Microsoft.Storage/storageAccounts@version

resource sbQueue queues {
}
resource storageQueue queueServices/queues {
}

or:

using Microsoft.ServiceBus/namespaces@version
using storageQueues=Microsoft.Storage/storageAccounts/queueServices/queues@version

resource sbQueue queues {
}
resource storageQueue storageQueues {
}

I'd keep / as namespace separator - a) to be consistent with reference b) just recently we decided to use :: for parent-child relation not to mix up with namespace separation sign: / :)

As for child API versions, we don't require them in ARM and here as well when we define using nesting, so I think this should be acceptable (use 'parent' namespace API version). If there will be a problem, you could narrow down 'namespace' with different api (even by using alias to use various types) or override the API in the resource declaration.

With good IDE support, when you start typing 'virtualMachines' and choose the proper one from dropdown, the using Microsoft.Compute@version should be added automatically on top of the file and you will even not notice it.

@anthony-c-martin
Copy link
Member

anthony-c-martin commented Mar 16, 2021

my concern with requiring imports is it just moves the complexity to the top of the file. I think in 99% of cases, user's don't need/want to worry about this level of detail, they simply "want a VM", so I think we need to have a set of defaults or something like that.

I don't have any great solutions, but I want to +1 this comment @alex-frankel. To me it doesn't feel like an in-file using syntax really addresses the complexity - it merely shifts it to another part of the file.

This is a bit of a half-baked thought, but In my mind I have something like a combination of #893 & #444 to allow constant type definitions in the same file or in another file which can be easily referenced and allow people to essentially build their own equivalent of the using statement if they want to - e.g.:

var types = import('./types.bicep')

resource sbQueue types.serviceBusQueues {
  ...
}
resource storageQueue types.storageQueues {
  ...
}

Or just:

var types = {
  serviceBusQueues: 'Microsoft.ServiceBus/namespaces@version'
  storageQueues: 'Microsoft.Storage/storageAccounts/queueServices/queues@version'
}

resource sbQueue types.serviceBusQueues {
  ...
}
resource storageQueue types.storageQueues {
  ...
}

The other thing I'd like to see is switching from strings to something which can be more easily composed (such as object references):

var vmType = Microsoft.Compute/virtualMachines
// having the ability to 'dot' into child types would be nice
var extensionType = vmType.extensions

@alex-frankel
Copy link
Collaborator Author

Another idea would be to still force user's to pick a version for a resource type, but map it to a semantic version instead of a date string. As we've discussed before, user's get confused by the date string and associate "old" dates with out-of-date versions which is not necessarily the case:

resource myVm virtualMachine@1 = { ... }

This borrows from how GitHub actions is handling versioning. It avoids "surprise" breaking changes by not requiring a "upgrade-everything-at-once" model.

@miqm
Copy link
Collaborator

miqm commented Mar 16, 2021

my concern with requiring imports

I think we shouldn’t require them, only allow them. Usually when we write bicep module files, we circle around only few providers, trying to separate parts from each other, so the amount shouldn’t be that high. Even simple using Microsoft.Web will reduce a lot of code. Plus, we have now in main the nested child resource definitions which reduces type boilerplate significantly.

removing the string part would definitely be helpful although it’s just semantics that intellisense will support devs with.

I wouldn’t use var keyword for types - it’s too much similar to objects and would be super confusing.

having to maintain one file with types, especially when you work with few devs on one projects will lead to lots of conflicts on merging. I’d rather stay with top of the file as it’s less common for 2 devs working on same area. Plus we can fold it in editor and I really think common case we’ll be to have up to 10 imports/usings per file.

@miqm
Copy link
Collaborator

miqm commented Mar 16, 2021

Another idea would be to still force user's to pick a version for a resource type, but map it to a semantic version instead of a date string.

That would be a killer when it would come do looking for the details in the docs, to align with cli/powershell rest command, resources.azure.com, and for people transitioning from ARM.

Also, I don't see how date can be confusing to choose the latest... Maybe... it might be due to the default popup width, where longer types (that have multiple childs) are truncated. If you do not expand the popup (using mouse), version is often hidden (as it's at the end) and it's easy to pick up not latest version, especially that VSCode does some "intelligent" sorting when you start typing. It does not respect the order we say the list should be. I've tried to understand this behaviour when I was fixing #737, but I couldn't find what was happening in VSCode, as the list we provided was ok.

Perhaps a solution to picking old versions accidentally would be to separate completions into parts, so the API version would no longer be truncated (I think this is covered by #1060)

@shenglol
Copy link
Contributor

what if we'd like to output variable called 'type'? Perhaps export keyword would be better?

Yeah good pt. Something like

var string = 'foo'
output type string = '...' // Are we exporting a type or a varable?

is definitely ambiguous.

I feel like the order should be changed. Isn't the type of the output a type just like string or object is a type? I would've expected:
output VM type = resource 'Microsoft.Compute/virtualMachines@2020-12-01'

To @alex-frankel's point, this will eliminate the ambiguity, but one thing I think we should discuss is that we use type as a keyword when defining local type aliases, but here it is used a sort of "type". Personally I'm more inclined to using export keyword as @miqm suggested.

type VM  = resource 'Microsoft.Compute/virtualMachines@2020-12-01'  // local type
export type VM = resource 'Microsoft.Compute/virtualMachines@2020-12-01' // exported type

@shenglol
Copy link
Contributor

@alex-frankel regarding custom types I was thinking more structure of objects and arrays of object.

However, for replacing allowed something like would be nice to work:

type MyValues = 'value1' | 'value2' | 'value3' | null

As for types of structures it would be very helpful in creating modules that consume arrays and create resources in a loop. Perhaps we'd need a bit different syntax, i.e.:

type serverFarmSku = 'S1' | 'S2' | 'P1V2' | 'P1V3'
type WebAppPlanConfig = struct {
  name: string
  sku: serverFarmSku
} 

param location string

// syntax option 1:
param plans WebAppPlanConfig[]

// syntax option 2:
@type(WebAppPlanConfig)
param plans array

resource webappServerFarm 'Microsoft.Web/serverfarms@2020-06-01' = [for item in plans: {
  name: item.name
  location: location
  properties: {
    reserved: true
  }
  kind: 'linux'
  sku: {
    name: item.sku    
  }
}

+1 something like type MyValues = 'value1' | 'value2' | 'value3' | null is nice, and I feel like option 1 makes more sense to me. As for custom object type definitions, I wonder if we could remove the struct keyword. I'm asking because I feel like it becomes a bit verbose when it comes to defining nested object types:

type myObj = struct {
  foo: struct {
    bar: struct {
      value: boolean
    }
  }
}

@shenglol
Copy link
Contributor

if we'd support custom types, would we output them into ARM (i.e. in metadata)?

That's a good question. Putting custom types in template metadata would make it possible to decompile them, but I'm not sure if it's worth doing it. Something to discuss in our team as well.

@miqm
Copy link
Collaborator

miqm commented May 24, 2021

+1 something like type MyValues = 'value1' | 'value2' | 'value3' | null is nice, and I feel like option 1 makes more sense to me. As for custom object type definitions, I wonder if we could remove the struct keyword. I'm asking because I feel like it becomes a bit verbose when it comes to defining nested object types:

type myObj = struct {
  foo: struct {
    bar: struct {
      value: boolean
    }
  }
}

Perhaps it could be omitted, but wouldn't parsing get too complicated? I added struct in the proposal to comply with resource keyword, its not a must.

@shenglol
Copy link
Contributor

It shouldn't be too complicated - I think we can even reuse the parsing logic to parse the custom object type as an ObjectSyntax.

@brwilkinson
Copy link
Collaborator

Being able to define custom struct objects would be so welcome... All of the objects that I like to pass to my templates are an array of a custom object with a subset of values to define resources. Many of them are optional, some are required, however there is no way to define these schema's at the moment at all, other than to provide examples of what the objects should look like and maintain documentation and examples.

I assume this will simply be to enhancing the authoring experience (only) ? i.e. allow for intellisense on object properties when defining inputs ? They would have no purpose outside of authoring or linting ?

@miqm
Copy link
Collaborator

miqm commented Jul 3, 2021

I assume this will simply be to enhancing the authoring experience (only) ? i.e. allow for intellisense on object properties when defining inputs ? They would have no purpose outside of authoring or linting ?

What other purpose do you have in mind?

@brwilkinson
Copy link
Collaborator

I assume this will simply be to enhancing the authoring experience (only) ? i.e. allow for intellisense on object properties when defining inputs ? They would have no purpose outside of authoring or linting ?

What other purpose do you have in mind?

There will be no deploy time validation OR blocking input if it doesn't match the defined struct? i.e. There is no way to define these structs once it's compiled to json.

@bganapa
Copy link
Member

bganapa commented Dec 14, 2021

I like the idea of importing a map of Resourcetypes:ApiVersion at the top of the file rather than aliases or using statements. similar to what @anthony-c-martin proposed. resourcetypes:apiversion map could be a separate .json file

Reourcetypes:ApiVersion map could be developer driven or it could be already published known map. A known map could be used for AzureStack API profile, relating to the issue #851.

Note it also comes with the complexity providing intellisense in vscode for various api versions of a resource type.

@stan-sz
Copy link
Contributor

stan-sz commented Aug 26, 2022

Can an existing resource declaration omit the api version?

resource rg 'Microsoft.Resources/resourceGroups' existing = {
  name: 'foo'
  scope: subscription('<guid>')
}

@alex-frankel
Copy link
Collaborator Author

Can an existing resource declaration omit the api version?

resource rg 'Microsoft.Resources/resourceGroups' existing = {
  name: 'foo'
  scope: subscription('<guid>')
}

As part of the implementation of this, we could cover existing as well.

@stan-sz
Copy link
Contributor

stan-sz commented Nov 15, 2022

Captured this in #9033 just in case.

@Rembrandtastic
Copy link

Still a need for this! Having to update multiple API resource references in Bicep templates is exhaustingly manual, and tedious. Has there been any more thought put into solving this tedious challenge? Terraform has +1 for not having to write it into every resource declaration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Todo
Development

No branches or pull requests