From cfef89b2292a36c09d61ea5084969f5741bff807 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Fri, 7 Nov 2025 20:49:09 +1100 Subject: [PATCH 1/2] First take at documented coding standards --- .github/copilot-instructions.md | 12 +++++-- CODING_STANDARDS.md | 64 +++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 CODING_STANDARDS.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 234be2e41..a9b072ecf 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,10 +1,16 @@ -You will be tasked to fix an issue from an open-source repository. This is a Go based repository hosting a Terrform provider for the elastic stack (elasticsearch and kibana) APIs. This repo currently supports both [plugin framework](https://developer.hashicorp.com/terraform/plugin/framework/getting-started/code-walkthrough) and [sdkv2](https://developer.hashicorp.com/terraform/plugin/sdkv2) resources. Unless you're told otherwise, all new resources _must_ use the plugin framework. +You will be writing or reviewing code for the Terraform provider for Elastic Stack (Elasticsearch, Kibana, Fleet, APM, and Logstash). This is a Go based repository hosting the provider source. -Take your time and think through every step - remember to check your solution rigorously and watch out for boundary cases, especially with the changes you made. Your solution must be perfect. If not, continue working on it. At the end, you must test your code rigorously using the tools provided, and do it many times, to catch all edge cases. If it is not robust, iterate more and make it perfect. Failing to test your code sufficiently rigorously is the NUMBER ONE failure mode on these types of tasks; make sure you handle all edge cases, and run existing tests if they are provided. +When writing code, you must adhere to the coding standards and conventions outlined in the [CODING_STANDARDS.md](../CODING_STANDARDS.md) document in this repository. + +When reviewing code, ensure that all changes comply with the coding standards and conventions specified in the [CODING_STANDARDS.md](../CODING_STANDARDS.md) document. Pay special attention to project structure, schema definitions, JSON handling, resource implementation, and testing practices. + +Take your time and think through every step - remember to check solutions rigorously and watch out for boundary cases, especially with the changes being made. + +When writing code, your solution must be perfect. If not, continue working on it. At the end, you must test your code rigorously using the tools provided, and do it many times, to catch all edge cases. If it is not robust, iterate more and make it perfect. Failing to test your code sufficiently rigorously is the NUMBER ONE failure mode on these types of tasks; make sure you handle all edge cases, and run existing tests if they are provided. Please see [README.md](../README.md) and the [CONTRIBUTING.md](../CONTRIBUTING.md) docs before getting started. -# Workflow +# Development Workflow ## High-Level Problem Solving Strategy diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md new file mode 100644 index 000000000..7f7b36756 --- /dev/null +++ b/CODING_STANDARDS.md @@ -0,0 +1,64 @@ +# Coding Standards + +This document outlines the coding standards and conventions used in the terraform-provider-elasticstack repository. + +## General Principles + +- Write idiomatic Go. + - [Effective Go](https://go.dev/doc/effective_go) + - [Code Review Comments](https://go.dev/wiki/CodeReviewComments) + - The [Google Styleguide](https://google.github.io/styleguide/go/index#about) + +## 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 +- Avoid adding extra functionality to the existing `utils` package. Instead: + - Code should live as close 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. + +## 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. +- 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 `resource-description.md` for examples. +- 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. + +## 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 + +- 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 + +## Testing + +- Use table-driven unit tests when possible with `t.Run()` for test cases +- Use testify library (`assert`, `require`) for test assertions +- Ensure that *every* resource attribute is covered by at least one acceptance test case whenever possible. + - Features that *require* external services are likely the only excuse to not include acceptance test coverage. +- Organize acceptance tests in `acc_test.go` files +- Test Terraform code should be vanilla, valid Terraform + - Store test Terraform modules in `testdata//` directories. + - Define any required variables within the module + - Reference the test code via `ConfigDirectory: acctest.NamedTestCaseDirectory("")` + - Define any required variables via `ConfigVariables` + +## API Client Usage + +- Use generated API clients from `generated/kbapi/` for new Kibana API interactions +- Avoid deprecated clients (`libs/go-kibana-rest`, `generated/alerting`, `generated/connectors`, `generated/slo`) \ No newline at end of file From 899aec99157dfb95bb1b5d5ff58573346d137812 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Fri, 7 Nov 2025 21:07:20 +1100 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 2 +- CODING_STANDARDS.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a9b072ecf..84d54a56a 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,4 +1,4 @@ -You will be writing or reviewing code for the Terraform provider for Elastic Stack (Elasticsearch, Kibana, Fleet, APM, and Logstash). This is a Go based repository hosting the provider source. +You will be writing or reviewing code for the Terraform provider for Elastic Stack (Elasticsearch, Kibana, Fleet, APM, and Logstash). This is a Go-based repository hosting the provider source. When writing code, you must adhere to the coding standards and conventions outlined in the [CODING_STANDARDS.md](../CODING_STANDARDS.md) document in this repository. diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index 7f7b36756..fe6661975 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -14,7 +14,7 @@ This document outlines the coding standards and conventions used in the terrafor - 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 - Avoid adding extra functionality to the existing `utils` package. Instead: - - Code should live as close the consumers. + - 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. @@ -25,7 +25,7 @@ This document outlines the coding standards and conventions used in the terrafor - Use `customtypes.DurationType{}` for duration-based string attributes. - Use `customtypes.JSONWithDefaultsType{}` 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 `resource-description.md` for examples. +- 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. - 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.