Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
81 changes: 81 additions & 0 deletions CODING_STANDARDS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# 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 [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 [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{}`](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. 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should be explicitly asking co-pilot to generate the OAS and consider this when generating schema definitions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could also consider committing this file? I suppose currently we are ignoring the client oas file. I'm sure this would lead to some annoying merge conflicts etc, but maybe is worth it in order to make it easier for copilot to consider the oas?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure this would lead to some annoying merge conflicts etc,

Now Renovate is handling the updates we shouldn't have humans cherry-picking changes, so I think these updates should be pretty clean. I'll add it in.

- 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/)
- Use `EnforceMinVersion` to ensure the backing Elastic Stack applications support the defined fields.
- The provider supports a wide range of Stack versions, and so newer features will not be available in all versions.
- See [`assertKafkaSupport`](./internal/fleet/output/models.go) for an example of how to handle the use of unsupported attributes.


## JSON Handling

- 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

- 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/<test_name>/<step_description>` directories.
- Define any required variables within the module
- Reference the test code via `ConfigDirectory: acctest.NamedTestCaseDirectory("<step description>")`
- Define any required variables via `ConfigVariables`

## API Client Usage

- 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`)
4 changes: 4 additions & 0 deletions internal/elasticsearch/security/system_user/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
}
Expand Down
2 changes: 1 addition & 1 deletion renovate.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading