PR feedback

Author: tobio

CODING_STANDARDS.md

@@ -12,38 +12,52 @@ This document outlines the coding standards and conventions used in the terrafor
 ## Project Structure
 
 - Use the Plugin Framework for all new resources (not SDKv2)
-- Follow the code organization pattern of `internal/elasticsearch/security/system_user` for new Plugin Framework resources
+- Follow the code organization pattern of [the `system_user` resource](./internal/elasticsearch/security/system_user) for new Plugin Framework resources
+  - [`testdata/`](./internal/elasticsearch/security/system_user/testdata) - This directory contains Terraform definitions used within the resource acceptance tests. In most cases, this will contain a subdirectory for each test, which then contain subdirectories for individual named test steps. 
+  - [`acc_test.go`](./internal/elasticsearch/security/system_user/acc_test.go) - Contains acceptance tests for the resource
+  - [`create.go`](./internal/elasticsearch/security/system_user/create.go) - Contains the resources `Create` method and any required logic. Depending on the underlying API, the create and update handlers may share a single code path. 
+  - [`delete.go`](./internal/elasticsearch/security/system_user/delete.go) - Contains the resources `Delete` method.
+  - [`models.go`](./internal/elasticsearch/security/system_user/models.go) - Contains Golang models used by the resource. At a minimum this will contain a model for reading plan/config/state from the Terraform plugin framework. Any non-trivial models should also define receivers for translating between Terraform models and API client models. 
+  - [`read.go`](./internal/elasticsearch/security/system_user/read.go) - Contains the resources `Read` method. This should also define an internal `read` function that can be re-used by the create/update paths to populate the final Terraform state after performing the create/update operation. 
+  - [`resource.go`](./internal/elasticsearch/security/system_user/resource.go) - Contains:
+    - A factory function for creating the resource (e.g `NewSystemUserResource`)
+    - `Metadata`, `Configure`, and optionally `ImportState` functions. 
+    - Type assertions ensuring the resource fully implement the relevant Plugin Framework interfaces (e.g `var _ resource.ResourceWithConfigure = &systemUserResource{}`)
+  - [`schema.go`](./internal/elasticsearch/security/system_user/schema.go) - Contains the `Schema` function fully defining the resources schema
+  - [`update.go`](./internal/elasticsearch/security/system_user/update.go) - Contains the `Update` method. Depending on the underlying API this may share significant logic with the `Create` method. 
+  - Some resources may define other files, for example:
+    - [`models_*.go`](./internal/kibana/security_detection_rule/) - Complex APIs may result in significant model related logic. Split these files as appropriate if they become large. 
+    - Custom [plan modifiers](./internal/elasticsearch/security/api_key/set_unknown_if_access_has_changes.go), [validators](./internal/elasticsearch/security/api_key/validators.go) and [types](./internal/elasticsearch/security/api_key/role_descriptor_defaults.go) - Resource specific plan modifiers and custom types should be contained within the resource package. 
+    - [`state_upgrade.go`](./internal/elasticsearch/security/api_key/state_upgrade.go) - Resources requiring state upgrades should place the `UpgradeState` method within this file.
 - Avoid adding extra functionality to the existing `utils` package. Instead:
   - Code should live as close to the consumers.
   - Resource, area, application specific shared logic should live at that level. For example within `internal/kibana` for Kibana specific shared logic.
-  - Provider wide shared logic should be packaged together by a logical concept. For example `internal/diagutil` contains shared code for managing Terraform Diagnostics, and translating between errors, SDKv2 diags, and Plugin Framework diags.
+  - Provider wide shared logic should be packaged together by a logical concept. For example [diagutil](./internal/diagutil) contains shared code for managing Terraform Diagnostics, and translating between errors, SDKv2 diags, and Plugin Framework diags.
+- Prefer using existing util functions over longer form, duplicated code:
+  - `utils.IsKnown(val)` instead of `!val.IsNull() && !val.IsUnknown()`
+  - `utils.ListTypeAs` instead of `val.ElementsAs` or similar for other collection types
 
 ## Schema Definitions
 
 - Use custom types to model attribute specific behaviour.
