Skip to content

Raising a Pull Request#

  1. Fork the GitHub repository allowing you to make the changes in your own copy of the repository.

  2. Create a branch using the following naming prefixes:

    • f = feature
    • b = bug fix
    • d = documentation
    • t = tests
    • td = technical debt
    • v = dependencies ("vendoring" previously)

    Some indicative example branch names would be f-aws_emr_instance_group-refactor or td-staticcheck-st1008

  3. Make the changes you would like to include in the provider, add new tests as required, and make sure that all relevant existing tests are passing.

  4. Create a pull request. Please ensure (if possible) that the 'Allow edits from maintainers' checkbox is checked. This will allow the maintainers to make changes and merge the PR without requiring action from the contributor. You are welcome to submit your pull request for commentary or review before it is fully completed by creating a draft pull request. Please include specific questions or items you'd like feedback on.

  5. Pull Request Best Practices

    • Descriptive Titles: When creating a pull request (PR), use a clear and descriptive title that highlights the primary change. If the change pertains to a specific resource or data source, include its name in the title.
    • Detailed Descriptions: Provide a comprehensive description that explains the reasoning behind the change, what was modified, and any expected changes to the user experience (if applicable).
    • Focused and Manageable Scope:
      • Keep pull requests small and focused on a single change.
      • For resource or data source additions, each PR should contain only one item and its corresponding tests.
      • Avoid bundling multiple resources or combining service client additions with resource changes in a single PR. Such combinations are significantly harder and more time-consuming for maintainers to review.

  6. Create a changelog entry following the process outlined here

  7. Once you believe your pull request is ready to be reviewed, ensure the pull request is not a draft pull request by marking it ready for review or removing [WIP] from the pull request title if necessary, and a maintainer will review it. Follow the checklists below to help ensure that your contribution can be easily reviewed and potentially merged.

  8. One of Terraform's provider team members will look over your contribution and either approve it or provide comments letting you know if there is anything left to do. We'll try to give you the opportunity to make the required changes yourself, but in some cases, we may perform the changes ourselves if it makes sense to (minor changes, or for urgent issues). We do our best to keep up with the volume of PRs waiting for review, but it may take some time depending on the complexity of the work.

  9. Once all outstanding comments and checklist items have been addressed, your contribution will be merged! Merged PRs will be included in the next Terraform release.

  10. In some cases, we might decide that a PR should be closed without merging. We'll make sure to provide clear reasoning when this happens.

Go Coding Style#

All Go code is automatically checked for compliance with various linters, such as gofmt. These tools can be installed using the GNUMakefile in this repository.

make tools

Check your code with the linters:

make lint

We use Semgrep to check for other code standards. This can be run directly on the command line, i.e.,

semgrep

or it can be run using Docker via the Makefile, i.e.,

make semgrep

gofmt will also fix many simple formatting issues for you. The Makefile includes a target for this:

make fmt

