From 8fec14017e1b357506faab615369e94f06bbe906 Mon Sep 17 00:00:00 2001 From: Manuel Vaas Date: Mon, 18 May 2026 10:51:29 +0200 Subject: [PATCH 1/2] feat(sfs): add labels to sfs resources relates to STACKITTPR-525 --- docs/data-sources/sfs_export_policy.md | 1 + docs/data-sources/sfs_resource_pool.md | 1 + docs/data-sources/sfs_share.md | 1 + docs/resources/sfs_export_policy.md | 1 + docs/resources/sfs_resource_pool.md | 1 + docs/resources/sfs_share.md | 1 + .../services/sfs/export-policy/datasource.go | 5 + .../services/sfs/export-policy/resource.go | 37 ++++- .../sfs/export-policy/resource_test.go | 128 +++++++++++++++- .../services/sfs/resourcepool/datasource.go | 15 +- .../sfs/resourcepool/datasource_test.go | 2 + .../services/sfs/resourcepool/resource.go | 25 +++ .../sfs/resourcepool/resource_test.go | 13 +- stackit/internal/services/sfs/sfs_acc_test.go | 36 ++++- .../internal/services/sfs/share/datasource.go | 14 +- .../services/sfs/share/datasource_test.go | 1 + .../internal/services/sfs/share/resource.go | 40 ++++- .../services/sfs/share/resource_test.go | 12 +- .../sfs/testdata/export-policy-max.tf | 4 + .../sfs/testdata/resource-pool-max.tf | 4 + .../services/sfs/testdata/share-max.tf | 4 + stackit/internal/utils/labels.go | 45 ++++++ stackit/internal/utils/labels_test.go | 133 ++++++++++++++++ stackit/internal/validate/labels.go | 36 +++++ stackit/internal/validate/labels_test.go | 144 ++++++++++++++++++ 25 files changed, 675 insertions(+), 29 deletions(-) create mode 100644 stackit/internal/utils/labels.go create mode 100644 stackit/internal/utils/labels_test.go create mode 100644 stackit/internal/validate/labels.go create mode 100644 stackit/internal/validate/labels_test.go diff --git a/docs/data-sources/sfs_export_policy.md b/docs/data-sources/sfs_export_policy.md index e60538044..0259fd021 100644 --- a/docs/data-sources/sfs_export_policy.md +++ b/docs/data-sources/sfs_export_policy.md @@ -37,6 +37,7 @@ data "stackit_sfs_export_policy" "example" { ### Read-Only - `id` (String) Terraform's internal resource ID. It is structured as "`project_id`,`region`,`policy_id`". +- `labels` (Map of String) Labels are key-value string pairs which can be attached to a resource pool - `name` (String) Name of the export policy. - `rules` (Attributes List) (see [below for nested schema](#nestedatt--rules)) diff --git a/docs/data-sources/sfs_resource_pool.md b/docs/data-sources/sfs_resource_pool.md index 1ef1e74ed..119848999 100644 --- a/docs/data-sources/sfs_resource_pool.md +++ b/docs/data-sources/sfs_resource_pool.md @@ -39,6 +39,7 @@ data "stackit_sfs_resource_pool" "resourcepool" { - `availability_zone` (String) Availability zone. - `id` (String) Terraform's internal resource ID. It is structured as "`project_id`,`resource_pool_id`". - `ip_acl` (List of String) List of IPs that can mount the resource pool in read-only; IPs must have a subnet mask (e.g. "172.16.0.0/24" for a range of IPs, or "172.16.0.250/32" for a specific IP). +- `labels` (Map of String) Labels are key-value string pairs which can be attached to a resource pool - `name` (String) Name of the resource pool. - `performance_class` (String) Name of the performance class. - `performance_class_downgradable_at` (String) Time when the performance class can be downgraded again. diff --git a/docs/data-sources/sfs_share.md b/docs/data-sources/sfs_share.md index 6cf5979f7..6c99ca7b4 100644 --- a/docs/data-sources/sfs_share.md +++ b/docs/data-sources/sfs_share.md @@ -43,6 +43,7 @@ Note that if this is not set, the Share can only be mounted in read only by clients with IPs matching the IP ACL of the Resource Pool hosting this Share. You can also assign a Share Export Policy after creating the Share - `id` (String) Terraform's internal resource ID. It is structured as "`project_id`,`share_id`". +- `labels` (Map of String) Labels are key-value string pairs which can be attached to a share - `mount_path` (String) Mount path of the Share, used to mount the Share - `name` (String) Name of the Share - `space_hard_limit_gigabytes` (Number) Space hard limit for the Share. diff --git a/docs/resources/sfs_export_policy.md b/docs/resources/sfs_export_policy.md index 08e24c026..8ed85a93f 100644 --- a/docs/resources/sfs_export_policy.md +++ b/docs/resources/sfs_export_policy.md @@ -44,6 +44,7 @@ import { ### Optional +- `labels` (Map of String) Labels are key-value string pairs which can be attached to the resource. - `region` (String) The resource region. If not defined, the provider region is used. - `rules` (Attributes List) (see [below for nested schema](#nestedatt--rules)) diff --git a/docs/resources/sfs_resource_pool.md b/docs/resources/sfs_resource_pool.md index 4f3760925..90dce59df 100644 --- a/docs/resources/sfs_resource_pool.md +++ b/docs/resources/sfs_resource_pool.md @@ -50,6 +50,7 @@ import { ### Optional +- `labels` (Map of String) Labels are key-value string pairs which can be attached to the resource. - `region` (String) The resource region. If not defined, the provider region is used. - `snapshot_policy` (Attributes) Name of the snapshot policy. (see [below for nested schema](#nestedatt--snapshot_policy)) - `snapshots_are_visible` (Boolean) If set to true, snapshots are visible and accessible to users. (default: false) diff --git a/docs/resources/sfs_share.md b/docs/resources/sfs_share.md index 57bd194fb..a571bd4b1 100644 --- a/docs/resources/sfs_share.md +++ b/docs/resources/sfs_share.md @@ -49,6 +49,7 @@ import { Note that if this is set to an empty string, the Share can only be mounted in read only by clients with IPs matching the IP ACL of the Resource Pool hosting this Share. You can also assign a Share Export Policy after creating the Share +- `labels` (Map of String) Labels are key-value string pairs which can be attached to the resource. - `region` (String) The resource region. If not defined, the provider region is used. ### Read-Only diff --git a/stackit/internal/services/sfs/export-policy/datasource.go b/stackit/internal/services/sfs/export-policy/datasource.go index 8d937a279..5de7df03d 100644 --- a/stackit/internal/services/sfs/export-policy/datasource.go +++ b/stackit/internal/services/sfs/export-policy/datasource.go @@ -142,6 +142,11 @@ func (d *exportPolicyDataSource) Schema(_ context.Context, _ datasource.SchemaRe Description: "Name of the export policy.", Computed: true, }, + "labels": schema.MapAttribute{ + Description: "Labels are key-value string pairs which can be attached to a resource pool", + ElementType: types.StringType, + Computed: true, + }, "rules": schema.ListNestedAttribute{ Computed: true, NestedObject: schema.NestedAttributeObject{ diff --git a/stackit/internal/services/sfs/export-policy/resource.go b/stackit/internal/services/sfs/export-policy/resource.go index 04280a718..e5f9908db 100644 --- a/stackit/internal/services/sfs/export-policy/resource.go +++ b/stackit/internal/services/sfs/export-policy/resource.go @@ -47,6 +47,7 @@ type Model struct { ProjectId types.String `tfsdk:"project_id"` ExportPolicyId types.String `tfsdk:"policy_id"` Name types.String `tfsdk:"name"` + Labels types.Map `tfsdk:"labels"` Rules types.List `tfsdk:"rules"` Region types.String `tfsdk:"region"` } @@ -188,6 +189,12 @@ func (r *exportPolicyResource) Schema(_ context.Context, _ resource.SchemaReques stringvalidator.LengthAtLeast(1), }, }, + "labels": schema.MapAttribute{ + Description: "Labels are key-value string pairs which can be attached to the resource.", + ElementType: types.StringType, + Optional: true, + Validators: validate.LabelValidators(), + }, "rules": schema.ListNestedAttribute{ Computed: true, Optional: true, @@ -269,7 +276,7 @@ func (r *exportPolicyResource) Create(ctx context.Context, req resource.CreateRe } } - payload, err := toCreatePayload(&model, rules) + payload, err := toCreatePayload(ctx, &model, rules) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating export policy", fmt.Sprintf("Creating API payload: %v", err)) return @@ -400,7 +407,7 @@ func (r *exportPolicyResource) Update(ctx context.Context, req resource.UpdateRe } } - payload, err := toUpdatePayload(&model, rules) + payload, err := toUpdatePayload(ctx, &model, rules) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating export policy", fmt.Sprintf("Creating API payload: %v", err)) return @@ -511,6 +518,12 @@ func mapFields(ctx context.Context, resp *sfs.GetShareExportPolicyResponse, mode return fmt.Errorf("export policy id not present") } + labels, err := utils.MapLabels(ctx, resp.ShareExportPolicy.Labels, model.Labels) + if err != nil { + return err + } + model.Labels = labels + // iterate over Rules from response if resp.ShareExportPolicy.Rules != nil { rulesList := []attr.Value{} @@ -564,7 +577,7 @@ func mapFields(ctx context.Context, resp *sfs.GetShareExportPolicyResponse, mode } // Build a CreateShareExportPolicyPayload from provider's model -func toCreatePayload(model *Model, rules []rulesModel) (*sfs.CreateShareExportPolicyPayload, error) { +func toCreatePayload(ctx context.Context, model *Model, rules []rulesModel) (*sfs.CreateShareExportPolicyPayload, error) { if model == nil { return nil, fmt.Errorf("nil model") } @@ -572,6 +585,11 @@ func toCreatePayload(model *Model, rules []rulesModel) (*sfs.CreateShareExportPo return nil, fmt.Errorf("nil rules") } + labels, err := utils.LabelsToPayload(ctx, model.Labels) + if err != nil { + return nil, err + } + // iterate over rules var tempRules []sfs.CreateShareExportPolicyRequestRule for _, rule := range rules { @@ -593,7 +611,8 @@ func toCreatePayload(model *Model, rules []rulesModel) (*sfs.CreateShareExportPo // name and rules result := &sfs.CreateShareExportPolicyPayload{ - Name: model.Name.ValueString(), + Name: model.Name.ValueString(), + Labels: &labels, } // Rules should only be set if tempRules has value. Otherwise, the payload would contain `{ "rules": null }` what should be prevented @@ -604,7 +623,7 @@ func toCreatePayload(model *Model, rules []rulesModel) (*sfs.CreateShareExportPo return result, nil } -func toUpdatePayload(model *Model, rules []rulesModel) (*sfs.UpdateShareExportPolicyPayload, error) { +func toUpdatePayload(ctx context.Context, model *Model, rules []rulesModel) (*sfs.UpdateShareExportPolicyPayload, error) { if model == nil { return nil, fmt.Errorf("nil model") } @@ -612,6 +631,11 @@ func toUpdatePayload(model *Model, rules []rulesModel) (*sfs.UpdateShareExportPo return nil, fmt.Errorf("nil rules") } + labels, err := utils.LabelsToPayload(ctx, model.Labels) + if err != nil { + return nil, err + } + // iterate over rules tempRules := make([]sfs.UpdateShareExportPolicyBodyRule, len(rules)) for i, rule := range rules { @@ -635,7 +659,8 @@ func toUpdatePayload(model *Model, rules []rulesModel) (*sfs.UpdateShareExportPo result := &sfs.UpdateShareExportPolicyPayload{ // Rules should *+never** result in a payload where they are defined as null, e.g. `{ "rules": null }`. Instead, // they should either be set to an array (with values or empty) or they shouldn't be present in the payload. - Rules: tempRules, + Rules: tempRules, + Labels: &labels, } return result, nil } diff --git a/stackit/internal/services/sfs/export-policy/resource_test.go b/stackit/internal/services/sfs/export-policy/resource_test.go index 4be266dd1..a8d797e43 100644 --- a/stackit/internal/services/sfs/export-policy/resource_test.go +++ b/stackit/internal/services/sfs/export-policy/resource_test.go @@ -71,6 +71,7 @@ func fixtureResponseModel(rulesModel basetypes.ListValue) *Model { Id: types.StringValue(project_id + ",region,uuid1"), ExportPolicyId: types.StringValue("uuid1"), Rules: rulesModel, + Labels: types.MapNull(types.StringType), Region: types.StringValue("region"), } } @@ -152,14 +153,16 @@ func fixtureRulesPayloadModel() []rulesModel { func fixtureExportPolicyCreatePayload(rules []sfs.CreateShareExportPolicyRequestRule) *sfs.CreateShareExportPolicyPayload { return &sfs.CreateShareExportPolicyPayload{ - Name: "createPayloadName", - Rules: rules, + Name: "createPayloadName", + Rules: rules, + Labels: &map[string]string{}, } } func fixtureExportPolicyUpdatePayload(rules []sfs.UpdateShareExportPolicyBodyRule) *sfs.UpdateShareExportPolicyPayload { return &sfs.UpdateShareExportPolicyPayload{ - Rules: rules, + Rules: rules, + Labels: &map[string]string{}, } } @@ -221,6 +224,84 @@ func TestMapFields(t *testing.T) { region: testRegion, isValid: true, }, + { + name: "Add Labels", + state: &Model{ + ProjectId: types.StringValue(project_id), + }, + input: &sfs.GetShareExportPolicyResponse{ + ShareExportPolicy: &sfs.ShareExportPolicy{ + Id: new("uuid1"), + Rules: fixtureRulesResponse(), + Labels: &map[string]string{ + "foo": "bar", + }, + }, + }, + expectedModel: &Model{ + ProjectId: types.StringValue(project_id), + Id: types.StringValue(project_id + ",region,uuid1"), + ExportPolicyId: types.StringValue("uuid1"), + Rules: fixtureRulesModel(), + Labels: types.MapValueMust(types.StringType, map[string]attr.Value{ + "foo": types.StringValue("bar"), + }), + Region: types.StringValue("region"), + }, + region: testRegion, + isValid: true, + }, + { + name: "Remove Labels through empty map", + state: &Model{ + ProjectId: types.StringValue(project_id), + Labels: types.MapValueMust(types.StringType, map[string]attr.Value{ + "foo": types.StringValue("bar"), + }), + }, + input: &sfs.GetShareExportPolicyResponse{ + ShareExportPolicy: &sfs.ShareExportPolicy{ + Id: new("uuid1"), + Rules: fixtureRulesResponse(), + Labels: &map[string]string{}, + }, + }, + expectedModel: &Model{ + ProjectId: types.StringValue(project_id), + Id: types.StringValue(project_id + ",region,uuid1"), + ExportPolicyId: types.StringValue("uuid1"), + Rules: fixtureRulesModel(), + Labels: types.MapValueMust(types.StringType, map[string]attr.Value{}), + Region: types.StringValue("region"), + }, + region: testRegion, + isValid: true, + }, + { + name: "Remove Labels through missing parameter", + state: &Model{ + ProjectId: types.StringValue(project_id), + Labels: types.MapValueMust(types.StringType, map[string]attr.Value{ + "foo": types.StringValue("bar"), + }), + }, + input: &sfs.GetShareExportPolicyResponse{ + ShareExportPolicy: &sfs.ShareExportPolicy{ + Id: new("uuid1"), + Rules: fixtureRulesResponse(), + }, + }, + expectedModel: &Model{ + ProjectId: types.StringValue(project_id), + Id: types.StringValue(project_id + ",region,uuid1"), + ExportPolicyId: types.StringValue("uuid1"), + Rules: fixtureRulesModel(), + Labels: types.MapValueMust(types.StringType, map[string]attr.Value{}), + Region: types.StringValue("region"), + }, + region: testRegion, + isValid: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -284,10 +365,29 @@ func TestToCreatePayload(t *testing.T) { expected: fixtureExportPolicyCreatePayload(fixtureRulesCreatePayload()), wantErr: false, }, + { + name: "valid label payload", + model: &Model{ + ProjectId: types.StringValue(project_id), + Name: types.StringValue("createPayloadName"), + Labels: types.MapValueMust(types.StringType, map[string]attr.Value{ + "foo": types.StringValue("bar"), + }), + }, + rules: fixtureRulesPayloadModel(), + expected: &sfs.CreateShareExportPolicyPayload{ + Name: "createPayloadName", + Rules: fixtureRulesCreatePayload(), + Labels: &map[string]string{ + "foo": "bar", + }, + }, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := toCreatePayload(tt.model, tt.rules) + got, err := toCreatePayload(context.Background(), tt.model, tt.rules) if (err != nil) != tt.wantErr { t.Errorf("toCreatePayload() error = %v, wantErr %v", err, tt.wantErr) return @@ -342,10 +442,28 @@ func TestToUpdatePayload(t *testing.T) { expected: fixtureExportPolicyUpdatePayload(fixtureRulesUpdatePayload()), wantErr: false, }, + { + name: "valid label payload", + model: &Model{ + ProjectId: types.StringValue(project_id), + Name: types.StringValue("createPayloadName"), + Labels: types.MapValueMust(types.StringType, map[string]attr.Value{ + "foo": types.StringValue("bar"), + }), + }, + rules: fixtureRulesPayloadModel(), + expected: &sfs.UpdateShareExportPolicyPayload{ + Rules: fixtureRulesUpdatePayload(), + Labels: &map[string]string{ + "foo": "bar", + }, + }, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := toUpdatePayload(tt.model, tt.rules) + got, err := toUpdatePayload(context.Background(), tt.model, tt.rules) if (err != nil) != tt.wantErr { t.Errorf("toUpdatePayload() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/stackit/internal/services/sfs/resourcepool/datasource.go b/stackit/internal/services/sfs/resourcepool/datasource.go index 646a8c6d1..65c2c89af 100644 --- a/stackit/internal/services/sfs/resourcepool/datasource.go +++ b/stackit/internal/services/sfs/resourcepool/datasource.go @@ -44,6 +44,7 @@ type dataSourceModel struct { Region types.String `tfsdk:"region"` SnapshotsAreVisible types.Bool `tfsdk:"snapshots_are_visible"` SnapshotPolicy *SnapshotPolicyModel `tfsdk:"snapshot_policy"` + Labels types.Map `tfsdk:"labels"` } type resourcePoolDataSource struct { @@ -210,7 +211,13 @@ func (r *resourcePoolDataSource) Schema(_ context.Context, _ datasource.SchemaRe // the region cannot be found automatically, so it has to be passed Optional: true, Description: "The resource region. Read-only attribute that reflects the provider region.", - }}, + }, + "labels": schema.MapAttribute{ + Description: "Labels are key-value string pairs which can be attached to a resource pool", + ElementType: types.StringType, + Computed: true, + }, + }, } } @@ -269,5 +276,11 @@ func mapDataSourceFields(ctx context.Context, region string, resourcePool *sfs.R } } + labels, err := utils.MapLabels(ctx, resourcePool.Labels, model.Labels) + if err != nil { + return err + } + model.Labels = labels + return nil } diff --git a/stackit/internal/services/sfs/resourcepool/datasource_test.go b/stackit/internal/services/sfs/resourcepool/datasource_test.go index ed5a775d2..0ca4f02c6 100644 --- a/stackit/internal/services/sfs/resourcepool/datasource_test.go +++ b/stackit/internal/services/sfs/resourcepool/datasource_test.go @@ -41,6 +41,7 @@ func TestMapDatasourceFields(t *testing.T) { AvailabilityZone: types.StringNull(), IpAcl: types.ListNull(types.StringType), Name: types.StringNull(), + Labels: types.MapNull(types.StringType), PerformanceClass: types.StringNull(), SizeGigabytes: types.Int32Null(), Region: testRegion, @@ -91,6 +92,7 @@ func TestMapDatasourceFields(t *testing.T) { types.StringValue("baz"), }), Name: types.StringValue("testname"), + Labels: types.MapNull(types.StringType), PerformanceClass: types.StringValue("performance"), SizeGigabytes: types.Int32Value(42), Region: testRegion, diff --git a/stackit/internal/services/sfs/resourcepool/resource.go b/stackit/internal/services/sfs/resourcepool/resource.go index 66e56669e..18e0f4f7a 100644 --- a/stackit/internal/services/sfs/resourcepool/resource.go +++ b/stackit/internal/services/sfs/resourcepool/resource.go @@ -48,6 +48,7 @@ type Model struct { AvailabilityZone types.String `tfsdk:"availability_zone"` IpAcl types.List `tfsdk:"ip_acl"` Name types.String `tfsdk:"name"` + Labels types.Map `tfsdk:"labels"` PerformanceClass types.String `tfsdk:"performance_class"` SizeGigabytes types.Int32 `tfsdk:"size_gigabytes"` SnapshotPolicy types.Object `tfsdk:"snapshot_policy"` @@ -156,6 +157,12 @@ func (r *resourcePoolResource) Schema(_ context.Context, _ resource.SchemaReques validate.NoSeparator(), }, }, + "labels": schema.MapAttribute{ + Description: "Labels are key-value string pairs which can be attached to the resource.", + ElementType: types.StringType, + Optional: true, + Validators: validate.LabelValidators(), + }, "region": schema.StringAttribute{ Optional: true, // must be computed to allow for storing the override value from the provider @@ -544,6 +551,12 @@ func mapFields(ctx context.Context, region string, resourcePool *sfs.ResourcePoo model.IpAcl = types.ListNull(types.StringType) } + labels, err := utils.MapLabels(ctx, resourcePool.Labels, model.Labels) + if err != nil { + return err + } + model.Labels = labels + model.Name = types.StringPointerValue(resourcePool.Name) if pc := resourcePool.PerformanceClass; pc != nil { model.PerformanceClass = types.StringPointerValue(pc.Name) @@ -591,10 +604,16 @@ func toCreatePayload(ctx context.Context, model *Model) (*sfs.CreateResourcePool } } + labels, err := utils.LabelsToPayload(ctx, model.Labels) + if err != nil { + return nil, err + } + result := &sfs.CreateResourcePoolPayload{ AvailabilityZone: model.AvailabilityZone.ValueString(), IpAcl: aclList, Name: model.Name.ValueString(), + Labels: &labels, PerformanceClass: model.PerformanceClass.ValueString(), SizeGigabytes: model.SizeGigabytes.ValueInt32(), SnapshotsAreVisible: model.SnapshotsAreVisible.ValueBoolPointer(), @@ -627,12 +646,18 @@ func toUpdatePayload(ctx context.Context, model *Model) (*sfs.UpdateResourcePool } } + labels, err := utils.LabelsToPayload(ctx, model.Labels) + if err != nil { + return nil, err + } + result := &sfs.UpdateResourcePoolPayload{ IpAcl: aclList, PerformanceClass: model.PerformanceClass.ValueStringPointer(), SizeGigabytes: *sfs.NewNullableInt32(model.SizeGigabytes.ValueInt32Pointer()), SnapshotsAreVisible: model.SnapshotsAreVisible.ValueBoolPointer(), SnapshotPolicyId: *sfs.NewNullableString(snapshotPolicy.Id.ValueStringPointer()), + Labels: &labels, } return result, nil } diff --git a/stackit/internal/services/sfs/resourcepool/resource_test.go b/stackit/internal/services/sfs/resourcepool/resource_test.go index 8a3b41779..2fbf699a0 100644 --- a/stackit/internal/services/sfs/resourcepool/resource_test.go +++ b/stackit/internal/services/sfs/resourcepool/resource_test.go @@ -51,6 +51,7 @@ func TestMapFields(t *testing.T) { AvailabilityZone: types.StringNull(), IpAcl: types.ListNull(types.StringType), Name: types.StringNull(), + Labels: types.MapNull(types.StringType), PerformanceClass: types.StringNull(), SizeGigabytes: types.Int32Null(), Region: testRegion, @@ -95,6 +96,7 @@ func TestMapFields(t *testing.T) { types.StringValue("baz"), }), Name: types.StringValue("testname"), + Labels: types.MapNull(types.StringType), PerformanceClass: types.StringValue("performance"), SizeGigabytes: types.Int32Value(42), Region: testRegion, @@ -147,6 +149,7 @@ func TestToCreatePayload(t *testing.T) { PerformanceClass: "performance", SizeGigabytes: 42, SnapshotPolicyId: new("snapshot-id"), + Labels: &map[string]string{}, }, false, }, @@ -168,14 +171,14 @@ func TestToCreatePayload(t *testing.T) { Name: "testname", PerformanceClass: "performance", SizeGigabytes: 42, + Labels: &map[string]string{}, }, false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - got, err := toCreatePayload(ctx, tt.model) + got, err := toCreatePayload(context.Background(), tt.model) if (err != nil) != tt.wantErr { t.Errorf("toCreatePayload() error = %v, wantErr %v", err, tt.wantErr) return @@ -213,6 +216,7 @@ func TestToUpdatePayload(t *testing.T) { SizeGigabytes: *sfs.NewNullableInt32(utils.Ptr[int32](42)), SnapshotsAreVisible: new(true), SnapshotPolicyId: *sfs.NewNullableString(nil), + Labels: &map[string]string{}, }, false, }, @@ -233,6 +237,7 @@ func TestToUpdatePayload(t *testing.T) { PerformanceClass: new("performance"), SizeGigabytes: *sfs.NewNullableInt32(utils.Ptr[int32](42)), SnapshotPolicyId: *sfs.NewNullableString(nil), + Labels: &map[string]string{}, }, false, }, @@ -253,14 +258,14 @@ func TestToUpdatePayload(t *testing.T) { PerformanceClass: new("performance"), SizeGigabytes: *sfs.NewNullableInt32(utils.Ptr[int32](42)), SnapshotPolicyId: *sfs.NewNullableString(nil), + Labels: &map[string]string{}, }, false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - got, err := toUpdatePayload(ctx, tt.model) + got, err := toUpdatePayload(context.Background(), tt.model) if (err != nil) != tt.wantErr { t.Errorf("toUpdatePayload() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/stackit/internal/services/sfs/sfs_acc_test.go b/stackit/internal/services/sfs/sfs_acc_test.go index 6e67866af..a065bff83 100644 --- a/stackit/internal/services/sfs/sfs_acc_test.go +++ b/stackit/internal/services/sfs/sfs_acc_test.go @@ -78,6 +78,7 @@ var testConfigExportPolicyVarsMax = config.Variables{ "second_rule_ip_acl_2": config.StringVariable("172.16.0.250/32"), "second_rule_read_only": config.BoolVariable(true), "second_rule_super_user": config.BoolVariable(false), + "label": config.StringVariable("foo"), } var testConfigExportPolicyVarsMaxUpdated = func() config.Variables { @@ -87,6 +88,7 @@ var testConfigExportPolicyVarsMaxUpdated = func() config.Variables { updatedConfig["first_rule_description"] = config.StringVariable("Some other description") updatedConfig["first_rule_ip_acl_1"] = config.StringVariable("172.17.0.0/24") updatedConfig["first_rule_ip_acl_2"] = config.StringVariable("172.17.0.250/32") + updatedConfig["label"] = config.StringVariable("bar") return updatedConfig } @@ -126,6 +128,7 @@ var testConfigResourcePoolVarsMax = config.Variables{ "size_gigabytes": config.IntegerVariable(512), "snapshots_are_visible": config.BoolVariable(true), "snapshot_policy_id": config.StringVariable("2b138c3b-2453-11f1-97cd-d039eac4b54e"), + "label": config.StringVariable("foo"), } var testConfigResourcePoolVarsMaxUpdated = func() config.Variables { @@ -136,6 +139,7 @@ var testConfigResourcePoolVarsMaxUpdated = func() config.Variables { updatedConfig["size_gigabytes"] = config.IntegerVariable(1024) updatedConfig["ip_acl_1"] = config.StringVariable("172.17.0.0/24") updatedConfig["ip_acl_2"] = config.StringVariable("172.17.0.250/32") + updatedConfig["label"] = config.StringVariable("bar") return updatedConfig } @@ -164,12 +168,14 @@ var testConfigShareVarsMax = config.Variables{ "resource_pool_name": config.StringVariable("tf-acc-" + acctest.RandStringFromCharSet(8, acctest.CharSetAlpha)), "space_hard_limit_gigabytes": config.IntegerVariable(42), "export_policy_name": config.StringVariable("tf-acc-" + acctest.RandStringFromCharSet(8, acctest.CharSetAlpha)), + "label": config.StringVariable("foo"), } var testConfigShareVarsMaxUpdated = func() config.Variables { updatedConfig := config.Variables{} maps.Copy(updatedConfig, testConfigShareVarsMax) updatedConfig["space_hard_limit_gigabytes"] = config.IntegerVariable(50) + updatedConfig["label"] = config.StringVariable("bar") return updatedConfig } @@ -196,6 +202,7 @@ func TestAccExportPolicyMin(t *testing.T) { resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "name", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMin["name"])), resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.#", "0"), + resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "labels.%", "0"), ), }, // Data source @@ -225,6 +232,7 @@ func TestAccExportPolicyMin(t *testing.T) { ), resource.TestCheckResourceAttr("data.stackit_sfs_export_policy.policy_data_test", "name", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMin["name"])), + resource.TestCheckResourceAttr("data.stackit_sfs_export_policy.policy_data_test", "labels.%", "0"), resource.TestCheckResourceAttr("data.stackit_sfs_export_policy.policy_data_test", "rules.#", "0"), ), }, @@ -262,6 +270,7 @@ func TestAccExportPolicyMin(t *testing.T) { resource.TestCheckResourceAttrSet("stackit_sfs_export_policy.exportpolicy", "policy_id"), resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "name", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMinUpdated()["name"])), + resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "labels.%", "0"), resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.#", "0"), ), }, @@ -304,6 +313,8 @@ func TestAccExportPolicyMax(t *testing.T) { resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.1.read_only", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMax["second_rule_read_only"])), resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.1.set_uuid", "false"), // default value resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.1.super_user", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMax["second_rule_super_user"])), + resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "labels.%", "1"), + resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "labels.label", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMax["label"])), ), }, // Data source @@ -351,6 +362,8 @@ func TestAccExportPolicyMax(t *testing.T) { resource.TestCheckResourceAttr("data.stackit_sfs_export_policy.policy_data_test", "rules.1.read_only", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMax["second_rule_read_only"])), resource.TestCheckResourceAttr("data.stackit_sfs_export_policy.policy_data_test", "rules.1.set_uuid", "false"), // default value resource.TestCheckResourceAttr("data.stackit_sfs_export_policy.policy_data_test", "rules.1.super_user", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMax["second_rule_super_user"])), + resource.TestCheckResourceAttr("data.stackit_sfs_export_policy.policy_data_test", "labels.%", "1"), + resource.TestCheckResourceAttr("data.stackit_sfs_export_policy.policy_data_test", "labels.label", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMax["label"])), ), }, // Import @@ -405,6 +418,8 @@ func TestAccExportPolicyMax(t *testing.T) { resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.1.read_only", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMaxUpdated()["second_rule_read_only"])), resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.1.set_uuid", "false"), // default value resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.1.super_user", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMaxUpdated()["second_rule_super_user"])), + resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "labels.%", "1"), + resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "labels.label", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMaxUpdated()["label"])), ), }, // Deletion is done by the framework implicitly @@ -435,6 +450,7 @@ func TestAccResourcePoolResourceMin(t *testing.T) { resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "ip_acl.1", testutil.ConvertConfigVariable(testConfigResourcePoolVarsMin["ip_acl_2"])), resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "snapshots_are_visible", "false"), resource.TestCheckNoResourceAttr("stackit_sfs_resource_pool.resourcepool", "snapshot_policy"), + resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "labels.%", "0"), ), }, // Data source @@ -471,6 +487,7 @@ func TestAccResourcePoolResourceMin(t *testing.T) { resource.TestCheckResourceAttr("data.stackit_sfs_resource_pool.resource_pool_ds", "ip_acl.1", testutil.ConvertConfigVariable(testConfigResourcePoolVarsMin["ip_acl_2"])), resource.TestCheckResourceAttr("data.stackit_sfs_resource_pool.resource_pool_ds", "snapshots_are_visible", "false"), resource.TestCheckNoResourceAttr("data.stackit_sfs_resource_pool.resource_pool_ds", "snapshot_policy"), + resource.TestCheckResourceAttr("data.stackit_sfs_resource_pool.resource_pool_ds", "labels.%", "0"), ), }, // Import @@ -514,6 +531,7 @@ func TestAccResourcePoolResourceMin(t *testing.T) { resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "ip_acl.1", testutil.ConvertConfigVariable(testConfigResourcePoolVarsMinUpdated()["ip_acl_2"])), resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "snapshots_are_visible", "false"), resource.TestCheckNoResourceAttr("stackit_sfs_resource_pool.resourcepool", "snapshot_policy"), + resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "labels.%", "0"), ), }, // Deletion is done by the framework implicitly @@ -545,6 +563,8 @@ func TestAccResourcePoolResourceMax(t *testing.T) { resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "snapshots_are_visible", testutil.ConvertConfigVariable(testConfigResourcePoolVarsMax["snapshots_are_visible"])), resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "snapshot_policy.id", testutil.ConvertConfigVariable(testConfigResourcePoolVarsMax["snapshot_policy_id"])), resource.TestCheckResourceAttrSet("stackit_sfs_resource_pool.resourcepool", "snapshot_policy.name"), + resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "labels.%", "1"), + resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "labels.label", testutil.ConvertConfigVariable(testConfigResourcePoolVarsMax["label"])), ), }, // Data source @@ -585,6 +605,8 @@ func TestAccResourcePoolResourceMax(t *testing.T) { "data.stackit_sfs_resource_pool.resource_pool_ds", "snapshot_policy.name", "stackit_sfs_resource_pool.resourcepool", "snapshot_policy.name", ), + resource.TestCheckResourceAttr("data.stackit_sfs_resource_pool.resource_pool_ds", "labels.%", "1"), + resource.TestCheckResourceAttr("data.stackit_sfs_resource_pool.resource_pool_ds", "labels.label", testutil.ConvertConfigVariable(testConfigResourcePoolVarsMax["label"])), ), }, // Import @@ -628,7 +650,10 @@ func TestAccResourcePoolResourceMax(t *testing.T) { resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "ip_acl.1", testutil.ConvertConfigVariable(testConfigResourcePoolVarsMaxUpdated()["ip_acl_2"])), resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "snapshots_are_visible", testutil.ConvertConfigVariable(testConfigResourcePoolVarsMaxUpdated()["snapshots_are_visible"])), resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "snapshot_policy.id", testutil.ConvertConfigVariable(testConfigResourcePoolVarsMax["snapshot_policy_id"])), - resource.TestCheckResourceAttrSet("stackit_sfs_resource_pool.resourcepool", "snapshot_policy.name")), + resource.TestCheckResourceAttrSet("stackit_sfs_resource_pool.resourcepool", "snapshot_policy.name"), + resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "labels.%", "1"), + resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "labels.label", testutil.ConvertConfigVariable(testConfigResourcePoolVarsMaxUpdated()["label"])), + ), }, // Deletion is done by the framework implicitly }, @@ -657,6 +682,7 @@ func TestAccShareResourceMin(t *testing.T) { resource.TestCheckResourceAttr("stackit_sfs_share.share", "space_hard_limit_gigabytes", testutil.ConvertConfigVariable(testConfigShareVarsMin["space_hard_limit_gigabytes"])), resource.TestCheckNoResourceAttr("stackit_sfs_share.share", "export_policy"), resource.TestCheckResourceAttrSet("stackit_sfs_share.share", "mount_path"), + resource.TestCheckResourceAttr("stackit_sfs_share.share", "labels.%", "0"), ), }, // Data source @@ -693,6 +719,7 @@ func TestAccShareResourceMin(t *testing.T) { resource.TestCheckResourceAttr("data.stackit_sfs_share.share_ds", "space_hard_limit_gigabytes", testutil.ConvertConfigVariable(testConfigShareVarsMin["space_hard_limit_gigabytes"])), resource.TestCheckNoResourceAttr("data.stackit_sfs_share.share_ds", "export_policy"), resource.TestCheckResourceAttrSet("data.stackit_sfs_share.share_ds", "mount_path"), + resource.TestCheckResourceAttr("data.stackit_sfs_share.share_ds", "labels.%", "0"), ), }, // Import @@ -739,6 +766,7 @@ func TestAccShareResourceMin(t *testing.T) { resource.TestCheckResourceAttr("stackit_sfs_share.share", "space_hard_limit_gigabytes", testutil.ConvertConfigVariable(testConfigShareVarsMinUpdated()["space_hard_limit_gigabytes"])), resource.TestCheckNoResourceAttr("stackit_sfs_share.share", "export_policy"), resource.TestCheckResourceAttrSet("stackit_sfs_share.share", "mount_path"), + resource.TestCheckResourceAttr("stackit_sfs_share.share", "labels.%", "0"), ), }, // Deletion is done by the framework implicitly @@ -771,6 +799,8 @@ func TestAccShareResourceMax(t *testing.T) { "stackit_sfs_export_policy.exportpolicy", "name", ), resource.TestCheckResourceAttrSet("stackit_sfs_share.share", "mount_path"), + resource.TestCheckResourceAttr("stackit_sfs_share.share", "labels.%", "1"), + resource.TestCheckResourceAttr("stackit_sfs_share.share", "labels.label", testutil.ConvertConfigVariable(testConfigShareVarsMax["label"])), ), }, // Data source @@ -810,6 +840,8 @@ func TestAccShareResourceMax(t *testing.T) { "stackit_sfs_export_policy.exportpolicy", "name", ), resource.TestCheckResourceAttrSet("data.stackit_sfs_share.share_ds", "mount_path"), + resource.TestCheckResourceAttr("data.stackit_sfs_share.share_ds", "labels.%", "1"), + resource.TestCheckResourceAttr("data.stackit_sfs_share.share_ds", "labels.label", testutil.ConvertConfigVariable(testConfigShareVarsMax["label"])), ), }, // Import @@ -859,6 +891,8 @@ func TestAccShareResourceMax(t *testing.T) { "stackit_sfs_export_policy.exportpolicy", "name", ), resource.TestCheckResourceAttrSet("stackit_sfs_share.share", "mount_path"), + resource.TestCheckResourceAttr("stackit_sfs_share.share", "labels.%", "1"), + resource.TestCheckResourceAttr("stackit_sfs_share.share", "labels.label", testutil.ConvertConfigVariable(testConfigShareVarsMaxUpdated()["label"])), ), }, // Deletion is done by the framework implicitly diff --git a/stackit/internal/services/sfs/share/datasource.go b/stackit/internal/services/sfs/share/datasource.go index a1c4a9cc9..28899614e 100644 --- a/stackit/internal/services/sfs/share/datasource.go +++ b/stackit/internal/services/sfs/share/datasource.go @@ -37,6 +37,7 @@ type dataSourceModel struct { SpaceHardLimitGigabytes types.Int32 `tfsdk:"space_hard_limit_gigabytes"` ExportPolicyName types.String `tfsdk:"export_policy"` Region types.String `tfsdk:"region"` + Labels types.Map `tfsdk:"labels"` } type shareDataSource struct { client *sfs.APIClient @@ -183,11 +184,16 @@ You can also assign a Share Export Policy after creating the Share`, Optional: true, Description: "The resource region. Read-only attribute that reflects the provider region.", }, + "labels": schema.MapAttribute{ + Description: "Labels are key-value string pairs which can be attached to a share", + ElementType: types.StringType, + Computed: true, + }, }, } } -func mapDataSourceFields(_ context.Context, region string, share *sfs.Share, model *dataSourceModel) error { +func mapDataSourceFields(ctx context.Context, region string, share *sfs.Share, model *dataSourceModel) error { if share == nil { return fmt.Errorf("share empty in response") } @@ -221,5 +227,11 @@ func mapDataSourceFields(_ context.Context, region string, share *sfs.Share, mod model.MountPath = types.StringPointerValue(share.MountPath) + labels, err := utils.MapLabels(ctx, share.Labels, model.Labels) + if err != nil { + return err + } + model.Labels = labels + return nil } diff --git a/stackit/internal/services/sfs/share/datasource_test.go b/stackit/internal/services/sfs/share/datasource_test.go index 533f6454e..3750ed7e6 100644 --- a/stackit/internal/services/sfs/share/datasource_test.go +++ b/stackit/internal/services/sfs/share/datasource_test.go @@ -43,6 +43,7 @@ func TestMapDatasourceFields(t *testing.T) { ResourcePoolId: testResourcePoolId, ShareId: testShareId, Name: types.StringValue("test-name"), + Labels: types.MapNull(types.StringType), ExportPolicyName: testPolicyName, SpaceHardLimitGigabytes: types.Int32Value(42), MountPath: types.StringValue("/testmount"), diff --git a/stackit/internal/services/sfs/share/resource.go b/stackit/internal/services/sfs/share/resource.go index 3cbcacda8..e5814636b 100644 --- a/stackit/internal/services/sfs/share/resource.go +++ b/stackit/internal/services/sfs/share/resource.go @@ -40,6 +40,7 @@ type Model struct { ResourcePoolId types.String `tfsdk:"resource_pool_id"` ShareId types.String `tfsdk:"share_id"` Name types.String `tfsdk:"name"` + Labels types.Map `tfsdk:"labels"` ExportPolicyName types.String `tfsdk:"export_policy"` SpaceHardLimitGigabytes types.Int32 `tfsdk:"space_hard_limit_gigabytes"` Region types.String `tfsdk:"region"` @@ -159,6 +160,12 @@ func (r *shareResource) Schema(_ context.Context, _ resource.SchemaRequest, resp validate.NoSeparator(), }, }, + "labels": schema.MapAttribute{ + Description: "Labels are key-value string pairs which can be attached to the resource.", + ElementType: types.StringType, + Optional: true, + Validators: validate.LabelValidators(), + }, "region": schema.StringAttribute{ Optional: true, // must be computed to allow for storing the override value from the provider @@ -219,7 +226,7 @@ func (r *shareResource) Create(ctx context.Context, req resource.CreateRequest, ctx = core.InitProviderContext(ctx) - payload, err := toCreatePayload(&model) + payload, err := toCreatePayload(ctx, &model) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Create Resourcepool", fmt.Sprintf("Cannot create payload: %v", err)) return @@ -227,7 +234,7 @@ func (r *shareResource) Create(ctx context.Context, req resource.CreateRequest, // Create new share share, err := r.client.DefaultAPI.CreateShare(ctx, projectId, region, resourcePoolId). - CreateSharePayload(payload). + CreateSharePayload(*payload). Execute() if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating share", fmt.Sprintf("Calling API: %v", err)) @@ -370,7 +377,7 @@ func (r *shareResource) Update(ctx context.Context, req resource.UpdateRequest, return } - payload, err := toUpdatePayload(&model) + payload, err := toUpdatePayload(ctx, &model) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Update share", fmt.Sprintf("cannot create payload: %v", err)) return @@ -485,7 +492,7 @@ func (r *shareResource) ImportState(ctx context.Context, req resource.ImportStat tflog.Info(ctx, "SFS share imported") } -func mapFields(_ context.Context, share *sfs.Share, region string, model *Model) error { +func mapFields(ctx context.Context, share *sfs.Share, region string, model *Model) error { if share == nil { return fmt.Errorf("share empty in response") } @@ -507,6 +514,12 @@ func mapFields(_ context.Context, share *sfs.Share, region string, model *Model) ) model.Name = types.StringPointerValue(share.Name) + labels, err := utils.MapLabels(ctx, share.Labels, model.Labels) + if err != nil { + return err + } + model.Labels = labels + if share.ExportPolicy.IsSet() { if policy := share.ExportPolicy.Get(); policy != nil { model.ExportPolicyName = types.StringPointerValue(policy.Name) @@ -519,26 +532,39 @@ func mapFields(_ context.Context, share *sfs.Share, region string, model *Model) return nil } -func toCreatePayload(model *Model) (ret sfs.CreateSharePayload, err error) { +func toCreatePayload(ctx context.Context, model *Model) (ret *sfs.CreateSharePayload, err error) { if model == nil { return ret, fmt.Errorf("nil model") } - result := sfs.CreateSharePayload{ + + labels, err := utils.LabelsToPayload(ctx, model.Labels) + if err != nil { + return nil, err + } + + result := &sfs.CreateSharePayload{ ExportPolicyName: *sfs.NewNullableString(model.ExportPolicyName.ValueStringPointer()), Name: model.Name.ValueString(), + Labels: &labels, SpaceHardLimitGigabytes: model.SpaceHardLimitGigabytes.ValueInt32(), } return result, nil } -func toUpdatePayload(model *Model) (*sfs.UpdateSharePayload, error) { +func toUpdatePayload(ctx context.Context, model *Model) (*sfs.UpdateSharePayload, error) { if model == nil { return nil, fmt.Errorf("nil model") } + labels, err := utils.LabelsToPayload(ctx, model.Labels) + if err != nil { + return nil, err + } + result := &sfs.UpdateSharePayload{ ExportPolicyName: *sfs.NewNullableString(model.ExportPolicyName.ValueStringPointer()), SpaceHardLimitGigabytes: *sfs.NewNullableInt32(model.SpaceHardLimitGigabytes.ValueInt32Pointer()), + Labels: &labels, } return result, nil } diff --git a/stackit/internal/services/sfs/share/resource_test.go b/stackit/internal/services/sfs/share/resource_test.go index 907fe8e8f..7aa7208d1 100644 --- a/stackit/internal/services/sfs/share/resource_test.go +++ b/stackit/internal/services/sfs/share/resource_test.go @@ -54,6 +54,7 @@ func TestMapFields(t *testing.T) { ResourcePoolId: testResourcePoolId, ShareId: testShareId, Name: types.StringValue("testname"), + Labels: types.MapNull(types.StringType), ExportPolicyName: testPolicyName, SpaceHardLimitGigabytes: types.Int32Value(42), Region: types.StringValue("eu01"), @@ -84,6 +85,7 @@ func TestMapFields(t *testing.T) { ProjectId: testProjectId, ResourcePoolId: testResourcePoolId, Name: types.StringValue("testname"), + Labels: types.MapNull(types.StringType), ShareId: testShareId, ExportPolicyName: testPolicyName, SpaceHardLimitGigabytes: types.Int32Value(42), @@ -112,7 +114,7 @@ func TestToCreatePayload(t *testing.T) { tests := []struct { name string model *Model - want sfs.CreateSharePayload + want *sfs.CreateSharePayload wantErr bool }{ { @@ -126,17 +128,18 @@ func TestToCreatePayload(t *testing.T) { ExportPolicyName: testPolicyName, SpaceHardLimitGigabytes: types.Int32Value(42), }, - sfs.CreateSharePayload{ + &sfs.CreateSharePayload{ ExportPolicyName: *sfs.NewNullableString(new("test-policy")), Name: "testname", SpaceHardLimitGigabytes: 42, + Labels: &map[string]string{}, }, false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := toCreatePayload(tt.model) + got, err := toCreatePayload(context.Background(), tt.model) if (err != nil) != tt.wantErr { t.Errorf("toCreatePayload() error = %v, wantErr %v", err, tt.wantErr) return @@ -172,13 +175,14 @@ func TestToUpdatePayload(t *testing.T) { &sfs.UpdateSharePayload{ ExportPolicyName: *sfs.NewNullableString(testPolicyName.ValueStringPointer()), SpaceHardLimitGigabytes: *sfs.NewNullableInt32(utils.Ptr[int32](42)), + Labels: &map[string]string{}, }, false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := toUpdatePayload(tt.model) + got, err := toUpdatePayload(context.Background(), tt.model) if (err != nil) != tt.wantErr { t.Errorf("toCreatePayload() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/stackit/internal/services/sfs/testdata/export-policy-max.tf b/stackit/internal/services/sfs/testdata/export-policy-max.tf index 7fd85b02b..7b089e923 100644 --- a/stackit/internal/services/sfs/testdata/export-policy-max.tf +++ b/stackit/internal/services/sfs/testdata/export-policy-max.tf @@ -10,6 +10,7 @@ variable "second_rule_ip_acl_1" {} variable "second_rule_ip_acl_2" {} variable "second_rule_read_only" {} variable "second_rule_super_user" {} +variable "label" {} resource "stackit_sfs_export_policy" "exportpolicy" { project_id = var.project_id @@ -32,4 +33,7 @@ resource "stackit_sfs_export_policy" "exportpolicy" { read_only = var.second_rule_read_only super_user = var.second_rule_super_user }] + labels = { + label = var.label + } } diff --git a/stackit/internal/services/sfs/testdata/resource-pool-max.tf b/stackit/internal/services/sfs/testdata/resource-pool-max.tf index 82fc0b76b..aa69a1051 100644 --- a/stackit/internal/services/sfs/testdata/resource-pool-max.tf +++ b/stackit/internal/services/sfs/testdata/resource-pool-max.tf @@ -9,6 +9,7 @@ variable "ip_acl_1" {} variable "ip_acl_2" {} variable "snapshots_are_visible" {} variable "snapshot_policy_id" {} +variable "label" {} resource "stackit_sfs_resource_pool" "resourcepool" { project_id = var.project_id @@ -21,6 +22,9 @@ resource "stackit_sfs_resource_pool" "resourcepool" { var.ip_acl_1, var.ip_acl_2 ] + labels = { + label = var.label + } snapshots_are_visible = var.snapshots_are_visible snapshot_policy = { id = var.snapshot_policy_id diff --git a/stackit/internal/services/sfs/testdata/share-max.tf b/stackit/internal/services/sfs/testdata/share-max.tf index ef1667284..81d85ad93 100644 --- a/stackit/internal/services/sfs/testdata/share-max.tf +++ b/stackit/internal/services/sfs/testdata/share-max.tf @@ -5,6 +5,7 @@ variable "resource_pool_name" {} variable "export_policy_name" {} variable "name" {} variable "space_hard_limit_gigabytes" {} +variable "label" {} resource "stackit_sfs_resource_pool" "resourcepool" { project_id = var.project_id @@ -27,4 +28,7 @@ resource "stackit_sfs_share" "share" { name = var.name export_policy = stackit_sfs_export_policy.exportpolicy.name space_hard_limit_gigabytes = var.space_hard_limit_gigabytes + labels = { + label = var.label + } } diff --git a/stackit/internal/utils/labels.go b/stackit/internal/utils/labels.go new file mode 100644 index 000000000..b209af87b --- /dev/null +++ b/stackit/internal/utils/labels.go @@ -0,0 +1,45 @@ +package utils + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" + + "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core" +) + +func MapLabels(ctx context.Context, responseLabels *map[string]string, currentLabels types.Map) (basetypes.MapValue, error) { // nolint:gocritic // responseLabels needs to be a pointer + // Labels can have a value {"foo": "bar"}, can be empty {} or can be not provided by the config. + // The last two states are identical for the API but have a different tfstate value. + // The goal of this function is to only apply a change to the values if they actually got changed. + labels := types.MapValueMust(types.StringType, map[string]attr.Value{}) + + if responseLabels != nil && len(*responseLabels) != 0 { + var diags diag.Diagnostics + labels, diags = types.MapValueFrom(ctx, types.StringType, *responseLabels) + if diags.HasError() { + return labels, fmt.Errorf("convert labels to string map: %w", core.DiagsToError(diags)) + } + } else if currentLabels.IsNull() { + labels = types.MapNull(types.StringType) + } + + return labels, nil +} + +func LabelsToPayload(ctx context.Context, modelLabels types.Map) (map[string]string, error) { + labels := map[string]string{} + + if !modelLabels.IsNull() { + diags := modelLabels.ElementsAs(ctx, &labels, false) + if diags.HasError() { + return nil, fmt.Errorf("converting from MapValue: %w", core.DiagsToError(diags)) + } + } + + return labels, nil +} diff --git a/stackit/internal/utils/labels_test.go b/stackit/internal/utils/labels_test.go new file mode 100644 index 000000000..a931c30c3 --- /dev/null +++ b/stackit/internal/utils/labels_test.go @@ -0,0 +1,133 @@ +package utils + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" +) + +func TestMapLabels(t *testing.T) { + type args struct { + currentLabels types.Map + responseLabels *map[string]string + } + tests := []struct { + name string + input args + expectedOutput basetypes.MapValue + isValid bool + }{ + { + name: "No labels, no map", + input: args{ + currentLabels: types.MapNull(types.StringType), + responseLabels: &map[string]string{}, + }, + expectedOutput: types.MapNull(types.StringType), + isValid: true, + }, + { + name: "No labels, empty map", + input: args{ + currentLabels: types.MapValueMust(types.StringType, map[string]attr.Value{}), + responseLabels: &map[string]string{}, + }, + expectedOutput: types.MapValueMust(types.StringType, map[string]attr.Value{}), + isValid: true, + }, + { + name: "Add Labels", + input: args{ + currentLabels: types.MapNull(types.StringType), + responseLabels: &map[string]string{ + "foo": "bar", + }, + }, + expectedOutput: types.MapValueMust(types.StringType, map[string]attr.Value{ + "foo": types.StringValue("bar"), + }), + isValid: true, + }, + { + name: "Remove Labels", + input: args{ + currentLabels: types.MapValueMust(types.StringType, map[string]attr.Value{ + "foo": types.StringValue("bar"), + }), + responseLabels: &map[string]string{}, + }, + expectedOutput: types.MapValueMust(types.StringType, map[string]attr.Value{}), + isValid: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output, err := MapLabels(context.Background(), tt.input.responseLabels, tt.input.currentLabels) + if !tt.isValid && err == nil { + t.Fatalf("Should have failed") + } + if tt.isValid && err != nil { + t.Fatalf("Should not have failed: %v", err) + } + if tt.isValid { + diff := cmp.Diff(output, tt.expectedOutput) + if diff != "" { + t.Fatalf("Data does not match: %s", diff) + } + } + }) + } +} + +func TestLabelsToPayload(t *testing.T) { + tests := []struct { + name string + input types.Map + expectedOutput map[string]string + isValid bool + }{ + { + name: "No labels, no map", + input: types.MapNull(types.StringType), + expectedOutput: map[string]string{}, + isValid: true, + }, + { + name: "No labels, empty map", + input: types.MapValueMust(types.StringType, map[string]attr.Value{}), + expectedOutput: map[string]string{}, + isValid: true, + }, + { + name: "Valid Labels", + input: types.MapValueMust(types.StringType, map[string]attr.Value{ + "foo": types.StringValue("bar"), + }), + expectedOutput: map[string]string{ + "foo": "bar", + }, + isValid: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output, err := LabelsToPayload(context.Background(), tt.input) + if !tt.isValid && err == nil { + t.Fatalf("Should have failed") + } + if tt.isValid && err != nil { + t.Fatalf("Should not have failed: %v", err) + } + if tt.isValid { + diff := cmp.Diff(output, tt.expectedOutput) + if diff != "" { + t.Fatalf("Data does not match: %s", diff) + } + } + }) + } +} diff --git a/stackit/internal/validate/labels.go b/stackit/internal/validate/labels.go new file mode 100644 index 000000000..c049b71e7 --- /dev/null +++ b/stackit/internal/validate/labels.go @@ -0,0 +1,36 @@ +package validate + +import ( + "regexp" + + "github.com/hashicorp/terraform-plugin-framework-validators/mapvalidator" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" +) + +func LabelValidators() []validator.Map { + return []validator.Map{ + mapvalidator.KeysAre( + stringvalidator.RegexMatches( + regexp.MustCompile(`^.{1,63}$`), + "must be between 1 and 63 characters long"), + stringvalidator.RegexMatches( + regexp.MustCompile(`^[-A-Za-z0-9_.]*$`), + "may only include alphanumerical characters, dashes, underscores and dots"), + stringvalidator.RegexMatches( + regexp.MustCompile(`^([A-Za-z0-9].*)?[A-Za-z0-9]$`), + "must begin and end with an alphanumerical character"), + ), + mapvalidator.ValueStringsAre( + stringvalidator.RegexMatches( + regexp.MustCompile(`^.{0,63}$`), + "must not be longer than 63 characters"), + stringvalidator.RegexMatches( + regexp.MustCompile(`^[-A-Za-z0-9_.]*$`), + "may only include alphanumerical characters, dashes, underscores and dots"), + stringvalidator.RegexMatches( + regexp.MustCompile(`^(([A-Za-z0-9].*)?[A-Za-z0-9])?$`), + "must begin and end with an alphanumerical character"), + ), + } +} diff --git a/stackit/internal/validate/labels_test.go b/stackit/internal/validate/labels_test.go new file mode 100644 index 000000000..36120f687 --- /dev/null +++ b/stackit/internal/validate/labels_test.go @@ -0,0 +1,144 @@ +package validate + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +func TestLabelValidators(t *testing.T) { + tests := []struct { + description string + input map[string]attr.Value + isValid bool + }{ + { + "ok", + map[string]attr.Value{ + "foo": types.StringValue("bar"), + }, + true, + }, + { + "all valid characters", + map[string]attr.Value{ + "abcdefghijklmnopqrstuvwxyz-_.0123456789": types.StringValue("abcdefghijklmnopqrstuvwxyz-_.0123456789"), + }, + true, + }, + { + "invalid character in key", + map[string]attr.Value{ + "foo!1": types.StringValue("bar"), + }, + false, + }, + { + "invalid start in key", + map[string]attr.Value{ + "_foo": types.StringValue("bar"), + }, + false, + }, + { + "invalid end in key", + map[string]attr.Value{ + "foo_": types.StringValue("bar"), + }, + false, + }, + { + "invalid character in value", + map[string]attr.Value{ + "foo": types.StringValue("bar!1"), + }, + false, + }, + { + "invalid start in value", + map[string]attr.Value{ + "foo": types.StringValue("_bar"), + }, + false, + }, + { + "invalid end in value", + map[string]attr.Value{ + "foo": types.StringValue("bar_"), + }, + false, + }, + { + "Max key length", + map[string]attr.Value{ + "123456789012345678901234567890123456789012345678901234567890123": types.StringValue("bar"), + }, + true, + }, + { + "Min key length", + map[string]attr.Value{ + "1": types.StringValue("bar"), + }, + true, + }, + { + "Key to long", + map[string]attr.Value{ + "1234567890123456789012345678901234567890123456789012345678901234": types.StringValue("bar"), + }, + false, + }, + { + "Key to short", + map[string]attr.Value{ + "": types.StringValue("bar"), + }, + false, + }, + { + "Max value length", + map[string]attr.Value{ + "foo": types.StringValue("123456789012345678901234567890123456789012345678901234567890123"), + }, + true, + }, + { + "Empty value", + map[string]attr.Value{ + "foo": types.StringValue(""), + }, + true, + }, + { + "Value to long", + map[string]attr.Value{ + "foo": types.StringValue("1234567890123456789012345678901234567890123456789012345678901234"), + }, + false, + }, + } + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + r := validator.MapResponse{} + + value, _ := types.MapValue(types.StringType, tt.input) + + for _, LabelValidator := range LabelValidators() { + LabelValidator.ValidateMap(context.Background(), validator.MapRequest{ + ConfigValue: value, + }, &r) + } + + if !tt.isValid && !r.Diagnostics.HasError() { + t.Fatalf("Should have failed") + } + if tt.isValid && r.Diagnostics.HasError() { + t.Fatalf("Should not have failed: %v", r.Diagnostics.Errors()) + } + }) + } +} From 66d6144d2b3e5986dfc11c06b22224fe373847aa Mon Sep 17 00:00:00 2001 From: Manuel Vaas Date: Mon, 18 May 2026 16:36:09 +0200 Subject: [PATCH 2/2] resolve comments --- docs/data-sources/sfs_export_policy.md | 2 +- docs/resources/sfs_export_policy.md | 3 +++ docs/resources/sfs_resource_pool.md | 3 +++ docs/resources/sfs_share.md | 3 +++ examples/resources/stackit_sfs_export_policy/resource.tf | 5 ++++- examples/resources/stackit_sfs_resource_pool/resource.tf | 5 ++++- examples/resources/stackit_sfs_share/resource.tf | 5 ++++- stackit/internal/services/sfs/export-policy/datasource.go | 2 +- stackit/internal/validate/labels_test.go | 6 +++--- 9 files changed, 26 insertions(+), 8 deletions(-) diff --git a/docs/data-sources/sfs_export_policy.md b/docs/data-sources/sfs_export_policy.md index 0259fd021..391f56299 100644 --- a/docs/data-sources/sfs_export_policy.md +++ b/docs/data-sources/sfs_export_policy.md @@ -37,7 +37,7 @@ data "stackit_sfs_export_policy" "example" { ### Read-Only - `id` (String) Terraform's internal resource ID. It is structured as "`project_id`,`region`,`policy_id`". -- `labels` (Map of String) Labels are key-value string pairs which can be attached to a resource pool +- `labels` (Map of String) Labels are key-value string pairs which can be attached to an export policy - `name` (String) Name of the export policy. - `rules` (Attributes List) (see [below for nested schema](#nestedatt--rules)) diff --git a/docs/resources/sfs_export_policy.md b/docs/resources/sfs_export_policy.md index 8ed85a93f..5fea32694 100644 --- a/docs/resources/sfs_export_policy.md +++ b/docs/resources/sfs_export_policy.md @@ -25,6 +25,9 @@ resource "stackit_sfs_export_policy" "example" { order = 1 } ] + labels = { + "foo" = "bar" + } } # Only use the import statement, if you want to import an existing export policy diff --git a/docs/resources/sfs_resource_pool.md b/docs/resources/sfs_resource_pool.md index 90dce59df..4a7e103fd 100644 --- a/docs/resources/sfs_resource_pool.md +++ b/docs/resources/sfs_resource_pool.md @@ -27,6 +27,9 @@ resource "stackit_sfs_resource_pool" "resourcepool" { "192.168.42.2/32" ] snapshots_are_visible = true + labels = { + "foo" = "bar" + } } # Only use the import statement, if you want to import an existing resource pool diff --git a/docs/resources/sfs_share.md b/docs/resources/sfs_share.md index a571bd4b1..dd70dee06 100644 --- a/docs/resources/sfs_share.md +++ b/docs/resources/sfs_share.md @@ -22,6 +22,9 @@ resource "stackit_sfs_share" "example" { name = "my-nfs-share" export_policy = "high-performance-class" space_hard_limit_gigabytes = 32 + labels = { + "foo" = "bar" + } } # Only use the import statement, if you want to import an existing sfs share diff --git a/examples/resources/stackit_sfs_export_policy/resource.tf b/examples/resources/stackit_sfs_export_policy/resource.tf index fada10fc8..1ad284ac2 100644 --- a/examples/resources/stackit_sfs_export_policy/resource.tf +++ b/examples/resources/stackit_sfs_export_policy/resource.tf @@ -7,10 +7,13 @@ resource "stackit_sfs_export_policy" "example" { order = 1 } ] + labels = { + "foo" = "bar" + } } # Only use the import statement, if you want to import an existing export policy import { to = stackit_sfs_export_policy.example id = "${var.project_id},${var.region},${var.policy_id}" -} \ No newline at end of file +} diff --git a/examples/resources/stackit_sfs_resource_pool/resource.tf b/examples/resources/stackit_sfs_resource_pool/resource.tf index 0fb901735..f63b55a9d 100644 --- a/examples/resources/stackit_sfs_resource_pool/resource.tf +++ b/examples/resources/stackit_sfs_resource_pool/resource.tf @@ -9,10 +9,13 @@ resource "stackit_sfs_resource_pool" "resourcepool" { "192.168.42.2/32" ] snapshots_are_visible = true + labels = { + "foo" = "bar" + } } # Only use the import statement, if you want to import an existing resource pool import { to = stackit_sfs_resource_pool.resourcepool id = "${var.project_id},${var.region},${var.resource_pool_id}" -} \ No newline at end of file +} diff --git a/examples/resources/stackit_sfs_share/resource.tf b/examples/resources/stackit_sfs_share/resource.tf index 9359ce06a..39741cd6f 100644 --- a/examples/resources/stackit_sfs_share/resource.tf +++ b/examples/resources/stackit_sfs_share/resource.tf @@ -4,10 +4,13 @@ resource "stackit_sfs_share" "example" { name = "my-nfs-share" export_policy = "high-performance-class" space_hard_limit_gigabytes = 32 + labels = { + "foo" = "bar" + } } # Only use the import statement, if you want to import an existing sfs share import { to = stackit_sfs_resource_pool.resourcepool id = "${var.project_id},${var.region},${var.resource_pool_id},${var.share_id}" -} \ No newline at end of file +} diff --git a/stackit/internal/services/sfs/export-policy/datasource.go b/stackit/internal/services/sfs/export-policy/datasource.go index 5de7df03d..c7d3cd0d5 100644 --- a/stackit/internal/services/sfs/export-policy/datasource.go +++ b/stackit/internal/services/sfs/export-policy/datasource.go @@ -143,7 +143,7 @@ func (d *exportPolicyDataSource) Schema(_ context.Context, _ datasource.SchemaRe Computed: true, }, "labels": schema.MapAttribute{ - Description: "Labels are key-value string pairs which can be attached to a resource pool", + Description: "Labels are key-value string pairs which can be attached to an export policy", ElementType: types.StringType, Computed: true, }, diff --git a/stackit/internal/validate/labels_test.go b/stackit/internal/validate/labels_test.go index 36120f687..a5627c9a1 100644 --- a/stackit/internal/validate/labels_test.go +++ b/stackit/internal/validate/labels_test.go @@ -86,14 +86,14 @@ func TestLabelValidators(t *testing.T) { true, }, { - "Key to long", + "Key too long", map[string]attr.Value{ "1234567890123456789012345678901234567890123456789012345678901234": types.StringValue("bar"), }, false, }, { - "Key to short", + "Key too short", map[string]attr.Value{ "": types.StringValue("bar"), }, @@ -114,7 +114,7 @@ func TestLabelValidators(t *testing.T) { true, }, { - "Value to long", + "Value too long", map[string]attr.Value{ "foo": types.StringValue("1234567890123456789012345678901234567890123456789012345678901234"), },