Skip to content
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.

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

fix: handle expiration date in regard to changed timezones #667

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
83 changes: 80 additions & 3 deletions stackit/internal/services/objectstorage/credential/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/conversion"
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core"
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils"
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/validate"

"github.com/hashicorp/terraform-plugin-framework/path"
Expand All @@ -29,6 +30,7 @@ var (
_ resource.Resource = &credentialResource{}
_ resource.ResourceWithConfigure = &credentialResource{}
_ resource.ResourceWithImportState = &credentialResource{}
_ resource.ResourceWithModifyPlan = &credentialResource{}
)

type Model struct {
Expand All @@ -52,6 +54,34 @@ type credentialResource struct {
client *objectstorage.APIClient
}

// ModifyPlan implements resource.ResourceWithModifyPlan.
func (r *credentialResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { // nolint:gocritic // function signature required by Terraform
p := path.Root("expiration_timestamp")
var (
stateDate time.Time
planDate time.Time
)

resp.Diagnostics.Append(utils.GetTimeFromStringAttribute(ctx, p, req.State, time.RFC3339, &stateDate)...)
if resp.Diagnostics.HasError() {
return
}
resp.Diagnostics.Append(utils.GetTimeFromStringAttribute(ctx, p, resp.Plan, time.RFC3339, &planDate)...)
if resp.Diagnostics.HasError() {
return
}

// replace the planned expiration time with the current state date, iff they represent
Fyusel marked this conversation as resolved.
Show resolved Hide resolved
// the same point in time (but perhaps with different textual representation)
// this will prevent no-op updates
if stateDate.Equal(planDate) {
resp.Diagnostics.Append(resp.Plan.SetAttribute(ctx, p, types.StringValue(stateDate.Format(time.RFC3339)))...)
if resp.Diagnostics.HasError() {
return
}
}
}

// Metadata returns the resource type name.
func (r *credentialResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
resp.TypeName = req.ProviderTypeName + "_objectstorage_credential"
Expand Down Expand Up @@ -167,6 +197,7 @@ func (r *credentialResource) Schema(_ context.Context, _ resource.SchemaRequest,
},
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
stringplanmodifier.RequiresReplace(),
},
},
},
Expand Down Expand Up @@ -218,6 +249,25 @@ func (r *credentialResource) Create(ctx context.Context, req resource.CreateRequ
core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating credential", fmt.Sprintf("Processing API payload: %v", err))
return
}

var (
actualDate time.Time
planDate time.Time
)
resp.Diagnostics.Append(utils.ToTime(ctx, time.RFC3339, model.ExpirationTimestamp, &actualDate)...)
if resp.Diagnostics.HasError() {
return
}
resp.Diagnostics.Append(utils.GetTimeFromStringAttribute(ctx, path.Root("expiration_timestamp"), req.Plan, time.RFC3339, &planDate)...)
if resp.Diagnostics.HasError() {
return
}
// replace the planned expiration date with the original date, iff
Fyusel marked this conversation as resolved.
Show resolved Hide resolved
// they represent the same point in time, (perhaps with different textual representations)
if actualDate.Equal(planDate) {
model.ExpirationTimestamp = types.StringValue(planDate.Format(time.RFC3339))
}

diags = resp.State.Set(ctx, model)
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
Expand Down Expand Up @@ -251,6 +301,26 @@ func (r *credentialResource) Read(ctx context.Context, req resource.ReadRequest,
resp.State.RemoveResource(ctx)
return
}
var (
currentApiDate time.Time
stateDate time.Time
)

resp.Diagnostics.Append(utils.ToTime(ctx, time.RFC3339, model.ExpirationTimestamp, &currentApiDate)...)
if resp.Diagnostics.HasError() {
return
}

resp.Diagnostics.Append(utils.GetTimeFromStringAttribute(ctx, path.Root("expiration_timestamp"), req.State, time.RFC3339, &stateDate)...)
if resp.Diagnostics.HasError() {
return
}

// replace the resulting expiration date with the original date, iff
Fyusel marked this conversation as resolved.
Show resolved Hide resolved
// they represent the same point in time, (perhaps with different textual representations)
if currentApiDate.Equal(stateDate) {
model.ExpirationTimestamp = types.StringValue(stateDate.Format(time.RFC3339))
}

// Set refreshed state
diags = resp.State.Set(ctx, model)
Expand All @@ -262,9 +332,16 @@ func (r *credentialResource) Read(ctx context.Context, req resource.ReadRequest,
}

