Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache location is not valid when cache_bucket_suffix_enabled = true #91

Closed
inhumantsar opened this issue Aug 1, 2021 · 7 comments · Fixed by #92
Closed

Cache location is not valid when cache_bucket_suffix_enabled = true #91

inhumantsar opened this issue Aug 1, 2021 · 7 comments · Fixed by #92
Labels
bug 🐛 An issue with the system

Comments

@inhumantsar
Copy link

Describe the Bug

When cache_bucket_suffix_enabled is True, this module raises an error:

❯ cd terraform-aws-codebuild/examples/complete
❯ terraform plan -var-file fixtures.us-east-2.tfvars -var cache_bucket_suffix_enabled=true
╷
│ Error: cache location is required when cache type is "S3"
│ 
│   with module.codebuild.aws_codebuild_project.default[0],
│   on ../../main.tf line 292, in resource "aws_codebuild_project" "default":
│  292: resource "aws_codebuild_project" "default" {
│ 
╵

❯ terraform plan -var-file fixtures.us-east-2.tfvars                                      

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create
 <= read (data resources)

Terraform will perform the following actions:

  # module.codebuild.data.aws_iam_policy_document.permissions_cache_bucket[0] will be read during apply
  # (config refers to values not yet known)
 <= data "aws_iam_policy_document" "permissions_cache_bucket"  {
      + id   = (known after apply)
      + json = (known after apply)

      + statement {
...

This is having downstream effects. eg: cloudposse/terraform-aws-ecs-codepipeline#71

Expected Behavior

No error

Steps to Reproduce

cd terraform-aws-codebuild/examples/complete
terraform plan -var-file fixtures.us-east-2.tfvars -var cache_bucket_suffix_enabled=true
@inhumantsar inhumantsar added the bug 🐛 An issue with the system label Aug 1, 2021
@inhumantsar
Copy link
Author

inhumantsar commented Aug 1, 2021

Did some troubleshooting and was able to narrow it down to random_string.

troubleshooting

this works:

  cache_options = {
    "S3" = {
      type     = "S3"
      location = this.module.id 
    },
...

but this does not...

  cache_bucket_name = "${module.this.id}${var.cache_bucket_suffix_enabled ? "-${join("", random_string.bucket_prefix.*.result)}" : ""}"
  cache_options = {
    "S3" = {
      type     = "S3"
      location = local.cache_bucket_name 
    },
...

this also works...

  moo = "moo"
  cache_options = {
    "S3" = {
      type     = "S3"
      location = local.moo
    },
...

this does not...

  cache_bucket_name   = "${module.this.id}-${join("", random_string.bucket_prefix.*.result)}"
  cache_options = {
    "S3" = {
      type     = "S3"
      location = local.cache_bucket_name
    },
...

this does...

  cache_bucket_name   = module.this.id
  cache_options = {
    "S3" = {
      type     = "S3"
      location = local.cache_bucket_name
    },
...

this does...

  cache_bucket_name   = "${module.this.id}-moo"
  cache_options = {
    "S3" = {
      type     = "S3"
      location = local.cache_bucket_name
    },
...

this does not...

  cache_bucket_name   = join("", random_string.bucket_prefix.*.result)
  cache_options = {
    "S3" = {
      type     = "S3"
      location = local.cache_bucket_name
    },
...

for whatever reason, calling that random_string resource causes this failure. just to be sure i used random_id with no count and got the same result:

  cache_bucket_name   = "${module.this.id}-${random_id.bucket_prefix.hex}"
  cache_options = {
    "S3" = {
      type     = "S3"
      location = local.cache_bucket_name
    },
...

updating the normalization logic to exclude the suffix, instead adding that to the cache_bucket resource fails as well:

  cache_bucket_name_normalised = substr(
    join("-", split("_", lower(module.this.id))),
    0,
    min(length(module.this.id), var.cache_bucket_suffix_enabled ? 50 : 63),
  )

...

resource "aws_s3_bucket" "cache_bucket" {
  count         = module.this.enabled && local.s3_cache_enabled ? 1 : 0
  bucket        = var.cache_bucket_suffix_enabled ? "${local.cache_bucket_name_normalised}-${random_id.bucket_prefix.hex}" : local.cache_bucket_name_normalised
...

Using the prefix arg for random_id also fails.

I even tried changing cache_bucket_suffix_enabled to accept a string and set cache_bucket_name to "${module.this.id}${var.cache_bucket_suffix_enabled}". Then I passed the result from random_string in from the complete example's main.tf and still got the same error:

resource "random_string" "bucket_prefix" {
  length  = 12
  number  = false
  upper   = false
  special = false
  lower   = true
}

module "codebuild" {
  source                      = "../../"
  cache_bucket_suffix_enabled = random_string.bucket_prefix.result
  environment_variables       = var.environment_variables
  cache_expiration_days       = var.cache_expiration_days
  cache_type                  = var.cache_type

  context = module.this.context
}
}

and same as before, changing it to a plain string worked fine...

module "codebuild" {
  source                      = "../../"
  cache_bucket_suffix_enabled = "moo"
  environment_variables       = var.environment_variables
  cache_expiration_days       = var.cache_expiration_days
  cache_type                  = var.cache_type

  context = module.this.context
}

The original usage is entirely in-line with the Terraform docs example, so I don't know why this would be happening.

# The following example shows how to generate a unique name for an AWS EC2
# instance that changes each time a new AMI id is selected.

resource "random_id" "server" {
  keepers = {
    # Generate a new id each time we switch to a new AMI id
    ami_id = "${var.ami_id}"
  }

  byte_length = 8
}

resource "aws_instance" "server" {
  tags = {
    Name = "web-server ${random_id.server.hex}"
  }

  # Read the AMI id "through" the random_id resource to ensure that
  # both will change together.
  ami = random_id.server.keepers.ami_id

  # ... (other aws_instance arguments) ...
}

In the debug logs I do see a warning about non-computed attributes but bug reports mentioning that say it's expected and ok. However the bit at the end caught my eye.

2021-08-01T13:56:04.483-0500 [WARN]  Provider "registry.terraform.io/hashicorp/random" produced an invalid plan for module.codebuild.random_string.bucket_prefix[0], but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .min_lower: planned value cty.NumberIntVal(0) for a non-computed attribute
      - .min_numeric: planned value cty.NumberIntVal(0) for a non-computed attribute
      - .min_special: planned value cty.NumberIntVal(0) for a non-computed attribute
      - .min_upper: planned value cty.NumberIntVal(0) for a non-computed attribute
2021-08-01T13:56:04.484-0500 [DEBUG] provider.stdio: received EOF, stopping recv loop: err="rpc error: code = Unavailable desc = transport is closing"
2021-08-01T13:56:04.485-0500 [DEBUG] provider: plugin process exited: path=.terraform/providers/registry.terraform.io/hashicorp/random/3.1.0/darwin_amd64/terraform-provider-random_v3.1.0_x5 pid=76169
2021-08-01T13:56:04.485-0500 [DEBUG] provider: plugin exited

There is a bug report open for the same error when using random. User found that setting the length to 15 fixed the issue. Updating random_string to use a length of 15 or 18 didn't work, neither did setting values for min_.

TL;DR: WTF?

@nitrocode
Copy link
Member

nitrocode commented Aug 2, 2021

In PR 92, the output of terratest shows the correct suffix attached to the bucket.

https://github.com/cloudposse/actions/runs/3220447314?check_suite_focus=true

Could you try doing a terraform plan using the examples/complete in the repo ?

git clone [email protected]:cloudposse/terraform-aws-codebuild.git
cd examples/complete
terraform plan -target -var-file=fixtures.us-east-2.tfvars

This is the plan I see

terraform plan
  # module.codebuild.data.aws_iam_policy_document.permissions_cache_bucket[0] will be read during apply
  # (config refers to values not yet known)
 <= data "aws_iam_policy_document" "permissions_cache_bucket"  {
      + id   = (known after apply)
      + json = (known after apply)

      + statement {
          + actions   = [
              + "s3:*",
            ]
          + effect    = "Allow"
          + resources = [
              + (known after apply),
              + (known after apply),
            ]
        }
    }

  # module.codebuild.aws_codebuild_project.default[0] will be created
  + resource "aws_codebuild_project" "default" {
      + arn            = (known after apply)
      + badge_enabled  = false
      + badge_url      = (known after apply)
      + build_timeout  = 60
      + description    = (known after apply)
      + encryption_key = (known after apply)
      + id             = (known after apply)
      + name           = "eg-test-codebuild-test"
      + queued_timeout = 480
      + service_role   = (known after apply)
      + tags           = {
          + "Name"      = "eg-test-codebuild-test"
          + "Namespace" = "eg"
          + "Stage"     = "test"
        }
      + tags_all       = {
          + "Name"      = "eg-test-codebuild-test"
          + "Namespace" = "eg"
          + "Stage"     = "test"
        }

      + artifacts {
          + encryption_disabled    = false
          + override_artifact_name = false
          + type                   = "CODEPIPELINE"
        }

      + cache {
          + location = (known after apply)
          + modes    = (known after apply)
          + type     = (known after apply)
        }

      + environment {
          + compute_type                = "BUILD_GENERAL1_SMALL"
          + image                       = "aws/codebuild/standard:2.0"
          + image_pull_credentials_type = "CODEBUILD"
          + privileged_mode             = false
          + type                        = "LINUX_CONTAINER"

          + environment_variable {
              + name  = "AWS_REGION"
              + type  = "PLAINTEXT"
              + value = "us-east-2"
            }
          + environment_variable {
              + name  = "AWS_ACCOUNT_ID"
              + type  = "PLAINTEXT"
              + value = "781590699830"
            }
          + environment_variable {
              + name  = "IMAGE_REPO_NAME"
              + type  = "PLAINTEXT"
              + value = "UNSET"
            }
          + environment_variable {
              + name  = "IMAGE_TAG"
              + type  = "PLAINTEXT"
              + value = "latest"
            }
          + environment_variable {
              + name  = "STAGE"
              + type  = "PLAINTEXT"
              + value = "test"
            }
          + environment_variable {
              + name  = "APP_URL"
              + type  = "PLAINTEXT"
              + value = "https://app.example.com"
            }
          + environment_variable {
              + name  = "COMPANY_NAME"
              + type  = "PLAINTEXT"
              + value = "Cloud Posse"
            }
          + environment_variable {
              + name  = "TIME_ZONE"
              + type  = "PLAINTEXT"
              + value = "America/Los_Angeles"
            }
        }

      + source {
          + report_build_status = false
          + type                = "CODEPIPELINE"
        }
    }

  # module.codebuild.aws_iam_policy.default[0] will be created
  + resource "aws_iam_policy" "default" {
      + arn       = (known after apply)
      + id        = (known after apply)
      + name      = "eg-test-codebuild-test"
      + path      = "/service-role/"
      + policy    = jsonencode(
            {
              + Statement = [
                  + {
                      + Action   = [
                          + "ssm:GetParameters",
                          + "secretsmanager:GetSecretValue",
                          + "logs:PutLogEvents",
                          + "logs:CreateLogStream",
                          + "logs:CreateLogGroup",
                          + "iam:PassRole",
                          + "ecs:RunTask",
                          + "ecr:UploadLayerPart",
                          + "ecr:PutImage",
                          + "ecr:InitiateLayerUpload",
                          + "ecr:GetAuthorizationToken",
                          + "ecr:CompleteLayerUpload",
                          + "ecr:BatchCheckLayerAvailability",
                          + "codecommit:GitPull",
                        ]
                      + Effect   = "Allow"
                      + Resource = "*"
                      + Sid      = ""
                    },
                ]
              + Version   = "2012-10-17"
            }
        )
      + policy_id = (known after apply)
      + tags_all  = (known after apply)
    }

  # module.codebuild.aws_iam_policy.default_cache_bucket[0] will be created
  + resource "aws_iam_policy" "default_cache_bucket" {
      + arn       = (known after apply)
      + id        = (known after apply)
      + name      = "eg-test-codebuild-test-cache-bucket"
      + path      = "/service-role/"
      + policy    = (known after apply)
      + policy_id = (known after apply)
      + tags_all  = (known after apply)
    }

  # module.codebuild.aws_iam_role.default[0] will be created
  + resource "aws_iam_role" "default" {
      + arn                   = (known after apply)
      + assume_role_policy    = jsonencode(
            {
              + Statement = [
                  + {
                      + Action    = "sts:AssumeRole"
                      + Effect    = "Allow"
                      + Principal = {
                          + Service = "codebuild.amazonaws.com"
                        }
                      + Sid       = ""
                    },
                ]
              + Version   = "2012-10-17"
            }
        )
      + create_date           = (known after apply)
      + force_detach_policies = true
      + id                    = (known after apply)
      + managed_policy_arns   = (known after apply)
      + max_session_duration  = 3600
      + name                  = "eg-test-codebuild-test"
      + path                  = "/"
      + tags                  = {
          + "Name"      = "eg-test-codebuild-test"
          + "Namespace" = "eg"
          + "Stage"     = "test"
        }
      + tags_all              = {
          + "Name"      = "eg-test-codebuild-test"
          + "Namespace" = "eg"
          + "Stage"     = "test"
        }
      + unique_id             = (known after apply)

      + inline_policy {
          + name   = (known after apply)
          + policy = (known after apply)
        }
    }

  # module.codebuild.aws_iam_role_policy_attachment.default[0] will be created
  + resource "aws_iam_role_policy_attachment" "default" {
      + id         = (known after apply)
      + policy_arn = (known after apply)
      + role       = (known after apply)
    }

  # module.codebuild.aws_iam_role_policy_attachment.default_cache_bucket[0] will be created
  + resource "aws_iam_role_policy_attachment" "default_cache_bucket" {
      + id         = (known after apply)
      + policy_arn = (known after apply)
      + role       = (known after apply)
    }

  # module.codebuild.aws_s3_bucket.cache_bucket[0] will be created
  + resource "aws_s3_bucket" "cache_bucket" {
      + acceleration_status         = (known after apply)
      + acl                         = "private"
      + arn                         = (known after apply)
      + bucket                      = (known after apply)
      + bucket_domain_name          = (known after apply)
      + bucket_regional_domain_name = (known after apply)
      + force_destroy               = true
      + hosted_zone_id              = (known after apply)
      + id                          = (known after apply)
      + region                      = (known after apply)
      + request_payer               = (known after apply)
      + tags                        = {
          + "Name"      = "eg-test-codebuild-test"
          + "Namespace" = "eg"
          + "Stage"     = "test"
        }
      + tags_all                    = {
          + "Name"      = "eg-test-codebuild-test"
          + "Namespace" = "eg"
          + "Stage"     = "test"
        }
      + website_domain              = (known after apply)
      + website_endpoint            = (known after apply)

      + lifecycle_rule {
          + enabled = true
          + id      = "codebuildcache"
          + prefix  = "/"
          + tags    = {
              + "Name"      = "eg-test-codebuild-test"
              + "Namespace" = "eg"
              + "Stage"     = "test"
            }

          + expiration {
              + days = 7
            }
        }

      + versioning {
          + enabled    = true
          + mfa_delete = false
        }
    }

  # module.codebuild.random_string.bucket_prefix[0] will be created
  + resource "random_string" "bucket_prefix" {
      + id          = (known after apply)
      + length      = 12
      + lower       = true
      + min_lower   = 0
      + min_numeric = 0
      + min_special = 0
      + min_upper   = 0
      + number      = false
      + result      = (known after apply)
      + special     = false
      + upper       = false
    }

Plan: 8 to add, 0 to change, 0 to destroy.

@inhumantsar
Copy link
Author

All of my testing was done using the complete example. I cannot get a plan to pass with the random_string in use.

The issue I referenced in the bug report is from another user who hit the exact same issue.

@inhumantsar
Copy link
Author

inhumantsar commented Aug 2, 2021

terraform plan using examples/complete
$ terraform-aws-codebuild/examples/complete 
$ git pull
remote: Enumerating objects: 29, done.
remote: Counting objects: 100% (29/29), done.
remote: Compressing objects: 100% (26/26), done.
remote: Total 29 (delta 12), reused 7 (delta 3), pack-reused 0
Unpacking objects: 100% (29/29), 18.40 KiB | 538.00 KiB/s, done.
From https://github.com/cloudposse/terraform-aws-codebuild
 * [new branch]      cache_bucket_suffix_enabled -> origin/cache_bucket_suffix_enabled
Already up to date.

$  git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
$ terraform plan -var-file fixtures.us-east-2.tfvars -var cache_bucket_suffix_enabled=true
╷
│ Error: cache location is required when cache type is "S3"
│ 
│   with module.codebuild.aws_codebuild_project.default[0],
│   on ../../main.tf line 292, in resource "aws_codebuild_project" "default":
│  292: resource "aws_codebuild_project" "default" {
│ 
╵

@inhumantsar
Copy link
Author

inhumantsar commented Aug 2, 2021

I put together a minimal test project using random_string and it passes planning no problem.

minimal test project using custom random_string
resource "random_string" "bucket_suffix" {
  length  = 12
  number  = false
  upper   = false
  special = false
  lower   = true
}

resource "aws_s3_bucket" "test" {
  bucket = "test-bucket-${random_string.bucket_suffix.result}"
}
terraform {
  required_version = ">= 0.13.0"

  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = ">= 2.0"
    }
    template = {
      source  = "hashicorp/template"
      version = ">= 2.0"
    }
    local = {
      source  = "hashicorp/local"
      version = ">= 1.2"
    }
    random = {
      source  = "hashicorp/random"
      version = ">= 2.1"
    }
    null = {
      source  = "hashicorp/null"
      version = ">= 2.0"
    }
  }
}
$ terraform plan
provider.aws.region
  The region where AWS operations will take place. Examples
  are us-east-1, us-west-2, etc.

  Enter a value: us-east-1


Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # aws_s3_bucket.test will be created
  + resource "aws_s3_bucket" "test" {
      + acceleration_status         = (known after apply)
      + acl                         = "private"
      + arn                         = (known after apply)
      + bucket                      = (known after apply)
      + bucket_domain_name          = (known after apply)
      + bucket_regional_domain_name = (known after apply)
      + force_destroy               = false
      + hosted_zone_id              = (known after apply)
      + id                          = (known after apply)
      + region                      = (known after apply)
      + request_payer               = (known after apply)
      + tags_all                    = (known after apply)
      + website_domain              = (known after apply)
      + website_endpoint            = (known after apply)

      + versioning {
          + enabled    = (known after apply)
          + mfa_delete = (known after apply)
        }
    }

  # random_string.bucket_suffix will be created
  + resource "random_string" "bucket_suffix" {
      + id          = (known after apply)
      + length      = 12
      + lower       = true
      + min_lower   = 0
      + min_numeric = 0
      + min_special = 0
      + min_upper   = 0
      + number      = false
      + result      = (known after apply)
      + special     = false
      + upper       = false
    }

Plan: 2 to add, 0 to change, 0 to destroy.

──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Note: You didn't use the -out option to save this plan, so Terraform can't guarantee to take exactly these actions if you run "terraform apply" now.

@nitrocode
Copy link
Member

If it's working in our tests but not for you, perhaps it has something to do with the version of terraform you're using.

I tested locally using terraform 1.0 and the tests use terraform 0.13.x.

@inhumantsar
Copy link
Author

it's not just me though: cloudposse/terraform-aws-ecs-codepipeline#71

❯ terraform --version
Terraform v1.0.1
on darwin_amd64
+ provider registry.terraform.io/hashicorp/aws v3.52.0
+ provider registry.terraform.io/hashicorp/random v3.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An issue with the system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants