Added support for Private Image Sharing #391
Conversation
ezilber-akamai
requested review from
Copilot,
jriddle-linode and
lgarber-akamai
and removed request for
a team
There was a problem hiding this comment.
Pull request overview
This PR adds support for automatically adding newly created private images to Image Share Groups in the Linode builder. This allows users to specify share group IDs in their configuration, and the builder will add the image to those groups immediately after creation.
Key changes:
- Added
image_share_group_idsconfiguration parameter to specify share groups - Implemented logic to add images to share groups after image creation
- Added comprehensive test coverage for the new configuration parameter
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| builder/linode/config.go | Added ImageShareGroupIDs field to Config struct with documentation |
| builder/linode/config.hcl2spec.go | Added HCL2 spec mapping for image_share_group_ids parameter |
| builder/linode/step_create_image.go | Implemented logic to add images to share groups after creation |
| builder/linode/builder_test.go | Added test case for ImageShareGroupIDs configuration |
| docs/builders/linode.mdx | Added example usage of image_share_group_ids in documentation |
| .web-docs/components/builder/linode/README.md | Documented the new image_share_group_ids parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _, err := s.client.ImageShareGroupAddImages( | ||
| ctx, | ||
| shareGroupID, | ||
| linodego.ImageShareGroupAddImagesOptions{ | ||
| Images: []linodego.ImageShareGroupImage{ | ||
| { | ||
| ID: image.ID, | ||
| }, | ||
| }, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
The return value from ImageShareGroupAddImages is being discarded. If the API returns important information (such as confirmation details or partial success status), it should be logged or handled appropriately.
There was a problem hiding this comment.
This returns essentially the same exact information that is already contained in the image, so I think it is fine to discard the return value. Happy to hear others' thoughts on this.
|
|
||
| // Add the image to Image Share Groups, if configured | ||
| if len(c.ImageShareGroupIDs) > 0 { | ||
| for _, shareGroupID := range c.ImageShareGroupIDs { |
There was a problem hiding this comment.
Images are being added to share groups sequentially. If multiple share groups are specified, this could be slow. Consider whether parallel addition or batch operations are supported by the API to improve performance.
There was a problem hiding this comment.
The API only supports adding images to one Share Group at a time so I think adding them sequentially is fine. Again, happy to hear others' thoughts.
jriddle-linode
left a comment
jriddle-linode
left a comment
There was a problem hiding this comment.
Looks good nice work!
3 participants
📝 Description
Added the option to add newly created Private Images directly to Image Share Groups.
✔️ How to Test
go test ./builder/linode -run TestBuilderPrepare_ImageShareGroupIDs