Skip to content

feat(sfs): add snapshot_policy_id to sfs_resource_pool #1444

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marceljk wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(sfs): add snapshot_policy_id to sfs_resource_pool #1444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/data-sources/sfs_resource_pool.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,13 @@ data "stackit_sfs_resource_pool" "resourcepool" {
- `performance_class_downgradable_at` (String) Time when the performance class can be downgraded again.
- `size_gigabytes` (Number) Size of the resource pool (unit: gigabytes)
- `size_reducible_at` (String) Time when the size can be reduced again.
- `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)

<a id="nestedatt--snapshot_policy"></a>
### Nested Schema for `snapshot_policy`

Read-Only:

- `id` (String) ID of the snapshot policy.
- `name` (String) Name of the snapshot policy.
12 changes: 12 additions & 0 deletions docs/resources/sfs_resource_pool.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,21 @@ import {
### Optional

- `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)

### Read-Only

- `id` (String) Terraform's internal resource ID. It is structured as "`project_id`,`region`,`resource_pool_id`".
- `resource_pool_id` (String) Resource pool ID

<a id="nestedatt--snapshot_policy"></a>
### Nested Schema for `snapshot_policy`

Optional:

- `id` (String) ID of the snapshot policy.

Read-Only:

- `name` (String) Name of the snapshot policy.
46 changes: 34 additions & 12 deletions stackit/internal/services/sfs/resourcepool/datasource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,19 @@ var (
)

type dataSourceModel struct {
Id types.String `tfsdk:"id"` // needed by TF
ProjectId types.String `tfsdk:"project_id"`
ResourcePoolId types.String `tfsdk:"resource_pool_id"`
AvailabilityZone types.String `tfsdk:"availability_zone"`
IpAcl types.List `tfsdk:"ip_acl"`
Name types.String `tfsdk:"name"`
PerformanceClass types.String `tfsdk:"performance_class"`
SizeGigabytes types.Int32 `tfsdk:"size_gigabytes"`
SizeReducibleAt types.String `tfsdk:"size_reducible_at"`
PerformanceClassDowngradableAt types.String `tfsdk:"performance_class_downgradable_at"`
Region types.String `tfsdk:"region"`
SnapshotsAreVisible types.Bool `tfsdk:"snapshots_are_visible"`
Id types.String `tfsdk:"id"` // needed by TF
ProjectId types.String `tfsdk:"project_id"`
ResourcePoolId types.String `tfsdk:"resource_pool_id"`
AvailabilityZone types.String `tfsdk:"availability_zone"`
IpAcl types.List `tfsdk:"ip_acl"`
Name types.String `tfsdk:"name"`
PerformanceClass types.String `tfsdk:"performance_class"`
SizeGigabytes types.Int32 `tfsdk:"size_gigabytes"`
SizeReducibleAt types.String `tfsdk:"size_reducible_at"`
PerformanceClassDowngradableAt types.String `tfsdk:"performance_class_downgradable_at"`
Region types.String `tfsdk:"region"`
SnapshotsAreVisible types.Bool `tfsdk:"snapshots_are_visible"`
SnapshotPolicy *SnapshotPolicyModel `tfsdk:"snapshot_policy"`
}

