diff --git a/internal/locality/regional/schemas_framework.go b/internal/locality/regional/schemas_framework.go index 4b287758bc..797d874f88 100644 --- a/internal/locality/regional/schemas_framework.go +++ b/internal/locality/regional/schemas_framework.go @@ -2,7 +2,9 @@ package regional import ( "github.com/hashicorp/terraform-plugin-framework/action/schema" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/scaleway/scaleway-sdk-go/scw" + "github.com/scaleway/terraform-provider-scaleway/v2/internal/verify" ) // AllRegions returns all valid Scaleway regions as strings @@ -16,9 +18,17 @@ func AllRegions() []string { } // SchemaAttribute returns a Plugin Framework schema attribute for a region field -func SchemaAttribute() schema.StringAttribute { +func SchemaAttribute(description ...string) schema.StringAttribute { + desc := "The region you want to attach the resource to" + if len(description) > 0 { + desc = description[0] + } + return schema.StringAttribute{ Optional: true, - Description: "The region you want to attach the resource to", + Description: desc, + Validators: []validator.String{ + verify.IsStringOneOfWithWarning(AllRegions()), + }, } } diff --git a/internal/locality/zonal/schemas_framework.go b/internal/locality/zonal/schemas_framework.go new file mode 100644 index 0000000000..66fbf47ddb --- /dev/null +++ b/internal/locality/zonal/schemas_framework.go @@ -0,0 +1,18 @@ +package zonal + +import ( + "github.com/hashicorp/terraform-plugin-framework/action/schema" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/scaleway/terraform-provider-scaleway/v2/internal/verify" +) + +// SchemaAttribute returns a Plugin Framework schema attribute for a zone field +func SchemaAttribute(description string) schema.StringAttribute { + return schema.StringAttribute{ + Optional: true, + Description: description, + Validators: []validator.String{ + verify.IsStringOneOfWithWarning(AllZones()), + }, + } +} diff --git a/internal/services/cockpit/action_trigger_test_alert_action.go b/internal/services/cockpit/action_trigger_test_alert_action.go index 8100fd8432..2f6238c66f 100644 --- a/internal/services/cockpit/action_trigger_test_alert_action.go +++ b/internal/services/cockpit/action_trigger_test_alert_action.go @@ -6,11 +6,13 @@ import ( "github.com/hashicorp/terraform-plugin-framework/action" "github.com/hashicorp/terraform-plugin-framework/action/schema" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/scaleway/scaleway-sdk-go/api/cockpit/v1" "github.com/scaleway/scaleway-sdk-go/scw" "github.com/scaleway/terraform-provider-scaleway/v2/internal/locality/regional" "github.com/scaleway/terraform-provider-scaleway/v2/internal/meta" + "github.com/scaleway/terraform-provider-scaleway/v2/internal/verify" ) var ( @@ -62,8 +64,11 @@ func (a *TriggerTestAlertAction) Schema(ctx context.Context, req action.SchemaRe "project_id": schema.StringAttribute{ Required: true, Description: "ID of the Project", + Validators: []validator.String{ + verify.IsStringUUID(), + }, }, - "region": regional.SchemaAttribute(), + "region": regional.SchemaAttribute("The region you want to attach the resource to"), }, } } diff --git a/internal/services/instance/action_server_action.go b/internal/services/instance/action_server_action.go index dcc1dc089b..da4a3cf962 100644 --- a/internal/services/instance/action_server_action.go +++ b/internal/services/instance/action_server_action.go @@ -12,7 +12,9 @@ import ( "github.com/scaleway/scaleway-sdk-go/api/instance/v1" "github.com/scaleway/scaleway-sdk-go/scw" "github.com/scaleway/terraform-provider-scaleway/v2/internal/locality" + "github.com/scaleway/terraform-provider-scaleway/v2/internal/locality/zonal" "github.com/scaleway/terraform-provider-scaleway/v2/internal/meta" + "github.com/scaleway/terraform-provider-scaleway/v2/internal/verify" ) var ( @@ -78,11 +80,11 @@ func (a *ServerAction) Schema(ctx context.Context, req action.SchemaRequest, res "server_id": schema.StringAttribute{ Required: true, Description: "Server id to send the action to", + Validators: []validator.String{ + verify.IsStringUUID(), + }, }, - "zone": schema.StringAttribute{ - Optional: true, - Description: "Zone of server to send the action to", - }, + "zone": zonal.SchemaAttribute("Zone of server to send the action to"), "wait": schema.BoolAttribute{ Optional: true, Description: "Wait for server to finish action", diff --git a/internal/services/jobs/action_start_job_definition_action.go b/internal/services/jobs/action_start_job_definition_action.go index e3be83b767..694cc1e08c 100644 --- a/internal/services/jobs/action_start_job_definition_action.go +++ b/internal/services/jobs/action_start_job_definition_action.go @@ -4,7 +4,6 @@ import ( "context" "fmt" - "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/action" "github.com/hashicorp/terraform-plugin-framework/action/schema" "github.com/hashicorp/terraform-plugin-framework/schema/validator" @@ -14,6 +13,7 @@ import ( "github.com/scaleway/terraform-provider-scaleway/v2/internal/locality" "github.com/scaleway/terraform-provider-scaleway/v2/internal/locality/regional" "github.com/scaleway/terraform-provider-scaleway/v2/internal/meta" + "github.com/scaleway/terraform-provider-scaleway/v2/internal/verify" ) var ( @@ -69,13 +69,10 @@ func (a *StartJobDefinitionAction) Schema(ctx context.Context, req action.Schema Required: true, Description: "ID of the job definition to start. Can be a plain UUID or a regional ID.", Validators: []validator.String{ - stringvalidator.LengthAtLeast(1), + verify.IsStringUUIDOrUUIDWithLocality(), }, }, - "region": schema.StringAttribute{ - Optional: true, - Description: "Region of the job definition. If not set, the region is derived from the job_definition_id when possible or from the provider configuration.", - }, + "region": regional.SchemaAttribute("Region of the job definition. If not set, the region is derived from the job_definition_id when possible or from the provider configuration."), "command": schema.StringAttribute{ Optional: true, Description: "Contextual startup command for this specific job run.", diff --git a/internal/services/keymanager/action_rotate_key_action.go b/internal/services/keymanager/action_rotate_key_action.go index b6535cf184..9faf7776b1 100644 --- a/internal/services/keymanager/action_rotate_key_action.go +++ b/internal/services/keymanager/action_rotate_key_action.go @@ -6,12 +6,14 @@ import ( "github.com/hashicorp/terraform-plugin-framework/action" "github.com/hashicorp/terraform-plugin-framework/action/schema" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" key_manager "github.com/scaleway/scaleway-sdk-go/api/key_manager/v1alpha1" "github.com/scaleway/scaleway-sdk-go/scw" "github.com/scaleway/terraform-provider-scaleway/v2/internal/locality" "github.com/scaleway/terraform-provider-scaleway/v2/internal/locality/regional" "github.com/scaleway/terraform-provider-scaleway/v2/internal/meta" + "github.com/scaleway/terraform-provider-scaleway/v2/internal/verify" ) var ( @@ -60,13 +62,13 @@ func NewRotateKeyAction() action.Action { func (a *RotateKeyAction) Schema(ctx context.Context, req action.SchemaRequest, resp *action.SchemaResponse) { resp.Schema = schema.Schema{ Attributes: map[string]schema.Attribute{ - "region": schema.StringAttribute{ - Optional: true, - Description: "Region of the key. If not set, the region is derived from the key_id when possible or from the provider configuration.", - }, + "region": regional.SchemaAttribute("Region of the key. If not set, the region is derived from the key_id when possible or from the provider configuration."), "key_id": schema.StringAttribute{ Required: true, Description: "ID of the key to rotate (UUID format)", + Validators: []validator.String{ + verify.IsStringUUIDOrUUIDWithLocality(), + }, }, }, } diff --git a/internal/verify/framework.go b/internal/verify/framework.go new file mode 100644 index 0000000000..2a763a0f4a --- /dev/null +++ b/internal/verify/framework.go @@ -0,0 +1,73 @@ +package verify + +import ( + "context" + "regexp" + + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" +) + +// Validators for schema.StringAttribute{} + +func IsStringUUID() validator.String { + return stringvalidator.RegexMatches( + regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`), + "must be a valid UUID", + ) +} + +func IsStringUUIDOrUUIDWithLocality() validator.String { + return stringvalidator.RegexMatches( + regexp.MustCompile(`^([a-zA-Z]{2}-[a-zA-Z]{3}/)?[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`), + "must be a valid UUID or UUID with locality prefix (format: aa-aaa-)", + ) +} + +// IsStringOneOfWithWarning only raises a warning if the string is not oneOf validValues +func IsStringOneOfWithWarning(validValues []string) validator.String { + return ErrorToWarningValidator( + stringvalidator.OneOf(validValues...), + ) +} + +// Converts errors from a validator into warnings +func ErrorToWarningValidator(validator validator.String) validator.String { + return errorToWarningValidator{validator: validator} +} + +type errorToWarningValidator struct { + validator validator.String +} + +func (v errorToWarningValidator) Description(ctx context.Context) string { + return v.validator.Description(ctx) +} + +func (v errorToWarningValidator) MarkdownDescription(ctx context.Context) string { + return v.validator.MarkdownDescription(ctx) +} + +func (v errorToWarningValidator) ValidateString(ctx context.Context, req validator.StringRequest, resp *validator.StringResponse) { + // Create a new response to capture the original diagnostics + validationResp := &validator.StringResponse{} + + // Run the original validator + v.validator.ValidateString(ctx, req, validationResp) + + // Convert any errors to warnings + for _, d := range validationResp.Diagnostics { + if d.Severity() == diag.SeverityError { + // Convert error to warning using the diag.NewWarningDiagnostic function + warningDiag := diag.NewWarningDiagnostic( + d.Summary(), + d.Detail(), + ) + resp.Diagnostics = append(resp.Diagnostics, warningDiag) + } else { + // Keep existing warnings or info + resp.Diagnostics = append(resp.Diagnostics, d) + } + } +} diff --git a/internal/verify/framework_test.go b/internal/verify/framework_test.go new file mode 100644 index 0000000000..782f5b6c05 --- /dev/null +++ b/internal/verify/framework_test.go @@ -0,0 +1,270 @@ +package verify_test + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/scaleway/terraform-provider-scaleway/v2/internal/verify" +) + +func TestStringValidatorUUID(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + testCases := map[string]struct { + errDesc string + value string + wantErr bool + }{ + "valid UUID": { + value: "123e4567-e89b-12d3-a456-426614174000", + wantErr: false, + }, + "valid UUID with all digits": { + value: "01234567-89ab-cdef-0123-456789abcdef", + wantErr: false, + }, + "invalid UUID - wrong format": { + value: "123e4567-e89b-12d3-a456-42661417400", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "invalid UUID - wrong characters": { + value: "123e4567-e89b-12d3-a456-426614174xxx", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "invalid UUID - too short": { + value: "123e4567-e89b-12d3-a456-426614174", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "invalid UUID - too long": { + value: "123e4567-e89b-12d3-a456-4266141740000", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "empty string": { + value: "", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "non-hex characters": { + value: "123e4567-e89b-12d3-a456-42661417400g", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + req := validator.StringRequest{ + ConfigValue: types.StringValue(tc.value), + } + + resp := validator.StringResponse{} + + verify.IsStringUUID().ValidateString(ctx, req, &resp) + + if tc.wantErr { + if !resp.Diagnostics.HasError() { + t.Fatal("expected error, got none") + } + + if tc.errDesc != "" { + errStr := resp.Diagnostics[0].Summary() + if errStr != tc.errDesc { + t.Fatalf("expected error description %q, got %q", tc.errDesc, errStr) + } + } + } else if resp.Diagnostics.HasError() { + t.Fatalf("unexpected error: %v", resp.Diagnostics[0].Summary()) + } + }) + } +} + +func TestStringValidatorUUIDOrUUIDWithLocality(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + testCases := map[string]struct { + errDesc string + value string + wantErr bool + }{ + "valid UUID": { + value: "123e4567-e89b-12d3-a456-426614174000", + wantErr: false, + }, + "valid UUID with all digits": { + value: "01234567-89ab-cdef-0123-456789abcdef", + wantErr: false, + }, + "invalid UUID - wrong format": { + value: "123e4567-e89b-12d3-a456-42661417400", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "invalid UUID - wrong characters": { + value: "123e4567-e89b-12d3-a456-426614174xxx", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "invalid UUID - too short": { + value: "123e4567-e89b-12d3-a456-426614174", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "invalid UUID - too long": { + value: "123e4567-e89b-12d3-a456-4266141740000", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "empty string": { + value: "", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "non-hex characters": { + value: "123e4567-e89b-12d3-a456-42661417400g", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "valid UUID with locality": { + value: "qw-ert/01234567-89ab-cdef-0123-456789abcdef", + wantErr: false, + }, + "valid UUID with uppercase locality": { + value: "YU-IOP/123e4567-e89b-12d3-a456-426614174000", + wantErr: false, + }, + "invalid - locality with invalid delimiter": { + value: "qw/ert/123e4567-e89b-12d3-a456-426614174000", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "invalid - locality with space": { + value: "qw ert/123e4567-e89b-12d3-a456-426614174000", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "invalid - missing uuid": { + value: "qw-ert/", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "invalid - UUID with locality containing special characters": { + value: "qw-@rt/123e4567-e89b-12d3-a456-426614174000", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "invalid - UUID with empty locality": { + value: "/123e4567-e89b-12d3-a456-426614174000", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "malformed UUID after valid prefix": { + value: "qw-ert/123e4567-e89b-12d3-a456-426614174xxx", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + "malformed UUID after valid prefix with slash": { + value: "qw-ert/123e4567-e89b-12d3-a456-426614174000/extra", + wantErr: true, + errDesc: "Invalid Attribute Value Match", + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + t.Parallel() // Add parallel execution for subtests + + req := validator.StringRequest{ + ConfigValue: types.StringValue(tc.value), + } + + resp := validator.StringResponse{} + + verify.IsStringUUIDOrUUIDWithLocality().ValidateString(ctx, req, &resp) + + if tc.wantErr { + if !resp.Diagnostics.HasError() { + t.Fatal("expected error, got none") + } + + if tc.errDesc != "" { + errStr := resp.Diagnostics[0].Summary() + if errStr != tc.errDesc { + t.Fatalf("expected error description %q, got %q", tc.errDesc, errStr) + } + } + } else if resp.Diagnostics.HasError() { + t.Fatalf("unexpected error: %v", resp.Diagnostics[0].Summary()) + } + }) + } +} + +func TestStringValidatorRegionWithWarning(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + validRegions := []string{"fr-par", "nl-ams", "pl-waw"} + + testCases := map[string]struct { + value string + wantWarn bool + }{ + "valid region": { + value: "fr-par", + wantWarn: false, + }, + "invalid region": { + value: "qw-ert", + wantWarn: true, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + req := validator.StringRequest{ + ConfigValue: types.StringValue(tc.value), + } + + resp := validator.StringResponse{} + + verify.IsStringOneOfWithWarning(validRegions).ValidateString(ctx, req, &resp) + + var hasWarning bool + + for _, d := range resp.Diagnostics { + if d.Severity() == diag.SeverityWarning { + hasWarning = true + + break + } + } + + if hasWarning != tc.wantWarn { + t.Fatalf("expected hasWarn=%v, got %v", tc.wantWarn, hasWarning) + } + + // Any raised error should have been turned into warnings + for _, d := range resp.Diagnostics { + if d.Severity() == diag.SeverityError { + t.Fatalf("unexpected error: %s", d.Summary()) + } + } + }) + } +}