cockroachdb / terraform-provider-cockroach

Terraform provider for CockroachDB Cloud
Apache License 2.0
57 stars 12 forks source link

Do not allow dedicated regions to be marked primary #118

Closed pjtatlow closed 1 year ago

pjtatlow commented 1 year ago

If a dedicated cluster has a primary region, the plan that's created doesn't match the state of the cluster. This change creates an error if a dedicated cluster's region is marked as primary, and adds some documentation to specify that only serverless regions should me marked primary.

Fixes #113

jaylim-crl commented 1 year ago

I believe this PR broke serverless cluster creation:

$ terraform plan
╷
│ Error: Invalid Attribute Combination
│
│   with cockroach_cluster.serverless,
│   on x.tf line 12, in resource "cockroach_cluster" "serverless":
│   12: resource "cockroach_cluster" "serverless" {
│
│ These attributes cannot be configured together: [regions[*].primary,dedicated]
╵
resource "cockroach_cluster" "serverless" {
  name           = "foo-123"
  cloud_provider = "AWS"
  serverless = {
      spend_limit = 0
  }
  regions = [
    {
      name = "us-east-1"
      primary = false
    },
    {
      name  = "eu-central-1"
      primary = true
    },
  ]
}

I suspect the conflict validator doesn't account for null values, though I could be wrong. Setting dedicated to null when specifying a primary region should be allowed.

jaylim-crl commented 1 year ago

It appears that the following should work:

resourcevalidator.Conflicting(
    path.MatchRoot("regions").AtAnyListIndex().AtName("primary"),
    path.MatchRoot("dedicated"),
),

Hm, not sure why that doesn't work as intended.

jaylim-crl commented 1 year ago

I see. This worked:

resource "cockroach_cluster" "serverless" {
  name           = "foo-123"
  cloud_provider = "AWS"
  serverless = {
      spend_limit = 0
  }
  regions = [
    {
      name = "us-east-1"
      primary = false
    },
    {
      name  = "eu-central-1"
    },
  ]
}

But not:

resource "cockroach_cluster" "serverless" {
  name           = "foo-123"
  cloud_provider = "AWS"
  serverless = {
      spend_limit = 0
  }
  regions = [
    {
      name = "us-east-1"
      primary = false
    },
    {
      name  = "eu-central-1"
      primary = true
    },
  ]
}

I think if we tried to set primary on more than one regions (even if they are false values), it would error out with the conflict error.