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.

Sign up for GitHub

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

Jump to bottom

Adding tags parameters with radapp defaults to Azure Recipes #60

Merged
merged 3 commits into from
Feb 21, 2024
Merged

Adding tags parameters with radapp defaults to Azure Recipes #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d55d50c
Adding tags parameters with radapp defaults to Azure Recipes
willvelida Feb 9, 2024
2ce46d3
Add radius-specific tags with optional tag parameters. null checking …
willvelida Feb 12, 2024
ff23921
Adding hyphen character in radiusTags object
willvelida Feb 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions azure/extender-servicebus.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,20 @@ param skuTier string = 'Standard'
@description('ISO 8601 Default message timespan to live value. This is the duration after which the message expires, starting from when the message is sent to Service Bus. This is the default value used when TimeToLive is not set on a message itself.')
param defaultMessageTimeToLive string = 'P14D'

@description('The user-defined tags that will be applied to the resource. Default is null')
param tags object = {}

@description('The Radius specific tags that will be applied to the resource')
var radiusTags = {
'radapp.io-environment': context.environment.id
'radapp.io-application': context.application == null ? '' : context.application.id
'radapp.io-resource': context.resource.id
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AaronCrawfis did you want these to be the names or ids? We used names here

For @willvelida the id will be a long string like /planes/radius/local/..../Applications.Core/applications/my-cool-app.

The name will be my-cool-app.

There's also a special-case to handle where the application can be null. You can see how we handled that here: https://github.com/radius-project/recipes/blob/main/local-dev/rabbitmqqueues.bicep#L60

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you want these to be the names or ids?

Since these are going to be used to identify an application within the Azure portal and not used for programmatic matching within a namespace, my preference is IDs. That way you can fully identity what's using this resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rynowak @AaronCrawfis I've kept the IDs in, but I've added null checking for the application. Hopefully this meets both requirements? 🙂


resource servicebus 'Microsoft.ServiceBus/namespaces@2021-06-01-preview' = {
name: 'servicebus-namespace-${uniqueString(context.resource.id, resourceGroup().id)}'
location: location
tags: union(radiusTags, tags)
sku: {
name: skuName
tier: skuTier
Expand Down
11 changes: 11 additions & 0 deletions azure/rediscaches.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,20 @@ param skuFamily string = 'C'
])
param skuName string = 'Basic'

@description('The user-defined tags that will be applied to the resource. Default is null')
param tags object = {}

@description('The Radius specific tags that will be applied to the resource')
var radiusTags = {
'radapp.io-environment': context.environment.id
'radapp.io-application': context.application == null ? '' : context.application.id
'radapp.io-resource': context.resource.id
}

resource azureCache 'Microsoft.Cache/redis@2022-06-01' = {
name: 'cache-${uniqueString(context.resource.id, resourceGroup().id)}'
location: location
tags: union(tags, radiusTags)
properties: {
sku: {
capacity: skuCapacity
Expand Down
12 changes: 12 additions & 0 deletions azure/sqldatabases.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,22 @@ param skuName string = 'Standard'
])
param skuTier string = 'Standard'

@description('The user-defined tags that will be applied to the resource. Default is null')
param tags object = {}

@description('The Radius specific tags that will be applied to the resource')
var radiusTags = {
'radapp.io-environment': context.environment.id
'radapp.io-application': context.application == null ? '' : context.application.id
'radapp.io-resource': context.resource.id
}

var mssqlPort = 1433

resource mssql 'Microsoft.Sql/servers@2021-02-01-preview' = {
name: '${context.resource.name}-${uniqueString(context.resource.id, resourceGroup().id)}'
location: location
tags: union(tags, radiusTags)
properties: {
administratorLogin: adminLogin
administratorLoginPassword: adminPassword
Expand All @@ -67,6 +78,7 @@ resource mssql 'Microsoft.Sql/servers@2021-02-01-preview' = {
resource db 'databases' = {
name: database
location: location
tags: union(tags, radiusTags)
sku: {
name: skuName
tier: skuTier
Expand Down
11 changes: 11 additions & 0 deletions azure/statestores.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,20 @@ param actorStateStore bool = false
@description('The name of the container to create within the Azure storage account and to reference within the Dapr component.')
var containerName = context.resource.name

@description('The user-defined tags that will be applied to the resource. Default is null')
param tags object = {}

@description('The Radius specific tags that will be applied to the resource')
var radiusTags = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just attempted to deploy this Recipe and got this error message:

{
          "code": "RecipeDeploymentFailed",
          "message": "failed to deploy recipe default of type Applications.Dapr/stateStores",
          "details": [
            {
              "code": "DeploymentFailed",
              "message": "At least one resource deployment operation failed. Please see the details for the specific operation that failed.",
              "target": "/planes/radius/local/resourceGroups/default/providers/Microsoft.Resources/deployments/recipe1707868070557672174",
              "details": [
                {
                  "code": "InvalidTagNameCharacters",
                  "message": "The tag names 'radapp.io/application, radapp.io/environment, radapp.io/resource' have reserved characters '\u003c,\u003e,%,\u0026,\\,?,/' or control characters. These characters are only allowed for tags that start with the prefix 'hidden, link'.",
                  "target": "/subscriptions/66d1209e-1382-45d3-99bb-650e6bf63fc0/resourcegroups/aacrawfi/providers/Microsoft.Storage/storageAccounts/recipe2lkqwvspyl24g"
                }
              ]
            }

Looking in the Azure portal I also get the same error message:
image

We should switch to using another character, maybe -? (radapp.io-application)

cc/ @rynowak

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AaronCrawfis I've changed the character now to a hyphen :)

Question - Do the tests just work for the local recipes? If so, could we introduce tests to test the Azure/AWS recipes as part of the workflow? Initially I thought using ephemeral environments for each cloud to do this, but not 100% on this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Currently yes, we just test local-dev Recipes as we can run them in-cluster without needing external cloud resources. We do want to add full integration tests for cloud resources, we just haven't gotten it setup yet. I opened #62 to track this work.

'radapp.io-environment': context.environment.id
'radapp.io-application': context.application == null ? '' : context.application.id
'radapp.io-resource': context.resource.id
}

resource storageAccount 'Microsoft.Storage/storageAccounts@2022-05-01' = {
name: 'recipe${uniqueString(context.resource.id, resourceGroup().id)}'
location: location
tags: union(tags, radiusTags)
sku: {
name: 'Standard_ZRS'
}
Expand Down
Loading