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

Add Service Account support #99

Merged
merged 12 commits into from
Nov 28, 2023
Merged

Add Service Account support #99

merged 12 commits into from
Nov 28, 2023

Conversation

tim-oster
Copy link
Contributor

This PR adds support for using an 1Password Service Account token as opposed to using a Connect Server setup (#79).

The implementation is best effort but most likely not ideal. It is a wrapper around the op CLI to be able to make use of the service account token. As of today, there seems to be no other option to interface with 1password other than the cli or a connect server.

The biggest issue in this implementation is, that updating an item will first delete it and then createa new one. The main reason for this is, that op does not support updating items using json. Ideally, we would use FroceNew for this in the provider but since it needs to be compatible with the Connect Server mode, this is not possible.

@franklouwers
Copy link

Does this mean that when updating an item, it will get a new UUID and new urls?

@tim-oster
Copy link
Contributor Author

@franklouwers yes, unfortunately.

It might be tweakable to use the actual op item edit command. The only downside I see is that it might not be fully compatible with everything that the json schema offers and that it would essentially leak all credentials of the command as arguments (more info).

@gabrielrinaldi
Copy link

@tim-oster does that mean the agent running this needs the op cli installed? We are using Terraform Cloud, which I think would not support that unfortunately. But excited to see progress on support for SAs.

@volodymyrZotov
Copy link
Contributor

@tim-oster Great contribution! Your work will help us a lot to release this feature even faster! 🚀
I'll review your pr shortly.

onepassword/provider.go Outdated Show resolved Hide resolved
onepassword/cli/exec.go Outdated Show resolved Hide resolved
onepassword/cli/exec.go Outdated Show resolved Hide resolved
onepassword/cli/op.go Outdated Show resolved Hide resolved
onepassword/connectctx/wrapper.go Outdated Show resolved Hide resolved
onepassword/data_source_onepassword_vault_test.go Outdated Show resolved Hide resolved
onepassword/resource_onepassword_item.go Outdated Show resolved Hide resolved
onepassword/provider.go Outdated Show resolved Hide resolved
onepassword/mock_client_test.go Show resolved Hide resolved
onepassword/cli/op.go Outdated Show resolved Hide resolved
@tim-oster
Copy link
Contributor Author

@volodymyrZotov thanks for the review!

@gabrielrinaldi yes, unfortunately using the CLI seems to be the only option at the moment. I am not too familiar with tf cloud, do you have access to a local file system? In that case, it might be worth investigating if we can bundle the cli binary together with the provider binary. Though this would drastically increase the provider's binary size.

@volodymyrZotov
Copy link
Contributor

@gabrielrinaldi Only Install Standalone Binaries article might be helpful to understand how you can add additional binaries.

And this one Install 1Password CLI might help to install. See Linux -> apt, or whatever is suitable for you.

Hope those will help you to figure out how to do that 😃 Let me know how it's going for you, please.

@gabrielrinaldi
Copy link

@volodymyrZotov amazing, thanks for pointing it out! Can't wait for this to be merged.

@volodymyrZotov
Copy link
Contributor

Hi folks! And especially @tim-oster 👋 Just want to put an update about the item edit.

Implementing that with the current cli version brings some difficulties, so we decided to add a piped input feature to the cli item edit command, which would allow us to pass the Item struct to perform the updates. So with the newer cli version you will be able to implement EditItem function in the same way as CreateItem.

I'm on that feature right now. I'll keep you posted about the progress. The initial aim is to have a new cli release with this feature later next week, but will see how it goes. 🚀

@tim-oster
Copy link
Contributor Author

@volodymyrZotov amazing, thanks for having a look at this so quickly! I'll update the PR as soon the CLI is released. Looking forward to getting this merged 💪

@volodymyrZotov
Copy link
Contributor

Hi @tim-oster. Just an update on that. We're still working on the item edit piped input feature. I'll let you know as soon as we have a new cli release with it and happy to help bring your PR to the finish line 🙂

@maxpowis-bp
Copy link

I just started a complex implementation an realize I might just be stuck at this stage due to this bug. Any ETA for the release ?

@speier
Copy link

speier commented Oct 11, 2023

In the meantime here is a hack using op cli locally..

module:

variable "item_ref" {}

locals {
  temp_dir = "${path.root}/op_tmp"
}

resource "local_file" "output" {
  filename = "${local.temp_dir}/${timestamp()}.txt"
  content  = ""
}

resource "null_resource" "op_read" {
  triggers = {
    once = timestamp()
  }
  provisioner "local-exec" {
    command = "op read ${var.item_ref} > ${local_file.output.filename}"
  }
}

data "local_file" "output" {
  depends_on = [null_resource.op_read]
  filename   = local_file.output.filename
}

resource "null_resource" "deleteLocalFile" {
  depends_on = [null_resource.op_read, data.local_file.output]
  triggers = {
    once = timestamp()
  }
  provisioner "local-exec" {
    command = "rm -rf ${local.temp_dir}"
  }
}

output "item_value" {
  value = data.local_file.output.content
}

usage example:

module "op" {
  source   = "../modules/op"
  item_ref = "op://myvault/fooprovider/api-token"
}

output "result" {
  value = module.op.item_value
}

@maxpowis-bp
Copy link

Thanks for the quick answer, I am using terraform-cloud though; probably won't do the trick :/

@hollow
Copy link

hollow commented Oct 12, 2023

Having to fall back to calling op from Terraform either through local-exec or from the provider code sounds like the wrong direction for this feature and will prevent Terraform Cloud users from using it.

@speier
Copy link

speier commented Oct 12, 2023

Sure, totally agree guys, local-exec is not the way. It's just a temp hack. Ideally a proper provider should work with 1Password API.

@volodymyrZotov
Copy link
Contributor

@tim-oster Hi. Some updates about op item edit piped input feature.
I know that your PR is blocked by that, but want to let you know that we're actively working on that feature on our side.
I'll drop a message here once it's released. Stay tuned. 😃

@volodymyrZotov
Copy link
Contributor

@tim-oster Hi! 👋 We plan to release a new CLI version that includes the feature to edit items using templates later this week.
Would you be able to continue the work on the PR? 😃

@dbrennand
Copy link

dbrennand commented Nov 19, 2023

@tim-oster @volodymyrZotov - Looks like the new CLI version was released 🙂 https://app-updates.agilebits.com/product_history/CLI2#v2230001

Looking forward to using this functionality! 🙂

dbrennand added a commit to dbrennand/home-ops that referenced this pull request Nov 19, 2023
@tim-oster
Copy link
Contributor Author

@volodymyrZotov @dbrennand the new CLI update is awesome! Works as expected!

@dbrennand
Copy link

dbrennand commented Nov 19, 2023

@volodymyrZotov @dbrennand the new CLI update is awesome! Works as expected!

Awesome 🙌🏻 - hopefully @volodymyrZotov can review soon and get this merged! 😃

Thanks for your hard work!

@volodymyrZotov
Copy link
Contributor

volodymyrZotov commented Nov 21, 2023

@tim-oster Hi. I pushed some commits to slightly change the error messages and address a couple of issues (see commits above).

While testing I found at least one issue that we need to address before moving forward. It can't create passwords when using CLI and throws an error:

[ERROR] 2023/11/21 17:32:33 unable to process line 1: Validation: (validateVaultItem failed to Validate), Couldn't validate the item: "[ItemValidator] has found 1 errors, 0 warnings: \nErrors:{1. Password item requires ps value}"

I'll continue looking into that tomorrow and continue testing to see if I can find any other issues.

@volodymyrZotov
Copy link
Contributor

Functional review: ✅

Performed intensive testing on that. Fixed several issues including #107.


Code review: ✅

The code looks good and straightforward.

Additional note: 💬

Users may encounter the following error op error: (409) Conflict: Internal server conflict when create/update/delete a bunch of items in the same vault as Terraform Provider handles each resource separately and therefore it makes a bunch of parallel requests using CLI for each of the resources.
There are some ways to avoid this:

  1. Use depends_on in your resource definition to make sure the Provider makes requests sequentially.
  2. After it fails with 409 error, they can do terraform apply again and again, till all the changes will be applied.
  3. Use Connect.
  4. Put items in the different vaults.

There is an issue to improve it #108
Resolves: #107

@volodymyrZotov
Copy link
Contributor

Added myself as a co-author to sign all the commits.

@volodymyrZotov volodymyrZotov merged commit ba2f48f into 1Password:main Nov 28, 2023
2 checks passed
@josh-burton
Copy link
Contributor

So is this able to be used on terraform cloud?

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.

9 participants