Rename input params for community and direct shared galleries & add validations #204
Conversation
… add validations and unit tests for mutual exclusiveness
|
@JenGoldstrich - pls take a look whenever you get time! This hopefully fixes any issues mentioned before! |
|
@JenGoldstrich @nywilken - can you help take a look? |
|
|
||
| // Id of the direct shared gallery image : /sharedGalleries/{cgName}/Images/{img}[/Versions/{}] (Versions part is optional) | ||
| SharedGalleryImageID string `mapstructure:"directSharedGallery_image_id" required:"false"` | ||
| // Id of the direct shared gallery image : /sharedGalleries/{galleryUniqueName}/Images/{img}[/Versions/{}] (Versions part is optional) |
There was a problem hiding this comment.
If an image version is provided in the community or direct shared gallery image does the value of ImageVersion get used?
Do they both need to be identical in order for things to work?
There was a problem hiding this comment.
I went ahead and updated the documentation to include these new configuration. I think there are a few inconsistencies that we should fast follow on. But I did want to get the documentation in a better place for now.
There was a problem hiding this comment.
@agrawalkshitij tanks for the quick follow up here. I left a few requests for clarifying the documentation. In doing so I came across a few questions about the value of the new gallery types and how they pertain to the exiting shared image gallery type.
builder/azure/arm/config.go
Outdated
| @@ -140,8 +140,8 @@ type Config struct { | |||
| // "gallery_name": "GalleryName", | |||
| // "image_name": "ImageName", | |||
| // "image_version": "1.0.0", | |||
| // "communityGallery_image_id": "/CommunityGalleries/{cg}/Images/{}/Versions/{}", | |||
| // "directSharedGallery_image_id": "/SharedGalleries/{cg}/Images/{}/Versions/{}" | |||
| // "community_gallery_image_id": "/CommunityGalleries/{galleryUniqueName}/Images/{}/Versions/{}", | |||
There was a problem hiding this comment.
This example is not usable as it contains all three gallery types. Instead of adding the values here. I would suggest adding a new section above the resource group usage section in the main builder docs to highlight the three types of shared image gallery sources.
I would also recommend putting correctly formatted example values with links to the azure documentation for the two additional gallery types.
| "image_name": "ignore", | ||
| "image_version": "ignore", | ||
| "community_gallery_image_id": "/CommunityGalleries/cg/Images/img", |
There was a problem hiding this comment.
Going back to a question I asked earlier here since it is in a test. The Shared Gallery requires a version and an image name. But community gallery id looks to contain an image name and could provider a version. If they are provided do they differ from the values in image_name and image_version?
If they don't we may just want to have community and direct shared gallery values by the name of the gallery. Or remove the need for having image_name and image_version be set when using a community or direct shared galleries. What do you think?
|
I updated the documentation to highlight the new Shared Image Gallery block configs. In the last commit I replace privately with private.
|
Includes the following changes:
Also clarified in comments why the gallery name and gallery unique names are 2 diff things, and thus the same fields should not be used.
Closes #203