Skip to content

Make tools module part of providers/azure #739

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
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Make tools module part of providers/azure #739

wants to merge 2 commits into from

Conversation

lucacavallaro
Copy link
Member

No description provided.

@Copilot Copilot AI review requested due to automatic review settings July 16, 2025 09:45
@lucacavallaro lucacavallaro requested a review from a team as a code owner July 16, 2025 09:46
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jul 16, 2025

⚠️ No Changeset found

Latest commit: c184513

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@lucacavallaro lucacavallaro changed the title Refactors/go-modules Make tools module part of providers/azure Jul 16, 2025
@lucacavallaro lucacavallaro requested a review from mamu0 July 16, 2025 09:46
Copilot
Copilot AI reviewed Jul 16, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

mamu0
mamu0 reviewed Jul 17, 2025
View reviewed changes
providers/azure/docs/functions/resource_name.md
Comment on lines -41 to -139
1. `configuration` (Map) A map containing the following keys: prefix, environment, location, domain (Optional), name, resource_type and instance_number.

|Name|Value Type|Required|Description|
|:---|:---:|:---:|:---|
|prefix|String|Yes|Prefix that define the repository domain (Max 2 characters)|
|environment|String|Yes|Environment where the resources will be deployed (d, u or p)|
|location|String|Yes|Location where the resources will be deployed (itn/italynorth or weu/westeurope)|
|domain|String|No|Optional value that specify the domain|
|name|String|Yes|Name of the resource|
|resource_type|String|Yes|Type of the resource (see the table below)|
|instance_number|Integer|Yes|Instance number of the resource (1-99)|

### Resource Types

The following table lists the resource types and their abbreviations used in the resource_name function:

| Type | Abbreviation |
| :------------------------------------- | :--------------: |
| ai_search | srch |
| api_management | apim |
| api_management_autoscale | apim-as |
| apim_network_security_group | apim-nsg |
| apim_subnet | apim-snet |
| app_gateway | agw |
| app_private_endpoint | app-pep |
| app_service | app |
| app_service_plan | asp |
| app_slot_private_endpoint | staging-app-pep |
| app_subnet | app-snet |
| application_insights | appi |
| blob_private_endpoint | blob-pep |
| function_blob_private_endpoint | func-blob-pep |
| dfunction_blob_private_endpoint | dfunc-blob-pep |
| blob_storage | blob |
| cdn_frontdoor_endpoint | fde |
| cdn_frontdoor_origin | fdo |
| cdn_frontdoor_origin_group | fdog |
| cdn_frontdoor_profile | afd |
| cdn_frontdoor_route | cdnr |
| cdn_monitor_diagnostic_setting | cdnp |
| monitor_alert_sbns_active | sbns-act-ma |
| monitor_alert_sbns_dlq | sbns-dlq-ma |
| container_app | ca |
| container_app_environment | cae |
| container_app_job | caj |
| container_app_private_endpoint | cae-pep |
| container_app_subnet | cae-snet |
| cosmos_db | cosmos |
| cosmos_db_nosql | cosno |
| cosmos_private_endpoint | cosno-pep |
| customer_key_storage_account | stcmk |
| durable_function_storage_account | stfd |
| eventhub_namespace | evhns |
| eventhub_private_endpoint | evhns-pep |
| file_private_endpoint | file-pep |
| function_file_private_endpoint | func-file-pep |
| dfunction_file_private_endpoint | dfunc-file-pep |
| file_storage | file |
| function_app | func |
| function_private_endpoint | func-pep |
| function_slot_private_endpoint | staging-func-pep |
| function_storage_account | stfn |
| function_subnet | func-snet |
| key_vault | kv |
| key_vault_private_endpoint | kv-pep |
| load_testing | lt |
| log_analytics | log |
| managed_identity | id |
| nat_gateway | ng |
| network_security_group | nsg |
| postgre_endpoint | psql-ep |
| postgre_private_endpoint | psql-pep |
| postgre_replica_private_endpoint | psql-pep-replica |
| postgresql | psql |
| postgresql_replica | psql-replica |
| private_endpoint | pep |
| private_endpoint_subnet | pep-snet |
| public_ip | pip |
| queue_private_endpoint | queue-pep |
| function_queue_private_endpoint | func-queue-pep |
| dfunction_queue_private_endpoint | dfunc-queue-pep |
| queue_storage | queue |
| redis_cache | redis |
| resource_group | rg |
| servicebus_namespace | sbns |
| servicebus_private_endpoint | sbns-pep |
| static_web_app | stapp |
| storage_account | st |
| subnet | snet |
| table_private_endpoint | table-pep |
| function_table_private_endpoint | func-table-pep |
| dfunction_table_private_endpoint | dfunc-table-pep |
| table_storage | table |
| virtual_machine | vm |
| virtual_network | vnet |
| dns_forwarding_ruleset | dnsfrs |
| dns_private_resolver | dnspr |
| dns_private_resolver_inbound_endpoint | in |
| dns_private_resolver_outbound_endpoint | out | No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

