feat(logs) onboard logs service #1096
Conversation
- add resource with examples - add data source with examples - add acceptance tests
|
|
||
| func (d *logsInstanceDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { | ||
| resp.Schema = schema.Schema{ | ||
| MarkdownDescription: features.AddBetaDescription("Logs instance resource schema.", core.Resource), |
There was a problem hiding this comment.
| MarkdownDescription: features.AddBetaDescription("Logs instance resource schema.", core.Resource), | |
| MarkdownDescription: features.AddBetaDescription("Logs instance data source schema.", core.Datasource), |
There was a problem hiding this comment.
Here it is still core.Resource
| core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading logs instance", fmt.Sprintf("Calling API: %v", err)) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
| ctx = core.LogResponse(ctx) |
| "acl": schema.ListAttribute{ | ||
| Description: schemaDescriptions["acl"], | ||
| ElementType: types.StringType, | ||
| Optional: true, |
There was a problem hiding this comment.
Should acl also be the Computed: true?
There was a problem hiding this comment.
Can be set on creation/update, not sure if Computed: true would be correct.
There was a problem hiding this comment.
hmm, as this is an optional field, if user do not defines sth, it might be that there are some server-side defaults. That is why I thought it should be optional: true and computed:true.
But maybe @marceljk, can double check this? :)
There was a problem hiding this comment.
Computed isn't needed here. This must only be set, if the backend would set acl to an default value, e.g. an empty list. But when it wasn't set during the creation, the api doesn't sent a value for acl and it remains to null.
Side note:
In some cases it's needed to set Computed and Optional to true. This can cause some issues in ListAttributes, when a user has set a value during creation and removes it then within an update. See https://www.github.com/stackitcloud/terraform-provider-stackit/issues/926
| if err != nil { | ||
| var oapiErr *oapierror.GenericOpenAPIError | ||
| ok := errors.As(err, &oapiErr) | ||
| if ok && oapiErr.StatusCode == http.StatusNotFound { |
There was a problem hiding this comment.
| if ok && oapiErr.StatusCode == http.StatusNotFound { | |
| if ok && (oapiErr.StatusCode == http.StatusNotFound || oapiErr.StatusCode == http.StatusGone) { |
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?
There was a problem hiding this comment.
Sometimes it is forgotten to add some status in the docs, I thought we can be on the safe side by adding the check. But it is no critical to add. You can also skip.
There was a problem hiding this comment.
We have only the http.StatusGone check for the DSA services. https://github.com/search?q=repo%3Astackitcloud%2Fterraform-provider-stackit%20http.StatusGone&type=code
So as long this isn't documented in the api docs of the logs service, I wouldn't add the check
- 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
|
When I run the following terraform config, the read of the data source fails resource "stackit_logs_instance" "example" {
display_name = "mj-test"
project_id = var.project_id
retention_days = 1
}
data "stackit_logs_instance" "example" {
project_id = var.project_id
instance_id = stackit_logs_instance.example.instance_id
}╷
│ Error: Provider produced null object
│
│ Provider "provider[\"registry.terraform.io/stackitcloud/stackit\"]" produced a null value for data.stackit_logs_instance.example.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵ |
There was a problem hiding this comment.
To add the examples to the generated docs, you need to rename the folder "stackit_logs" to the resource/datasource name "stackit_logs_instance" and rerun "make generate-docs"
thanks, fixed |
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
|
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)