-
Notifications
You must be signed in to change notification settings - Fork 67
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
feat: deployment freezes #822
base: main
Are you sure you want to change the base?
Conversation
internal/errors/error.go
Outdated
@@ -32,20 +32,20 @@ func ProcessApiError(ctx context.Context, d *schema.ResourceData, err error, res | |||
return diag.FromErr(err) | |||
} | |||
|
|||
func DeleteFromStateV2(ctx context.Context, resp *resource.ReadResponse, resource schemas.IResourceModel, resourceDescription string) error { | |||
func DeleteFromStateV2(ctx context.Context, state *tfsdk.State, resource schemas.IResourceModel, resourceDescription string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the changes in this file allow this functionality to be used in the delete and read functions
this also accounts for the majority of small changes
if useSourceForDates { | ||
state.Start = types.StringValue(deploymentFreeze.Start.Format(time.RFC3339)) | ||
state.End = types.StringValue(deploymentFreeze.End.Format(time.RFC3339)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the time contains a timezone the value for the times are returned with a server local timezone, for create and update operations we just store the value provided in the plan, for a refresh (read) operation it will store the value from the api as it may have been changed.
@@ -432,3 +433,14 @@ func GetSensitiveResourceSchema(description string, isRequired bool) resourceSch | |||
|
|||
return s | |||
} | |||
|
|||
func GetDateTimeResourceSchema(description string, isRequired bool) resourceSchema.Attribute { | |||
regex := "^((?:(\\d{4}-\\d{2}-\\d{2})T(\\d{2}:\\d{2}:\\d{2}(?:\\.\\d+)?))(?:Z|[\\+-]\\d{2}:\\d{2})?)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the RFC3339 specification
https://regex101.com/r/qH0sU7/1
func convertMapStringToMapAttrValue(m map[string]string) map[string]attr.Value { | ||
result := make(map[string]attr.Value, len(m)) | ||
for k, v := range m { | ||
result[k] = types.StringValue(v) | ||
} | ||
return result | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was moved to util.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some questions about the error handling. Did we mean to fall through the error conditions?
Also please add and addition step to the test so we cover update.
|
||
updatedFreeze, err = deploymentfreezes.Update(f.Config.Client, updatedFreeze) | ||
if err != nil { | ||
resp.Diagnostics.AddError("error while updating deployment freeze", err.Error()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing return?
|
||
updatedFreeze, err := mapFromState(plan) | ||
if err != nil { | ||
resp.Diagnostics.AddError("error while mapping deployment freeze", err.Error()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing return?
|
||
err = deploymentfreezes.Delete(f.Config.Client, freeze) | ||
if err != nil { | ||
resp.Diagnostics.AddError("unable to delete deployment freeze", err.Error()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return?
resource.TestCheckResourceAttr(resourceName, "start", start), | ||
resource.TestCheckResourceAttr(resourceName, "end", end)), | ||
Config: testDeploymentFreezeBasic(localName, name, start, end, spaceName, environmentName, projectName, projectGroupName, lifecycleName), | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add one more step so we can also test update.
… deployment freeze datasource
} | ||
|
||
plan.ID = types.StringValue(schemas.BuildTenantProjectID(spaceId, plan.TenantID.ValueString(), plan.ProjectID.ValueString())) | ||
plan.ID = types.StringValue(util.BuildCompositeId(spaceId, plan.TenantID.ValueString(), plan.ProjectID.ValueString())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to shared function
@@ -83,7 +82,7 @@ func (t *tenantProjectResource) Read(ctx context.Context, req resource.ReadReque | |||
return | |||
} | |||
|
|||
bits := strings.Split(data.ID.ValueString(), ":") | |||
bits := util.SplitCompositeId(data.ID.ValueString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to shared function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, left some comments
resource.TestCheckResourceAttr(resourceName, "end", end)), | ||
Config: testDeploymentFreezeBasic(localName, name, start, end, spaceName, []string{environmentName1}, projectName, projectGroupName, lifecycleName), | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would appreciate if we run a 2nd step with an updated config so we can test the update function for the resource
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put in an update as a second resource.Test
block. I will move it into the same set and make the update do more changes
…e' into bp/deployment-freeze
Note: for some reason running
tfplugindocs
resulted in an update to remove some whitespace from most of the docs files.Source
Create
Update
end = "2024-12-27T00:00:00+10:00"
Destroy
Datasource
Output: