buildkite / terraform-provider-buildkite

Terraform provider for Buildkite
https://registry.terraform.io/providers/buildkite/buildkite/latest
MIT License
56 stars 36 forks source link

Provider errors upgrading from 0.27.0 to 1.0.0 #410

Closed ianchesal closed 11 months ago

ianchesal commented 11 months ago

Describe the bug I'm using a modular approach to defining my pipelines and I'm getting a high number of errors for instances of the module when I move my provider version from 0.27.0 to 1.0.0 and follow the upgrade steps.

On plan, for each instance of the module, I see:

│ Error: Invalid attribute value
│
│   with buildkite_pipeline.pipeline["persona-identity"],
│   on main.tf line 1, in resource "buildkite_pipeline" "pipeline":
│    1: resource "buildkite_pipeline" "pipeline" {
│
│ Invalid use when repository is: git@bitbucket.org:persona-id/persona-identity.git

And:

│ Error: Invalid Attribute Value
│
│   with buildkite_pipeline.pipeline["persona-identity"],
│   on main.tf line 1, in resource "buildkite_pipeline" "pipeline":
│    1: resource "buildkite_pipeline" "pipeline" {
│
│ Attribute provider_settings.pull_request_branch_filter_enabled true, got: false

In the plan output. I get both errors for every instance of the module.

To Reproduce

Here is my modular definition block:

resource "buildkite_pipeline" "pipeline" {
  # See: https://registry.terraform.io/providers/buildkite/buildkite/latest/docs/resources/pipeline
  for_each = local.pipelines

  # Required settings
  default_branch = each.value.default_branch
  description    = each.value.description
  name           = each.key
  repository     = each.value.repository

  # Immutable settings
  steps = lookup(each.value, "custom_steps", lookup(each.value, "deflake_mode", false) ? file("./deflake-steps.yaml") : file("./default-steps.yaml"))

  # Optional settings
  allow_rebuilds             = lookup(each.value, "allow_rebuilds", true)
  branch_configuration       = lookup(each.value, "branch_configuration", "")
  default_timeout_in_minutes = lookup(each.value, "default_timeout_in_minutes", 0)
  maximum_timeout_in_minutes = lookup(each.value, "maximum_timeout_in_minutes", 0)
  tags                       = lookup(each.value, "tags", [])

  skip_intermediate_builds                 = lookup(each.value, "only_build_latest_branch", lookup(each.value, "skip_intermediate_builds", true))
  skip_intermediate_builds_branch_filter   = lookup(each.value, "only_build_latest_branch_filter", lookup(each.value, "skip_intermediate_builds_branch_filter", ""))
  cancel_intermediate_builds               = lookup(each.value, "only_build_latest_branch", lookup(each.value, "cancel_intermediate_builds", true))
  cancel_intermediate_builds_branch_filter = lookup(each.value, "only_build_latest_branch_filter", lookup(each.value, "cancel_intermediate_builds_branch_filter", ""))

  provider_settings = {
    build_branches                           = lookup(each.value, "build_branches", true)
    build_pull_requests                      = lookup(each.value, "build_pull_requests", true)
    build_tags                               = lookup(each.value, "build_tags", false)
    filter_condition                         = lookup(each.value, "filter_condition", "")
    filter_enabled                           = lookup(each.value, "filter_enabled", false)
    publish_commit_status                    = lookup(each.value, "publish_commit_status", true)
    publish_commit_status_per_step           = lookup(each.value, "publish_commit_status_per_step", false)
    pull_request_branch_filter_configuration = lookup(each.value, "pull_request_branch_filter_configuration", "")
    pull_request_branch_filter_enabled = lookup(each.value,
    "pull_request_branch_filter_enabled", false)
    skip_pull_request_builds_for_existing_commits = lookup(each.value, "skip_pull_request_builds_for_existing_commits", true)
  }
}

And here are my variables.tf for one instance of the module:

locals {
  pipelines = {
    "persona-identity" = {
      repository     = "git@bitbucket.org:persona-id/persona-identity.git"
      description    = "Persona Identity Service"
      default_branch = "master"
    },
}

Expected behavior Following the upgrade guide, I was expecting a fairly straightforward upgrade to 1.0.0 from 0.27.0. I didn't think any of our structure here was really pushing limits.

Additional context Since we're not actually overriding the default value of pull_request_branch_filter_enabled in this instance of the module, I deleted it to see if I could get it to plan/apply. Terraform just complains about one of the other settings in the provider_settings block when I do that. Feels like it is not so much about the setting, and more that we're doing a lookup with a default value for the setting in our case.

jradtilbrook commented 11 months ago

Thanks @ianchesal I got you to move this but now this does seem more related to #409 than I originally thought. That's okay though, we'll keep you updated here as well.

I think this comment also applies then: https://github.com/buildkite/terraform-provider-buildkite/issues/409#issuecomment-1742862386

jradtilbrook commented 11 months ago

Hi @ianchesal, we've released v1.0.2 with a fix for this. Can you give it a shot and let us know if it still doesn't work or raise a new issue if some other bug comes up

ianchesal commented 11 months ago

I'm still seeing invalid attribute value errors for attributes in the provider_setting block when I run terraform refresh during the upgrade.

For the example above it's:

╷
│ Error: Invalid Attribute Value
│
│   with buildkite_pipeline.pipeline["persona-identity"],
│   on main.tf line 1, in resource "buildkite_pipeline" "pipeline":
│    1: resource "buildkite_pipeline" "pipeline" {
│
│ Attribute provider_settings.filter_enabled true, got: false
╵
╷
│ Error: Invalid Attribute Value
│
│   with buildkite_pipeline.pipeline["persona-identity"],
│   on main.tf line 1, in resource "buildkite_pipeline" "pipeline":
│    1: resource "buildkite_pipeline" "pipeline" {
│
│ Attribute provider_settings.pull_request_branch_filter_enabled true, got: false
╵

Based on the docs, filter_enabled seems like it should be fine to use here. We're BitBucket, not Github, so trigger_mode = "code" shouldn't apply.

For pull_request_branch_filter_enabled -- docs say it's only valid if build_pull_requests = true but does that mean I can't include this if build_pull_requests = false? Would seem odd to not be able to have it there even if it's redundant.

These problems should be reproducible with the example I provided in the first post on this thread.

jradtilbrook commented 11 months ago

Oops I see we have missed a piece of validation in the logic. I'll create a release a fix for that

For pull_request_branch_filter_enabled -- docs say it's only valid if build_pull_requests = true but does that mean I can't include this if build_pull_requests = false? Would seem odd to not be able to have it there even if it's redundant.

That's correct, unless build_pull_requests is set and set to true, you cannot specify the filter enabled and by extension the filter condition

jradtilbrook commented 11 months ago

@ianchesal We've discussed as a team and opted to remove the validation on the provider_settings attribute. It was not working correctly and quite complex to get it correct. It's been released as 1.0.3 so you should be able to try that out now

ianchesal commented 11 months ago

@jradtilbrook thank you! I'll give 1.0.3 a go. I like the thought of doing pre-API call validation, but branching logic is ugly-hard in Terraform so I appreciate you helping me avoiding having to attempt that in my module.

ianchesal commented 11 months ago

@jradtilbrook 1.0.3 seems to be working for me! Thanks!