Add infrastructure tags#46
Conversation
simonkurtz-MSFT
deleted the
feature/tag-and-identify-infrastructure-resource-groups
branch
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for applying custom tags to Azure resource groups and Bicep deployments.
- Adds
build_infrastructure_tagshelper to generate standard and custom tags. - Extends
create_resource_groupandcreate_bicep_deployment_groupto accept and apply tags. - Updates sample notebooks to generate and pass
rg_tagswhen invoking deployments.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| shared/python/utils.py | Added tag-building helper; updated RG and Bicep deployment APIs to support tags. |
| tests/python/test_utils.py | Added unit tests covering infrastructure tag generation and RG creation with tags. |
| infrastructure/*/create.ipynb | Updated notebooks to call build_infrastructure_tags and pass rg_tags to deployments. |
Comments suppressed due to low confidence (1)
shared/python/utils.py:41
- [nitpick] The docstring refers to "infrastructure name tags" plural but only a single
infrastructuretag is produced; consider clarifying this description.
"""
| """ | ||
|
|
||
| # Convert infrastructure enum to string value if needed | ||
| if hasattr(infrastructure, 'value'): |
There was a problem hiding this comment.
[nitpick] Instead of using hasattr to detect an enum, consider using isinstance(infrastructure, INFRASTRUCTURE) for clearer intent and to avoid false positives on objects with a value attribute.
| if hasattr(infrastructure, 'value'): | |
| if isinstance(infrastructure, INFRASTRUCTURE): |
|
|
||
| run(f"az group create --name {rg_name} --location {resource_group_location} --tags source=apim-sample", | ||
| # Build the tags string for the Azure CLI command | ||
| tag_string = "source=apim-sample" |
There was a problem hiding this comment.
[nitpick] Wrap the default source tag value in quotes (e.g. source="apim-sample") for consistency with how custom tag values are quoted, and to prevent issues if the value ever contains special characters.
| tag_string = "source=apim-sample" | |
| tag_string = "source=\"apim-sample\"" |
| # Build the tags string for the Azure CLI command | ||
| tag_string = "source=apim-sample" | ||
| if tags: | ||
| for key, value in tags.items(): |
There was a problem hiding this comment.
[nitpick] Iterating tags.items() can produce a non-deterministic order; consider iterating over sorted(tags) to keep the CLI command consistent across runs and easier to test.
| for key, value in tags.items(): | |
| for key, value in sorted(tags.items()): |
| escaped_value = value.replace('"', '\\"') if isinstance(value, str) else str(value) | ||
| tag_string += f" {key}=\"{escaped_value}\"" |
There was a problem hiding this comment.
Manual string escaping may miss other shell metacharacters; using shlex.quote(value) would more robustly escape any special characters for the shell.
| escaped_value = value.replace('"', '\\"') if isinstance(value, str) else str(value) | |
| tag_string += f" {key}=\"{escaped_value}\"" | |
| escaped_value = shlex.quote(value) if isinstance(value, str) else shlex.quote(str(value)) | |
| tag_string += f" {key}={escaped_value}" |
Resolves #45
In draft now as it still requires unit tests to be added. Will be added early next week before changing draft to full PR.