-
Notifications
You must be signed in to change notification settings - Fork 47
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
Migration to Terraform provider framework #167
Conversation
Run the following commands: - `go get -u ./...` - `go mod tidy` - `go mod vendor` With this, we also update Go version to 1.21.
- Move `github.com/hashicorp/terraform-plugin-framework` package from indirect dependency to direct dependency - `go get -u ./...` - `go mod tidy` - `go mod vendor`
Currently, the provider doesn't implement any functions. Therefore, these files are removed.
This client will be used by the provider to create, read, update and delete items using either Connect or 1Password CLI. This client is also capable of fetching vault data.
Changes made compared to `onepassword/connectctx/wrapper.go` - NewClient function is now responsible of creating the Connect client with user agent. - Renaming the struct for clarity and cleanliness.
Changes made compared to the code in `internal/onepassword/cli` directory: - New function is renamed to NewClient and implements the initializeCLI function - Enable password generation when updating an item (to be consistent with the Connect client).
This function will return either the Connect or the CLI client.
- Name the struct as OnePasswordProvider - Implement provider schema - Implement provider data model What changed from the previous implementation is that we marked the service account token and connect token as sensitive.
- Load default values from environment variables - Initialize the 1Password client (either Connect or CLI client based on the configuration)
This is used to add validators to schemas (e.g. dependencies between attributes). Some of these will be used by the vault data source.
With the vault data source fully implemented, we add it to the list of data sources the provider supports.
Changes made here as part of the migration: - Add a new constant `terraformItemIDDescription` which will be used for both item resource and data source for consistency. - The slices of categories, field purposes, and field types are now based on the constants available in the Connect Go SDK to eliminate hardcoding the string values (these apply to both Connect and CLI clients).
These new ones are used by the item resource schema.
With the new framework, Terraform wants to be more predictable, therefore any computed values will be recomputed whenever something in the resource changes. What this modifier does is that it will use the existing value in the state unless one of the following two scenarios happen: - The value is set to a specific new one in the plan. - The password recipe (which was used to generate it) is changed.
This converts an item into a data source model. It’s a rewrite of the same function that exists in the old provider code. A couple of improvements have been added as part of the migration: - No longer throw cause Terraform to trigger a change or an inconsistency error if the tags are the same, but in different order. This is because 1Password sorts the tags alphabetically, which can have a different order than the one tags passed in Terraform have. - With the new framework, Terraform will throw an inconsistency error if the planned value is null (i.e. not set) and the result is empty string). That's why we use the custom `setStringValue` function to set a value on each attribute based on the actual value in 1Password.
This converts the Terraform data into a 1Password item. It's a rewrite of the same function that exists in the old provider code. We also implemented the functions associated to it: - parseGeneratorRecipe - currently missing the case in which the attribute is not defined (or Nil) - addRecipe - identical to the one in the old provider code
This function extracts the vault and item UUIDs from the terraform ID. This matches the function with the same name from the old provider code.
Implement configure, create, read, update and delete functions for item resource
These will help with the following: - easily generate the items that we will use for testing. - have a mock server that will act as a Connect server during the tests. Co-authored-by: jillianwilson <[email protected]>
Co-authored-by: jillianwilson <[email protected]>
Co-authored-by: jillianwilson <[email protected]>
Co-authored-by: jillianwilson <[email protected]>
Co-authored-by: jillianwilson <[email protected]>
This came as part of the scaffolding. We remove it since the 1Password Terraform provider is under the MIT license.
- Add make command for running acceptance tests. - Update conditional compilation for tools package. Since Go 1.18 the new conditional compilation is `//go:build` instead of `// + build`. - Add terraform-registry-manifest.json file. This file is part of the new framework structure. It's used by the terraform registry to get additional data about the provider being published. For details check this link: https://developer.hashicorp.com/terraform/registry/providers/publishing#terraform-registry-manifest-file - Update goreleaser to use the registry manifest file
Update the docs based on the migrated provider and add small improvements to existing examples.
We also do the following adjustments: - Update Go version in the pipeline to 1.21. - Run the test step with TF_ACC=1 to enable acceptance tests (which are the ones we implemented in the migration). - Switch the `paultyng/[email protected]` action to `crazy-max/ghaction-import-gpg@v6` since the previous one is deprecated and it was recommended to use the upstream action that we now use.
Field of type OTP require a special field ID. Adding this validator achieves the following: - Ensures that the user will have a functional OTP field. - Terraform will not throw inconsistency errors when the CLI tries to correct the custom ID for OTP field.
✅ Tested it and could confirm that this fixes the issue with printing sensitive data in the sections. ✅ In addition, I tested other scenarios (create/read/update/delete) items, and it works well after migration.
Nevertheless, there is an error in the console, I see that the Secure Note item is created in 1Password. I approve this PR as it contains an important fix to not print sensitive data in the sections, but let's address the Security Note issue in the following PR. |
Filed an issue for the secure note #173 |
Migration to Terraform provider framework
TL;DR
Due to a bug in the Hashicorp
terraform-plugin-sdk
it is possible for sensitive data pulled from 1Password items to be shown in plaintext when a user runsterraform plan
. This only affects the sensitive data pulled from custom sections within 1Password items.Affected sensitive data pulled from 1Password, that isn’t marked sensitive in the Terraform plan, is only shown to the user running
terraform plan
or any third-party terraform providers used to run the plan. The sensitive data is otherwise not unmasked to any other sources.Previously the 1Password Terraform provider was built with the latest version of the terraform-plugin-sdk which was impacted by this bug. We have since upgraded our provider to use the terraform-plugin-framework which no longer has this bug.
Determining impact
In a Terraform plan, a block can be marked as sensitive so that fields cannot be shown in plaintext. Due to the bug in terraform-plugin-sdk, if nested blocks are marked as sensitive, the property is ignored and sensitive fields can be shown in plaintext.
Have the team responsible for the implementation of the Terraform provider determine if — in their Terraform configuration file — they have any provider that consumes data from an item’s section or if they output it.
This looks something like this:
Or like this:
Note that if your configuration is impacted, sensitive data is only viewable in plaintext in authenticated flows. This means that only the users and third-party integrations that are authorized to read those secrets from 1Password can print or read them as plaintext. Affected sensitive data is not otherwise unmasked to any other sources.
What this PR does
The PR migrates the 1Password Terraform provider from using the terraform-plugin-sdk to using terraform-plugin-framework. The new framework has a more robust handling of the schema and nested blocks, thereby no longer ignoring the sensitivity of a field’s value from a custom section. The outcome is that now, Terraform will take appropriate action to mask the field and mark it as sensitive. As an example, if a user tries to print an item’s section, Terraform will display an appropriate error.
This fix will be available in the upcoming Terraform provider release v2.0.0 shortly. Please update to that version if you are affected. This update does not contain any breaking changes. In the meantime, if applicable, remove all outputs of Terraform plans and configurations that may contain sensitive data.
Additional improvements as part of this PR
With the migration to the new terraform-plugin-framework, we've had the chance to improve the provider. Here's a list with the improvements:
Secure Note
items. Resolves Support for Secure Note #149.note_value
attribute representing a 1Password Item'snotes
field. Resolves Edit notes field #57.OTP
are better handled when user provides a custom ID for them. Terraform will throw an error if the custom ID doesn't have theTOTP_
prefix, which is required for this field type.server
field (previously known ashostname
) will populate the Terraformhostname
attribute. This ensures that the data from new Database items is mapped as expected. Resolves For database items hostname is not set because label is server not hostname as the item schema expects #76.How to test the PR
Setup
Checkout this branch:
Build the provider:
Get your full path to the locally-built provider:
Configure your
~/.terraformrc
file to use the local 1Password Terraform provider:Test that the bug is fixed
In a Terraform file, add the following snippets:
Get an item from a vault:
Print an item's section to the output:
Run a terraform plan:
It shuold throw the following error:
Test that the migration didn't cause any breaking changes
Vault data source
Create a Terraform file, add the following snippet:
Make a plan for the configuration:
The
Changes to outputs
section should look like this one:Item data source
Create a Terraform file, add the following snippet:
Make a plan for the configuration:
Item resource
In a Terraform file, add a complex
onepassword_item
resource. You can use the snippets below as an inspiration.Login item example snippet
Database item example snippet
Secure Note example snippet (new feature)
Complex item example snippet
Apply the changes to your configuration to create the 1Password item:
Check the created item in 1Password
Make a change to the
onepassword_item
resource. A couple of examples:note_value
to the item (new feature)Apply the changes again and then check the item in 1Password again.
Remove the code for the
onepassword_item
resource and apply the changes again.