type resourcePoolDataSource struct {
Expand Down Expand Up @@ -191,6 +192,20 @@ func (r *resourcePoolDataSource) Schema(_ context.Context, _ datasource.SchemaRe
Computed: true,
Description: "If set to true, snapshots are visible and accessible to users. (default: false)",
},
"snapshot_policy": schema.SingleNestedAttribute{
Description: `Name of the snapshot policy.`,
Computed: true,
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Description: "ID of the snapshot policy.",
Computed: true,
},
"name": schema.StringAttribute{
Description: "Name of the snapshot policy.",
Computed: true,
},
},
},
"region": schema.StringAttribute{
// the region cannot be found automatically, so it has to be passed
Optional: true,
Expand Down Expand Up @@ -247,5 +262,12 @@ func mapDataSourceFields(ctx context.Context, region string, resourcePool *sfs.R
model.SizeReducibleAt = types.StringValue(t.Format(time.RFC3339))
}

if snapshotPolicy := resourcePool.SnapshotPolicy.Get(); snapshotPolicy != nil {
model.SnapshotPolicy = &SnapshotPolicyModel{
Id: types.StringPointerValue(snapshotPolicy.Id),
Name: types.StringPointerValue(snapshotPolicy.Name),
}
}

return nil
}
8 changes: 8 additions & 0 deletions stackit/internal/services/sfs/resourcepool/datasource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ func TestMapDatasourceFields(t *testing.T) {
Space: &sfs.ResourcePoolSpace{
SizeGigabytes: utils.Ptr[int32](42),
},
SnapshotPolicy: *sfs.NewNullableResourcePoolSnapshotPolicy(&sfs.ResourcePoolSnapshotPolicy{
Id: new("snapshot-id"),
Name: new("snapshot-name"),
}),
State: new("state"),
},
expected: &dataSourceModel{
Expand All @@ -92,6 +96,10 @@ func TestMapDatasourceFields(t *testing.T) {
Region: testRegion,
SizeReducibleAt: testTimePlus1h,
PerformanceClassDowngradableAt: testTime,
SnapshotPolicy: &SnapshotPolicyModel{
Id: types.StringValue("snapshot-id"),
Name: types.StringValue("snapshot-name"),
},
},
isValid: true,
},
Expand Down
97 changes: 90 additions & 7 deletions stackit/internal/services/sfs/resourcepool/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,19 @@ import (
"strings"

"github.com/hashicorp/terraform-plugin-framework-validators/listvalidator"
"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/objectdefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/stackitcloud/stackit-sdk-go/core/oapierror"
sfs "github.com/stackitcloud/stackit-sdk-go/services/sfs/v1api"
Expand All @@ -27,6 +32,7 @@ import (
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/features"
sfsUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/sfs/utils"
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils"
stringplanmodifierUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/planmodifiers/stringplanmodifier"
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/validate"
)

Expand All @@ -38,6 +44,9 @@ var (
_ resource.ResourceWithModifyPlan = &resourcePoolResource{}
)

// defaultSnapshotPolicyId is an empty string, which removes any snapshot policy within updates
const defaultSnapshotPolicyId = ""

type Model struct {
Id types.String `tfsdk:"id"` // needed by TF
ProjectId types.String `tfsdk:"project_id"`
Expand All @@ -47,10 +56,21 @@ type Model struct {
Name types.String `tfsdk:"name"`
PerformanceClass types.String `tfsdk:"performance_class"`
SizeGigabytes types.Int32 `tfsdk:"size_gigabytes"`
SnapshotPolicy types.Object `tfsdk:"snapshot_policy"`
Region types.String `tfsdk:"region"`
SnapshotsAreVisible types.Bool `tfsdk:"snapshots_are_visible"`
}

type SnapshotPolicyModel struct {
Id types.String `tfsdk:"id"`
Name types.String `tfsdk:"name"`
}

var snapshotPolicyTypes = map[string]attr.Type{
"id": basetypes.StringType{},
"name": basetypes.StringType{},
}

// NewResourcePoolResource is a helper function to simplify the provider implementation.
func NewResourcePoolResource() resource.Resource {
return &resourcePoolResource{}
Expand Down Expand Up @@ -200,6 +220,39 @@ func (r *resourcePoolResource) Schema(_ context.Context, _ resource.SchemaReques
Computed: true,
Default: booldefault.StaticBool(false),
},
"snapshot_policy": schema.SingleNestedAttribute{
Description: `Name of the snapshot policy.`,
Computed: true,
Optional: true,
Default: objectdefault.StaticValue(
types.ObjectValueMust(snapshotPolicyTypes, map[string]attr.Value{
"id": types.StringValue(defaultSnapshotPolicyId),
Copy link
Copy Markdown
Member

@rubenhoenle rubenhoenle May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect the API to return an error for an empty string value as a snapshot policy ID.

At least I didn't get any snapshot policy with id "" from https://docs.api.stackit.cloud/documentation/sfs/version/v1#operation/ListSnapshotPolicies

Copy link
Copy Markdown
Member

@rubenhoenle rubenhoenle May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API design is the one thing. But why to we send an empty string? I don't quite get it tbh 😅

Copy link
Copy Markdown
Contributor Author

@marceljk marceljk May 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The snapshot policy ID is in the PATCH request optional, but not nullable. So to remove the snapshot policy from an existing resource pool, we need to set it to an empty string. The empty string, is also the default "snapshot_policy_id", when no id was set within the creation.

I set it on default to an empty string, to automatically remove the snapshot_policy_id, when someone removes the object in their terraform config. Otherwise terraform wouldn't trigger an update

Copy link
Copy Markdown
Member

@rubenhoenle rubenhoenle May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the empty string thing is a workaround for an API issue?

"name": types.StringUnknown(),
}),
),
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Description: "ID of the snapshot policy.",
Optional: true,
Computed: true,
// ID can be either an empty string or a UUID
Validators: []validator.String{
stringvalidator.Any(
stringvalidator.OneOf(""),
validate.UUID(),
),
},
Default: stringdefault.StaticString(defaultSnapshotPolicyId),
},
"name": schema.StringAttribute{
Description: "Name of the snapshot policy.",
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifierUtils.UseStateForUnknownIf(stringplanmodifierUtils.StringChanged, "id", "sets `UseStateForUnknown` only if `id` has not changed"),
},
},
},
},
},
}
}
Expand All @@ -221,7 +274,7 @@ func (r *resourcePoolResource) Create(ctx context.Context, req resource.CreateRe

ctx = core.InitProviderContext(ctx)

payload, err := toCreatePayload(&model)
payload, err := toCreatePayload(ctx, &model)
if err != nil {
core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", fmt.Sprintf("Cannot create payload: %v", err))
return
Expand Down Expand Up @@ -314,9 +367,8 @@ func (r *resourcePoolResource) Read(ctx context.Context, req resource.ReadReques

response, err := r.client.DefaultAPI.GetResourcePool(ctx, projectId, region, resourcePoolId).Execute()
if err != nil {
var openapiError *oapierror.GenericOpenAPIError
if errors.As(err, &openapiError) {
if openapiError.StatusCode == http.StatusNotFound {
if openapiError, ok := errors.AsType[*oapierror.GenericOpenAPIError](err); ok {
if openapiError.StatusCode == http.StatusNotFound || openapiError.StatusCode == http.StatusGone {
resp.State.RemoveResource(ctx)
return
}
Expand Down Expand Up @@ -368,7 +420,7 @@ func (r *resourcePoolResource) Update(ctx context.Context, req resource.UpdateRe
return
}

payload, err := toUpdatePayload(&model)
payload, err := toUpdatePayload(ctx, &model)
if err != nil {
core.LogAndAddError(ctx, &resp.Diagnostics, "Update resource pool", fmt.Sprintf("cannot create payload: %v", err))
return
Expand Down Expand Up @@ -520,10 +572,22 @@ func mapFields(ctx context.Context, region string, resourcePool *sfs.ResourcePoo
model.SizeGigabytes = types.Int32PointerValue(resourcePool.Space.SizeGigabytes)
}

model.SnapshotPolicy = types.ObjectNull(snapshotPolicyTypes)
if snapshotPolicy := resourcePool.SnapshotPolicy.Get(); snapshotPolicy != nil {
snapshotPolicyTf, diags := types.ObjectValue(snapshotPolicyTypes, map[string]attr.Value{
"id": types.StringPointerValue(snapshotPolicy.Id),
"name": types.StringPointerValue(snapshotPolicy.Name),
})
if diags.HasError() {
return fmt.Errorf("failed to map snapshot policy: %w", core.DiagsToError(diags))
}
model.SnapshotPolicy = snapshotPolicyTf
}

return nil
}

func toCreatePayload(model *Model) (*sfs.CreateResourcePoolPayload, error) {
func toCreatePayload(ctx context.Context, model *Model) (*sfs.CreateResourcePoolPayload, error) {
if model == nil {
return nil, fmt.Errorf("nil model")
}
Expand All @@ -538,18 +602,28 @@ func toCreatePayload(model *Model) (*sfs.CreateResourcePoolPayload, error) {
aclList = tmp
}

snapshotPolicy := &SnapshotPolicyModel{}
if !utils.IsUndefined(model.SnapshotPolicy) {
diags := model.SnapshotPolicy.As(ctx, snapshotPolicy, basetypes.ObjectAsOptions{})
if diags.HasError() {
return nil, fmt.Errorf("cannot convert snapshot policy: %w", core.DiagsToError(diags))
}
}

result := &sfs.CreateResourcePoolPayload{
AvailabilityZone: model.AvailabilityZone.ValueString(),
IpAcl: aclList,
Name: model.Name.ValueString(),
PerformanceClass: model.PerformanceClass.ValueString(),
SizeGigabytes: model.SizeGigabytes.ValueInt32(),
SnapshotsAreVisible: model.SnapshotsAreVisible.ValueBoolPointer(),
SnapshotPolicyId: snapshotPolicy.Id.ValueStringPointer(),
}

return result, nil
}

func toUpdatePayload(model *Model) (*sfs.UpdateResourcePoolPayload, error) {
func toUpdatePayload(ctx context.Context, model *Model) (*sfs.UpdateResourcePoolPayload, error) {
if model == nil {
return nil, fmt.Errorf("nil model")
}
Expand All @@ -564,11 +638,20 @@ func toUpdatePayload(model *Model) (*sfs.UpdateResourcePoolPayload, error) {
aclList = tmp
}

snapshotPolicy := &SnapshotPolicyModel{}
if !utils.IsUndefined(model.SnapshotPolicy) {
diags := model.SnapshotPolicy.As(ctx, snapshotPolicy, basetypes.ObjectAsOptions{})
if diags.HasError() {
return nil, fmt.Errorf("cannot convert snapshot policy: %w", core.DiagsToError(diags))
}
}

result := &sfs.UpdateResourcePoolPayload{
IpAcl: aclList,
PerformanceClass: model.PerformanceClass.ValueStringPointer(),
SizeGigabytes: *sfs.NewNullableInt32(model.SizeGigabytes.ValueInt32Pointer()),
SnapshotsAreVisible: model.SnapshotsAreVisible.ValueBoolPointer(),
SnapshotPolicyId: snapshotPolicy.Id.ValueStringPointer(),
}
return result, nil
}
11 changes: 9 additions & 2 deletions stackit/internal/services/sfs/resourcepool/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,18 @@ func TestToCreatePayload(t *testing.T) {
Name: types.StringValue("testname"),
PerformanceClass: types.StringValue("performance"),
SizeGigabytes: types.Int32Value(42),
SnapshotPolicy: types.ObjectValueMust(snapshotPolicyTypes, map[string]attr.Value{
"id": types.StringValue("snapshot-id"),
"name": types.StringNull(),
}),
},
&sfs.CreateResourcePoolPayload{
AvailabilityZone: testAvailabilityZone.ValueString(),
IpAcl: []string{"foo", "bar", "baz"},
Name: "testname",
PerformanceClass: "performance",
SizeGigabytes: 42,
SnapshotPolicyId: new("snapshot-id"),
},
false,
},
Expand Down Expand Up @@ -169,7 +174,8 @@ func TestToCreatePayload(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := toCreatePayload(tt.model)
ctx := context.Background()
got, err := toCreatePayload(ctx, tt.model)
if (err != nil) != tt.wantErr {
t.Errorf("toCreatePayload() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down Expand Up @@ -250,7 +256,8 @@ func TestToUpdatePayload(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := toUpdatePayload(tt.model)
ctx := context.Background()
got, err := toUpdatePayload(ctx, tt.model)
if (err != nil) != tt.wantErr {
t.Errorf("toUpdatePayload() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down
Loading
Loading