// Update updates the resource and sets the updated Terraform state on success.
func (r *credentialResource) Update(ctx context.Context, _ resource.UpdateRequest, resp *resource.UpdateResponse) { // nolint:gocritic // function signature required by Terraform
// Update shouldn't be called
core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating credential", "Credential can't be updated")
func (r *credentialResource) Update(_ context.Context, _ resource.UpdateRequest, _ *resource.UpdateResponse) { // nolint:gocritic // function signature required by Terraform
/*
While a credential cannot be updated, the Update call must not be prevented with an error:
When the expiration timestamp has been updated to the same point in time, but e.g. with a different timezone,
terraform will still trigger an Update due to the computed attributes. These will not change,
but terraform has no way of knowing this without calling this function. So it is
still updated as a no-op.
A possible enhancement would be to emit an error, if it is attempted to change one of the not computed attributes
and abort with an error in this case.
*/
}

// Delete deletes the resource and removes the Terraform state on success.
Expand Down
46 changes: 46 additions & 0 deletions stackit/internal/utils/attributes.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package utils

import (
"context"
"fmt"
"time"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core"
)

type attributeGetter interface {
GetAttribute(ctx context.Context, attributePath path.Path, target interface{}) diag.Diagnostics
}

func ToTime(ctx context.Context, format string, val types.String, target *time.Time) (diags diag.Diagnostics) {
var err error
text := val.ValueString()
*target, err = time.Parse(format, text)
if err != nil {
core.LogAndAddError(ctx, &diags, "cannot parse date", fmt.Sprintf("cannot parse date %q with format %q: %v", text, format, err))
return diags
}
return diags
}

// GetTimeFromStringAttribute retrieves a string attribute from e.g. a [plan.Plan], [tfsdk.Config] or a [tfsdk.State] and
// converts it to a [time.Time] object with a given format, if possible.
func GetTimeFromStringAttribute(ctx context.Context, attributePath path.Path, source attributeGetter, dateFormat string, target *time.Time) (diags diag.Diagnostics) {
var date types.String
diags.Append(source.GetAttribute(ctx, attributePath, &date)...)
if diags.HasError() {
return diags
}
if date.IsNull() || date.IsUnknown() {
return diags
}
diags.Append(ToTime(ctx, dateFormat, date, target)...)
if diags.HasError() {
return diags
}

return diags
}
88 changes: 88 additions & 0 deletions stackit/internal/utils/attributes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package utils

import (
"context"
"log"
"testing"
"time"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/types"
)

type attributeGetterFunc func(ctx context.Context, attributePath path.Path, target interface{}) diag.Diagnostics

func (a attributeGetterFunc) GetAttribute(ctx context.Context, attributePath path.Path, target interface{}) diag.Diagnostics {
return a(ctx, attributePath, target)
}

func mustLocation(name string) *time.Location {
loc, err := time.LoadLocation(name)
if err != nil {
log.Panicf("cannot load location %s: %v", name, err)
}
return loc
}

func TestGetTimeFromString(t *testing.T) {
type args struct {
path path.Path
source attributeGetterFunc
dateFormat string
}
tests := []struct {
name string
args args
wantErr bool
want time.Time
}{
{
name: "simple string",
args: args{
path: path.Root("foo"),
source: func(_ context.Context, _ path.Path, target interface{}) diag.Diagnostics {
t, ok := target.(*types.String)
if !ok {
log.Panicf("wrong type %T", target)
}
*t = types.StringValue("2025-02-06T09:41:00+01:00")
return nil
},
dateFormat: time.RFC3339,
},
want: time.Date(2025, 2, 6, 9, 41, 0, 0, mustLocation("Europe/Berlin")),
},
{
name: "invalid type",
args: args{
path: path.Root("foo"),
source: func(_ context.Context, p path.Path, _ interface{}) (diags diag.Diagnostics) {
diags.AddAttributeError(p, "kapow", "kapow")
return diags
},
dateFormat: time.RFC3339,
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var target time.Time
gotDiags := GetTimeFromStringAttribute(context.Background(), tt.args.path, tt.args.source, tt.args.dateFormat, &target)
if tt.wantErr {
if !gotDiags.HasError() {
t.Errorf("expected error")
}
} else {
if gotDiags.HasError() {
t.Errorf("expected no errors, but got %v", gotDiags)
} else {
if want, got := tt.want, target; !want.Equal(got) {
t.Errorf("got wrong date, want %s but got %s", want, got)
}
}
}
})
}
}
10 changes: 10 additions & 0 deletions stackit/internal/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,13 @@ func QuoteValues(values []string) []string {
func IsLegacyProjectRole(role string) bool {
return utils.Contains(LegacyProjectRoles, role)
}

type value interface {
IsUnknown() bool
IsNull() bool
}

// IsUndefined checks if a passed value is unknown or null
func IsUndefined(val value) bool {
return val.IsUnknown() || val.IsNull()
}