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