Merge pull request #13 from platform9/feat/images-properties

feat(images): settable custom image properties

Author: PF9-pushkar

CHANGELOG.md

@@ -26,8 +26,8 @@ All notable changes to this project are documented here. The format is based on
   `pcd_identity_user`, `pcd_identity_role_assignment`, `pcd_identity_application_credential`.
 - Identity data sources: `pcd_identity_project`, `pcd_identity_user`, `pcd_identity_role`.
 - Images (Glance v2): `pcd_images_image` resource (local-file upload + web-download import,
-  status waiter, checksum verify, unprotect-before-delete) and `pcd_images_image` /
-  `pcd_images_image_ids` data sources.
+  status waiter, checksum verify, unprotect-before-delete, settable custom `properties`
+  metadata) and `pcd_images_image` / `pcd_images_image_ids` data sources.
 - Networking (Neutron v2): resources `pcd_networking_network`, `_subnet`, `_secgroup`,
   `_secgroup_rule`, `_router`, `_router_interface`; data sources `pcd_networking_network`,
   `_subnet`, `_secgroup`.

DECISIONS.md

@@ -17,6 +17,7 @@ yet passable on this lab (reason noted). Generated registry docs are not committ
 | Identity (DS) | `pcd_identity_project`, `_user`, `_role` | **VALIDATED** |
 | Images | `pcd_images_image` (local-file upload); web-download import path also exercised | **VALIDATED** |
 | Images (DS) | `pcd_images_image`, `_image_ids` | **VALIDATED** |
+| Images | `pcd_images_image` settable `properties` (custom metadata) | **PENDING** — code-complete; add/replace/remove via JSON-patch, echo-only Read filters Glance system properties to avoid perpetual diff. Acc test extended (create/update/import). Not yet run live (credentials unavailable this session). |
 | Networking | `pcd_networking_network`, `_subnet`, `_secgroup`, `_secgroup_rule`, `_router`, `_router_interface` | **VALIDATED** |
 | Networking (DS) | `pcd_networking_network`, `_subnet`, `_secgroup` | **VALIDATED** |
 | Networking | `pcd_networking_port` | **PENDING** — code-complete, build/vet/lint/docs clean; acceptance test written (create/update/import). Not yet run live: lab credentials were unavailable in this session. No lab-side blocker expected. |

internal/services/images/image_resource.go

