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