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

Throw error when 1Password client item creation fails #178

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

edif2008
Copy link
Member

@edif2008 edif2008 commented May 29, 2024

Summary

We add to diagnostics the error returned when the 1Password client fails to create an item. However, we weren't stopping the process, which caused the provider to try to add the "failed" created item in the state which led to a crash of the provider.

Relates: #174

How to test

  1. Checkout this branch:
    git pull && git checkout fix/throw-err-on-item-creation-fail
  2. Build the provider:
    make build
  3. Ensure that your environment is configured to use the local provider. Specifically, your .terraform.rc on macOS/Linux or terraform.rc on Windows contains this:
    provider_installation {
      dev_overrides {
          # other overrides
          "1Password/onepassword" = "path/to/terraform-provider-onepassword/dist"
      }
      # For all other providers, install them directly from their origin provider
      # registries as normal. If you omit this, Terraform will _only_ use
      # the dev_overrides block, and so no other providers will be available.
      direct {}
    }
    
  4. Create a main.tf file with this content:
    terraform {
      required_providers {
        onepassword = {
          source = "1Password/onepassword"
          version = "~> 2.0"
        }
      }
    }
    
    data "onepassword_vault" "vault" {
      name = "demo"
    }
    
    resource "onepassword_item" "demo_login" {
      vault = data.onepassword_vault.vault.id
    
      title    = "Demo Terraform Login"
      category = "login"
      username = "test@example.com"
    }
  5. Export the necessary environment variables for the authentication method that you want (Connect, service account or regular user)
  6. Run terraform apply
    • The provider should not crash
    • The provider should throw an error like this one:
      ╷
      │ Error: 1Password Item create error
      │ 
      │   with onepassword_item.demo_login,
      │   on main.tf line 14, in resource "onepassword_item" "demo_login":
      │   14: resource "onepassword_item" "demo_login" {
      │ 
      │ Error creating 1Password item, got error op error: unable to process line 1: "vaults/ba7ntbqcum4tsrcc5ciem5wfsu" isn't a vault in this account. Specify the vault with its ID or name.
      ╵
      

We add to diagnostics the error returned when the 1Password client fails to create an item. However, we weren't stopping the process, which caused the provider to try to add the "failed" created item in the state which led to a crash of the provider.
@back-2-95
Copy link

I tested this PR and provider did not crash anymore. But I'm having issues with inconsistent results (they vary on different results) AND defining vault (tried name, id, uuid).

@edif2008
Copy link
Member Author

edif2008 commented Jun 7, 2024

Hey @back-2-95! 👋🏻

It sounds like this PR addressed the issue of the provider crashing, which is its purpose.
What you're describing seems to be something else that might be better suited for a separate issue. Would you be kind to open a new issue with the following details?

  • The use case (can provide snippets that can be used to reproduce the issue you're facing)
  • What you were expecting to get
  • What is the actual outcome.

That would be much appreciated. 😄

@back-2-95
Copy link

Yes, I think those other issues are already mentioned in some other issues by others.

@volodymyrZotov
Copy link
Contributor

✅ Tested this and confirmed that the provider doesn't crash anymore.

@edif2008 edif2008 merged commit bcbfdb0 into main Jun 17, 2024
7 checks passed
@edif2008 edif2008 deleted the fix/throw-err-on-item-creation-fail branch June 17, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants