From bd0e9c5569edaa479eefe87d63cec9a61a5d7669 Mon Sep 17 00:00:00 2001 From: Jan Obernberger Date: Tue, 11 Aug 2026 17:21:44 +0200 Subject: [PATCH 1/6] feat(iaas): first version --- .../internal/services/iaas/image/resource.go | 189 ++++++++++++++++-- stackit/internal/services/iaas/utils/util.go | 27 +++ 2 files changed, 199 insertions(+), 17 deletions(-) diff --git a/stackit/internal/services/iaas/image/resource.go b/stackit/internal/services/iaas/image/resource.go index 2be96d4df..c2c540b1c 100644 --- a/stackit/internal/services/iaas/image/resource.go +++ b/stackit/internal/services/iaas/image/resource.go @@ -3,10 +3,13 @@ package image import ( "bufio" "context" + "crypto/md5" "errors" "fmt" + "io" "net/http" "os" + "path/filepath" "strings" "time" @@ -45,20 +48,36 @@ var ( ) type Model struct { - Id types.String `tfsdk:"id"` // needed by TF - ProjectId types.String `tfsdk:"project_id"` - Region types.String `tfsdk:"region"` - ImageId types.String `tfsdk:"image_id"` - Name types.String `tfsdk:"name"` - DiskFormat types.String `tfsdk:"disk_format"` - MinDiskSize types.Int64 `tfsdk:"min_disk_size"` - MinRAM types.Int64 `tfsdk:"min_ram"` - Protected types.Bool `tfsdk:"protected"` - Scope types.String `tfsdk:"scope"` - Config types.Object `tfsdk:"config"` - Checksum types.Object `tfsdk:"checksum"` - Labels types.Map `tfsdk:"labels"` - LocalFilePath types.String `tfsdk:"local_file_path"` + Id types.String `tfsdk:"id"` // needed by TF + ProjectId types.String `tfsdk:"project_id"` + Region types.String `tfsdk:"region"` + ImageId types.String `tfsdk:"image_id"` + Name types.String `tfsdk:"name"` + DiskFormat types.String `tfsdk:"disk_format"` + MinDiskSize types.Int64 `tfsdk:"min_disk_size"` + MinRAM types.Int64 `tfsdk:"min_ram"` + Protected types.Bool `tfsdk:"protected"` + Scope types.String `tfsdk:"scope"` + Config types.Object `tfsdk:"config"` + Checksum types.Object `tfsdk:"checksum"` + Labels types.Map `tfsdk:"labels"` + LocalFilePath types.String `tfsdk:"local_file_path"` + ImageFile *imageFileModel `tfsdk:"image_file"` +} + +type localModel struct { + Path types.String `tfsdk:"file_path"` + DisablePlanValidation types.Bool `tfsdk:"disable_plan_validation"` +} + +type downloadModel struct { + URL types.String `tfsdk:"url"` + CachePath types.String `tfsdk:"cache_path"` +} + +type imageFileModel struct { + Local *localModel `tfsdk:"local"` + Download *downloadModel `tfsdk:"download"` } // Struct corresponding to Model.Config @@ -225,7 +244,7 @@ func (r *imageResource) Schema(_ context.Context, _ resource.SchemaRequest, resp }, "local_file_path": schema.StringAttribute{ Description: "The filepath of the raw image file to be uploaded.", - Required: true, + Optional: true, PlanModifiers: []planmodifier.String{ stringplanmodifier.RequiresReplace(), }, @@ -407,6 +426,49 @@ func (r *imageResource) Schema(_ context.Context, _ resource.SchemaRequest, resp ElementType: types.StringType, Optional: true, }, + "image_file": schema.SingleNestedAttribute{ + Description: "Representation of an image file.", + Computed: true, + Optional: true, + PlanModifiers: []planmodifier.Object{ + objectplanmodifier.UseStateForUnknown(), + }, + Attributes: map[string]schema.Attribute{ + "local": schema.SingleNestedAttribute{ + Description: "Representation of a local image file.", + Optional: true, + Attributes: map[string]schema.Attribute{ + "file_path": schema.StringAttribute{ + Description: "Path to the local file.", + Required: true, + Validators: []validator.String{ + // Validating that the file exists in the plan is useful to avoid + // creating an image resource where the local image upload will fail + validate.FileExists(), + }, + }, + "disable_plan_validation": schema.BoolAttribute{ + Description: "Wheter to disable plan-time validation.", + Optional: true, + }, + }, + }, + "download": schema.SingleNestedAttribute{ + Description: "Remote file download settings.", + Optional: true, + Attributes: map[string]schema.Attribute{ + "url": schema.StringAttribute{ + Description: "URL to downlioad the image from.", + Required: true, + }, + "cache_path": schema.StringAttribute{ + Description: "Local path to cache the downloaded image.", + Required: true, + }, + }, + }, + }, + }, }, } } @@ -428,6 +490,22 @@ func (r *imageResource) Create(ctx context.Context, req resource.CreateRequest, ctx = core.InitProviderContext(ctx) + if model.ImageFile == nil || model.ImageFile.Download != nil && model.ImageFile.Local != nil || model.ImageFile.Download == nil && model.ImageFile.Local == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating image", "Error in config") + return + } + var filename string + var err error + if model.ImageFile.Download != nil { + filename, err = downloadImage(ctx, &resp.Diagnostics, model.ImageFile.Download.CachePath.ValueString(), model.ImageFile.Download.URL.ValueString()) + if err != nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error downloading image", fmt.Sprintf("Downloading Image: %v", err)) + return + } + } else { + filename = model.ImageFile.Local.Path.ValueString() + } + // Generate API request body from model payload, err := toCreatePayload(ctx, &model) if err != nil { @@ -468,7 +546,7 @@ func (r *imageResource) Create(ctx context.Context, req resource.CreateRequest, } // Upload image - err = uploadImage(ctx, &resp.Diagnostics, model.LocalFilePath.ValueString(), imageCreateResp.UploadUrl) + err = uploadImage(ctx, &resp.Diagnostics, filename, imageCreateResp.UploadUrl) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating image", fmt.Sprintf("Uploading image: %v", err)) return @@ -875,6 +953,13 @@ func uploadImage(ctx context.Context, diags *diag.Diagnostics, filePath, uploadU if err != nil { return fmt.Errorf("open file: %w", err) } + + defer func() { + if closeErr := file.Close(); closeErr != nil { + core.LogAndAddError(ctx, diags, "Error closing file", closeErr.Error()) + } + }() + stat, err := file.Stat() if err != nil { return fmt.Errorf("stat file: %w", err) @@ -902,6 +987,76 @@ func uploadImage(ctx context.Context, diags *diag.Diagnostics, filePath, uploadU if resp.StatusCode != http.StatusOK { return fmt.Errorf("upload image: %s", resp.Status) } - return nil } + +// file zurückgeben - unit test mock server (dummy file), file pointer checken | diags raus +func downloadImage(ctx context.Context, diags *diag.Diagnostics, cachePath, downloadURL string) (string, error) { + if downloadURL == "" { + return "", fmt.Errorf("upload URL is empty") + } + md5sum := fmt.Sprintf("%x", md5.Sum([]byte(downloadURL))) + // uuid? + // go tmp verzeichnis pro ressource -> kein konflikt + filename := filepath.Join(cachePath, md5sum+".img") + delFile := func() { + if err := os.Remove(filename); err != nil { + tflog.Debug(ctx, "failed to cleanup file") + } + } + unlock := iaasUtils.LockimageDownload(filename) + defer unlock() + // TODO: retry + req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) + if err != nil { + return "", fmt.Errorf("create download request: %w", err) + } + + client := &http.Client{} + resp, err := client.Do(req) + if err != nil { + return "", fmt.Errorf("download image: %w", err) + } + + defer func() { + err = resp.Body.Close() + if err != nil { + // can test if handled, return caller should care + core.LogAndAddError(ctx, diags, "Error downloading image", fmt.Sprintf("Closing response body: %v", err)) + } + }() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("upload image: %s", resp.Status) + } + + info, err := os.Stat(filename) + if err != nil && !os.IsNotExist(err) { + return "", fmt.Errorf("accessing file %q: %w", filename, err) + } + + if info != nil { + if info.Size() != 0 { + // cache hit + return filename, nil + } + delFile() + } + + file, err := os.Create(filename) + if err != nil { + delFile() + return "", fmt.Errorf("creating file: %w", err) + } + defer func() { + err = resp.Body.Close() + if err != nil { + core.LogAndAddError(ctx, diags, "Error uploading image", fmt.Sprintf("Closing response body: %v", err)) + } + }() + _, err = io.Copy(file, resp.Body) + if err != nil { + return "", fmt.Errorf("writing to file: %w", err) + } + return filename, nil +} diff --git a/stackit/internal/services/iaas/utils/util.go b/stackit/internal/services/iaas/utils/util.go index a5f846de2..7237bf313 100644 --- a/stackit/internal/services/iaas/utils/util.go +++ b/stackit/internal/services/iaas/utils/util.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "sync" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" @@ -84,3 +85,29 @@ func ReadXRequestId(ctx context.Context) (string, error) { } return "", fmt.Errorf("no response with type `**http.Response` found in context") } + +// Global map to hold locks for specific imageDownload IDs +// This ensures that creating the same imageDownload in parallel waits for the first one to finish +var ( + imageDownloadLocksMu sync.Mutex + imageDownloadLocks = make(map[string]*sync.Mutex) +) + +// LockimageDownload acquires a lock for a specific imageDownload identifier. +// It returns an unlock function that must be deferred. +func LockimageDownload(id string) func() { + imageDownloadLocksMu.Lock() + mu, ok := imageDownloadLocks[id] + if !ok { + mu = &sync.Mutex{} + imageDownloadLocks[id] = mu + } + imageDownloadLocksMu.Unlock() + + mu.Lock() + + // Return the cleanup function + return func() { + mu.Unlock() + } +} From 1db22dd8a5ad4a60ba9988e314393bc31f5d023b Mon Sep 17 00:00:00 2001 From: Jan Obernberger Date: Mon, 24 Aug 2026 13:35:54 +0200 Subject: [PATCH 2/6] switched to types.object for complex attributes --- .../internal/services/iaas/image/resource.go | 72 +++++++++++++------ 1 file changed, 50 insertions(+), 22 deletions(-) diff --git a/stackit/internal/services/iaas/image/resource.go b/stackit/internal/services/iaas/image/resource.go index c2c540b1c..d9efdbf35 100644 --- a/stackit/internal/services/iaas/image/resource.go +++ b/stackit/internal/services/iaas/image/resource.go @@ -48,21 +48,21 @@ var ( ) type Model struct { - Id types.String `tfsdk:"id"` // needed by TF - ProjectId types.String `tfsdk:"project_id"` - Region types.String `tfsdk:"region"` - ImageId types.String `tfsdk:"image_id"` - Name types.String `tfsdk:"name"` - DiskFormat types.String `tfsdk:"disk_format"` - MinDiskSize types.Int64 `tfsdk:"min_disk_size"` - MinRAM types.Int64 `tfsdk:"min_ram"` - Protected types.Bool `tfsdk:"protected"` - Scope types.String `tfsdk:"scope"` - Config types.Object `tfsdk:"config"` - Checksum types.Object `tfsdk:"checksum"` - Labels types.Map `tfsdk:"labels"` - LocalFilePath types.String `tfsdk:"local_file_path"` - ImageFile *imageFileModel `tfsdk:"image_file"` + Id types.String `tfsdk:"id"` // needed by TF + ProjectId types.String `tfsdk:"project_id"` + Region types.String `tfsdk:"region"` + ImageId types.String `tfsdk:"image_id"` + Name types.String `tfsdk:"name"` + DiskFormat types.String `tfsdk:"disk_format"` + MinDiskSize types.Int64 `tfsdk:"min_disk_size"` + MinRAM types.Int64 `tfsdk:"min_ram"` + Protected types.Bool `tfsdk:"protected"` + Scope types.String `tfsdk:"scope"` + Config types.Object `tfsdk:"config"` + Checksum types.Object `tfsdk:"checksum"` + Labels types.Map `tfsdk:"labels"` + LocalFilePath types.String `tfsdk:"local_file_path"` + ImageFile types.Object `tfsdk:"image_file"` } type localModel struct { @@ -76,8 +76,8 @@ type downloadModel struct { } type imageFileModel struct { - Local *localModel `tfsdk:"local"` - Download *downloadModel `tfsdk:"download"` + Local types.Object `tfsdk:"local"` + Download types.Object `tfsdk:"download"` } // Struct corresponding to Model.Config @@ -428,7 +428,7 @@ func (r *imageResource) Schema(_ context.Context, _ resource.SchemaRequest, resp }, "image_file": schema.SingleNestedAttribute{ Description: "Representation of an image file.", - Computed: true, + Computed: false, Optional: true, PlanModifiers: []planmodifier.Object{ objectplanmodifier.UseStateForUnknown(), @@ -490,20 +490,48 @@ func (r *imageResource) Create(ctx context.Context, req resource.CreateRequest, ctx = core.InitProviderContext(ctx) - if model.ImageFile == nil || model.ImageFile.Download != nil && model.ImageFile.Local != nil || model.ImageFile.Download == nil && model.ImageFile.Local == nil { + if model.ImageFile.IsNull() || model.ImageFile.IsUnknown() { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating image", "Error in config") + } + + var imageFile imageFileModel + diags = model.ImageFile.As(ctx, &imageFile, basetypes.ObjectAsOptions{}) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating image", "Error in config") + } + + isLocal := imageFile.Download.IsNull() || imageFile.Download.IsUnknown() + isDownload := imageFile.Local.IsNull() || imageFile.Local.IsUnknown() + if isDownload && isLocal || !isDownload && !isLocal { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating image", "Error in config") return } + var filename string var err error - if model.ImageFile.Download != nil { - filename, err = downloadImage(ctx, &resp.Diagnostics, model.ImageFile.Download.CachePath.ValueString(), model.ImageFile.Download.URL.ValueString()) + var downloadModel downloadModel + var localModel localModel + if isDownload { + diags = imageFile.Download.As(ctx, &downloadModel, basetypes.ObjectAsOptions{}) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating image", "Error in config") + return + } + filename, err = downloadImage(ctx, &resp.Diagnostics, downloadModel.CachePath.ValueString(), downloadModel.URL.ValueString()) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error downloading image", fmt.Sprintf("Downloading Image: %v", err)) return } } else { - filename = model.ImageFile.Local.Path.ValueString() + diags = imageFile.Download.As(ctx, &localModel, basetypes.ObjectAsOptions{}) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating image", "Error in config") + return + } + filename = localModel.Path.ValueString() } // Generate API request body from model From 49f582872244f6d4d50e2940b2fefed6236d3499 Mon Sep 17 00:00:00 2001 From: Jan Obernberger Date: Tue, 25 Aug 2026 10:59:32 +0200 Subject: [PATCH 3/6] switch to file pointer --- .../internal/services/iaas/image/resource.go | 42 +++++++------------ 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/stackit/internal/services/iaas/image/resource.go b/stackit/internal/services/iaas/image/resource.go index d9efdbf35..19458bfce 100644 --- a/stackit/internal/services/iaas/image/resource.go +++ b/stackit/internal/services/iaas/image/resource.go @@ -519,11 +519,12 @@ func (r *imageResource) Create(ctx context.Context, req resource.CreateRequest, core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating image", "Error in config") return } - filename, err = downloadImage(ctx, &resp.Diagnostics, downloadModel.CachePath.ValueString(), downloadModel.URL.ValueString()) + file, err := downloadImage(ctx, &resp.Diagnostics, downloadModel.URL.ValueString()) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error downloading image", fmt.Sprintf("Downloading Image: %v", err)) return } + filename = file.Name() } else { diags = imageFile.Download.As(ctx, &localModel, basetypes.ObjectAsOptions{}) resp.Diagnostics.Append(diags...) @@ -1019,31 +1020,33 @@ func uploadImage(ctx context.Context, diags *diag.Diagnostics, filePath, uploadU } // file zurückgeben - unit test mock server (dummy file), file pointer checken | diags raus -func downloadImage(ctx context.Context, diags *diag.Diagnostics, cachePath, downloadURL string) (string, error) { +func downloadImage(ctx context.Context, diags *diag.Diagnostics, downloadURL string) (*os.File, error) { if downloadURL == "" { - return "", fmt.Errorf("upload URL is empty") + return nil, fmt.Errorf("upload URL is empty") } md5sum := fmt.Sprintf("%x", md5.Sum([]byte(downloadURL))) // uuid? // go tmp verzeichnis pro ressource -> kein konflikt - filename := filepath.Join(cachePath, md5sum+".img") + tmpDir, err := os.MkdirTemp("", "tf-prodiver-download-*") + if err != nil { + return nil, fmt.Errorf("failed to create temp dir: %w", err) + } + filename := filepath.Join(tmpDir, md5sum+".img") delFile := func() { if err := os.Remove(filename); err != nil { tflog.Debug(ctx, "failed to cleanup file") } } - unlock := iaasUtils.LockimageDownload(filename) - defer unlock() // TODO: retry req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) if err != nil { - return "", fmt.Errorf("create download request: %w", err) + return nil, fmt.Errorf("create download request: %w", err) } client := &http.Client{} resp, err := client.Do(req) if err != nil { - return "", fmt.Errorf("download image: %w", err) + return nil, fmt.Errorf("download image: %w", err) } defer func() { @@ -1055,26 +1058,13 @@ func downloadImage(ctx context.Context, diags *diag.Diagnostics, cachePath, down }() if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("upload image: %s", resp.Status) - } - - info, err := os.Stat(filename) - if err != nil && !os.IsNotExist(err) { - return "", fmt.Errorf("accessing file %q: %w", filename, err) - } - - if info != nil { - if info.Size() != 0 { - // cache hit - return filename, nil - } - delFile() + return nil, fmt.Errorf("upload image: %s", resp.Status) } - file, err := os.Create(filename) + file, err := os.CreateTemp("", filename) if err != nil { delFile() - return "", fmt.Errorf("creating file: %w", err) + return nil, fmt.Errorf("creating file: %w", err) } defer func() { err = resp.Body.Close() @@ -1084,7 +1074,7 @@ func downloadImage(ctx context.Context, diags *diag.Diagnostics, cachePath, down }() _, err = io.Copy(file, resp.Body) if err != nil { - return "", fmt.Errorf("writing to file: %w", err) + return nil, fmt.Errorf("writing to file: %w", err) } - return filename, nil + return file, nil } From 60c955b24d2e5447b576a33e3566d2b5ee335c56 Mon Sep 17 00:00:00 2001 From: Jan Obernberger Date: Tue, 25 Aug 2026 14:31:51 +0200 Subject: [PATCH 4/6] added unit test --- .../internal/services/iaas/image/resource.go | 59 ++++++----- .../services/iaas/image/resource_test.go | 100 ++++++++++++++++++ 2 files changed, 135 insertions(+), 24 deletions(-) diff --git a/stackit/internal/services/iaas/image/resource.go b/stackit/internal/services/iaas/image/resource.go index 19458bfce..007b22a71 100644 --- a/stackit/internal/services/iaas/image/resource.go +++ b/stackit/internal/services/iaas/image/resource.go @@ -519,7 +519,7 @@ func (r *imageResource) Create(ctx context.Context, req resource.CreateRequest, core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating image", "Error in config") return } - file, err := downloadImage(ctx, &resp.Diagnostics, downloadModel.URL.ValueString()) + file, err := downloadImage(ctx, downloadModel.URL.ValueString()) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error downloading image", fmt.Sprintf("Downloading Image: %v", err)) return @@ -1019,62 +1019,73 @@ func uploadImage(ctx context.Context, diags *diag.Diagnostics, filePath, uploadU return nil } -// file zurückgeben - unit test mock server (dummy file), file pointer checken | diags raus -func downloadImage(ctx context.Context, diags *diag.Diagnostics, downloadURL string) (*os.File, error) { +func downloadImage(ctx context.Context, downloadURL string) (*os.File, error) { if downloadURL == "" { - return nil, fmt.Errorf("upload URL is empty") + return nil, fmt.Errorf("download URL is empty") } + md5sum := fmt.Sprintf("%x", md5.Sum([]byte(downloadURL))) - // uuid? - // go tmp verzeichnis pro ressource -> kein konflikt - tmpDir, err := os.MkdirTemp("", "tf-prodiver-download-*") + + tmpDir, err := os.MkdirTemp("", "tf-provider-download-*") if err != nil { return nil, fmt.Errorf("failed to create temp dir: %w", err) } + filename := filepath.Join(tmpDir, md5sum+".img") - delFile := func() { - if err := os.Remove(filename); err != nil { - tflog.Debug(ctx, "failed to cleanup file") + + cleanupOnErr := func() { + if err := os.RemoveAll(tmpDir); err != nil { + tflog.Warn(ctx, "failed to cleanup temp directory", map[string]interface{}{ + "dir": tmpDir, + "error": err.Error(), + }) } } - // TODO: retry + req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) if err != nil { + cleanupOnErr() return nil, fmt.Errorf("create download request: %w", err) } client := &http.Client{} resp, err := client.Do(req) if err != nil { + cleanupOnErr() return nil, fmt.Errorf("download image: %w", err) } defer func() { - err = resp.Body.Close() - if err != nil { - // can test if handled, return caller should care - core.LogAndAddError(ctx, diags, "Error downloading image", fmt.Sprintf("Closing response body: %v", err)) + if err := resp.Body.Close(); err != nil { + tflog.Debug(ctx, "failed to close HTTP response body", map[string]interface{}{ + "error": err.Error(), + }) } }() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("upload image: %s", resp.Status) + cleanupOnErr() + return nil, fmt.Errorf("download image unexpected status: %s", resp.Status) } - file, err := os.CreateTemp("", filename) + file, err := os.Create(filename) if err != nil { - delFile() + cleanupOnErr() return nil, fmt.Errorf("creating file: %w", err) } - defer func() { - err = resp.Body.Close() - if err != nil { - core.LogAndAddError(ctx, diags, "Error uploading image", fmt.Sprintf("Closing response body: %v", err)) - } - }() + _, err = io.Copy(file, resp.Body) if err != nil { + file.Close() + cleanupOnErr() return nil, fmt.Errorf("writing to file: %w", err) } + + if _, err := file.Seek(0, 0); err != nil { + file.Close() + cleanupOnErr() + return nil, fmt.Errorf("seeking file: %w", err) + } + return file, nil } diff --git a/stackit/internal/services/iaas/image/resource_test.go b/stackit/internal/services/iaas/image/resource_test.go index e3e157f87..a59a3a0ef 100644 --- a/stackit/internal/services/iaas/image/resource_test.go +++ b/stackit/internal/services/iaas/image/resource_test.go @@ -1,11 +1,13 @@ package image import ( + "bytes" "context" "fmt" "net/http" "net/http/httptest" "net/url" + "os" "testing" "github.com/google/go-cmp/cmp" @@ -405,3 +407,101 @@ func Test_UploadImage(t *testing.T) { }) } } + +func Test_DownloadImage_EdgeCases(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/404": + w.WriteHeader(http.StatusNotFound) + case "/empty": + w.WriteHeader(http.StatusOK) + case "/large": + w.WriteHeader(http.StatusOK) + _, _ = w.Write(bytes.Repeat([]byte("A"), 1024*1024)) + case "/drop-conn": + hj, ok := w.(http.Hijacker) + if !ok { + http.Error(w, "webserver doesn't support hijacking", http.StatusInternalServerError) + return + } + conn, _, _ := hj.Hijack() + _ = conn.Close() + default: + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("dummy content")) + } + })) + t.Cleanup(server.Close) + + tests := []struct { + name string + ctx context.Context + downloadURL string + wantBytes []byte + wantErr bool + }{{ + name: "ok", + ctx: context.Background(), + downloadURL: server.URL, + wantBytes: []byte("dummy content"), + wantErr: false, + }, + { + name: "invalid_url_format", + ctx: context.Background(), + downloadURL: "http://127.0.0.1:0/invalid", + wantErr: true, + }, + { + name: "status_404_not_found", + ctx: context.Background(), + downloadURL: server.URL + "/404", + wantErr: true, + }, + { + name: "empty_body_200_ok", + ctx: context.Background(), + downloadURL: server.URL + "/empty", + wantBytes: []byte(""), + wantErr: false, + }, + { + name: "large_file_stream", + ctx: context.Background(), + downloadURL: server.URL + "/large", + wantBytes: bytes.Repeat([]byte("A"), 1024*1024), + wantErr: false, + }, + { + name: "connection_dropped_mid_stream", + ctx: context.Background(), + downloadURL: server.URL + "/drop-conn", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + file, err := downloadImage(tt.ctx, tt.downloadURL) + if (err != nil) != tt.wantErr { + t.Fatalf("downloadImage() error = %v, wantErr %v", err, tt.wantErr) + } + + if file != nil { + t.Cleanup(func() { + _ = file.Close() + _ = os.Remove(file.Name()) + }) + + gotBytes, err := os.ReadFile(file.Name()) + if err != nil { + t.Fatalf("failed to read downloaded file: %v", err) + } + + if !bytes.Equal(gotBytes, tt.wantBytes) { + t.Errorf("byte mismatch: got length %d, want length %d", len(gotBytes), len(tt.wantBytes)) + } + } + }) + } +} From 8e00c272ea456a5b9be74bb09511aa477eca9ad2 Mon Sep 17 00:00:00 2001 From: Jan Obernberger Date: Tue, 25 Aug 2026 14:40:59 +0200 Subject: [PATCH 5/6] reverse iaas utils changes --- stackit/internal/services/iaas/utils/util.go | 27 -------------------- 1 file changed, 27 deletions(-) diff --git a/stackit/internal/services/iaas/utils/util.go b/stackit/internal/services/iaas/utils/util.go index 7237bf313..a5f846de2 100644 --- a/stackit/internal/services/iaas/utils/util.go +++ b/stackit/internal/services/iaas/utils/util.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net/http" - "sync" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" @@ -85,29 +84,3 @@ func ReadXRequestId(ctx context.Context) (string, error) { } return "", fmt.Errorf("no response with type `**http.Response` found in context") } - -// Global map to hold locks for specific imageDownload IDs -// This ensures that creating the same imageDownload in parallel waits for the first one to finish -var ( - imageDownloadLocksMu sync.Mutex - imageDownloadLocks = make(map[string]*sync.Mutex) -) - -// LockimageDownload acquires a lock for a specific imageDownload identifier. -// It returns an unlock function that must be deferred. -func LockimageDownload(id string) func() { - imageDownloadLocksMu.Lock() - mu, ok := imageDownloadLocks[id] - if !ok { - mu = &sync.Mutex{} - imageDownloadLocks[id] = mu - } - imageDownloadLocksMu.Unlock() - - mu.Lock() - - // Return the cleanup function - return func() { - mu.Unlock() - } -} From 0f6321b5d35d00732b968850767e3c548255cb9e Mon Sep 17 00:00:00 2001 From: Jan Obernberger Date: Tue, 25 Aug 2026 18:35:29 +0200 Subject: [PATCH 6/6] remove leftover from tests and add cleanup to caller of downloadImage --- stackit/internal/services/iaas/image/resource.go | 2 ++ stackit/internal/services/iaas/image/resource_test.go | 8 +------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/stackit/internal/services/iaas/image/resource.go b/stackit/internal/services/iaas/image/resource.go index 007b22a71..ae5b71673 100644 --- a/stackit/internal/services/iaas/image/resource.go +++ b/stackit/internal/services/iaas/image/resource.go @@ -524,6 +524,8 @@ func (r *imageResource) Create(ctx context.Context, req resource.CreateRequest, core.LogAndAddError(ctx, &resp.Diagnostics, "Error downloading image", fmt.Sprintf("Downloading Image: %v", err)) return } + defer file.Close() + defer os.RemoveAll(filepath.Dir(file.Name())) filename = file.Name() } else { diags = imageFile.Download.As(ctx, &localModel, basetypes.ObjectAsOptions{}) diff --git a/stackit/internal/services/iaas/image/resource_test.go b/stackit/internal/services/iaas/image/resource_test.go index a59a3a0ef..619f62e57 100644 --- a/stackit/internal/services/iaas/image/resource_test.go +++ b/stackit/internal/services/iaas/image/resource_test.go @@ -408,7 +408,7 @@ func Test_UploadImage(t *testing.T) { } } -func Test_DownloadImage_EdgeCases(t *testing.T) { +func Test_DownloadImage(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/404": @@ -441,40 +441,34 @@ func Test_DownloadImage_EdgeCases(t *testing.T) { wantErr bool }{{ name: "ok", - ctx: context.Background(), downloadURL: server.URL, wantBytes: []byte("dummy content"), wantErr: false, }, { name: "invalid_url_format", - ctx: context.Background(), downloadURL: "http://127.0.0.1:0/invalid", wantErr: true, }, { name: "status_404_not_found", - ctx: context.Background(), downloadURL: server.URL + "/404", wantErr: true, }, { name: "empty_body_200_ok", - ctx: context.Background(), downloadURL: server.URL + "/empty", wantBytes: []byte(""), wantErr: false, }, { name: "large_file_stream", - ctx: context.Background(), downloadURL: server.URL + "/large", wantBytes: bytes.Repeat([]byte("A"), 1024*1024), wantErr: false, }, { name: "connection_dropped_mid_stream", - ctx: context.Background(), downloadURL: server.URL + "/drop-conn", wantErr: true, },