Logging and error handling improvements, bug fixes (#21)

- Uniformed logs and diagnostics:
  - Logging and adding to diagnostics is done by the highest level function (Create/Read/Update/Delete/Import) using `LogAndAddError`
  - Lower-level routines' signature changed to return error instead of writing to diagnostics
  - Standardize summary and details across services
  - Removed manual adding of relevant variables to details (they're in the context, TF adds them to logs)
- Changed validators to be closer to official implementation
- Fix logging wrong output after wait
- Fix Argus checking wrong diagnostics
- Fix Resource Manager not updating state after project update
- Fix unnecessary pointer in LogAndAddError

docs/data-sources/logme_instance.md

@@ -33,7 +33,7 @@ data "stackit_logme_instance" "example" {
 - `cf_organization_guid` (String)
 - `cf_space_guid` (String)
 - `dashboard_url` (String)
-- `id` (String) Terraform's internal resource identifier. It is structured as "`project_id`,`zone_id`".
+- `id` (String) Terraform's internal resource identifier. It is structured as "`project_id`,`instance_id`".
 - `image_url` (String)
 - `name` (String) Instance name.
 - `parameters` (Attributes) (see [below for nested schema](#nestedatt--parameters))

docs/data-sources/mariadb_credentials.md

@@ -34,7 +34,7 @@ data "stackit_mariadb_credentials" "example" {
 - `host` (String)
 - `hosts` (List of String)
 - `http_api_uri` (String)
-- `id` (String) Terraform's internal resource ID. It is structured as "`project_id`,`instance_id`,`credentials_id`".
+- `id` (String) Terraform's internal resource identifier. It is structured as "`project_id`,`instance_id`,`credentials_id`".
 - `name` (String)
 - `password` (String, Sensitive)
 - `port` (Number)

docs/data-sources/mariadb_instance.md

@@ -33,7 +33,7 @@ data "stackit_mariadb_instance" "example" {
 - `cf_organization_guid` (String)
 - `cf_space_guid` (String)
 - `dashboard_url` (String)
-- `id` (String) Terraform's internal resource ID. It is structured as "`project_id`,`instance_id`".
+- `id` (String) Terraform's internal resource identifier. It is structured as "`project_id`,`instance_id`".
 - `image_url` (String)
 - `name` (String) Instance name.
 - `parameters` (Attributes) (see [below for nested schema](#nestedatt--parameters))

docs/resources/mariadb_credentials.md

@@ -33,7 +33,7 @@ resource "stackit_mariadb_credentials" "example" {
 - `host` (String)
 - `hosts` (List of String)
 - `http_api_uri` (String)
-- `id` (String) Terraform's internal resource ID. It is structured as "`project_id`,`instance_id`,`credentials_id`".
+- `id` (String) Terraform's internal resource identifier. It is structured as "`project_id`,`instance_id`,`credentials_id`".
 - `name` (String)
 - `password` (String, Sensitive)
 - `port` (Number)

stackit/core/core.go

@@ -52,5 +52,5 @@ func DiagsToError(diags diag.Diagnostics) error {
 // LogAndAddError Logs the error and adds it to the diags
 func LogAndAddError(ctx context.Context, diags *diag.Diagnostics, summary, detail string) {
 	tflog.Error(ctx, summary)
-	(*diags).AddError(summary, detail)
+	diags.AddError(summary, detail)
 }

stackit/provider.go

@@ -2,6 +2,7 @@ package stackit
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/hashicorp/terraform-plugin-framework/datasource"
 	"github.com/hashicorp/terraform-plugin-framework/provider"
@@ -225,10 +226,7 @@ func (p *Provider) Configure(ctx context.Context, req provider.ConfigureRequest,
 	}
 	roundTripper, err := sdkauth.SetupAuth(sdkConfig)
 	if err != nil {
-		resp.Diagnostics.AddError(
-			"Unable to Setup SDK",
-			err.Error(),
-		)
+		core.LogAndAddError(ctx, &resp.Diagnostics, "Error configuring provider", fmt.Sprintf("Setting up authentication: %v", err))
 		return
 	}
 
@@ -274,7 +272,7 @@ func (p *Provider) Resources(_ context.Context) []func() resource.Resource {
 		postgresInstance.NewInstanceResource,
 		postgresCredentials.NewCredentialsResource,
 		logMeInstance.NewInstanceResource,
-		logMeCredentials.NewlogmeCredentialsResource,
+		logMeCredentials.NewCredentialsResource,
 		mariaDBInstance.NewInstanceResource,
 		mariaDBCredentials.NewCredentialsResource,
 		openSearchInstance.NewInstanceResource,

stackit/services/argus/credential/resource.go

@@ -49,15 +49,15 @@ func (r *credentialResource) Metadata(_ context.Context, req resource.MetadataRe
 }
 
 // Configure adds the provider configured client to the resource.
-func (r *credentialResource) Configure(_ context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) {
+func (r *credentialResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) {
 	// Prevent panic if the provider has not been configured.
 	if req.ProviderData == nil {
 		return
 	}
 
 	providerData, ok := req.ProviderData.(core.ProviderData)
 	if !ok {
-		resp.Diagnostics.AddError("Unexpected Data Source Configure Type", fmt.Sprintf("Expected stackit.ProviderData, got %T", req.ProviderData))
+		core.LogAndAddError(ctx, &resp.Diagnostics, "Error configuring API client", fmt.Sprintf("Expected configure type stackit.ProviderData, got %T", req.ProviderData))
 		return
 	}
 
@@ -76,10 +76,12 @@ func (r *credentialResource) Configure(_ context.Context, req resource.Configure
 	}
 
 	if err != nil {
-		resp.Diagnostics.AddError("Could not Configure API Client", err.Error())
+		core.LogAndAddError(ctx, &resp.Diagnostics, "Error configuring API client", err.Error())
 		return
 	}
+
 	r.client = apiClient
+	tflog.Info(ctx, "Argus credential client configured")
 }
 
 func (r *credentialResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) {
@@ -148,17 +150,20 @@ func (r *credentialResource) Create(ctx context.Context, req resource.CreateRequ
 
 	got, err := r.client.CreateCredential(ctx, instanceId, projectId).Execute()
 	if err != nil {
-		resp.Diagnostics.AddError("Error creating credential", fmt.Sprintf("Calling API: %v", err))
+		core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating credential", fmt.Sprintf("Calling API: %v", err))
 		return
 	}
 	err = mapFields(got.Credentials, &model)
 	if err != nil {
-		core.LogAndAddError(ctx, &resp.Diagnostics, "Error mapping fields", err.Error())
+		core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating credential", fmt.Sprintf("Processing API payload: %v", err))
 		return
 	}
 	diags = resp.State.Set(ctx, &model)
 	resp.Diagnostics.Append(diags...)
-	tflog.Info(ctx, "ARGUS credential created")
+	if resp.Diagnostics.HasError() {
+		return
+	}
+	tflog.Info(ctx, "Argus credential created")
 }
 
 func mapFields(r *argus.Credential, model *Model) error {
@@ -202,17 +207,20 @@ func (r *credentialResource) Read(ctx context.Context, req resource.ReadRequest,
 	userName := model.Username.ValueString()
 	_, err := r.client.GetCredential(ctx, instanceId, projectId, userName).Execute()
 	if err != nil {
-		resp.Diagnostics.AddError("Error reading credential", fmt.Sprintf("Project id = %s, instance id = %s, username = %s: %v", projectId, instanceId, userName, err))
+		core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading credential", fmt.Sprintf("Calling API: %v", err))
 		return
 	}
 	diags = resp.State.Set(ctx, model)
 	resp.Diagnostics.Append(diags...)
-	tflog.Info(ctx, "ARGUS credential read")
+	if resp.Diagnostics.HasError() {
+		return
+	}
+	tflog.Info(ctx, "Argus credential read")
 }
 
-func (r *credentialResource) Update(_ context.Context, _ resource.UpdateRequest, resp *resource.UpdateResponse) { // nolint:gocritic // function signature required by Terraform
+func (r *credentialResource) Update(ctx context.Context, _ resource.UpdateRequest, resp *resource.UpdateResponse) { // nolint:gocritic // function signature required by Terraform
 	// Update shouldn't be called
-	resp.Diagnostics.AddError("Error updating credentials", "credentials can't be updated")
+	core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating credential", "Credential can't be updated")
 }
 
 // Delete deletes the resource and removes the Terraform state on success.
@@ -229,8 +237,8 @@ func (r *credentialResource) Delete(ctx context.Context, req resource.DeleteRequ
 	userName := model.Username.ValueString()
 	_, err := r.client.DeleteCredential(ctx, instanceId, projectId, userName).Execute()
 	if err != nil {
-		resp.Diagnostics.AddError("Error deleting credential", "project id = "+projectId+", instance id = "+instanceId+", username = "+userName+", "+err.Error())
+		core.LogAndAddError(ctx, &resp.Diagnostics, "Error deleting credential", fmt.Sprintf("Calling API: %v", err))
 		return
 	}
-	tflog.Info(ctx, "ARGUS credential deleted")
+	tflog.Info(ctx, "Argus credential deleted")
 }

stackit/services/argus/instance/datasource.go

@@ -9,6 +9,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/types"
+	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/stackitcloud/stackit-sdk-go/core/config"
 	"github.com/stackitcloud/stackit-sdk-go/services/argus"
 	"github.com/stackitcloud/terraform-provider-stackit/stackit/core"
@@ -35,7 +36,7 @@ func (d *instanceDataSource) Metadata(_ context.Context, req datasource.Metadata
 	resp.TypeName = req.ProviderTypeName + "_argus_instance"
 }
 
-func (d *instanceDataSource) Configure(_ context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) {
+func (d *instanceDataSource) Configure(ctx context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) {
 	// Prevent panic if the provider has not been configured.
 	if req.ProviderData == nil {
 		return
@@ -46,7 +47,7 @@ func (d *instanceDataSource) Configure(_ context.Context, req datasource.Configu
 
 	providerData, ok := req.ProviderData.(core.ProviderData)
 	if !ok {
-		resp.Diagnostics.AddError("Unexpected Data Source Configure Type", fmt.Sprintf("Expected stackit.ProviderData, got %T. Please report this issue to the provider developers.", req.ProviderData))
+		core.LogAndAddError(ctx, &resp.Diagnostics, "Error configuring API client", fmt.Sprintf("Expected configure type stackit.ProviderData, got %T", req.ProviderData))
 		return
 	}
 
@@ -62,13 +63,11 @@ func (d *instanceDataSource) Configure(_ context.Context, req datasource.Configu
 		)
 	}
 	if err != nil {
-		resp.Diagnostics.AddError(
-			"Could not Configure API Client",
-			err.Error(),
-		)
+		core.LogAndAddError(ctx, &resp.Diagnostics, "Error configuring API client", fmt.Sprintf("Configuring client: %v", err))
 		return
 	}
 	d.client = apiClient
+	tflog.Info(ctx, "Argus instance client configured")
 }
 
 // Schema defines the schema for the data source.
@@ -202,28 +201,29 @@ func (d *instanceDataSource) Schema(_ context.Context, _ datasource.SchemaReques
 
 // Read refreshes the Terraform state with the latest data.
 func (d *instanceDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { // nolint:gocritic // function signature required by Terraform
-	var state Model
-	diags := req.Config.Get(ctx, &state)
+	var model Model
+	diags := req.Config.Get(ctx, &model)
 	resp.Diagnostics.Append(diags...)
 	if resp.Diagnostics.HasError() {
 		return
 	}
-	projectId := state.ProjectId.ValueString()
-	instanceId := state.InstanceId.ValueString()
+	projectId := model.ProjectId.ValueString()
+	instanceId := model.InstanceId.ValueString()
 	instanceResponse, err := d.client.GetInstance(ctx, instanceId, projectId).Execute()
 	if err != nil {
-		core.LogAndAddError(ctx, &diags, "Unable to read instance", err.Error())
+		core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading instance", fmt.Sprintf("Calling API: %v", err))
 		return
 	}
 
-	err = mapFields(ctx, instanceResponse, &state)
+	err = mapFields(ctx, instanceResponse, &model)
 	if err != nil {
-		core.LogAndAddError(ctx, &diags, "Mapping fields", err.Error())
+		core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading instance", fmt.Sprintf("Processing API payload: %v", err))
 		return
 	}
-	diags = resp.State.Set(ctx, state)
+	diags = resp.State.Set(ctx, model)
 	resp.Diagnostics.Append(diags...)
 	if resp.Diagnostics.HasError() {
 		return
 	}
+	tflog.Info(ctx, "Argus instance read")
 }