Skip to content

Commit

Permalink
Add acm certificate lifecycle create before destroy rule (#202)
Browse files Browse the repository at this point in the history
* Add acm certificate lifecycle create before destroy rule

* Fix broken links
  • Loading branch information
AleksaC authored Dec 2, 2021
1 parent c34979e commit 131fddd
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 4 deletions.
5 changes: 3 additions & 2 deletions docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ These rules warn of possible errors that can occur at `terraform apply`. Rules m
|aws_elasticache_cluster_invalid_parameter_group|Disallow using invalid parameter group|||
|aws_elasticache_cluster_invalid_security_group|Disallow using invalid security groups|||
|aws_elasticache_cluster_invalid_subnet_group|Disallow using invalid subnet group|||
|[aws_elasticache_cluster_invalid_type](aws_elasticache_cluster_invalid_type)|Disallow using invalid node type|||
|[aws_elasticache_replication_group_invalid_type](aws_elasticache_replication_group_invalid_type)|Disallow using invalid node type|||
|[aws_elasticache_cluster_invalid_type](aws_elasticache_cluster_invalid_type.md)|Disallow using invalid node type|||
|[aws_elasticache_replication_group_invalid_type](aws_elasticache_replication_group_invalid_type.md)|Disallow using invalid node type|||
|aws_elb_invalid_instance|Disallow using invalid instances|||
|aws_elb_invalid_security_group|Disallow using invalid security groups|||
|aws_elb_invalid_subnet|Disallow using invalid subnets|||
Expand All @@ -48,6 +48,7 @@ These rules enforce best practices and naming conventions:

|Rule|Description|Enabled by default|
| --- | --- | --- |
|[aws_acm_certificate_lifecycle](aws_acm_certificate_lifecycle.md)|Disallow adding `aws_acm_certificate` resource without setting `create_before_destroy = true` in `lifecycle` block ||
|[aws_db_instance_previous_type](aws_db_instance_previous_type.md)|Disallow using previous generation instance types||
|[aws_db_instance_default_parameter_group](aws_db_instance_default_parameter_group.md)|Disallow using default DB parameter group||
|[aws_elasticache_cluster_previous_type](aws_elasticache_cluster_previous_type.md)|Disallow using previous node types||
Expand Down
5 changes: 3 additions & 2 deletions docs/rules/README.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ These rules warn of possible errors that can occur at `terraform apply`. Rules m
|aws_elasticache_cluster_invalid_parameter_group|Disallow using invalid parameter group|✔|✔|
|aws_elasticache_cluster_invalid_security_group|Disallow using invalid security groups|✔|✔|
|aws_elasticache_cluster_invalid_subnet_group|Disallow using invalid subnet group|✔|✔|
|[aws_elasticache_cluster_invalid_type](aws_elasticache_cluster_invalid_type)|Disallow using invalid node type||✔|
|[aws_elasticache_replication_group_invalid_type](aws_elasticache_replication_group_invalid_type)|Disallow using invalid node type||✔|
|[aws_elasticache_cluster_invalid_type](aws_elasticache_cluster_invalid_type.md)|Disallow using invalid node type||✔|
|[aws_elasticache_replication_group_invalid_type](aws_elasticache_replication_group_invalid_type.md)|Disallow using invalid node type||✔|
|aws_elb_invalid_instance|Disallow using invalid instances|✔|✔|
|aws_elb_invalid_security_group|Disallow using invalid security groups|✔|✔|
|aws_elb_invalid_subnet|Disallow using invalid subnets|✔|✔|
Expand All @@ -48,6 +48,7 @@ These rules enforce best practices and naming conventions:

|Rule|Description|Enabled by default|
| --- | --- | --- |
|[aws_acm_certificate_lifecycle](aws_acm_certificate_lifecycle.md)|Disallow adding `aws_acm_certificate` resource without setting `create_before_destroy = true` in `lifecycle` block |✔|
|[aws_db_instance_previous_type](aws_db_instance_previous_type.md)|Disallow using previous generation instance types|✔|
|[aws_db_instance_default_parameter_group](aws_db_instance_default_parameter_group.md)|Disallow using default DB parameter group|✔|
|[aws_elasticache_cluster_previous_type](aws_elasticache_cluster_previous_type.md)|Disallow using previous node types|✔|
Expand Down
47 changes: 47 additions & 0 deletions docs/rules/aws_acm_certificate_lifecycle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# aws_acm_certificate_lifecycle

This rule ensures lifecycle [`create_before_destroy`](https://www.terraform.io/docs/language/meta-arguments/lifecycle.html#create_before_destroy)
argument is set to `true` for acm certificates as per
[`aws_acm_certificate`](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/acm_certificate)
official docs:

> It's recommended to specify `create_before_destroy = true` in a
> lifecycle block to replace a certificate which is currently in use

## Example

```hcl
resource "aws_acm_certificate" "test" {
domain_name = var.domain_name
validation_method = "DNS"
}
```

```console
$ tflint
1 issue(s) found:

Warning: resource `aws_acm_certificate` needs to contain `create_before_destroy = true` in `lifecycle` block (aws_acm_certificate_lifecycle)

on test.tf line 1:
1: resource "aws_acm_certificate" "test" {
```

## Why

If you don't add this, terraform will timeout when trying to replace a certificate
that's in use at the moment of apply.

## How To Fix

```hcl
resource "aws_acm_certificate" "test" {
domain_name = local.assets_domain_name
validation_method = "DNS"
lifecycle {
create_before_destroy = true
}
}
```
54 changes: 54 additions & 0 deletions rules/aws_acm_certificate_lifecycle.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package rules

import (
"github.com/terraform-linters/tflint-plugin-sdk/terraform/configs"
"github.com/terraform-linters/tflint-plugin-sdk/tflint"
"github.com/terraform-linters/tflint-ruleset-aws/project"
)

// AwsAcmCertificateLifecycleRule checks whether `create_before_destroy = true` is set in a lifecycle block of
// `aws_acm_certificate` resource
type AwsAcmCertificateLifecycleRule struct {
resourceType string
attributeName string
}

// NewAwsAcmCertificateLifecycleRule returns new rule with default attributes
func NewAwsAcmCertificateLifecycleRule() *AwsAcmCertificateLifecycleRule {
return &AwsAcmCertificateLifecycleRule{
resourceType: "aws_acm_certificate",
attributeName: "lifecycle",
}
}

// Name returns the rule name
func (r *AwsAcmCertificateLifecycleRule) Name() string {
return "aws_acm_certificate_lifecycle"
}

// Enabled returns whether the rule is enabled by default
func (r *AwsAcmCertificateLifecycleRule) Enabled() bool {
return true
}

// Severity returns the rule severity
func (r *AwsAcmCertificateLifecycleRule) Severity() string {
return tflint.WARNING
}

// Link returns the rule reference link
func (r *AwsAcmCertificateLifecycleRule) Link() string {
return project.ReferenceLink(r.Name())
}

// Check checks whether the aws_acm_certificate resource contains create_before_destroy = true in lifecycle block
func (r *AwsAcmCertificateLifecycleRule) Check(runner tflint.Runner) error {
return runner.WalkResources("aws_acm_certificate", func(resource *configs.Resource) error {
if !resource.Managed.CreateBeforeDestroy {
if err := runner.EmitIssue(r, "resource `aws_acm_certificate` needs to contain `create_before_destroy = true` in `lifecycle` block", resource.DeclRange); err != nil {
return err
}
}
return nil
})
}
80 changes: 80 additions & 0 deletions rules/aws_acm_certificate_lifecycle_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package rules

import (
"testing"

hcl "github.com/hashicorp/hcl/v2"
"github.com/terraform-linters/tflint-plugin-sdk/helper"
)

func Test_AwsAcmCertificateLifecycle(t *testing.T) {
cases := []struct {
Name string
Content string
Expected helper.Issues
}{
{
Name: "no lifecycle block",
Content: `
resource "aws_acm_certificate" "test" {
domain_name = var.domain_name
validation_method = "DNS"
}`,
Expected: helper.Issues{
{
Rule: NewAwsAcmCertificateLifecycleRule(),
Message: "resource `aws_acm_certificate` needs to contain `create_before_destroy = true` in `lifecycle` block",
Range: hcl.Range{
Filename: "resource.tf",
Start: hcl.Pos{Line: 2, Column: 1},
End: hcl.Pos{Line: 2, Column: 38},
},
},
},
},
{
Name: "no create_before_destroy attribute in lifecycle block",
Content: `
resource "aws_acm_certificate" "test" {
domain_name = var.domain_name
validation_method = "DNS"
lifecycle {}
}`,
Expected: helper.Issues{
{
Rule: NewAwsAcmCertificateLifecycleRule(),
Message: "resource `aws_acm_certificate` needs to contain `create_before_destroy = true` in `lifecycle` block",
Range: hcl.Range{
Filename: "resource.tf",
Start: hcl.Pos{Line: 2, Column: 1},
End: hcl.Pos{Line: 2, Column: 38},
},
},
},
},
{
Name: "create_before_destroy = false",
Content: `
resource "aws_acm_certificate" "test" {
domain_name = var.domain_name
validation_method = "DNS"
lifecycle {
create_before_destroy = true
}
}`,
Expected: helper.Issues{},
},
}

rule := NewAwsAcmCertificateLifecycleRule()

for _, tc := range cases {
runner := helper.TestRunner(t, map[string]string{"resource.tf": tc.Content})

if err := rule.Check(runner); err != nil {
t.Fatalf("Unexpected error occurred: %s", err)
}

helper.AssertIssues(t, tc.Expected, runner.Issues)
}
}
1 change: 1 addition & 0 deletions rules/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@ var Rules = append([]tflint.Rule{
NewAwsIAMPolicyTooLongPolicyRule(),
NewAwsLambdaFunctionDeprecatedRuntimeRule(),
NewAwsIAMGroupPolicyTooLongRule(),
NewAwsAcmCertificateLifecycleRule(),
}, models.Rules...)

0 comments on commit 131fddd

Please sign in to comment.