review changes

Author: h3adex

docs/data-sources/image.md

@@ -24,15 +24,14 @@ data "stackit_image" "name_match" {
 }
 
 data "stackit_image" "name_regex_latest" {
-  project_id      = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
-  name_regex      = "^Ubuntu .*"
-  sort_descending = true
+  project_id = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
+  name_regex = "^Ubuntu .*"
 }
 
 data "stackit_image" "name_regex_oldest" {
-  project_id      = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
-  name_regex      = "^Ubuntu .*"
-  sort_descending = false
+  project_id     = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
+  name_regex     = "^Ubuntu .*"
+  sort_ascending = true
 }
 
 data "stackit_image" "filter_distro_version" {
@@ -55,9 +54,9 @@ data "stackit_image" "filter_distro_version" {
 
 - `filter` (Attributes) Additional filtering options based on image properties. Can be used independently or in conjunction with `name` or `name_regex`. (see [below for nested schema](#nestedatt--filter))
 - `image_id` (String) Image ID to fetch directly
-- `name` (String) Exact image name to match. Optionally applies a `filter` block to further refine results in case multiple images share the same name. The first match is returned, optionally sorted by name in descending order. Cannot be used together with `name_regex`.
-- `name_regex` (String) Regular expression to match against image names. Optionally applies a `filter` block to narrow down results when multiple image names match the regex. The first match is returned, optionally sorted by name in descending order. Cannot be used together with `name`.
-- `sort_descending` (Boolean) If set to `true`, images are sorted in descending lexicographical order by image name before selecting the first match. Defaults to `false` (ascending).
+- `name` (String) Exact image name to match. Optionally applies a `filter` block to further refine results in case multiple images share the same name. The first match is returned, optionally sorted by name in ascending order. Cannot be used together with `name_regex`.
+- `name_regex` (String) Regular expression to match against image names. Optionally applies a `filter` block to narrow down results when multiple image names match the regex. The first match is returned, optionally sorted by name in ascending order. Cannot be used together with `name`.
+- `sort_ascending` (Boolean) If set to `true`, images are sorted in ascending lexicographical order by image name (such as `Ubuntu 18.04`, `Ubuntu 20.04`, `Ubuntu 22.04`) before selecting the first match. Defaults to `false` (descending such as `Ubuntu 22.04`, `Ubuntu 20.04`, `Ubuntu 18.04`).
 
 ### Read-Only
 

examples/data-sources/stackit_image/data-source.tf

@@ -9,15 +9,14 @@ data "stackit_image" "name_match" {
 }
 
 data "stackit_image" "name_regex_latest" {
-  project_id      = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
-  name_regex      = "^Ubuntu .*"
-  sort_descending = true
+  project_id = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
+  name_regex = "^Ubuntu .*"
 }
 
 data "stackit_image" "name_regex_oldest" {
-  project_id      = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
-  name_regex      = "^Ubuntu .*"
-  sort_descending = false
+  project_id     = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
+  name_regex     = "^Ubuntu .*"
+  sort_ascending = true
 }
 
 data "stackit_image" "filter_distro_version" {

stackit/internal/services/iaas/iaas_acc_test.go

@@ -3332,46 +3332,29 @@ func TestAccImageMin(t *testing.T) {
 				Config: fmt.Sprintf(`
 					%s
 					%s
-					
-					data "stackit_image" "image_id_match" {
+
+					data "stackit_image" "image" {
 						project_id = stackit_image.image.project_id
 						image_id = stackit_image.image.image_id
 					}
-
-					data "stackit_image" "exact_match" {
-					  project_id = var.project_id
-					  name       = "Ubuntu 22.04"
-					}
-
-					data "stackit_image" "ubuntu_latest" {
-					  project_id       = stackit_image.image.project_id
-					  name_regex       = "^Ubuntu .*"
-					  sort_descending  = true
-					}
-				
-					data "stackit_image" "ubuntu_oldest" {
-					  project_id       = stackit_image.image.project_id
-					  name_regex       = "^Ubuntu .*"
-					  sort_descending  = false
-					}
 					`,
 					resourceImageMinConfig, testutil.IaaSProviderConfig(),
 				),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					// Instance
-					resource.TestCheckResourceAttr("data.stackit_image.image_id_match", "project_id", testutil.ConvertConfigVariable(testConfigImageVarsMin["project_id"])),
-					resource.TestCheckResourceAttrPair("data.stackit_image.image_id_match", "image_id", "stackit_image.image", "image_id"),
-					resource.TestCheckResourceAttrPair("data.stackit_image.image_id_match", "name", "stackit_image.image", "name"),
-					resource.TestCheckResourceAttrPair("data.stackit_image.image_id_match", "disk_format", "stackit_image.image", "disk_format"),
-					resource.TestCheckResourceAttrPair("data.stackit_image.image_id_match", "min_disk_size", "stackit_image.image", "min_disk_size"),
-					resource.TestCheckResourceAttrPair("data.stackit_image.image_id_match", "min_ram", "stackit_image.image", "min_ram"),
-					resource.TestCheckResourceAttrPair("data.stackit_image.image_id_match", "protected", "stackit_image.image", "protected"),
-					resource.TestCheckResourceAttrSet("data.stackit_image.image_id_match", "protected"),
-					resource.TestCheckResourceAttrSet("data.stackit_image.image_id_match", "scope"),
-					resource.TestCheckResourceAttrSet("data.stackit_image.image_id_match", "checksum.algorithm"),
-					resource.TestCheckResourceAttrSet("data.stackit_image.image_id_match", "checksum.digest"),
-					resource.TestCheckResourceAttrSet("data.stackit_image.image_id_match", "checksum.algorithm"),
-					resource.TestCheckResourceAttrSet("data.stackit_image.image_id_match", "checksum.digest"),
+					resource.TestCheckResourceAttr("data.stackit_image.image", "project_id", testutil.ConvertConfigVariable(testConfigImageVarsMin["project_id"])),
+					resource.TestCheckResourceAttrPair("data.stackit_image.image", "image_id", "stackit_image.image", "image_id"),
+					resource.TestCheckResourceAttrPair("data.stackit_image.image", "name", "stackit_image.image", "name"),
+					resource.TestCheckResourceAttrPair("data.stackit_image.image", "disk_format", "stackit_image.image", "disk_format"),
+					resource.TestCheckResourceAttrPair("data.stackit_image.image", "min_disk_size", "stackit_image.image", "min_disk_size"),
+					resource.TestCheckResourceAttrPair("data.stackit_image.image", "min_ram", "stackit_image.image", "min_ram"),
+					resource.TestCheckResourceAttrPair("data.stackit_image.image", "protected", "stackit_image.image", "protected"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.image", "protected"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.image", "scope"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.image", "checksum.algorithm"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.image", "checksum.digest"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.image", "checksum.algorithm"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.image", "checksum.digest"),
 				),
 			},
 			// Import
@@ -3582,6 +3565,16 @@ func TestAccImageDatasourceSearchVariants(t *testing.T) {
 					resource.TestCheckResourceAttrSet("data.stackit_image.name_match_ubuntu_22_04", "checksum.algorithm"),
 					resource.TestCheckResourceAttrSet("data.stackit_image.name_match_ubuntu_22_04", "checksum.digest"),
 
+					resource.TestCheckResourceAttr("data.stackit_image.ubuntu_by_image_id", "project_id", testutil.ConvertConfigVariable(testConfigImageVarsMax["project_id"])),
+					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_by_image_id", "image_id"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_by_image_id", "name"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_by_image_id", "min_disk_size"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_by_image_id", "min_ram"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_by_image_id", "protected"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_by_image_id", "scope"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_by_image_id", "checksum.algorithm"),
+					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_by_image_id", "checksum.digest"),
+
 					resource.TestCheckResourceAttr("data.stackit_image.regex_match_ubuntu_22_04", "project_id", testutil.ConvertConfigVariable(testConfigImageVarsMax["project_id"])),
 					resource.TestCheckResourceAttrSet("data.stackit_image.regex_match_ubuntu_22_04", "image_id"),
 					resource.TestCheckResourceAttrSet("data.stackit_image.regex_match_ubuntu_22_04", "name"),
@@ -3650,7 +3643,30 @@ func TestAccImageDatasourceSearchVariants(t *testing.T) {
 					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_arm64_oldest", "protected"),
 					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_arm64_oldest", "scope"),
 					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_arm64_oldest", "checksum.algorithm"),
-					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_arm64_oldest", "checksum.digest")),
+					resource.TestCheckResourceAttrSet("data.stackit_image.ubuntu_arm64_oldest", "checksum.digest"),
+
+					// e2e test that ascending sort is working
+					func(s *terraform.State) error {
+						latest := s.RootModule().Resources["data.stackit_image.ubuntu_arm64_latest"]
+						oldest := s.RootModule().Resources["data.stackit_image.ubuntu_arm64_oldest"]
+
+						if latest == nil {
+							return fmt.Errorf("datasource 'data.stackit_image.ubuntu_arm64_latest' not found")
+						}
+						if oldest == nil {
+							return fmt.Errorf("datasource 'data.stackit_image.ubuntu_arm64_oldest' not found")
+						}
+
+						nameLatest := latest.Primary.Attributes["name"]
+						nameOldest := oldest.Primary.Attributes["name"]
+
+						if nameLatest == nameOldest {
+							return fmt.Errorf("expected image names to differ, but both are %q", nameLatest)
+						}
+
+						return nil
+					},
+				),
 			},
 		},
 	})

stackit/internal/services/iaas/image/datasource.go

@@ -34,13 +34,13 @@ var (
 )
 
 type DataSourceModel struct {
-	Id             types.String `tfsdk:"id"` // needed by TF
-	ProjectId      types.String `tfsdk:"project_id"`
-	ImageId        types.String `tfsdk:"image_id"`
-	Name           types.String `tfsdk:"name"`
-	NameRegex      types.String `tfsdk:"name_regex"`
-	SortDescending types.Bool   `tfsdk:"sort_descending"`
-	Filter         types.Object `tfsdk:"filter"`
+	Id            types.String `tfsdk:"id"` // needed by TF
+	ProjectId     types.String `tfsdk:"project_id"`
+	ImageId       types.String `tfsdk:"image_id"`
+	Name          types.String `tfsdk:"name"`
+	NameRegex     types.String `tfsdk:"name_regex"`
+	SortAscending types.Bool   `tfsdk:"sort_ascending"`
+	Filter        types.Object `tfsdk:"filter"`
 
 	DiskFormat  types.String `tfsdk:"disk_format"`
 	MinDiskSize types.Int64  `tfsdk:"min_disk_size"`
@@ -133,15 +133,15 @@ func (d *imageDataSource) Schema(_ context.Context, _ datasource.SchemaRequest,
 				},
 			},
 			"name": schema.StringAttribute{
-				Description: "Exact image name to match. Optionally applies a `filter` block to further refine results in case multiple images share the same name. The first match is returned, optionally sorted by name in descending order. Cannot be used together with `name_regex`.",
+				Description: "Exact image name to match. Optionally applies a `filter` block to further refine results in case multiple images share the same name. The first match is returned, optionally sorted by name in ascending order. Cannot be used together with `name_regex`.",
 				Optional:    true,
 			},
 			"name_regex": schema.StringAttribute{
-				Description: "Regular expression to match against image names. Optionally applies a `filter` block to narrow down results when multiple image names match the regex. The first match is returned, optionally sorted by name in descending order. Cannot be used together with `name`.",
+				Description: "Regular expression to match against image names. Optionally applies a `filter` block to narrow down results when multiple image names match the regex. The first match is returned, optionally sorted by name in ascending order. Cannot be used together with `name`.",
 				Optional:    true,
 			},
-			"sort_descending": schema.BoolAttribute{
-				Description: "If set to `true`, images are sorted in descending lexicographical order by image name before selecting the first match. Defaults to `false` (ascending).",
+			"sort_ascending": schema.BoolAttribute{
+				Description: "If set to `true`, images are sorted in ascending lexicographical order by image name (such as `Ubuntu 18.04`, `Ubuntu 20.04`, `Ubuntu 22.04`) before selecting the first match. Defaults to `false` (descending such as `Ubuntu 22.04`, `Ubuntu 20.04`, `Ubuntu 18.04`).",
 				Optional:    true,
 			},
 			"filter": schema.SingleNestedAttribute{
@@ -284,7 +284,7 @@ func (d *imageDataSource) Read(ctx context.Context, req datasource.ReadRequest,
 	imageID := model.ImageId.ValueString()
 	name := model.Name.ValueString()
 	nameRegex := model.NameRegex.ValueString()
-	sortDescending := model.SortDescending.ValueBool()
+	sortAscending := model.SortAscending.ValueBool()
 
 	var filter Filter
 	if !model.Filter.IsNull() && !model.Filter.IsUnknown() {
@@ -298,7 +298,7 @@ func (d *imageDataSource) Read(ctx context.Context, req datasource.ReadRequest,
 	ctx = tflog.SetField(ctx, "image_id", imageID)
 	ctx = tflog.SetField(ctx, "name", name)
 	ctx = tflog.SetField(ctx, "name_regex", nameRegex)
-	ctx = tflog.SetField(ctx, "sort_descending", sortDescending)
+	ctx = tflog.SetField(ctx, "sort_ascending", sortAscending)
 
 	var imageResp *iaas.Image
 	var err error
@@ -350,21 +350,9 @@ func (d *imageDataSource) Read(ctx context.Context, req datasource.ReadRequest,
 			}
 		}
 
-		// Step 2: Sort matched images by name (optional, based on sortDescending flag)
+		// Step 2: Sort matched images by name (optional, based on sortAscending flag)
 		if len(matchedImages) > 1 {
-			sort.SliceStable(matchedImages, func(i, j int) bool {
-				a, b := matchedImages[i].Name, matchedImages[j].Name
-				if a == nil {
-					return false
-				}
-				if b == nil {
-					return true
-				}
-				if sortDescending {
-					return *a > *b
-				}
-				return *a < *b
-			})
+			sortImagesByName(matchedImages, sortAscending)
 		}
 
 		// Step 3: Apply additional filtering based on OS, distro, version, UEFI, secure boot, etc.
@@ -517,37 +505,56 @@ func imageMatchesFilter(img *iaas.Image, filter *Filter) bool {
 
 	cfg := img.Config
 
-	if !filter.OS.IsNull() {
-		if cfg.OperatingSystem == nil || filter.OS.ValueString() != *cfg.OperatingSystem {
-			return false
-		}
+	if !filter.OS.IsNull() &&
+		(cfg.OperatingSystem == nil || filter.OS.ValueString() != *cfg.OperatingSystem) {
+		return false
 	}
 
-	if !filter.Distro.IsNull() {
-		if cfg.OperatingSystemDistro == nil || cfg.OperatingSystemDistro.Get() == nil ||
-			filter.Distro.ValueString() != *cfg.OperatingSystemDistro.Get() {
-			return false
-		}
+	if !filter.Distro.IsNull() &&
+		(cfg.OperatingSystemDistro == nil || cfg.OperatingSystemDistro.Get() == nil ||
+			filter.Distro.ValueString() != *cfg.OperatingSystemDistro.Get()) {
+		return false
 	}
 
-	if !filter.Version.IsNull() {
-		if cfg.OperatingSystemVersion == nil || cfg.OperatingSystemVersion.Get() == nil ||
-			filter.Version.ValueString() != *cfg.OperatingSystemVersion.Get() {
-			return false
-		}
+	if !filter.Version.IsNull() &&
+		(cfg.OperatingSystemVersion == nil || cfg.OperatingSystemVersion.Get() == nil ||
+			filter.Version.ValueString() != *cfg.OperatingSystemVersion.Get()) {
+		return false
 	}
 
-	if !filter.UEFI.IsNull() {
-		if cfg.Uefi == nil || filter.UEFI.ValueBool() != *cfg.Uefi {
-			return false
-		}
+	if !filter.UEFI.IsNull() &&
+		(cfg.Uefi == nil || filter.UEFI.ValueBool() != *cfg.Uefi) {
+		return false
 	}
 
-	if !filter.SecureBoot.IsNull() {
-		if cfg.SecureBoot == nil || filter.SecureBoot.ValueBool() != *cfg.SecureBoot {
-			return false
-		}
+	if !filter.SecureBoot.IsNull() &&
+		(cfg.SecureBoot == nil || filter.SecureBoot.ValueBool() != *cfg.SecureBoot) {
+		return false
 	}
 
 	return true
 }
+
+// sortImagesByName sorts a slice of images by name, respecting nils and order direction.
+func sortImagesByName(images []*iaas.Image, sortAscending bool) {
+	if len(images) <= 1 {
+		return
+	}
+
+	sort.SliceStable(images, func(i, j int) bool {
+		a, b := images[i].Name, images[j].Name
+
+		switch {
+		case a == nil && b == nil:
+			return false // Equal
+		case a == nil:
+			return false // Nil goes after non-nil
+		case b == nil:
+			return true // Non-nil goes before nil
+		case sortAscending:
+			return *a < *b
+		default:
+			return *a > *b
+		}
+	})
+}