The import statement in a Go file follows these rules (see #15903):

  1. Import declarations are grouped into a maximum of three groups in the following order:
    • Standard packages (also called short import path or built-in packages)
    • Third-party packages (also called long import path packages)
    • Local packages
  2. Groups are separated by a single blank line
  3. Packages within each group are alphabetized

Check your imports:

make import-lint

For greater detail, the following Go language resources provide common coding preferences that may be referenced during review, if not automatically handled by the project's linting tools.

Resource Contribution Guidelines#

The following resource checks need to be addressed before your contribution can be merged. The exclusion of any applicable check may result in a delayed time to merge. Some of these are not handled by the automated code testing that occurs during submission, so reviewers (even those outside the maintainers) are encouraged to reach out to contributors about any issues to save time.

This Contribution Guide also includes separate sections on topics such as Error Handling, which also applies to contributions.

  • Passes Testing: All code and documentation changes must pass unit testing, code linting, and website link testing. Resource code changes must pass all acceptance testing for the resource.
  • Avoids API Calls Across Account, Region, and Service Boundaries: Resources should not implement cross-account, cross-region, or cross-service API calls.
  • Does Not Set Optional or Required for Non-Configurable Attributes: Resource schema definitions for read-only attributes must not include Optional: true or Required: true.
  • Avoids retry.RetryContext() without retry.RetryableError(): Resource logic should only implement retry.Retry() if there is a retryable condition (e.g., return retry.RetryableError(err)).
  • Avoids Reusing Resource Read Function in Data Source Read Function: Data sources should fully implement their own resource Read functionality.
  • Avoids Reading Schema Structure in Resource Code: The resource Schema should not be read in resource Create/Read/Update/Delete functions to perform looping or otherwise complex attribute logic. Use d.Get() and d.Set() directly with individual attributes instead.
  • Avoids ResourceData.GetOkExists(): Resource logic should avoid using ResourceData.GetOkExists() as its expected functionality is not guaranteed in all scenarios.
  • Calls Read After Create and Update: Except where API eventual consistency prohibits immediate reading of resources or updated attributes, resource Create and Update functions should return the resource Read function.
  • Implements Immediate Resource ID Set During Create: Immediately after calling the API creation function, the resource ID should be set with d.SetId() before other API operations or returning the Read function.
  • Implements Attribute Refreshes During Read: All attributes available in the API should have d.Set() called their values in the Terraform state during the Read function.
  • Performs Error Checks with Non-Primitive Attribute Refreshes: When using d.Set() with non-primitive types (schema.TypeList, schema.TypeSet, or schema.TypeMap), perform error checking to prevent issues where the code is not properly able to refresh the Terraform state.
  • Implements Import Acceptance Testing and Documentation: Support for resource import (Importer in resource schema) must include ImportState acceptance testing (see also the Acceptance Testing Guidelines) and ## Import section in resource documentation.
  • Implements Customizable Timeouts Documentation: Support for customizable timeouts (Timeouts in resource schema) must include ## Timeouts section in resource documentation.
  • Implements State Migration When Adding New Virtual Attribute: For new "virtual" attributes (those only in Terraform and not in the API), the schema should implement State Migration to prevent differences for existing configurations that upgrade.
  • Uses AWS Go SDK Constants: Many AWS services provide string constants for value enumerations, error codes, and status types. See also the "Constants" sections under each of the service packages in the AWS Go SDK documentation.
  • Uses AWS Go SDK Pointer Conversion Functions: Many APIs return pointer types and these functions return the zero value for the type if the pointer is nil. This prevents potential panics from unchecked * pointer dereferences and can eliminate boilerplate nil checking in many cases. See also the aws package in the AWS Go SDK documentation.
  • Uses AWS Go SDK Types: Use available SDK structs instead of implementing custom types with indirection.
  • Uses Existing Validation Functions: Schema definitions including ValidateFunc for attribute validation should use available Terraform helper/validation package functions. All()/Any() can be used for combining multiple validation function behaviors.
  • Uses tfresource.TimedOut() with retry.Retry(): Resource logic implementing retry.Retry() should error check with tfresource.TimedOut(err error) and potentially unset the error before returning the error. For example:
var output *kms.CreateKeyOutput
err := retry.Retry(1*time.Minute, func() *retry.RetryError {
  var err error

  output, err = conn.CreateKey(input)

  /* ... */

  return nil
})

if tfresource.TimedOut(err) {
  output, err = conn.CreateKey(input)
}

if err != nil {
  return fmt.Errorf("creating KMS External Key: %s", err)
}
  • Uses id.UniqueId(): API fields for concurrency protection such as CallerReference and IdempotencyToken should use id.UniqueId(). The implementation includes a monotonic counter which is safer for concurrent operations than solutions such as time.Now().
  • Skips id Attribute: The id attribute is implicit for all Terraform resources and does not need to be defined in the schema.

The below are style-based items that may be noted during review and are recommended for simplicity, consistency, and quality assurance:

  • Implements arn Attribute: APIs that return an ARN should implement arn as an attribute. Alternatively, the ARN can be synthesized using the AWS Go SDK arn.ARN structure. For example:
// Direct Connect Virtual Interface ARN.
// See https://docs.aws.amazon.com/directconnect/latest/UserGuide/security_iam_service-with-iam.html#security_iam_service-with-iam-id-based-policies-resources.
arn := arn.ARN{
    Partition: meta.(*conns.AWSClient).Partition,
    Region:    meta.(*conns.AWSClient).Region,
    Service:   "directconnect",
    AccountID: meta.(*conns.AWSClient).AccountID,
    Resource:  fmt.Sprintf("dxvif/%s", d.Id()),
}.String()
d.Set("arn", arn)

When the arn attribute is synthesized this way, add the resource to the list of those affected by the provider's skip_requesting_account_id attribute.

  • Implements Warning Logging With Resource State Removal: If a resource is removed outside of Terraform (e.g., via a different tool, API, or web UI), d.SetId("") and return nil can be used in the resource Read function to trigger resource recreation. When this occurs, a warning log message should be printed beforehand: log.Printf("[WARN] {SERVICE} {THING} (%s) not found, removing from state", d.Id())
  • Uses American English for Attribute Naming: For any ambiguity with attribute naming, prefer American English over British English. e.g., color without the British u.
  • Skips Timestamp Attributes: Generally, creation and modification dates from the API should be omitted from the schema.
  • Uses Paginated AWS Go SDK Functions When Iterating Over a Collection of Objects: When the API for listing a collection of objects provides a paginated function, use it instead of looping until the next page token is not set. For example, with the EC2 API, DescribeInstancesPages should be used instead of DescribeInstances when more than one result is expected.
  • Adds Paginated Functions Missing from the AWS Go SDK to Internal Service Package: If the AWS Go SDK does not define a paginated equivalent for a function to list a collection of objects, it should be added to a per-service internal package using the listpages generator. A support case should also be opened with AWS to have the paginated functions added to the AWS Go SDK.