1Password / terraform-provider-onepassword

Use the 1Password Terraform Provider to reference, create, or update items in your 1Password Vaults.
https://developer.1password.com/docs/terraform/
MIT License
322 stars 44 forks source link

1Password Item update error when inserting fields #190

Open DavidS-ovm opened 2 months ago

DavidS-ovm commented 2 months ago

Your environment

Terraform v1.5.7 on linux_amd64

What happened?

When inserting fields at the beginning of a section:

Error: 1Password Item update error
  with onepassword_item.environment_variables,
  on 1password.tf line 75, in resource "onepassword_item" "environment_variables":
  75: resource "onepassword_item" "environment_variables" {
Could not update item 'xkjcynmzjjkwjfenfmkd6mffuy' from vault
'5rmq6vebgi3c2iejytskh52d6y', got error: op error: unable to process line 1:
Validation: (validateVaultItem failed to Validate), Couldn't validate the
item: "[ItemValidator] has found 3 errors, 0 warnings: \nErrors:{1.
details.sections[9].fields[13] has non-unique name}, {2.
details.sections[9].fields[14] has non-unique name}, {3.
details.sections[9].fields[15] has non-unique name}"
Releasing state lock. This may take a few moments...

What did you expect to happen?

New fields get correctly inserted.

Notes

the same configuration change was applied successfully after moving the new fields to the bottom of their section.

dustin-ruetz commented 2 months ago

Hi @DavidS-ovm :wave: Thanks for your interest in our Terraform provider and for filing this issue.

I’m hoping to clarify a few things:

  1. You mention that when inserting new fields at the beginning of a section that the 1Password Item update error occurs. Just to rephrase the steps to ensure that I'm understanding it correctly: are you referring to a flow where a 1Password item is created with a section and a number of fields, saved, and then modified afterwards where new fields are added to the above-mentioned section and placed at its beginning?
  2. I see from the error message you included that several details.sections[9].fields[$INDEX] has non-unique name errors are being logged. Just to confirm, during your testing did you double-check that each field’s name was a unique string? (The main reason I ask to confirm is because personally I've done this myself many times before; if I have an item with a lot of fields on it and I’m making a lot of changes to them it’s very common for me to accidentally end up with duplicate field keys at the end.)
  3. Could you kindly provide us with the full set of commands/actions that you ran in order to help us reproduce this error?

I can also suggest the following workaround (which I will be the first to admit is not the most ideal solution, but I’ll include it here anyway just in case it’s helpful for your situation/anyone else who may come upon this issue): if the order of fields within the section is important, you could make note of the section’s names and values and then re-create the section on the item with the fields sorted in your preferred order.

DavidS-ovm commented 2 months ago

Hi @DavidS-ovm 👋 Thanks for your interest in our Terraform provider and for filing this issue.

I’m hoping to clarify a few things:

1. You mention that when inserting new fields at the beginning of a section that the `1Password Item update error` occurs. Just to rephrase the steps to ensure that I'm understanding it correctly: are you referring to a flow where a 1Password item is created with a section and a number of fields, saved, and then modified afterwards where new fields are added to the above-mentioned section and placed at its beginning?

Yes.

2. I see from the error message you included that several `details.sections[9].fields[$INDEX] has non-unique name` errors are being logged. Just to confirm, during your testing did you double-check that each field’s name was a unique string? (The main reason I ask to confirm is because personally I've done this myself many times before; if I have an item with a lot of fields on it and I’m making a lot of changes to them it’s very common for me to accidentally end up with duplicate field keys at the end.)

It applied successfully when I moved the same additional fields to the end of the section.

3. Could you kindly provide us with the full set of commands/actions that you ran in order to help us reproduce this error?

my terraform manifest looks approximately like this:

provider "onepassword" {
}

data "onepassword_vault" "env_vault" {
  name = var.terraform_env_name
}

resource "onepassword_item" "environment_variables" {
  vault = data.onepassword_vault.env_vault.uuid

  title      = var.terraform_env_name
  category   = "secure_note"
  note_value = "MANAGED BY TERRAFORM - DO NOT EDIT MANUALLY\n\nAll the values for the ${var.terraform_env_name} environment"

  section {
    label = "generated"
    field {
      label = "description"
      type  = "STRING"
      value = "These values are generated by the terraform deployment and any changes will be overwritten"
    }

    field {
      label = "some_secret"
      type  = "STRING"
      value = module.some_module.some_secret
    }

our CI pipelines for this basically boil down to terraform plan/terraform apply.

I can also suggest the following workaround (which I will be the first to admit is not the most ideal solution, but I’ll include it here anyway just in case it’s helpful for your situation/anyone else who may come upon this issue): if the order of fields within the section is important, you could make note of the section’s names and values and then re-create the section on the item with the fields sorted in your preferred order.

dannysauer commented 1 month ago

I'd like to note that I encountered this issue after updating today as well. In my case, I have a loop which records tokens in a secure note. They're Docker account PATs assigned to individual customers who fetch a private image, so tokens with the unique email as a key. I have a local variable which contains a list of users. I for_each over the list, storing the generated token in a secure note section.

When I insert a new value, instead of just creating a new entry in the item's section, what happens is all of the lexicographically subsequent entries have their label and value changed. I suspect this is because the elements in a section are handled as an ordered list instead of a dictionary/hash/associative array. The final element is created as new, and all of the other elements are changed. Which is kinda weird, since the provider enforcing unique labels implies that the elements should be able to just be treated as a dict with the label as a key instead of the field ID.

Relevant code looks like this:

  section {
    label = "user_tokens"
    dynamic "field" {
      for_each = local.dev_emails
      content {
        label = field.key
        type  = "STRING"
        value = dockerhub_token.token[field.key].token
      }
    }
  }

It'd be nice if the provider could consider a label existing on another ID as being that element instead of insisting on generating a chain of changes. Perhaps generate a set of fields keyed on label and calculate a difference instead of just treating it as a linear array.

I actually don't care about the order, but the plan sequence seems to run into a problem where creating the "new" field conflicts with an existing field which is going to be relabeled. My current workaround is to delete the entire section manually and then apply. I suppose I could use the time provider and sort by the creation time, but I'd very much rather not . :D