github.com/magodo/terraform@v0.11.12-beta1/.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