- Notifications
You must be signed in to change notification settings - Fork3
fix: avoid sending template metadata update ifmax_port_share_level is default#190
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This stack of pull requests is managed byGraphite. Learn more aboutstacking. |
| @@ -573,6 +573,8 @@ func (r *TemplateResource) Create(ctx context.Context, req resource.CreateReques | |||
| // deployment running `v2.15.0` or later. | |||
| if data.MaxPortShareLevel.IsUnknown() { | |||
| data.MaxPortShareLevel = types.StringValue(string(templateResp.MaxPortShareLevel)) | |||
| } else if data.MaxPortShareLevel.ValueString() == string(templateResp.MaxPortShareLevel) { | |||
| tflog.Info(ctx, "max port share level set to default, not updating") | |||
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.
not necessarily the default, just the current value
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 only runs onCreate, so the current value is always the default
1d19415 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Closes#188.
For context, the
MaxPortShareLeveltemplate metadata value was not present on thecoderdcreate template request prior to 2.15. As such, during template creation we need to send an update request for backwards compatibility (though we could probably remove this soon), see#110.Whilst we don't send this update request is the attribute is omitted on the resource, we were sending a spurious update request if the value was explicitly configured to the default value ('owner' for enterprise+, 'public' otherwise). This was causing the error in the linked issue. An example is seen in the newly added test.
The fix is to just not send that update request if the configured value is:
note: I'd recommend hiding the whitespace when reviewing the diff