|
| 1 | +# Coding Standards |
| 2 | + |
| 3 | +This document outlines the coding standards and conventions used in the terraform-provider-elasticstack repository. |
| 4 | + |
| 5 | +## General Principles |
| 6 | + |
| 7 | +- Write idiomatic Go. |
| 8 | + - [Effective Go](https://go.dev/doc/effective_go) |
| 9 | + - [Code Review Comments](https://go.dev/wiki/CodeReviewComments) |
| 10 | + - The [Google Styleguide](https://google.github.io/styleguide/go/index#about) |
| 11 | + |
| 12 | +## Project Structure |
| 13 | + |
| 14 | +- Use the Plugin Framework for all new resources (not SDKv2) |
| 15 | +- Follow the code organization pattern of [the `system_user` resource](./internal/elasticsearch/security/system_user) for new Plugin Framework resources |
| 16 | + - [`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. |
| 17 | + - [`acc_test.go`](./internal/elasticsearch/security/system_user/acc_test.go) - Contains acceptance tests for the resource |
| 18 | + - [`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. |
| 19 | + - [`delete.go`](./internal/elasticsearch/security/system_user/delete.go) - Contains the resources `Delete` method. |
| 20 | + - [`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. |
| 21 | + - [`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. |
| 22 | + - [`resource.go`](./internal/elasticsearch/security/system_user/resource.go) - Contains: |
| 23 | + - A factory function for creating the resource (e.g `NewSystemUserResource`) |
| 24 | + - `Metadata`, `Configure`, and optionally `ImportState` functions. |
| 25 | + - Type assertions ensuring the resource fully implement the relevant Plugin Framework interfaces (e.g `var _ resource.ResourceWithConfigure = &systemUserResource{}`) |
| 26 | + - [`schema.go`](./internal/elasticsearch/security/system_user/schema.go) - Contains the `Schema` function fully defining the resources schema |
| 27 | + - [`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. |
| 28 | + - Some resources may define other files, for example: |
| 29 | + - [`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. |
| 30 | + - 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. |
| 31 | + - [`state_upgrade.go`](./internal/elasticsearch/security/api_key/state_upgrade.go) - Resources requiring state upgrades should place the `UpgradeState` method within this file. |
| 32 | +- Avoid adding extra functionality to the existing `utils` package. Instead: |
| 33 | + - Code should live as close to the consumers. |
| 34 | + - Resource, area, application specific shared logic should live at that level. For example within `internal/kibana` for Kibana specific shared logic. |
| 35 | + - 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. |
| 36 | +- Prefer using existing util functions over longer form, duplicated code: |
| 37 | + - `utils.IsKnown(val)` instead of `!val.IsNull() && !val.IsUnknown()` |
| 38 | + - `utils.ListTypeAs` instead of `val.ElementsAs` or similar for other collection types |
| 39 | + |
| 40 | +## Schema Definitions |
| 41 | + |
| 42 | +- Use custom types to model attribute specific behaviour. |
| 43 | + - 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. |
| 44 | + - Use [`customtypes.DurationType{}`](./internal/utils/customtypes/duration_type.go) for duration-based string attributes. |
| 45 | + - Use [`customtypes.JSONWithDefaultsType{}`](./internal/utils/customtypes/json_with_defaults_type.go) to allow users to specify only a subset of a JSON blob. |
| 46 | +- Always include comprehensive descriptions for all resources, and attributes. |
| 47 | +- 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). |
| 48 | +- Use schema validation wherever possible. Only perform validation within create/read/update functions as a last resort. |
| 49 | + - For example, any validation that relies on the actual Elastic Stack components (e.g Elasticsearch version) |
| 50 | + can only be performed during the create/read/update phase. |
| 51 | +- 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). |
| 52 | + - Further details may be found in the [API documentation](https://www.elastic.co/docs/api/doc/kibana/v9/) |
| 53 | +- Elasticsearch resources will be backed by the [go-elasticsearch](https://github.com/elastic/go-elasticsearch) client. |
| 54 | + - Further details may be found in the [API documentation](https://www.elastic.co/docs/api/doc/elasticsearch/) |
| 55 | +- Use `EnforceMinVersion` to ensure the backing Elastic Stack applications support the defined fields. |
| 56 | + - The provider supports a wide range of Stack versions, and so newer features will not be available in all versions. |
| 57 | + - See [`assertKafkaSupport`](./internal/fleet/output/models.go) for an example of how to handle the use of unsupported attributes. |
| 58 | + |
| 59 | + |
| 60 | +## JSON Handling |
| 61 | + |
| 62 | +- 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. |
| 63 | +- Use [`customtypes.JSONWithDefaultsType{}`](./internal/utils/customtypes/json_with_defaults_type.go) if API level defaults may be applied automatically. |
| 64 | + |
| 65 | +## Testing |
| 66 | + |
| 67 | +- Use table-driven unit tests when possible with `t.Run()` for test cases |
| 68 | +- Use testify library (`assert`, `require`) for test assertions |
| 69 | +- Ensure that *every* resource attribute is covered by at least one acceptance test case whenever possible. |
| 70 | + - Features that *require* external services are likely the only excuse to not include acceptance test coverage. |
| 71 | +- Organize acceptance tests in `acc_test.go` files |
| 72 | +- Test Terraform code should be vanilla, valid Terraform |
| 73 | + - Store test Terraform modules in `testdata/<test_name>/<step_description>` directories. |
| 74 | + - Define any required variables within the module |
| 75 | + - Reference the test code via `ConfigDirectory: acctest.NamedTestCaseDirectory("<step description>")` |
| 76 | + - Define any required variables via `ConfigVariables` |
| 77 | + |
| 78 | +## API Client Usage |
| 79 | + |
| 80 | +- Use generated API clients from [`generated/kbapi/`](./generated/kbapi/) for new Kibana API interactions |
| 81 | +- Avoid deprecated clients (`libs/go-kibana-rest`, `generated/alerting`, `generated/connectors`, `generated/slo`) |
0 commit comments