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

Customise module naming #3838

Merged
merged 10 commits into from
Jan 10, 2025
Merged

Customise module naming #3838

merged 10 commits into from
Jan 10, 2025

Conversation

danielrbradley
Copy link
Member

@danielrbradley danielrbradley commented Jan 8, 2025 •
edited
Loading

Choosing default versions (#3448) is hard where there's multiple versions mixed into a single module. This is sometimes because we structure our modules around Azure's namespaces rather than the SDK folders. This PR allows us to selectively restructure problematic modules to match Azure's SDK structure (#690) and avoid mixing multiple versions into a single module. An example of the result of turning on the feature flag can be seen in 14db6d8 where we split out Dns, DnsResolver, FrontDoor, PrivateDns and TrafficManager from the Network module.

Side note: when splitting the modules out, there's all previous versions generated for the new module. These will be reduced when we run the squeeze process.

Manual report of all specification folders and the namespaces within them: spec-dirs.csv

For the first Network module split, we've hard coded the new module names to use because the casing is not able to be derived from the folder name alone. In a follow-up PR we can investigate parsing the Readme.md files from the specifications to automatically infer the correct names and avoid hard-coding.

Generate implementation approach:

  1. Make naming overrides a functions instead of a map so we can make decisions based on the version switch and other information.
  2. When overriding a module name, return the old name too and add aliases so it's easy for users to migrate.

@danielrbradley danielrbradley self-assigned this Jan 8, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 8, 2025

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@codecov Codecov
Copy link

codecov bot commented Jan 8, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 98.71795% with 1 line in your changes missing coverage. Please review.

Project coverage is 56.98%. Comparing base (d40719a) to head (34f6aef).
Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
provider/pkg/gen/schema.go 97.22% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3838      +/-   ##
==========================================
+ Coverage   56.87%   56.98%   +0.10%     
==========================================
  Files          79       79              
  Lines       12253    12284      +31     
==========================================
+ Hits         6969     7000      +31     
  Misses       4759     4759              
  Partials      525      525

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielrbradley danielrbradley force-pushed the provider-naming-func branch 2 times, most recently from 0fbf242 to 9c27adc Compare January 8, 2025 15:57
@danielrbradley danielrbradley changed the title Add function to decide on module naming exceptions Customise module naming Jan 8, 2025
@danielrbradley danielrbradley marked this pull request as ready for review January 9, 2025 09:24
@danielrbradley
Copy link
Member Author

Added last tests, rebased and updated example commit for the impact when enabled: 14db6d8

@danielrbradley
Refactor name overrides into function
4e2d2de 
This will allow us to extend our rules to conditionally split larger namespaces.
@danielrbradley
Return old name when overridden
aaae478 
Allow us to build aliases as required for renames between major versions.
@danielrbradley
Fix case insensitive comparison
0915233
@danielrbradley
Move folderModulePattern
292ed31 
Move to near its only usage.
@danielrbradley
Make oldProvider accessible from generateAliases function
85781fa 
Add PreviousProviderName to ResourceSpec which is embedded into resourceVariant and passed into `func (g *packageGenerator) generateAliases(...)`
@danielrbradley
Prefer simple functions over class methods
eb9af14 
Prefer pure functions for simple string manipulation so the inputs are transparent to the caller.

Make pulumi provider name a constant rather than referencing from the packageGenerator object.

Simplify & test goModuleName

1. Add test of old function.
2. Add goModuleRepoPath prefixing to implementation & test.
3. Simplify implementation.
@danielrbradley
Refactor generateAliases
8fc71cd 
Replace makeTypeAlias with local function so it can also add to the slice directly. Allow setting the module name too so we can add aliases between modules.

Add previous module variations of aliases

Remove makeTypeAlias test
@danielrbradley
Add manual split of Network module
90c3d7d 
Note: We might be able to do this from the specification metadata at some point instead of hard-coding the corrected cases.
@danielrbradley
Refactor getNameOverride()
0406603 
- Use maps instead of conditionals for conciseness & to get us to 100% coverage.
- Move fetching major version number to call in Discover module so we can unit test both v2 and v3 paths.
- Rename resourceProvider() to findNamespaceWithoutPrefixFromPath()
- Rename fileProvider and apiProvider to be more explicit as to their string content.
@danielrbradley
Systematically test alias generation
34f6aef
@danielrbradley danielrbradley merged commit 59ec1ee into master Jan 10, 2025
23 checks passed
@danielrbradley danielrbradley deleted the provider-naming-func branch January 10, 2025 07:49
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v2.82.0.

@danielrbradley danielrbradley mentioned this pull request Jan 30, 2025
Merged
danielrbradley added a commit that referenced this pull request Jan 30, 2025
@danielrbradley
Previous module name aliases cleanup (#3910)
e78802a 
Stacked on top of #3906 

This was only added in
#3838 before changing
direction in #3906

This removes a few aliases in v3 only where the explicit version was not
actually present in the previous major version due to squeezing:

```diff
diff --git a/provider/cmd/pulumi-resource-azure-native/schema.json b/provider/cmd/pulumi-resource-azure-native/schema.json
--- a/provider/cmd/pulumi-resource-azure-native/schema.json
+++ b/provider/cmd/pulumi-resource-azure-native/schema.json
@@ -693311,21 +693311,6 @@
                 {
                     "type": "azure-native:dns/v20230701preview:RecordSet"
                 },
-                {
-                    "type": "azure-native:network/v20150504preview:RecordSet"
-                },
-                {
-                    "type": "azure-native:network/v20160401:RecordSet"
-                },
-                {
-                    "type": "azure-native:network/v20170901:RecordSet"
-                },
-                {
-                    "type": "azure-native:network/v20171001:RecordSet"
-                },
-                {
-                    "type": "azure-native:network/v20180301preview:RecordSet"
-                },
                 {
                     "type": "azure-native:network/v20180501:RecordSet"
                 },
@@ -693499,21 +693484,6 @@
                 {
                     "type": "azure-native:dns/v20230701preview:Zone"
                 },
-                {
-                    "type": "azure-native:network/v20150504preview:Zone"
-                },
-                {
-                    "type": "azure-native:network/v20160401:Zone"
-                },
-                {
-                    "type": "azure-native:network/v20170901:Zone"
-                },
-                {
-                    "type": "azure-native:network/v20171001:Zone"
-                },
-                {
-                    "type": "azure-native:network/v20180301preview:Zone"
-                },
                 {
                     "type": "azure-native:network/v20180501:Zone"
                 },
@@ -693749,9 +693719,6 @@
                 {
                     "type": "azure-native:dnsresolver/v20230701preview:DnsResolver"
                 },
-                {
-                    "type": "azure-native:network/v20200401preview:DnsResolver"
-                },
                 {
                     "type": "azure-native:network/v20220701:DnsResolver"
                 },
@@ -694326,9 +694293,6 @@
                 {
                     "type": "azure-native:dnsresolver/v20230701preview:ForwardingRule"
                 },
-                {
-                    "type": "azure-native:network/v20200401preview:ForwardingRule"
-                },
                 {
                     "type": "azure-native:network/v20220701:ForwardingRule"
                 },
@@ -709971,21 +709935,6 @@
                 {
                     "type": "azure-native:frontdoor/v20210601:FrontDoor"
                 },
-                {
-                    "type": "azure-native:network/v20190401:FrontDoor"
-                },
-                {
-                    "type": "azure-native:network/v20190501:FrontDoor"
-                },
-                {
-                    "type": "azure-native:network/v20200101:FrontDoor"
-                },
-                {
-                    "type": "azure-native:network/v20200401:FrontDoor"
-                },
-                {
-                    "type": "azure-native:network/v20200501:FrontDoor"
-                },
                 {
                     "type": "azure-native:network/v20210601:FrontDoor"
                 },
@@ -710244,18 +710193,6 @@
                 {
                     "type": "azure-native:frontdoor/v20240201:Policy"
                 },
-                {
-                    "type": "azure-native:network/v20190301:Policy"
-                },
-                {
-                    "type": "azure-native:network/v20191001:Policy"
-                },
-                {
-                    "type": "azure-native:network/v20200401:Policy"
-                },
-                {
-                    "type": "azure-native:network/v20201101:Policy"
-                },
                 {
                     "type": "azure-native:network/v20210601:Policy"
                 },
@@ -710342,15 +710279,6 @@
                 {
                     "type": "azure-native:frontdoor/v20210601:RulesEngine"
                 },
-                {
-                    "type": "azure-native:network/v20200101:RulesEngine"
-                },
-                {
-                    "type": "azure-native:network/v20200401:RulesEngine"
-                },
-                {
-                    "type": "azure-native:network/v20200501:RulesEngine"
-                },
                 {
                     "type": "azure-native:network/v20210601:RulesEngine"
                 },
@@ -759350,27 +759278,15 @@
                 {
                     "type": "azure-native:documentdb/v20240301preview:FirewallRule"
                 },
-                {
-                    "type": "azure-native:documentdb/v20240301preview:MongoClusterFirewallRule"
-                },
                 {
                     "type": "azure-native:documentdb/v20240601preview:FirewallRule"
                 },
-                {
-                    "type": "azure-native:documentdb/v20240601preview:MongoClusterFirewallRule"
-                },
                 {
                     "type": "azure-native:documentdb/v20240701:FirewallRule"
                 },
-                {
-                    "type": "azure-native:documentdb/v20240701:MongoClusterFirewallRule"
-                },
                 {
                     "type": "azure-native:documentdb/v20241001preview:FirewallRule"
                 },
-                {
-                    "type": "azure-native:documentdb/v20241001preview:MongoClusterFirewallRule"
-                },
                 {
                     "type": "azure-native:documentdb:FirewallRule"
                 },
@@ -759447,33 +759363,18 @@
                 "resourceGroupName"
             ],
             "aliases": [
-                {
-                    "type": "azure-native:documentdb/v20240301preview:MongoClusterPrivateEndpointConnection"
-                },
                 {
                     "type": "azure-native:documentdb/v20240301preview:PrivateEndpointConnection"
                 },
-                {
-                    "type": "azure-native:documentdb/v20240601preview:MongoClusterPrivateEndpointConnection"
-                },
                 {
                     "type": "azure-native:documentdb/v20240601preview:PrivateEndpointConnection"
                 },
-                {
-                    "type": "azure-native:documentdb/v20240701:MongoClusterPrivateEndpointConnection"
-                },
                 {
                     "type": "azure-native:documentdb/v20240701:PrivateEndpointConnection"
                 },
-                {
-                    "type": "azure-native:documentdb/v20241001preview:MongoClusterPrivateEndpointConnection"
-                },
                 {
                     "type": "azure-native:documentdb/v20241001preview:PrivateEndpointConnection"
                 },
-                {
-                    "type": "azure-native:documentdb:MongoClusterPrivateEndpointConnection"
-                },
                 {
                     "type": "azure-native:mongocluster/v20240301preview:MongoClusterPrivateEndpointConnection"
                 },
@@ -801431,12 +801332,6 @@
                 "resourceGroupName"
             ],
             "aliases": [
-                {
-                    "type": "azure-native:network/v20180901:PrivateRecordSet"
-                },
-                {
-                    "type": "azure-native:network/v20200101:PrivateRecordSet"
-                },
                 {
                     "type": "azure-native:network/v20200601:PrivateRecordSet"
                 },
@@ -801560,12 +801455,6 @@
                 "resourceGroupName"
             ],
             "aliases": [
-                {
-                    "type": "azure-native:network/v20180901:PrivateZone"
-                },
-                {
-                    "type": "azure-native:network/v20200101:PrivateZone"
-                },
                 {
                     "type": "azure-native:network/v20200601:PrivateZone"
                 },
@@ -801698,12 +801587,6 @@
                 "resourceGroupName"
             ],
             "aliases": [
-                {
-                    "type": "azure-native:network/v20180901:VirtualNetworkLink"
-                },
-                {
-                    "type": "azure-native:network/v20200101:VirtualNetworkLink"
-                },
                 {
                     "type": "azure-native:network/v20200601:VirtualNetworkLink"
                 },
@@ -869461,27 +869344,6 @@
                 "resourceGroupName"
             ],
             "aliases": [
-                {
-                    "type": "azure-native:network/v20151101:Endpoint"
-                },
-                {
-                    "type": "azure-native:network/v20170301:Endpoint"
-                },
-                {
-                    "type": "azure-native:network/v20170501:Endpoint"
-                },
-                {
-                    "type": "azure-native:network/v20180201:Endpoint"
-                },
-                {
-                    "type": "azure-native:network/v20180301:Endpoint"
-                },
-                {
-                    "type": "azure-native:network/v20180401:Endpoint"
-                },
-                {
-                    "type": "azure-native:network/v20180801:Endpoint"
-                },
                 {
                     "type": "azure-native:network/v20220401:Endpoint"
                 },
@@ -869694,27 +869556,6 @@
                 "resourceGroupName"
             ],
             "aliases": [
-                {
-                    "type": "azure-native:network/v20151101:Profile"
-                },
-                {
-                    "type": "azure-native:network/v20170301:Profile"
-                },
-                {
-                    "type": "azure-native:network/v20170501:Profile"
-                },
-                {
-                    "type": "azure-native:network/v20180201:Profile"
-                },
-                {
-                    "type": "azure-native:network/v20180301:Profile"
-                },
-                {
-                    "type": "azure-native:network/v20180401:Profile"
-                },
-                {
-                    "type": "azure-native:network/v20180801:Profile"
-                },
                 {
                     "type": "azure-native:network/v20220401:Profile"
                 },
@@ -869771,12 +869612,6 @@
             },
             "type": "object",
             "aliases": [
-                {
-                    "type": "azure-native:network/v20180401:TrafficManagerUserMetricsKey"
-                },
-                {
-                    "type": "azure-native:network/v20180801:TrafficManagerUserMetricsKey"
-                },
                 {
                     "type": "azure-native:network/v20220401:TrafficManagerUserMetricsKey"
                 },
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomas11 thomas11 thomas11 approved these changes

Assignees

@danielrbradley danielrbradley

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@danielrbradley @thomas11 @pulumi-bot