Preparation for devcenter gallery resource generation #3271

jiaweitao001 · 2023-11-02T08:38:19Z

tools/importer-rest-api-specs: adding support for shared image gallery as a dependency.

hc-github-team-tf-azure · 2023-11-02T08:39:01Z

17 Resource ID(s) found for DevCenter:

ID	File
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/projects/{projectName}/pools/{poolName}/schedules/{scheduleName}`	`.../DevCenter/v2023_04_01/Schedules/ResourceId-ScheduleId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/devCenters/{devCenterName}/galleries/{galleryName}/images/{imageName}`	`.../DevCenter/v2023_04_01/ImageVersions/ResourceId-ImageId.cs` `.../DevCenter/v2023_04_01/Images/ResourceId-ImageId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/devCenters/{devCenterName}/galleries/{galleryName}/images/{imageName}/versions/{versionName}`	`.../DevCenter/v2023_04_01/ImageVersions/ResourceId-VersionId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/projects/{projectName}/pools/{poolName}`	`.../DevCenter/v2023_04_01/Pools/ResourceId-PoolId.cs` `.../DevCenter/v2023_04_01/Schedules/ResourceId-PoolId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/devCenters/{devCenterName}/catalogs/{catalogName}`	`.../DevCenter/v2023_04_01/Catalogs/ResourceId-CatalogId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/projects/{projectName}/allowedEnvironmentTypes/{allowedEnvironmentTypeName}`	`.../DevCenter/v2023_04_01/EnvironmentTypes/ResourceId-AllowedEnvironmentTypeId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/networkConnections/{networkConnectionName}`	`.../DevCenter/v2023_04_01/NetworkConnection/ResourceId-NetworkConnectionId.cs` `.../DevCenter/v2023_04_01/NetworkConnections/ResourceId-NetworkConnectionId.cs`
`/subscriptions/{subscriptionId}/providers/Microsoft.DevCenter/locations/{locationName}`	`.../DevCenter/v2023_04_01/Usages/ResourceId-LocationId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/devCenters/{devCenterName}/devBoxDefinitions/{devBoxDefinitionName}`	`.../DevCenter/v2023_04_01/DevBoxDefinitions/ResourceId-DevCenterDevBoxDefinitionId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/devCenters/{devCenterName}/galleries/{galleryName}`	`.../DevCenter/v2023_04_01/Galleries/ResourceId-GalleryId.cs` `.../DevCenter/v2023_04_01/Images/ResourceId-GalleryId.cs`
`/subscriptions/{subscriptionId}`	`.../DevCenter/v2023_04_01/CheckNameAvailability/ResourceId-SubscriptionId.cs` `.../DevCenter/v2023_04_01/DevCenters/ResourceId-SubscriptionId.cs` `.../DevCenter/v2023_04_01/NetworkConnections/ResourceId-SubscriptionId.cs` `.../DevCenter/v2023_04_01/Projects/ResourceId-SubscriptionId.cs` `.../DevCenter/v2023_04_01/SKUs/ResourceId-SubscriptionId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/projects/{projectName}/devBoxDefinitions/{devBoxDefinitionName}`	`.../DevCenter/v2023_04_01/DevBoxDefinitions/ResourceId-DevBoxDefinitionId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}`	`.../DevCenter/v2023_04_01/DevCenters/ResourceId-ResourceGroupId.cs` `.../DevCenter/v2023_04_01/NetworkConnections/ResourceId-ResourceGroupId.cs` `.../DevCenter/v2023_04_01/Projects/ResourceId-ResourceGroupId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/devCenters/{devCenterName}/environmentTypes/{environmentTypeName}`	`.../DevCenter/v2023_04_01/EnvironmentTypes/ResourceId-DevCenterEnvironmentTypeId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/projects/{projectName}/environmentTypes/{environmentTypeName}`	`.../DevCenter/v2023_04_01/EnvironmentTypes/ResourceId-EnvironmentTypeId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/devCenters/{devCenterName}`	`.../DevCenter/v2023_04_01/AttachedNetworkConnections/ResourceId-DevCenterId.cs` `.../DevCenter/v2023_04_01/Catalogs/ResourceId-DevCenterId.cs` `.../DevCenter/v2023_04_01/DevBoxDefinitions/ResourceId-DevCenterId.cs` `.../DevCenter/v2023_04_01/DevCenters/ResourceId-DevCenterId.cs` `.../DevCenter/v2023_04_01/EnvironmentTypes/ResourceId-DevCenterId.cs` `.../DevCenter/v2023_04_01/Galleries/ResourceId-DevCenterId.cs` `.../DevCenter/v2023_04_01/Images/ResourceId-DevCenterId.cs`
`/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.DevCenter/projects/{projectName}`	`.../DevCenter/v2023_04_01/AttachedNetworkConnections/ResourceId-ProjectId.cs` `.../DevCenter/v2023_04_01/DevBoxDefinitions/ResourceId-ProjectId.cs` `.../DevCenter/v2023_04_01/EnvironmentTypes/ResourceId-ProjectId.cs` `.../DevCenter/v2023_04_01/Pools/ResourceId-ProjectId.cs` `.../DevCenter/v2023_04_01/Projects/ResourceId-ProjectId.cs`

stephybun

Thanks for this PR @jiaweitao001.

Overall it looks like this resource is a good candidate for generation. There are some bits that we have to workout though.

It seems that you've commited all of the changes that happen when you run the importer with your updates. You need to remove these since these will be added by an automatically generated PR which updates the data under data/Pandora.Definitions.ResourceManager.
To repeat: we should not be committing any changes to data/Pandora.Definitions.ResourceManager ourselves.
Since it's a dependency, we probably want to add the shared_gallery_id as a commonid in pandora as well as go-azure-helpers before finalising the generation of this resource.

If you look through the PR history on pandora and go-azure-helpers you should be able to find examples on how that is done.

Do reach out if you have questions or need help. Thanks!

stephybun · 2023-11-02T09:54:27Z

tools/importer-rest-api-specs/components/testing/dependencies.go

@@ -160,6 +168,7 @@ func DetermineDependencies(field, providerPrefix string, dependencies *testDepen
 	dependencyMapping := map[string]dependencyDefinition{
 		"application_insights_id":       {dependencies.setNeedsApplicationInsights, fmt.Sprintf("%s_application_insights.test.id", providerPrefix)},
 		"dev_center_id":                 {dependencies.setNeedsDevCenter, fmt.Sprintf("azurerm_dev_center.test.id")},
+		"gallery_resource_id":           {dependencies.setNeedsSharedImageGallery, fmt.Sprintf("%s_shared_image_gallery.test.id", providerPrefix)},


This name doesn't follow the convention in the provider or for any of the other dependencies in this list - it's implicit that everything is a resource. This should be called

Suggested change

"gallery_resource_id": {dependencies.setNeedsSharedImageGallery, fmt.Sprintf("%s_shared_image_gallery.test.id", providerPrefix)},

"shared_gallery_id": {dependencies.setNeedsSharedImageGallery, fmt.Sprintf("%s_shared_image_gallery.test.id", providerPrefix)},

jiaweitao001 · 2023-11-03T06:27:31Z

Thanks for this PR @jiaweitao001.

Overall it looks like this resource is a good candidate for generation. There are some bits that we have to workout though.

It seems that you've commited all of the changes that happen when you run the importer with your updates. You need to remove these since these will be added by an automatically generated PR which updates the data under data/Pandora.Definitions.ResourceManager.
To repeat: we should not be committing any changes to data/Pandora.Definitions.ResourceManager ourselves.

Since it's a dependency, we probably want to add the shared_gallery_id as a commonid in pandora as well as go-azure-helpers before finalising the generation of this resource.

If you look through the PR history on pandora and go-azure-helpers you should be able to find examples on how that is done.

Do reach out if you have questions or need help. Thanks!

Hi @stephybun , thanks for the feedback. I've updated the PR as suggested.

jiaweitao001 · 2023-11-03T06:58:33Z

Hi @stephybun , after changing the name of gallery_resource_id to shared_gallery_id in the dependency mapping, Pandora cannot correctly map the resource. Do we have to change the name in Swagger also to follow the naming convention?

stephybun · 2023-11-03T07:02:36Z

@jiaweitao001 point 2. in my comment will fix this issue.

Since it's a dependency, we probably want to add the shared_gallery_id as a commonid in pandora as well as go-azure-helpers before finalising the generation of this resource.

We have a doc/guide on this topic which you can follow: commonids

jiaweitao001 · 2023-11-08T05:36:12Z

@jiaweitao001 point 2. in my comment will fix this issue.

Since it's a dependency, we probably want to add the shared_gallery_id as a commonid in pandora as well as go-azure-helpers before finalising the generation of this resource.

We have a doc/guide on this topic which you can follow: commonids

Hi @stephybun , I have the commonid related PRs merged: hashicorp/go-azure-helpers#191 , #3285
Does that mean we can get back to this one and generate code?

jiaweitao001 · 2023-11-14T05:10:53Z

@jiaweitao001 point 2. in my comment will fix this issue.

Since it's a dependency, we probably want to add the shared_gallery_id as a commonid in pandora as well as go-azure-helpers before finalising the generation of this resource.

We have a doc/guide on this topic which you can follow: commonids

Hi @stephybun , I tried to rebase this PR but it looks like the error is still the same. Could you please kindly point out where the commonid related change do its magic and resolves the resource mapping issue? Thanks!

jiaweitao001 · 2023-11-20T09:05:40Z

@jiaweitao001 point 2. in my comment will fix this issue.

Since it's a dependency, we probably want to add the shared_gallery_id as a commonid in pandora as well as go-azure-helpers before finalising the generation of this resource.

We have a doc/guide on this topic which you can follow: commonids

@stephybun , in my understanding the mapping of this property is directly mapping the name in Swagger to the resource name in azurerm. I'm not very sure how this is related to the commonids change I did weeks ago. Can you plz point me where they're related and how can I resolve the missing dependency problem hanging for a few weeks? Looking forward to your reply. Thanks.

stephybun · 2023-11-20T09:58:24Z

@jiaweitao001 it looks like some additional changes are needed since the property gallery_resource_id isn't defined as a resource ID within the swagger spec. I need to check whether there's already functionality available that will allow us to override the name or whether we can add that in quickly.

Since it's nearing thanksgiving in the US we have a few people out so a lot going on. Please bear with me, I will try take a look this week.

stephybun · 2023-11-28T10:42:18Z

@jiaweitao001 I pushed a commit that adds the missing piece, which is to rename gallery_resource_id to shared_gallery_id.

This should generate now without issues, however before we can merge this we need to add the Dev Center Resource ID as a common ID as well. Pandora is currently correctly identifying that Dev Center Gallery is a child resource of Dev Center, so the generated resource uses the Parser defined in commonids (see snippet below from the generated resource)

devCenterId, err := commonids.ParseDevCenterID(config.DevCenterId)
if err != nil {
	return err
}

This throws a linting error because this method doesn't exist.

Can you please add the Dev Center ID as a common ID, then rebase this PR once that's been merged. This should be good to go then.

jiaweitao001 · 2023-11-29T08:14:02Z

@jiaweitao001 I pushed a commit that adds the missing piece, which is to rename gallery_resource_id to shared_gallery_id.

This should generate now without issues, however before we can merge this we need to add the Dev Center Resource ID as a common ID as well. Pandora is currently correctly identifying that Dev Center Gallery is a child resource of Dev Center, so the generated resource uses the Parser defined in commonids (see snippet below from the generated resource)
devCenterId, err := commonids.ParseDevCenterID(config.DevCenterId)
if err != nil {
	return err
}
This throws a linting error because this method doesn't exist.

Can you please add the Dev Center ID as a common ID, then rebase this PR once that's been merged. This should be good to go then.

@stephybun Here is the PR for Dev Center ID as common ID: hashicorp/go-azure-helpers#199

jiaweitao001 · 2024-02-04T09:27:27Z

Hi @stephybun , looks like the checks have all passed. Is there any chance that this PR can get merged?

stephybun · 2024-02-05T17:10:50Z

Thanks @jiaweitao001. I took a look and the resource definition looks fine. Generating the resource works and the output looks good, however:

The basic acceptance test fails because of a naming restriction on the name property

        "code": "InvalidGalleryName",
        "target": "Name",
        "message": "Gallery name is not valid. It must be between 1 and 80 characters, can only include alphanumeric characters, underscores and periods, and can not start or end with '.' or '_'.",

After fixing the value for name the next error suggests that there are some permissions missing

        "code": "DevCenterIsNotAuthorizedToGallery",
        "target": "",
        "message": "The DevCenter MSI does not have access to the provided Compute Gallery resource.",

The easiest way to work around this would be to handwrite the tests but that would require the ability to toggle generation of the tests, unfortunately we do not have this functionality yet.

I recognise that it's cumbersome at the moment to work with generated resources, testing simple and small changes require several manual steps and intervention. Since it's quite time consuming, until we have more automation and tooling for Pandora we would appreciate it if in future you would verify the viability of the resource by generating it and running the tests on your end.

Generally speaking it is also prudent to manually test the API (even seemingly simple ones) to ensure there is a full understanding of it's requirements and behaviour. This makes it possible for us to identify any potential gaps in Pandora ahead of time that could end up being blockers in the end.

Despite all of the work put into this it looks like this will need to be handwritten after all.

stephybun · 2024-02-06T08:05:25Z

Closing in favour of hashicorp/terraform-provider-azurerm#23760

jiaweitao001 · 2024-02-06T08:05:53Z

Thanks @jiaweitao001. I took a look and the resource definition looks fine. Generating the resource works and the output looks good, however:

The basic acceptance test fails because of a naming restriction on the name property
        "code": "InvalidGalleryName",
        "target": "Name",
        "message": "Gallery name is not valid. It must be between 1 and 80 characters, can only include alphanumeric characters, underscores and periods, and can not start or end with '.' or '_'.",
After fixing the value for name the next error suggests that there are some permissions missing
        "code": "DevCenterIsNotAuthorizedToGallery",
        "target": "",
        "message": "The DevCenter MSI does not have access to the provided Compute Gallery resource.",
The easiest way to work around this would be to handwrite the tests but that would require the ability to toggle generation of the tests, unfortunately we do not have this functionality yet.

I recognise that it's cumbersome at the moment to work with generated resources, testing simple and small changes require several manual steps and intervention. Since it's quite time consuming, until we have more automation and tooling for Pandora we would appreciate it if in future you would verify the viability of the resource by generating it and running the tests on your end.

Generally speaking it is also prudent to manually test the API (even seemingly simple ones) to ensure there is a full understanding of it's requirements and behaviour. This makes it possible for us to identify any potential gaps in Pandora ahead of time that could end up being blockers in the end.

Despite all of the work put into this it looks like this will need to be handwritten after all.

Hi @stephybun , thanks for all the hard work and time spent on this issue. I totally understand that we are now at the exploring stage on code gen with Pandora. At first this resource seems like a pretty simple one and a good candidate for code gen, even things went south at last but still it's a good practice for us to grok how we can use this tool. Really appreciate your help on this!

This [isn't actually a generated resource](hashicorp/pandora#3271) and so shouldn't be marked as such else it'll be removed by the automation as in #24805

This was referenced Nov 2, 2023

New Resource: azurerm_dev_center_gallery hashicorp/terraform-provider-azurerm#23760

Merged

New Resource: azurerm_dev_center_gallery hashicorp/terraform-provider-azurerm#23735

Closed

stephybun requested changes Nov 2, 2023

View reviewed changes

jiaweitao001 force-pushed the add_gallery_dependency branch from 6a34c3f to c6a6ec0 Compare November 3, 2023 06:07

jiaweitao001 requested a review from stephybun November 3, 2023 06:27

jiaweitao001 force-pushed the add_gallery_dependency branch from b759a59 to b26a553 Compare November 14, 2023 03:14

jiaweitao001 force-pushed the add_gallery_dependency branch from 13e8755 to 8e29544 Compare December 12, 2023 08:55

tombuildsstuff mentioned this pull request Dec 13, 2023

dependencies: updating to v0.65.0 of github.com/hashicorp/go-azure-helpers hashicorp/go-azure-sdk#768

Merged

jiaweitao001 force-pushed the add_gallery_dependency branch from 8e29544 to cabf0e8 Compare January 9, 2024 08:26

jiaweitao001 force-pushed the add_gallery_dependency branch from cabf0e8 to 55248cd Compare January 18, 2024 05:50

jiaweitao001 added 3 commits January 24, 2024 16:29

Preparation for devcenter gallery resource generation

3ef5bc9

rename to follow naming convention

d51e3be

update overrides of gallery resource id

4937e25

jiaweitao001 force-pushed the add_gallery_dependency branch from 55248cd to 4937e25 Compare January 24, 2024 09:17

stephybun closed this Feb 6, 2024

tombuildsstuff mentioned this pull request Feb 7, 2024

refactor: azurerm_dev_center_gallery is no longer a generated resource hashicorp/terraform-provider-azurerm#24806

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preparation for devcenter gallery resource generation #3271

Preparation for devcenter gallery resource generation #3271

jiaweitao001 commented Nov 2, 2023

hc-github-team-tf-azure commented Nov 2, 2023

stephybun left a comment

stephybun Nov 2, 2023

jiaweitao001 commented Nov 3, 2023

jiaweitao001 commented Nov 3, 2023

stephybun commented Nov 3, 2023

jiaweitao001 commented Nov 8, 2023

jiaweitao001 commented Nov 14, 2023

jiaweitao001 commented Nov 20, 2023

stephybun commented Nov 20, 2023

stephybun commented Nov 28, 2023

jiaweitao001 commented Nov 29, 2023

jiaweitao001 commented Feb 4, 2024

stephybun commented Feb 5, 2024

stephybun commented Feb 6, 2024

jiaweitao001 commented Feb 6, 2024

	"gallery_resource_id": {dependencies.setNeedsSharedImageGallery, fmt.Sprintf("%s_shared_image_gallery.test.id", providerPrefix)},
	"shared_gallery_id": {dependencies.setNeedsSharedImageGallery, fmt.Sprintf("%s_shared_image_gallery.test.id", providerPrefix)},

Preparation for devcenter gallery resource generation #3271

Preparation for devcenter gallery resource generation #3271

Conversation

jiaweitao001 commented Nov 2, 2023

hc-github-team-tf-azure commented Nov 2, 2023

stephybun left a comment

Choose a reason for hiding this comment

stephybun Nov 2, 2023

Choose a reason for hiding this comment

jiaweitao001 commented Nov 3, 2023

jiaweitao001 commented Nov 3, 2023

stephybun commented Nov 3, 2023

jiaweitao001 commented Nov 8, 2023

jiaweitao001 commented Nov 14, 2023

jiaweitao001 commented Nov 20, 2023

stephybun commented Nov 20, 2023

stephybun commented Nov 28, 2023

jiaweitao001 commented Nov 29, 2023

jiaweitao001 commented Feb 4, 2024

stephybun commented Feb 5, 2024

stephybun commented Feb 6, 2024

jiaweitao001 commented Feb 6, 2024