@@ -28,6 +28,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/mapplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/setplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
@@ -100,7 +101,7 @@ func (r *imageResource) Schema(_ context.Context, _ resource.SchemaRequest, resp
 			"hidden":           schema.BoolAttribute{Optional: true, Computed: true, Default: booldefault.StaticBool(false), MarkdownDescription: "Whether the image is hidden from the default list."},
 			"tags":             schema.SetAttribute{Optional: true, Computed: true, ElementType: types.StringType, MarkdownDescription: "Tags applied to the image.", PlanModifiers: []planmodifier.Set{setplanmodifier.UseStateForUnknown()}},
 			"verify_checksum":  schema.BoolAttribute{Optional: true, Computed: true, Default: booldefault.StaticBool(true), MarkdownDescription: "Verify the uploaded file's md5 against the Glance checksum (local_file_path only)."},
-			"properties":       schema.MapAttribute{Computed: true, ElementType: types.StringType, MarkdownDescription: "Image properties reported by Glance (read-only in this release).", PlanModifiers: []planmodifier.Map{}},
+			"properties":       schema.MapAttribute{Optional: true, Computed: true, ElementType: types.StringType, MarkdownDescription: "User-defined key/value image properties (custom Glance metadata). Only keys you set here are managed; Glance system/read-only properties are not tracked.", PlanModifiers: []planmodifier.Map{mapplanmodifier.UseStateForUnknown()}},
 			"checksum":         schema.StringAttribute{Computed: true, MarkdownDescription: "md5 checksum of the image data."},
 			"size_bytes":       schema.Int64Attribute{Computed: true, MarkdownDescription: "Size of the image data in bytes."},
 			"status":           schema.StringAttribute{Computed: true, MarkdownDescription: "Glance status (e.g. active)."},
@@ -170,6 +171,14 @@ func (r *imageResource) Create(ctx context.Context, req resource.CreateRequest,
 		vis := images.ImageVisibility(v)
 		createOpts.Visibility = &vis
 	}
+	if !plan.Properties.IsNull() && !plan.Properties.IsUnknown() {
+		var userProps map[string]string
+		resp.Diagnostics.Append(plan.Properties.ElementsAs(ctx, &userProps, false)...)
+		if resp.Diagnostics.HasError() {
+			return
+		}
+		createOpts.Properties = userProps
+	}
 
 	img, err := images.Create(ctx, client, createOpts).Extract()
 	if err != nil {
@@ -274,6 +283,30 @@ func (r *imageResource) Update(ctx context.Context, req resource.UpdateRequest,
 		}
 		patch = append(patch, images.ReplaceImageTags{NewTags: tags})
 	}
+	if !plan.Properties.Equal(state.Properties) {
+		var planProps, stateProps map[string]string
+		if !plan.Properties.IsNull() && !plan.Properties.IsUnknown() {
+			resp.Diagnostics.Append(plan.Properties.ElementsAs(ctx, &planProps, false)...)
+		}
+		if !state.Properties.IsNull() && !state.Properties.IsUnknown() {
+			resp.Diagnostics.Append(state.Properties.ElementsAs(ctx, &stateProps, false)...)
+		}
+		if resp.Diagnostics.HasError() {
+			return
+		}
+		for k, v := range planProps {
+			if sv, ok := stateProps[k]; !ok {
+				patch = append(patch, images.UpdateImageProperty{Op: images.AddOp, Name: k, Value: v})
+			} else if sv != v {
+				patch = append(patch, images.UpdateImageProperty{Op: images.ReplaceOp, Name: k, Value: v})
+			}
+		}
+		for k := range stateProps {
+			if _, ok := planProps[k]; !ok {
+				patch = append(patch, images.UpdateImageProperty{Op: images.RemoveOp, Name: k})
+			}
+		}
+	}
 
 	if len(patch) > 0 {
 		if _, err := images.Update(ctx, client, plan.ID.ValueString(), patch).Extract(); err != nil {
@@ -383,9 +416,23 @@ func (r *imageResource) flatten(ctx context.Context, img *images.Image, m *image
 	diags = append(diags, d...)
 	m.Tags = tags
 
-	props := make(map[string]string, len(img.Properties))
+	// Echo-only: Glance returns many system/read-only properties (os_hash_*, stores,
+	// direct_url, owner_specified.*, ...) via RemainingKeys. Track only the keys the
+	// user manages (already present in the model), or every apply would try to
+	// modify read-only props and the plan would never converge.
+	managed := map[string]struct{}{}
+	if !m.Properties.IsNull() && !m.Properties.IsUnknown() {
+		var cur map[string]string
+		diags = append(diags, m.Properties.ElementsAs(ctx, &cur, false)...)
+		for k := range cur {
+			managed[k] = struct{}{}
+		}
+	}
+	props := make(map[string]string, len(managed))
 	for k, v := range img.Properties {
-		props[k] = fmt.Sprintf("%v", v)
+		if _, ok := managed[k]; ok {
+			props[k] = fmt.Sprintf("%v", v)
+		}
 	}
 	propsMap, d := types.MapValueFrom(ctx, types.StringType, props)
 	diags = append(diags, d...)

internal/services/images/image_resource_test.go

@@ -29,31 +29,42 @@ func TestAccImagesImageResource_basic(t *testing.T) {
 		CheckDestroy:             testAccCheckImageDestroy(t),
 		Steps: []resource.TestStep{
 			{
-				Config: testAccImageConfig(imgFile, "tf-acc-image", 0, false),
+				Config: testAccImageConfig(imgFile, "tf-acc-image", 0, false, `properties = { role = "web", env = "test" }`),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					testAccCheckImageExists(t, resourceName),
 					resource.TestCheckResourceAttr(resourceName, "name", "tf-acc-image"),
 					resource.TestCheckResourceAttr(resourceName, "status", "active"),
 					resource.TestCheckResourceAttr(resourceName, "disk_format", "raw"),
 					resource.TestCheckResourceAttrSet(resourceName, "checksum"),
 					resource.TestCheckResourceAttrSet(resourceName, "size_bytes"),
+					// Only user-set keys are tracked (no os_hash_* system props leaking in).
+					resource.TestCheckResourceAttr(resourceName, "properties.%", "2"),
+					resource.TestCheckResourceAttr(resourceName, "properties.role", "web"),
+					resource.TestCheckResourceAttr(resourceName, "properties.env", "test"),
 					resource.TestCheckResourceAttrPair("data.pcd_images_image.by_name", "id", resourceName, "id"),
 					resource.TestCheckResourceAttr("data.pcd_images_image_ids.by_name", "ids.#", "1"),
 				),
 			},
 			{
-				Config: testAccImageConfig(imgFile, "tf-acc-image-updated", 1, true),
+				// Replace role, remove env, add tier.
+				Config: testAccImageConfig(imgFile, "tf-acc-image-updated", 1, true, `properties = { role = "app", tier = "1" }`),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "name", "tf-acc-image-updated"),
 					resource.TestCheckResourceAttr(resourceName, "min_disk_gb", "1"),
 					resource.TestCheckResourceAttr(resourceName, "protected", "true"),
+					resource.TestCheckResourceAttr(resourceName, "properties.%", "2"),
+					resource.TestCheckResourceAttr(resourceName, "properties.role", "app"),
+					resource.TestCheckResourceAttr(resourceName, "properties.tier", "1"),
+					resource.TestCheckNoResourceAttr(resourceName, "properties.env"),
 				),
 			},
 			{
-				ResourceName:            resourceName,
-				ImportState:             true,
-				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"local_file_path", "image_source_url", "verify_checksum"},
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+				// properties is echo-only, so an imported resource starts with no managed
+				// keys (state has no prior key set); the first apply re-adds them.
+				ImportStateVerifyIgnore: []string{"local_file_path", "image_source_url", "verify_checksum", "properties"},
 			},
 		},
 	})
@@ -72,7 +83,7 @@ func writeTempImage(t *testing.T) string {
 	return p
 }
 
-func testAccImageConfig(path, name string, minDiskGB int, protected bool) string {
+func testAccImageConfig(path, name string, minDiskGB int, protected bool, propsHCL string) string {
 	return fmt.Sprintf(`
 resource "pcd_images_image" "test" {
   name             = %q
@@ -81,6 +92,7 @@ resource "pcd_images_image" "test" {
   local_file_path  = %q
   min_disk_gb      = %d
   protected        = %t
+  %s
 }
 
 data "pcd_images_image" "by_name" {
@@ -91,7 +103,7 @@ data "pcd_images_image" "by_name" {
 data "pcd_images_image_ids" "by_name" {
   name = pcd_images_image.test.name
 }
-`, name, path, minDiskGB, protected)
+`, name, path, minDiskGB, protected, propsHCL)
 }
 
 func testAccCheckImageExists(t *testing.T, n string) resource.TestCheckFunc {