feat(logs) onboard logs service #1096
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- add resource with examples - add data source with examples - add acceptance tests
GokceGK
reviewed
Jan 7, 2026
| description = "Example description" | ||
| } | ||
|
|
||
| # Only use the import statement, if you want to import an existing git resource |
Contributor
There was a problem hiding this comment.
Suggested change
| # Only use the import statement, if you want to import an existing git resource | |
| # Only use the import statement, if you want to import an existing logs instance |
GokceGK
reviewed
Jan 7, 2026
| @@ -0,0 +1,23 @@ | |||
| resource "stackit_logs_instance" "git" { | |||
Contributor
There was a problem hiding this comment.
Suggested change
| resource "stackit_logs_instance" "git" { | |
| resource "stackit_logs_instance" "logs" { |
GokceGK
reviewed
Jan 7, 2026
| retention_days = 30 | ||
| } | ||
|
|
||
| resource "stackit_logs_instance" "logs" { |
Contributor
There was a problem hiding this comment.
Suggested change
| resource "stackit_logs_instance" "logs" { | |
| resource "stackit_logs_instance" "logs2" { |
GokceGK
reviewed
Jan 7, 2026
|
|
||
| func (d *logsInstanceDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { | ||
| resp.Schema = schema.Schema{ | ||
| MarkdownDescription: features.AddBetaDescription("Logs instance resource schema.", core.Resource), |
Contributor
There was a problem hiding this comment.
Suggested change
| MarkdownDescription: features.AddBetaDescription("Logs instance resource schema.", core.Resource), | |
| MarkdownDescription: features.AddBetaDescription("Logs instance data source schema.", core.Datasource), |
GokceGK
reviewed
Jan 7, 2026
| func (d *logsInstanceDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { | ||
| resp.Schema = schema.Schema{ | ||
| MarkdownDescription: features.AddBetaDescription("Logs instance resource schema.", core.Resource), | ||
| Description: fmt.Sprintf("Logs instance resource schema. %s", core.ResourceRegionFallbackDocstring), |
Contributor
There was a problem hiding this comment.
Suggested change
| Description: fmt.Sprintf("Logs instance resource schema. %s", core.ResourceRegionFallbackDocstring), | |
| Description: fmt.Sprintf("Logs instance data source schema. %s", core.DatasourceRegionFallbackDocstring), |
GokceGK
reviewed
Jan 7, 2026
| "instance_id": schema.StringAttribute{ | ||
| Description: schemaDescriptions["instance_id"], | ||
| Required: true, | ||
| Validators: []validator.String{validate.UUID()}, |
Contributor
There was a problem hiding this comment.
Suggested change
| Validators: []validator.String{validate.UUID()}, | |
| Validators: []validator.String{ | |
| validate.UUID(), | |
| validate.NoSeparator(), | |
| }, |
GokceGK
reviewed
Jan 7, 2026
| }, | ||
| "project_id": schema.StringAttribute{ | ||
| Description: schemaDescriptions["project_id"], | ||
| Required: true, |
Contributor
There was a problem hiding this comment.
Suggested change
| Required: true, | |
| Required: true, | |
| Validators: []validator.String{ | |
| validate.UUID(), | |
| validate.NoSeparator(), | |
| }, |
GokceGK
reviewed
Jan 7, 2026
| Description: schemaDescriptions["description"], | ||
| Computed: true, | ||
| }, | ||
| "display_name": schema.StringAttribute{ |
Contributor
There was a problem hiding this comment.
I think we also need here a validator like
Validators: []validator.String{
stringvalidator.LengthAtLeast(1),
},
GokceGK
reviewed
Jan 7, 2026
| Description: schemaDescriptions["retention_days"], | ||
| Computed: true, | ||
| }, | ||
| "status": schema.StringAttribute{ |
Contributor
There was a problem hiding this comment.
Status has enum values. It will be good if we can mention the values in the description.
Sth like
statusOptions := []string{"active","deleting","reconciling"}
... utils.FormatPossibleValues(statusOptions)...
GokceGK
reviewed
Jan 7, 2026
| core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading logs instance", fmt.Sprintf("Calling API: %v", err)) | ||
| return | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Suggested change
| ctx = core.LogResponse(ctx) |
GokceGK
reviewed
Jan 7, 2026
| if resp.Diagnostics.HasError() { | ||
| return | ||
| } | ||
| tflog.Info(ctx, "Logs Instance read") |
Contributor
There was a problem hiding this comment.
Suggested change
| tflog.Info(ctx, "Logs Instance read") | |
| tflog.Info(ctx, fmt.Sprintf("read Logs instance %s", instanceID)) |
GokceGK
reviewed
Jan 7, 2026
| "id": "Terraform's internal resource identifier. It is structured as \"`project_id`,`region`,`instance_id`\".", | ||
| "instance_id": "The Logs instance ID", | ||
| "region": "STACKIT region name the resource is located in. If not defined, the provider region is used.", | ||
| "project_id": "STACKIT project ID associated with the logs instance", |
Contributor
There was a problem hiding this comment.
Suggested change
| "project_id": "STACKIT project ID associated with the logs instance", | |
| "project_id": "STACKIT project ID associated with the Logs instance", |
GokceGK
reviewed
Jan 7, 2026
| "instance_id": "The Logs instance ID", | ||
| "region": "STACKIT region name the resource is located in. If not defined, the provider region is used.", | ||
| "project_id": "STACKIT project ID associated with the logs instance", | ||
| "acl": "ACL entries for the logs instance", |
Contributor
There was a problem hiding this comment.
Suggested change
| "acl": "ACL entries for the logs instance", | |
| "acl": "The access control list for the Logs instance", |
GokceGK
reviewed
Jan 7, 2026
| "region": "STACKIT region name the resource is located in. If not defined, the provider region is used.", | ||
| "project_id": "STACKIT project ID associated with the logs instance", | ||
| "acl": "ACL entries for the logs instance", | ||
| "created": "Time when the distribution was created", |
Contributor
There was a problem hiding this comment.
Suggested change
| "created": "Time when the distribution was created", | |
| "created": "The date and time the creation of the Logs instance was triggered", |
GokceGK
reviewed
Jan 7, 2026
| "query_range_url": "The Logs instance's query range URL", | ||
| "query_url": "The Logs instance's query URL", | ||
| "retention_days": "The log retention time in days", | ||
| "status": "The status of the Logs instance", |
Contributor
There was a problem hiding this comment.
As mentioned in the datasource, it will be good to mention the possible enum values
GokceGK
reviewed
Jan 7, 2026
| "status": "The status of the Logs instance", | ||
| } | ||
|
|
||
| type Model struct { |
Contributor
There was a problem hiding this comment.
I think comments are not needed for the fields. Descriptions are already very clear.
GokceGK
reviewed
Jan 7, 2026
| Attributes: map[string]schema.Attribute{ | ||
| "id": schema.StringAttribute{ | ||
| Description: schemaDescriptions["id"], | ||
| Computed: true, |
Contributor
There was a problem hiding this comment.
Suggested change
| Computed: true, | |
| Computed: true, | |
| PlanModifiers: []planmodifier.String{ | |
| stringplanmodifier.UseStateForUnknown(), | |
| }, |
GokceGK
reviewed
Jan 7, 2026
| "instance_id": schema.StringAttribute{ | ||
| Description: schemaDescriptions["instance_id"], | ||
| Computed: true, | ||
| Validators: []validator.String{validate.UUID()}, |
Contributor
There was a problem hiding this comment.
Suggested change
| Validators: []validator.String{validate.UUID()}, | |
| Validators: []validator.String{ | |
| validate.UUID(), | |
| validate.NoSeparator(), | |
| }, |
GokceGK
reviewed
Jan 7, 2026
| Required: true, | ||
| PlanModifiers: []planmodifier.String{ | ||
| stringplanmodifier.RequiresReplace(), | ||
| }, |
Contributor
There was a problem hiding this comment.
Suggested change
| }, | |
| }, | |
| Validators: []validator.String{ | |
| validate.UUID(), | |
| validate.NoSeparator(), | |
| }, |
GokceGK
reviewed
Jan 7, 2026
| "acl": schema.ListAttribute{ | ||
| Description: schemaDescriptions["acl"], | ||
| ElementType: types.StringType, | ||
| Optional: true, |
Contributor
There was a problem hiding this comment.
Should acl also be the Computed: true?
Contributor
Author
There was a problem hiding this comment.
Can be set on creation/update, not sure if Computed: true would be correct.
GokceGK
reviewed
Jan 7, 2026
| if err != nil { | ||
| var oapiErr *oapierror.GenericOpenAPIError | ||
| ok := errors.As(err, &oapiErr) | ||
| if ok && oapiErr.StatusCode == http.StatusNotFound { |
Contributor
There was a problem hiding this comment.
Suggested change
| if ok && oapiErr.StatusCode == http.StatusNotFound { | |
| if ok && (oapiErr.StatusCode == http.StatusNotFound || oapiErr.StatusCode == http.StatusGone) { |
Contributor
Author
There was a problem hiding this comment.
Gone is not listed as a possible status in the API docs.
What's the reasoning for this change?
GokceGK
reviewed
Jan 7, 2026
| core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading logs instance", fmt.Sprintf("Calling API: %v", err)) | ||
| return | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
I think we are missing this ctx = core.LogResponse(ctx)
GokceGK
reviewed
Jan 7, 2026
| if resp.Diagnostics.HasError() { | ||
| return | ||
| } | ||
| tflog.Info(ctx, "Logs Instance read") |
Contributor
There was a problem hiding this comment.
Suggested change
| tflog.Info(ctx, "Logs Instance read") | |
| tflog.Info(ctx, fmt.Sprintf("read logs instance %s", instanceID)) |
GokceGK
reviewed
Jan 7, 2026
| if resp.Diagnostics.HasError() { | ||
| return | ||
| } | ||
| tflog.Info(ctx, "Logs Instance updated") |
Contributor
There was a problem hiding this comment.
Suggested change
| tflog.Info(ctx, "Logs Instance updated") | |
| tflog.Info(ctx, fmt.Sprintf("update logs instance %s", instanceID)) |
GokceGK
reviewed
Jan 7, 2026
| return nil, fmt.Errorf("missing model") | ||
| } | ||
|
|
||
| var acls []string |
Contributor
There was a problem hiding this comment.
I think we can use here .ElementsAs and avoid that code block. Sth like the following maybe
if !(model.ACL.IsNull() || model.ACL.IsUnknown()) {
var acl []string
aclDiags := model.ACL.ElementsAs(ctx, &acl, false)
diags.Append(aclDiags...)
if !aclDiags.HasError() {
payload.Acl = &acl
}
}
Contributor
Author
There was a problem hiding this comment.
way nicer, thanks!
GokceGK
reviewed
Jan 7, 2026
| return nil, fmt.Errorf("missing model") | ||
| } | ||
|
|
||
| var acls []string |
Contributor
There was a problem hiding this comment.
If you decide to change, same here like i mentioned in CreatePayload
GokceGK
reviewed
Jan 7, 2026
| "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils" | ||
| ) | ||
|
|
||
| func ConfigureClient(ctx context.Context, providerData *core.ProviderData, diags *diag.Diagnostics) *logs.APIClient { |
Contributor
There was a problem hiding this comment.
Missing unit test for this one
| "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils" | ||
| ) | ||
|
|
||
| func ConfigureClient(ctx context.Context, providerData *core.ProviderData, diags *diag.Diagnostics) *logs.APIClient { |
Contributor
There was a problem hiding this comment.
Missing unit test for this one
Contributor
Author
There was a problem hiding this comment.
duplicated comment?
- rename git to logs in example - add NoSeparator validator for instance-/project-id - log responses and instance_ids - rm redundant struct field docs - mention possible status values in description - test ConfigureClient - use ElementsAs
…ogs-instance-datasource
Merging this branch changes the coverage (2 decrease, 2 increase)
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
relates to STACKITTPR-448
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)