github.com/nathanielks/terraform@v0.6.1-0.20170509030759-13e1a62319dc/.github/CONTRIBUTING.md (about)

     1  # Contributing to Terraform
     2  
     3  **First:** if you're unsure or afraid of _anything_, just ask
     4  or submit the issue or pull request anyways. You won't be yelled at for
     5  giving your best effort. The worst that can happen is that you'll be
     6  politely asked to change something. We appreciate any sort of contributions,
     7  and don't want a wall of rules to get in the way of that.
     8  
     9  However, for those individuals who want a bit more guidance on the
    10  best way to contribute to the project, read on. This document will cover
    11  what we're looking for. By addressing all the points we're looking for,
    12  it raises the chances we can quickly merge or address your contributions.
    13  
    14  Specifically, we have provided checklists below for each type of issue and pull
    15  request that can happen on the project. These checklists represent everything
    16  we need to be able to review and respond quickly.
    17  
    18  ## HashiCorp vs. Community Providers
    19  
    20  We separate providers out into what we call "HashiCorp Providers" and
    21  "Community Providers".
    22  
    23  HashiCorp providers are providers that we'll dedicate full time resources to
    24  improving, supporting the latest features, and fixing bugs. These are providers
    25  we understand deeply and are confident we have the resources to manage
    26  ourselves.
    27  
    28  Community providers are providers where we depend on the community to
    29  contribute fixes and enhancements to improve. HashiCorp will run automated
    30  tests and ensure these providers continue to work, but will not dedicate full
    31  time resources to add new features to these providers. These providers are
    32  available in official Terraform releases, but the functionality is primarily
    33  contributed.
    34  
    35  The current list of HashiCorp Providers is as follows:
    36  
    37   * `aws`
    38   * `azurerm`
    39   * `google`
    40  
    41  Our testing standards are the same for both HashiCorp and Community providers,
    42  and HashiCorp runs full acceptance test suites for every provider nightly to
    43  ensure Terraform remains stable.
    44  
    45  We make the distinction between these two types of providers to help
    46  highlight the vast amounts of community effort that goes in to making Terraform
    47  great, and to help contributors better understand the role HashiCorp employees
    48  play in the various areas of the code base.
    49  
    50  ## Issues
    51  
    52  ### Issue Reporting Checklists
    53  
    54  We welcome issues of all kinds including feature requests, bug reports, and
    55  general questions. Below you'll find checklists with guidelines for well-formed
    56  issues of each type.
    57  
    58  #### Bug Reports
    59  
    60   - [ ] __Test against latest release__: Make sure you test against the latest
    61     released version. It is possible we already fixed the bug you're experiencing.
    62  
    63   - [ ] __Search for possible duplicate reports__: It's helpful to keep bug
    64     reports consolidated to one thread, so do a quick search on existing bug
    65     reports to check if anybody else has reported the same thing. You can scope
    66     searches by the label "bug" to help narrow things down.
    67  
    68   - [ ] __Include steps to reproduce__: Provide steps to reproduce the issue,
    69     along with your `.tf` files, with secrets removed, so we can try to
    70     reproduce it. Without this, it makes it much harder to fix the issue.
    71  
    72   - [ ] __For panics, include `crash.log`__: If you experienced a panic, please
    73     create a [gist](https://gist.github.com) of the *entire* generated crash log
    74     for us to look at. Double check no sensitive items were in the log.
    75  
    76  #### Feature Requests
    77  
    78   - [ ] __Search for possible duplicate requests__: It's helpful to keep requests
    79     consolidated to one thread, so do a quick search on existing requests to
    80     check if anybody else has reported the same thing. You can scope searches by
    81     the label "enhancement" to help narrow things down.
    82  
    83   - [ ] __Include a use case description__: In addition to describing the
    84     behavior of the feature you'd like to see added, it's helpful to also lay
    85     out the reason why the feature would be important and how it would benefit
    86     Terraform users.
    87  
    88  #### Questions
    89  
    90   - [ ] __Search for answers in Terraform documentation__: We're happy to answer
    91     questions in GitHub Issues, but it helps reduce issue churn and maintainer
    92     workload if you work to find answers to common questions in the
    93     documentation. Often times Question issues result in documentation updates
    94     to help future users, so if you don't find an answer, you can give us
    95     pointers for where you'd expect to see it in the docs.
    96  
    97  ### Issue Lifecycle
    98  
    99  1. The issue is reported.
   100  
   101  2. The issue is verified and categorized by a Terraform collaborator.
   102     Categorization is done via GitHub labels. We generally use a two-label
   103     system of (1) issue/PR type, and (2) section of the codebase. Type is
   104     usually "bug", "enhancement", "documentation", or "question", and section
   105     can be any of the providers or provisioners or "core".
   106  
   107  3. Unless it is critical, the issue is left for a period of time (sometimes
   108     many weeks), giving outside contributors a chance to address the issue.
   109  
   110  4. The issue is addressed in a pull request or commit. The issue will be
   111     referenced in the commit message so that the code that fixes it is clearly
   112     linked.
   113  
   114  5. The issue is closed. Sometimes, valid issues will be closed to keep
   115     the issue tracker clean. The issue is still indexed and available for
   116     future viewers, or can be re-opened if necessary.
   117  
   118  ## Pull Requests
   119  
   120  Thank you for contributing! Here you'll find information on what to include in
   121  your Pull Request to ensure it is accepted quickly.
   122  
   123   * For pull requests that follow the guidelines, we expect to be able to review
   124     and merge very quickly.
   125   * Pull requests that don't follow the guidelines will be annotated with what
   126     they're missing. A community or core team member may be able to swing around
   127     and help finish up the work, but these PRs will generally hang out much
   128     longer until they can be completed and merged.
   129  
   130  ### Pull Request Lifecycle
   131  
   132  1. You are welcome to submit your pull request for commentary or review before
   133     it is fully completed. Please prefix the title of your pull request with
   134     "[WIP]" to indicate this. It's also a good idea to include specific
   135     questions or items you'd like feedback on.
   136  
   137  2. Once you believe your pull request is ready to be merged, you can remove any
   138     "[WIP]" prefix from the title and a core team member will review. Follow
   139     [the checklists below](#checklists-for-contribution) to help ensure that
   140     your contribution will be merged quickly.
   141  
   142  3. One of Terraform's core team members will look over your contribution and
   143     either provide comments letting you know if there is anything left to do. We
   144     do our best to provide feedback in a timely manner, but it may take some
   145     time for us to respond.
   146  
   147  4. Once all outstanding comments and checklist items have been addressed, your
   148     contribution will be merged! Merged PRs will be included in the next
   149     Terraform release. The core team takes care of updating the CHANGELOG as
   150     they merge.
   151  
   152  5. In rare cases, we might decide that a PR should be closed. We'll make sure
   153     to provide clear reasoning when this happens.
   154  
   155  ### Checklists for Contribution
   156  
   157  There are several different kinds of contribution, each of which has its own
   158  standards for a speedy review. The following sections describe guidelines for
   159  each type of contribution.
   160  
   161  #### Documentation Update
   162  
   163  Because [Terraform's website][website] is in the same repo as the code, it's
   164  easy for anybody to help us improve our docs.
   165  
   166   - [ ] __Reasoning for docs update__: Including a quick explanation for why the
   167     update needed is helpful for reviewers.
   168   - [ ] __Relevant Terraform version__: Is this update worth deploying to the
   169     site immediately, or is it referencing an upcoming version of Terraform and
   170     should get pushed out with the next release?
   171  
   172  #### Enhancement/Bugfix to a Resource
   173  
   174  Working on existing resources is a great way to get started as a Terraform
   175  contributor because you can work within existing code and tests to get a feel
   176  for what to do.
   177  
   178   - [ ] __Acceptance test coverage of new behavior__: Existing resources each
   179     have a set of [acceptance tests][acctests] covering their functionality.
   180     These tests should exercise all the behavior of the resource. Whether you are
   181     adding something or fixing a bug, the idea is to have an acceptance test that
   182     fails if your code were to be removed. Sometimes it is sufficient to
   183     "enhance" an existing test by adding an assertion or tweaking the config
   184     that is used, but often a new test is better to add. You can copy/paste an
   185     existing test and follow the conventions you see there, modifying the test
   186     to exercise the behavior of your code.
   187   - [ ] __Documentation updates__: If your code makes any changes that need to
   188     be documented, you should include those doc updates in the same PR. The
   189     [Terraform website][website] source is in this repo and includes
   190     instructions for getting a local copy of the site up and running if you'd
   191     like to preview your changes.
   192   - [ ] __Well-formed Code__: Do your best to follow existing conventions you
   193     see in the codebase, and ensure your code is formatted with `go fmt`. (The
   194     Travis CI build will fail if `go fmt` has not been run on incoming code.)
   195     The PR reviewers can help out on this front, and may provide comments with
   196     suggestions on how to improve the code.
   197  
   198  #### New Resource
   199  
   200  Implementing a new resource is a good way to learn more about how Terraform
   201  interacts with upstream APIs. There are plenty of examples to draw from in the
   202  existing resources, but you still get to implement something completely new.
   203  
   204   - [ ] __Minimal LOC__: It can be inefficient for both the reviewer
   205     and author to go through long feedback cycles on a big PR with many
   206     resources. We therefore encourage you to only submit **1 resource at a time**.
   207   - [ ] __Acceptance tests__: New resources should include acceptance tests
   208     covering their behavior. See [Writing Acceptance
   209     Tests](#writing-acceptance-tests) below for a detailed guide on how to
   210     approach these.
   211   - [ ] __Documentation__: Each resource gets a page in the Terraform
   212     documentation. The [Terraform website][website] source is in this
   213     repo and includes instructions for getting a local copy of the site up and
   214     running if you'd like to preview your changes. For a resource, you'll want
   215     to add a new file in the appropriate place and add a link to the sidebar for
   216     that page.
   217   - [ ] __Well-formed Code__: Do your best to follow existing conventions you
   218     see in the codebase, and ensure your code is formatted with `go fmt`. (The
   219     Travis CI build will fail if `go fmt` has not been run on incoming code.)
   220     The PR reviewers can help out on this front, and may provide comments with
   221     suggestions on how to improve the code.
   222  
   223  #### New Provider
   224  
   225  Implementing a new provider gives Terraform the ability to manage resources in
   226  a whole new API. It's a larger undertaking, but brings major new functionality
   227  into Terraform.
   228  
   229   - [ ] __Minimal initial LOC__: Some providers may be big and it can be
   230     inefficient for both reviewer & author to go through long feedback cycles
   231     on a big PR with many resources. We encourage you to only submit
   232     the necessary minimum in a single PR, ideally **just the first resource**
   233     of the provider.
   234   - [ ] __Acceptance tests__: Each provider should include an acceptance test
   235     suite with tests for each resource should include acceptance tests covering
   236     its behavior. See [Writing Acceptance Tests](#writing-acceptance-tests) below
   237     for a detailed guide on how to approach these.
   238   - [ ] __Documentation__: Each provider has a section in the Terraform
   239     documentation. The [Terraform website][website] source is in this repo and
   240     includes instructions for getting a local copy of the site up and running if
   241     you'd like to preview your changes. For a provider, you'll want to add new
   242     index file and individual pages for each resource.
   243   - [ ] __Well-formed Code__: Do your best to follow existing conventions you
   244     see in the codebase, and ensure your code is formatted with `go fmt`. (The
   245     Travis CI build will fail if `go fmt` has not been run on incoming code.)
   246     The PR reviewers can help out on this front, and may provide comments with
   247     suggestions on how to improve the code.
   248  
   249  #### Core Bugfix/Enhancement
   250  
   251  We are always happy when any developer is interested in diving into Terraform's
   252  core to help out! Here's what we look for in smaller Core PRs.
   253  
   254   - [ ] __Unit tests__: Terraform's core is covered by hundreds of unit tests at
   255     several different layers of abstraction. Generally the best place to start
   256     is with a "Context Test". These are higher level test that interact
   257     end-to-end with most of Terraform's core. They are divided into test files
   258     for each major action (plan, apply, etc.). Getting a failing test is a great
   259     way to prove out a bug report or a new enhancement. With a context test in
   260     place, you can work on implementation and lower level unit tests. Lower
   261     level tests are largely context dependent, but the Context Tests are almost
   262     always part of core work.
   263   - [ ] __Documentation updates__: If the core change involves anything that
   264     needs to be reflected in our documentation, you can make those changes in
   265     the same PR. The [Terraform website][website] source is in this repo and
   266     includes instructions for getting a local copy of the site up and running if
   267     you'd like to preview your changes.
   268   - [ ] __Well-formed Code__: Do your best to follow existing conventions you
   269     see in the codebase, and ensure your code is formatted with `go fmt`. (The
   270     Travis CI build will fail if `go fmt` has not been run on incoming code.)
   271     The PR reviewers can help out on this front, and may provide comments with
   272     suggestions on how to improve the code.
   273  
   274  #### Core Feature
   275  
   276  If you're interested in taking on a larger core feature, it's a good idea to
   277  get feedback early and often on the effort.
   278  
   279   - [ ] __Early validation of idea and implementation plan__: Terraform's core
   280     is complicated enough that there are often several ways to implement
   281     something, each of which has different implications and tradeoffs. Working
   282     through a plan of attack with the team before you dive into implementation
   283     will help ensure that you're working in the right direction.
   284   - [ ] __Unit tests__: Terraform's core is covered by hundreds of unit tests at
   285     several different layers of abstraction. Generally the best place to start
   286     is with a "Context Test". These are higher level test that interact
   287     end-to-end with most of Terraform's core. They are divided into test files
   288     for each major action (plan, apply, etc.). Getting a failing test is a great
   289     way to prove out a bug report or a new enhancement. With a context test in
   290     place, you can work on implementation and lower level unit tests. Lower
   291     level tests are largely context dependent, but the Context Tests are almost
   292     always part of core work.
   293   - [ ] __Documentation updates__: If the core change involves anything that
   294     needs to be reflected in our documentation, you can make those changes in
   295     the same PR. The [Terraform website][website] source is in this repo and
   296     includes instructions for getting a local copy of the site up and running if
   297     you'd like to preview your changes.
   298   - [ ] __Well-formed Code__: Do your best to follow existing conventions you
   299     see in the codebase, and ensure your code is formatted with `go fmt`. (The
   300     Travis CI build will fail if `go fmt` has not been run on incoming code.)
   301     The PR reviewers can help out on this front, and may provide comments with
   302     suggestions on how to improve the code.
   303  
   304  ### Writing Acceptance Tests
   305  
   306  Terraform includes an acceptance test harness that does most of the repetitive
   307  work involved in testing a resource.
   308  
   309  #### Acceptance Tests Often Cost Money to Run
   310  
   311  Because acceptance tests create real resources, they often cost money to run.
   312  Because the resources only exist for a short period of time, the total amount
   313  of money required is usually a relatively small. Nevertheless, we don't want
   314  financial limitations to be a barrier to contribution, so if you are unable to
   315  pay to run acceptance tests for your contribution, simply mention this in your
   316  pull request. We will happily accept "best effort" implementations of
   317  acceptance tests and run them for you on our side. This might mean that your PR
   318  takes a bit longer to merge, but it most definitely is not a blocker for
   319  contributions.
   320  
   321  #### Running an Acceptance Test
   322  
   323  Acceptance tests can be run using the `testacc` target in the Terraform
   324  `Makefile`. The individual tests to run can be controlled using a regular
   325  expression. Prior to running the tests provider configuration details such as
   326  access keys must be made available as environment variables.
   327  
   328  For example, to run an acceptance test against the Azure Resource Manager
   329  provider, the following environment variables must be set:
   330  
   331  ```sh
   332  export ARM_SUBSCRIPTION_ID=...
   333  export ARM_CLIENT_ID=...
   334  export ARM_CLIENT_SECRET=...
   335  export ARM_TENANT_ID=...
   336  ```
   337  
   338  Tests can then be run by specifying the target provider and a regular
   339  expression defining the tests to run:
   340  
   341  ```sh
   342  $ make testacc TEST=./builtin/providers/azurerm TESTARGS='-run=TestAccAzureRMPublicIpStatic_update'
   343  ==> Checking that code complies with gofmt requirements...
   344  go generate ./...
   345  TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMPublicIpStatic_update -timeout 120m
   346  === RUN   TestAccAzureRMPublicIpStatic_update
   347  --- PASS: TestAccAzureRMPublicIpStatic_update (177.48s)
   348  PASS
   349  ok      github.com/hashicorp/terraform/builtin/providers/azurerm    177.504s
   350  ```
   351  
   352  Entire resource test suites can be targeted by using the naming convention to
   353  write the regular expression. For example, to run all tests of the
   354  `azurerm_public_ip` resource rather than just the update test, you can start
   355  testing like this:
   356  
   357  ```sh
   358  $ make testacc TEST=./builtin/providers/azurerm TESTARGS='-run=TestAccAzureRMPublicIpStatic'
   359  ==> Checking that code complies with gofmt requirements...
   360  go generate ./...
   361  TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMPublicIpStatic -timeout 120m
   362  === RUN   TestAccAzureRMPublicIpStatic_basic
   363  --- PASS: TestAccAzureRMPublicIpStatic_basic (137.74s)
   364  === RUN   TestAccAzureRMPublicIpStatic_update
   365  --- PASS: TestAccAzureRMPublicIpStatic_update (180.63s)
   366  PASS
   367  ok      github.com/hashicorp/terraform/builtin/providers/azurerm    318.392s
   368  ```
   369  
   370  #### Writing an Acceptance Test
   371  
   372  Terraform has a framework for writing acceptance tests which minimises the
   373  amount of boilerplate code necessary to use common testing patterns. The entry
   374  point to the framework is the `resource.Test()` function.
   375  
   376  Tests are divided into `TestStep`s. Each `TestStep` proceeds by applying some
   377  Terraform configuration using the provider under test, and then verifying that
   378  results are as expected by making assertions using the provider API. It is
   379  common for a single test function to exercise both the creation of and updates
   380  to a single resource. Most tests follow a similar structure.
   381  
   382  1. Pre-flight checks are made to ensure that sufficient provider configuration
   383     is available to be able to proceed - for example in an acceptance test
   384     targeting AWS, `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` must be set prior
   385     to running acceptance tests. This is common to all tests exercising a single
   386     provider.
   387  
   388  Each `TestStep` is defined in the call to `resource.Test()`. Most assertion
   389  functions are defined out of band with the tests. This keeps the tests
   390  readable, and allows reuse of assertion functions across different tests of the
   391  same type of resource. The definition of a complete test looks like this:
   392  
   393  ```go
   394  func TestAccAzureRMPublicIpStatic_update(t *testing.T) {
   395  	resource.Test(t, resource.TestCase{
   396  		PreCheck:     func() { testAccPreCheck(t) },
   397  		Providers:    testAccProviders,
   398  		CheckDestroy: testCheckAzureRMPublicIpDestroy,
   399  		Steps: []resource.TestStep{
   400  			resource.TestStep{
   401  				Config: testAccAzureRMVPublicIpStatic_basic,
   402  				Check: resource.ComposeTestCheckFunc(
   403  					testCheckAzureRMPublicIpExists("azurerm_public_ip.test"),
   404  				),
   405  			},
   406          },
   407      })
   408  }
   409  ```
   410  
   411  When executing the test, the following steps are taken for each `TestStep`:
   412  
   413  1. The Terraform configuration required for the test is applied. This is
   414     responsible for configuring the resource under test, and any dependencies it
   415     may have. For example, to test the `azurerm_public_ip` resource, an
   416     `azurerm_resource_group` is required. This results in configuration which
   417     looks like this:
   418  
   419      ```hcl
   420      resource "azurerm_resource_group" "test" {
   421          name = "acceptanceTestResourceGroup1"
   422          location = "West US"
   423      }
   424  
   425      resource "azurerm_public_ip" "test" {
   426          name = "acceptanceTestPublicIp1"
   427          location = "West US"
   428          resource_group_name = "${azurerm_resource_group.test.name}"
   429          public_ip_address_allocation = "static"
   430      }
   431      ```
   432  
   433  1. Assertions are run using the provider API. These use the provider API
   434     directly rather than asserting against the resource state. For example, to
   435     verify that the `azurerm_public_ip` described above was created
   436     successfully, a test function like this is used:
   437  
   438      ```go
   439      func testCheckAzureRMPublicIpExists(name string) resource.TestCheckFunc {
   440          return func(s *terraform.State) error {
   441              // Ensure we have enough information in state to look up in API
   442              rs, ok := s.RootModule().Resources[name]
   443              if !ok {
   444                  return fmt.Errorf("Not found: %s", name)
   445              }
   446  
   447              publicIPName := rs.Primary.Attributes["name"]
   448              resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"]
   449              if !hasResourceGroup {
   450                  return fmt.Errorf("Bad: no resource group found in state for public ip: %s", availSetName)
   451              }
   452  
   453              conn := testAccProvider.Meta().(*ArmClient).publicIPClient
   454  
   455              resp, err := conn.Get(resourceGroup, publicIPName, "")
   456              if err != nil {
   457                  return fmt.Errorf("Bad: Get on publicIPClient: %s", err)
   458              }
   459  
   460              if resp.StatusCode == http.StatusNotFound {
   461                  return fmt.Errorf("Bad: Public IP %q (resource group: %q) does not exist", name, resourceGroup)
   462              }
   463  
   464              return nil
   465          }
   466      }
   467      ```
   468  
   469     Notice that the only information used from the Terraform state is the ID of
   470     the resource - though in this case it is necessary to split the ID into
   471     constituent parts in order to use the provider API. For computed properties,
   472     we instead assert that the value saved in the Terraform state was the
   473     expected value if possible. The testing framework provides helper functions
   474     for several common types of check - for example:
   475  
   476      ```go
   477      resource.TestCheckResourceAttr("azurerm_public_ip.test", "domain_name_label", "mylabel01"),
   478      ```
   479  
   480  1. The resources created by the test are destroyed. This step happens
   481     automatically, and is the equivalent of calling `terraform destroy`.
   482  
   483  1. Assertions are made against the provider API to verify that the resources
   484     have indeed been removed. If these checks fail, the test fails and reports
   485     "dangling resources". The code to ensure that the `azurerm_public_ip` shown
   486     above looks like this:
   487  
   488      ```go
   489      func testCheckAzureRMPublicIpDestroy(s *terraform.State) error {
   490          conn := testAccProvider.Meta().(*ArmClient).publicIPClient
   491  
   492          for _, rs := range s.RootModule().Resources {
   493              if rs.Type != "azurerm_public_ip" {
   494                  continue
   495              }
   496  
   497              name := rs.Primary.Attributes["name"]
   498              resourceGroup := rs.Primary.Attributes["resource_group_name"]
   499  
   500              resp, err := conn.Get(resourceGroup, name, "")
   501  
   502              if err != nil {
   503                  return nil
   504              }
   505  
   506              if resp.StatusCode != http.StatusNotFound {
   507                  return fmt.Errorf("Public IP still exists:\n%#v", resp.Properties)
   508              }
   509          }
   510  
   511          return nil
   512      }
   513      ```
   514  
   515     These functions usually test only for the resource directly under test: we
   516     skip the check that the `azurerm_resource_group` has been destroyed when
   517     testing `azurerm_resource_group`, under the assumption that
   518     `azurerm_resource_group` is tested independently in its own acceptance
   519     tests.
   520  
   521  [website]: https://github.com/hashicorp/terraform/tree/master/website
   522  [acctests]: https://github.com/hashicorp/terraform#acceptance-tests
   523  [ml]: https://groups.google.com/group/terraform-tool