From ac3a8718bd81086e3295f437dfdfbaa17b0d3510 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Tue, 27 Feb 2024 09:02:35 +1300 Subject: [PATCH 1/7] Extend description regex --- .../internal/codegen/pipeline/add_cross_resource_references.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go b/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go index cab0fa95e46..d23daf34e01 100644 --- a/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go +++ b/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go @@ -20,7 +20,7 @@ import ( // TransformCrossResourceReferencesStageID is the unique identifier for this pipeline stage const TransformCrossResourceReferencesStageID = "transformCrossResourceReferences" -var armIDDescriptionRegex = regexp.MustCompile("(?i)(.*/subscriptions/.*?/resourceGroups/.*|ARM ID|Resource ID|resourceId)") +var armIDDescriptionRegex = regexp.MustCompile("(?i)(.*/subscriptions/.*?/resourceGroups/.*|ARMID|ARM ID|Resource ID|resourceId)") var idRegex = regexp.MustCompile("^(.*)I[d|D]s?$") // TransformCrossResourceReferences replaces cross resource references with genruntime.ResourceReference. From 83835dc8773fb584d781a214bb73878da0b41ecb Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Tue, 27 Feb 2024 09:02:51 +1300 Subject: [PATCH 2/7] Add name regex --- .../apply_cross_resource_references_from_config.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go b/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go index 9aa6c86894b..bd3b0e5ea11 100644 --- a/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go +++ b/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go @@ -7,10 +7,10 @@ package pipeline import ( "context" - "github.com/go-logr/logr" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" + "regexp" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" @@ -174,6 +174,8 @@ func MakeARMIDPropertyTypeVisitor( return visitor } +var armIDNameRegex = regexp.MustCompile("(?i)(^Id$|ResourceID|ARMID)") + // DoesPropertyLookLikeARMReference uses a simple heuristic to determine if a property looks like it might be an ARM reference. // This can be used for logging/reporting purposes to discover references which we missed. func DoesPropertyLookLikeARMReference(prop *astmodel.PropertyDefinition) bool { @@ -187,9 +189,10 @@ func DoesPropertyLookLikeARMReference(prop *astmodel.PropertyDefinition) bool { return false } + hasMatchingName := armIDNameRegex.MatchString(prop.PropertyName().String()) hasMatchingDescription := armIDDescriptionRegex.MatchString(prop.Description()) - namedID := prop.HasName("Id") - if hasMatchingDescription || namedID { + + if hasMatchingName || hasMatchingDescription { return true } From efd6d3df1d00ef5ed139ae2edde46fea91e10f62 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Tue, 27 Feb 2024 09:03:05 +1300 Subject: [PATCH 3/7] Update configuration --- v2/azure-arm.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/v2/azure-arm.yaml b/v2/azure-arm.yaml index c2fcb111361..645d720381a 100644 --- a/v2/azure-arm.yaml +++ b/v2/azure-arm.yaml @@ -1109,6 +1109,8 @@ objectModelConfiguration: $supportedFrom: v2.0.0-alpha.2 $defaultAzureName: false RoleAssignmentProperties: + DelegatedManagedIdentityResourceId: + $armReference: false # Actually, this *IS* a resource id, but we want to avoid breaking changes so we're fibbing here RoleDefinitionId: $armReference: true PrincipalId: @@ -1119,6 +1121,8 @@ objectModelConfiguration: $supportedFrom: v2.4.0 $defaultAzureName: false RoleAssignmentProperties: + DelegatedManagedIdentityResourceId: + $armReference: false # Actually, this *IS* a resource id, but we want to avoid breaking changes so we're fibbing here RoleDefinitionId: $armReference: true PrincipalId: @@ -1728,6 +1732,9 @@ objectModelConfiguration: FlexibleServers_FirewallRule: $exportAs: FlexibleServersFirewallRule $supportedFrom: v2.0.0-alpha.2 + ServerProperties: + SourceServerResourceId: + $armReference: false # Actually, this *IS* a resource id, but we want to avoid breaking changes so we're fibbing here 2022-01-01: FlexibleServers_Administrator: $exportAs: FlexibleServersAdministrator @@ -2152,6 +2159,12 @@ objectModelConfiguration: AksNetworkingConfiguration: SubnetId: $armReference: true + IdentityForCmk: + UserAssignedIdentity: + $armReference: false # Actually, this *IS* a resource id, but we want to avoid breaking changes so we're fibbing here + KeyVaultProperties: + KeyVaultArmId: + $armReference: false # Actually, this *IS* a resource id, but we want to avoid breaking changes so we're fibbing here Kubernetes: ResourceId: $armReference: true From ac35811e38a4aebec7d628465c09531b7bb54f81 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Wed, 28 Feb 2024 13:34:40 +1300 Subject: [PATCH 4/7] Make property type checks more robust --- ...y_cross_resource_references_from_config.go | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go b/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go index bd3b0e5ea11..683366fefab 100644 --- a/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go +++ b/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go @@ -180,19 +180,31 @@ var armIDNameRegex = regexp.MustCompile("(?i)(^Id$|ResourceID|ARMID)") // This can be used for logging/reporting purposes to discover references which we missed. func DoesPropertyLookLikeARMReference(prop *astmodel.PropertyDefinition) bool { // The property must be a string, optional string, list of strings, or map[string]string - isString := astmodel.TypeEquals(prop.PropertyType(), astmodel.StringType) - isOptionalString := astmodel.TypeEquals(prop.PropertyType(), astmodel.OptionalStringType) - isStringSlice := astmodel.TypeEquals(prop.PropertyType(), astmodel.NewArrayType(astmodel.StringType)) - isStringMap := astmodel.TypeEquals(prop.PropertyType(), astmodel.MapOfStringStringType) + mightBeReference := false + if pt, ok := astmodel.AsPrimitiveType(prop.PropertyType()); ok { + // Might be a reference if we have a primitive type that's a string + mightBeReference = pt == astmodel.StringType + } + + if at, ok := astmodel.AsArrayType(prop.PropertyType()); ok { + // Might be references if we have an array of strings + elementType, elementTypeIsPrimitive := astmodel.AsPrimitiveType(at.Element()) + mightBeReference = elementTypeIsPrimitive && + elementType == astmodel.StringType + } - if !isString && !isOptionalString && !isStringSlice && !isStringMap { - return false + if mt, ok := astmodel.AsMapType(prop.PropertyType()); ok { + // Might be references if we have a map of strings to strings + keyType, keyTypeIsPrimitive := astmodel.AsPrimitiveType(mt.KeyType()) + valueType, valueTypeIsPrimitive := astmodel.AsPrimitiveType(mt.ValueType()) + mightBeReference = keyTypeIsPrimitive && valueTypeIsPrimitive && + keyType == astmodel.StringType && valueType == astmodel.StringType } hasMatchingName := armIDNameRegex.MatchString(prop.PropertyName().String()) hasMatchingDescription := armIDDescriptionRegex.MatchString(prop.Description()) - if hasMatchingName || hasMatchingDescription { + if mightBeReference && (hasMatchingName || hasMatchingDescription) { return true } From 86ed154dd739da1c87b353bc0943c412b9417f38 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 1 Mar 2024 11:48:23 +1300 Subject: [PATCH 5/7] Move regex based on PR feedback --- .../codegen/pipeline/add_cross_resource_references.go | 1 - .../pipeline/apply_cross_resource_references_from_config.go | 5 ++++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go b/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go index d23daf34e01..ad7a5282f07 100644 --- a/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go +++ b/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go @@ -20,7 +20,6 @@ import ( // TransformCrossResourceReferencesStageID is the unique identifier for this pipeline stage const TransformCrossResourceReferencesStageID = "transformCrossResourceReferences" -var armIDDescriptionRegex = regexp.MustCompile("(?i)(.*/subscriptions/.*?/resourceGroups/.*|ARMID|ARM ID|Resource ID|resourceId)") var idRegex = regexp.MustCompile("^(.*)I[d|D]s?$") // TransformCrossResourceReferences replaces cross resource references with genruntime.ResourceReference. diff --git a/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go b/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go index 683366fefab..6e2f24c7e15 100644 --- a/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go +++ b/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go @@ -174,7 +174,10 @@ func MakeARMIDPropertyTypeVisitor( return visitor } -var armIDNameRegex = regexp.MustCompile("(?i)(^Id$|ResourceID|ARMID)") +var ( + armIDNameRegex = regexp.MustCompile("(?i)(^Id$|ResourceID|ARMID)") + armIDDescriptionRegex = regexp.MustCompile("(?i)(.*/subscriptions/.*?/resourceGroups/.*|ARMID|ARM ID|Resource ID|resourceId)") +) // DoesPropertyLookLikeARMReference uses a simple heuristic to determine if a property looks like it might be an ARM reference. // This can be used for logging/reporting purposes to discover references which we missed. From 0ef10514599acbb8efa6868b52c7fa2f960a5a09 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 1 Mar 2024 11:52:10 +1300 Subject: [PATCH 6/7] Update imports --- .../pipeline/apply_cross_resource_references_from_config.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go b/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go index 6e2f24c7e15..fe7d8bb4f1e 100644 --- a/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go +++ b/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go @@ -7,10 +7,12 @@ package pipeline import ( "context" + "regexp" + "github.com/go-logr/logr" "github.com/pkg/errors" + kerrors "k8s.io/apimachinery/pkg/util/errors" - "regexp" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" From ecb29c25e03ee844180f4099f055917acf8bfb29 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 1 Mar 2024 12:11:49 +1300 Subject: [PATCH 7/7] Fix merge issue --- .../codegen/pipeline/add_cross_resource_references.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go b/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go index fb5fda93bd0..d2c78b96647 100644 --- a/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go +++ b/v2/tools/generator/internal/codegen/pipeline/add_cross_resource_references.go @@ -20,10 +20,7 @@ import ( // TransformCrossResourceReferencesStageID is the unique identifier for this pipeline stage const TransformCrossResourceReferencesStageID = "transformCrossResourceReferences" -var ( - armIDDescriptionRegex = regexp.MustCompile("(?i)(.*/subscriptions/.*?/resourceGroups/.*|ARM ID|Resource ID|resourceId)") - idRegex = regexp.MustCompile("^(.*)I[d|D]s?$") -) +var idRegex = regexp.MustCompile("^(.*)I[d|D]s?$") // TransformCrossResourceReferences replaces cross resource references with genruntime.ResourceReference. func TransformCrossResourceReferences(configuration *config.Configuration, idFactory astmodel.IdentifierFactory) *Stage {