-    - Use `jsontypes.NormalizedType{}` custom type for string attributes containing JSON blobs.
-    - Use `customtypes.DurationType{}` for duration-based string attributes.
-    - Use `customtypes.JSONWithDefaultsType{}` to allow users to specify only a subset of a JSON blob.
+    - Use [`jsontypes.NormalizedType{}`](https://github.com/hashicorp/terraform-plugin-framework-jsontypes/blob/main/jsontypes/normalized_type.go) custom type for string attributes containing JSON blobs.
+    - Use [`customtypes.DurationType{}`](./internal/utils/customtypes/duration_type.go) for duration-based string attributes.
+    - Use [`customtypes.JSONWithDefaultsType{}`](./internal/utils/customtypes/json_with_defaults_type.go) to allow users to specify only a subset of a JSON blob.
 - Always include comprehensive descriptions for all resources, and attributes. 
-- Long, multiline descriptions should be stored in an external markdown file, which is imported via Golang embedding. See `internal/elasticsearch/security/system_user/resource-description.md` for an example location.
+- Long, multiline descriptions should be stored in an external markdown file, which is imported via Golang embedding. For [example](./internal/elasticsearch/security/system_user/resource-description.md).
 - Use schema validation wherever possible. Only perform validation within create/read/update functions as a last resort. 
   - For example, any validation that relies on the actual Elastic Stack components (e.g Elasticsearch version)
     can only be performed during the create/read/update phase. 
+- Kibana and Fleet resources will be backed by the Kibana API. The schema definition should closely follow the defined API request/response models defined in the [OpenAPI specification](./generated/kbapi/oas-filtered.yaml).
+  - Further details may be found in the [API documentation](https://www.elastic.co/docs/api/doc/kibana/v9/)
+- Elasticsearch resources will be backed by the [go-elasticsearch](https://github.com/elastic/go-elasticsearch) client. 
+  - Further details may be found in the [API documentation](https://www.elastic.co/docs/api/doc/elasticsearch/)
 
-## JSON Handling
-
-- Use `jsontypes.NormalizedType{}` for JSON string attributes to ensure proper normalization and comparison.
-- Use `customtypes.JSONWithDefaultsType{}` if API level defaults may be applied automatically. 
 
-## Resource Implementation
+## JSON Handling
 
-- Follow the pattern: `resource.go`, `schema.go`, `models.go`, `create.go`, `read.go`, `update.go`, `delete.go`
-- Use factory functions like `NewSystemUserResource()` to create resource instances
-- Ensure appropriate interface assertions are included alongside the resource definition. 
-  - For example, if a resource supports imports, include `var _ resource.ResourceWithImportState = &resource{}` or similar.
-- Prefer using existing util functions over longer form, duplicated code:
-  - `utils.IsKnown(val)` instead of `!val.IsNull() && !val.IsUnknown()`
-  - `utils.ListTypeAs` instead of `val.ElementsAs` or similar for other collection types
+- Use [`jsontypes.NormalizedType{}`](https://github.com/hashicorp/terraform-plugin-framework-jsontypes/blob/main/jsontypes/normalized_type.go) for JSON string attributes to ensure proper normalization and comparison.
+- Use [`customtypes.JSONWithDefaultsType{}`](./internal/utils/customtypes/json_with_defaults_type.go) if API level defaults may be applied automatically. 
 
 ## Testing
 
@@ -60,5 +74,5 @@ This document outlines the coding standards and conventions used in the terrafor
 
 ## API Client Usage
 
-- Use generated API clients from `generated/kbapi/` for new Kibana API interactions
+- Use generated API clients from [`generated/kbapi/`](./generated/kbapi/) for new Kibana API interactions
 - Avoid deprecated clients (`libs/go-kibana-rest`, `generated/alerting`, `generated/connectors`, `generated/slo`)

internal/elasticsearch/security/system_user/resource.go

@@ -7,6 +7,10 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 )
 
+// Ensure provider defined types fully satisfy framework interfaces
+var _ resource.Resource = &systemUserResource{}
+var _ resource.ResourceWithConfigure = &systemUserResource{}
+
 func NewSystemUserResource() resource.Resource {
 	return &systemUserResource{}
 }

renovate.json

@@ -16,7 +16,7 @@
   ],
   "postUpgradeTasks": {
     "commands": ["make -C generated/kbapi all"],
-    "fileFilters": ["generated/kbapi/kibana.gen.go"]
+    "fileFilters": ["generated/kbapi/kibana.gen.go", "generated/kbapi/oas-filtered.yaml"]
   },
   "automerge": true,
   "automergeStrategy": "squash",