feat(telemetry link): Onboarding TelemetryLink#1456
Conversation
ozanichkovsky
changed the title
Fyusel
left a comment
Fyusel
left a comment
There was a problem hiding this comment.
Added some first review comments, need to have a deeper look
| @@ -0,0 +1,5 @@ | |||
| data "stackit_telemetrylink_link" "link" { | |||
There was a problem hiding this comment.
the name here is doubled "link"
| @@ -0,0 +1,24 @@ | |||
| resource "stackit_telemetrylink_link" "link" { | |||
There was a problem hiding this comment.
same here: "link" is doubled
|
|
||
| func (d *telemetryLinkLinkDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { | ||
| resp.Schema = schema.Schema{ | ||
| Description: fmt.Sprintf("TelemetryLink Link data source schema. %s", core.DatasourceRegionFallbackDocstring), |
| validate.NoSeparator(), | ||
| }, | ||
| }, | ||
| "resource_id": schema.StringAttribute{ |
There was a problem hiding this comment.
nitpick: Can you please put the required fields on top after the id (so that it is sorted - required, optional)
| resp.State.RemoveResource(ctx) | ||
| return | ||
| } | ||
| core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading TelemetryLink link", fmt.Sprintf("Calling API: %v", err)) |
There was a problem hiding this comment.
doubled "link" (check everywhere please) won't comment every occurrence
| return | ||
| } | ||
|
|
||
| regionId := r.providerData.GetRegionWithOverride(model.Region) |
There was a problem hiding this comment.
This two lines are present in every case. Please put this before the switch:
region := r.providerData.GetRegionWithOverride(model.Region)
ctx = tflog.SetField(ctx, "region", region)
| switch resourceType { | ||
| case resourceTypeOrganization: | ||
| response, err = r.client.DefaultAPI.GetOrganizationTelemetryLink(ctx, resourceID, region).Execute() | ||
| if err != nil { |
There was a problem hiding this comment.
This error block is the same for all cases, please move it out of the switch case
| return | ||
| } | ||
|
|
||
| regionId := r.providerData.GetRegionWithOverride(model.Region) |
| ctx = tflog.SetField(ctx, "region", regionId) | ||
| _, err = r.client.DefaultAPI.CreateOrUpdateOrganizationTelemetryLink(ctx, resourceID, regionId).CreateOrUpdateOrganizationTelemetryLinkPayload(*payload).Execute() | ||
| if err != nil { | ||
| core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating TelemetryLink Link", fmt.Sprintf("Calling API: %v", err)) |
There was a problem hiding this comment.
Those messages contain "creating" but it is "updating" (same for the following)
| }) | ||
| } | ||
|
|
||
| func (r *telemetryLinkInstanceResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { // nolint:gocritic // function signature required by Terraform |
There was a problem hiding this comment.
as far as I can see: create and update are the same.
Shouldn't be partial update possible?
| "display_name": config.StringVariable("tf-acc-test-link-max"), | ||
| "description": config.StringVariable("tf-acc-test-link-description"), | ||
| "access_token": config.StringVariable("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30"), | ||
| "telemetry_router_id": config.StringVariable("97272f10-87ec-4715-b280-195a4ab1856c"), |
There was a problem hiding this comment.
This won't work here. Where does this ID come from? Manually created telemetry router?
Those acc tests should work without manual steps so if there are additional resources required you need to add those to the resource-min.tf and resource-max.tf. Same for access_token
3 participants
Description
TelemetryLink service resources and datasources.
Checklist
make fmtexamples/directory)make generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)