I’d suggest keeping this section, since it’s the page that appears in the registry to document the resource:
https://registry.terraform.io/providers/pagopa-dx/azure/latest/docs/functions/resource_name
Without it, it’s unclear how the resource is meant to be used.

mamu0
mamu0 reviewed Jul 17, 2025
View reviewed changes
providers/azure/docs/resources/available_subnet_cidr.md
@@ -1,82 +0,0 @@
---
page_title: "dx_available_subnet_cidr Resource - terraform-provider-azure"
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto
this is the page about the subnet cidr resource
https://registry.terraform.io/providers/pagopa-dx/azure/latest/docs/resources/available_subnet_cidr

mamu0
mamu0 reviewed Jul 17, 2025
View reviewed changes
providers/azure/docs/resources/dx_available_subnet_cidr.md
@@ -0,0 +1,5 @@
---
# generated by https://github.com/hashicorp/terraform-plugin-docs
Copy link
Contributor

Choose a reason for hiding this comment

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

this file i think is not complete

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the meaning of this file?

Copy link
Contributor

Choose a reason for hiding this comment

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

This file already exists under a different name: available_subnet_cidr.md

is the documentation for the subnet CIDR resource that's visible on the Terraform Registry

@mamu0
Copy link
Contributor

mamu0 commented Jul 17, 2025

I believe the outcome of this PR is partly due to running a clean execution of the go generate command in the tools module.
However, while the command correctly generates the base structure for the documentation, it also overwrites any existing files.
So if we’ve added custom information to the documentation, running the command will reset it to its default (i.e., mostly empty), at least, that’s what happened in my case.

To avoid this, I used to make a backup copy of the existing docs for already defined resources, restore them after generation, and then complete the templates for the new ones.

Keep in mind that all the pages under the docs folder are the ones published on the Terraform registry:
https://registry.terraform.io/providers/pagopa-dx/azure/latest/docs
So we should make sure they’re always up-to-date with all the useful information needed to use our resources properly.

@lucacavallaro
Copy link
Member Author

To avoid this, I used to make a backup copy of the existing docs for already defined resources, restore them after generation, and then complete the templates for the new ones.

@mamu0 so, before merging this we have to find a way to do this step automatically without the need to backup and copy paste files. I will think about it.

@mamu0
Copy link
Contributor

mamu0 commented Jul 17, 2025

... find a way to do this step automatically without the need to backup and copy paste files. I will think about it.

That makes sense! But keep in mind that adding a new resource isn’t something we do very often, and documentation updates are made directly in the files themselves.
So if it’s too complex to make the tool ignore existing files, we might consider handling it manually in those rare cases, it’s a small trade-off for keeping things simple.

@mamu0
Copy link
Contributor

mamu0 commented Oct 8, 2025

Is this PR still active, or would it make sense to move it to draft or close it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Krusty93 Krusty93 Krusty93 left review comments

@mamu0 mamu0 mamu0 left review comments

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lucacavallaro @mamu0 @Krusty93