feat(data-source): expose root-level field_map and file_map on onepassword_item#350
feat(data-source): expose root-level field_map and file_map on onepassword_item#350benjamin-pilgrim wants to merge 7 commits into
Conversation
…sword_item Add two new computed attributes to the onepassword_item data source: - field_map: a map of all root-level fields (not in any section), keyed by field label. Includes both standard fields (username, password, etc.) and any custom fields added at the root level. - file_map: a map of all root-level files (not in any section), keyed by file name. Exposes id, content, and content_base64. Previously, custom root-level fields were silently ignored. The only workaround was to move fields into a named section and use section_map, or to repurpose standard fields. Closes 1Password#337
|
Broken CI seems to be unrelated to the PR |
ci: add setup-terraform to build and e2e jobs
|
Hey @benjamin-pilgrim 👋 I just merged a fix for the broken CI. Are you able to merge in changes to main and I can finish reviewing this PR this week! |
…sword_item Add two new computed attributes to the onepassword_item data source: - field_map: a map of all root-level fields (not in any section), keyed by field label. Includes both standard fields (username, password, etc.) and any custom fields added at the root level. - file_map: a map of all root-level files (not in any section), keyed by file name. Exposes id, content, and content_base64. Previously, custom root-level fields were silently ignored. The only workaround was to move fields into a named section and use section_map, or to repurpose standard fields. Closes 1Password#337
…lgrim/terraform-provider-onepassword into feat/root-field-map-file-map
|
/ok-to-test sha=926ab75 |
|
❌ E2E tests failed. |
|
✅ E2E tests passed. |
JillRegan
left a comment
There was a problem hiding this comment.
Hey @benjamin-pilgrim 👋 Thank you for your contribution! I've left a few comments on the PR. We may need to think about how we want to approach a couple of them but open to community feedback/discussion.
Also wanted to call out one thing... in the 1Password UI, using Add more without putting the field in a named section still usually stores it under the built in add more section in the API, not as a truly “root” field. The same tends to happen with op and other integrations unless they create the field and explicitly set the SectionId to an empty string to overwrite it being put into this built in add more section.
So these fields would show up under section_map not top-level field_map as technically they do have a section, even though in the UI it may not appear that way.
| fieldMap[f.Label] = OnePasswordItemFieldMapModel{ | ||
| ID: types.StringValue(f.ID), | ||
| Type: types.StringValue(string(f.Type)), | ||
| Value: types.StringValue(f.Value), |
There was a problem hiding this comment.
The inner case username/ password only run for fields that land in default of the outer switch f.Purpose. They apply when the field was not classified as Purpose == USERNAME, PASSWORD, or NOTES in that outer switch. They are a fallback for root fields whose id is username or password when purpose is not applied to the field (some items types like Database have a username field which does not have a purpose, whereas Login item type username field has PURPOSE type USERNAME).
The purpose based fields (USERNAME / PASSWORD / NOTES) are handled in the dedicated cases above so they would not appear in the field_map so it wouldn't end up being all root level fields in that case.
There was a problem hiding this comment.
That makes sense. USERNAME, PASSWORD, and NOTES purpose fields were bypassing the root field_map population.
I’ve fixed this by populating field_map for every field whose SectionID is empty before handling the top-level purpose/id-specific attributes. I also added a regression test covering root-level username, password, and notesPlain fields with purpose set.
| data.Type = types.StringValue(f.Value) | ||
| case "public_key": | ||
| data.PublicKey = types.StringValue(f.Value) | ||
| case "private_key": |
There was a problem hiding this comment.
There’s extra handling for private_key to handle private_key_openssh from the stored value, but root field_map below would only use the stored value. We may want to add handling for this in the field_map for consistencies.
There was a problem hiding this comment.
Both field_map maps use the same field model: id, type, and value, where value is the stored 1Password field value. Adding private_key_openssh only to the root map would make the two field_map structures diverge, and section-level private key fields still would not have the derived value.
| PrivateKeyOpenSSH types.String `tfsdk:"private_key_openssh"` | ||
| SectionList []OnePasswordItemSectionListModel `tfsdk:"section"` | ||
| SectionMap map[string]OnePasswordItemSectionMapModel `tfsdk:"section_map"` | ||
| FieldMap map[string]OnePasswordItemFieldMapModel `tfsdk:"field_map"` |
There was a problem hiding this comment.
We now have the same attribute name in two places, root field_map and section_map.*.field_map. I don’t feel strongly either way, it’s consistent with fields as a map but it could read as the same thing twice. Happy to defer to whatever feels clearest for users if others find it confusing.
There was a problem hiding this comment.
I’m inclined to keep the root attribute as field_map. The Terraform path disambiguates the scope: root field_map is item-level, while section_map.*.field_map is section-level.
I updated the root field_map docs to make the scope explicit, including the API SectionID behavior. I’m happy to make changes if maintainers or the wider community feel differently, though.
|
Thanks, that distinction makes sense. I’ve updated the docs to define root-level as API root-level: fields whose For context, I have run into this in my own usage with existing items, where custom fields were returned with an empty The docs now also call out that fields which appear unsectioned in the 1Password UI may still be stored under the built-in |
Summary
field_mapattribute to theonepassword_itemdata source: a map of all root-level fields (not in any section), keyed by field label. Includes both standard fields and custom fields.file_mapattribute to theonepassword_itemdata source: a map of all root-level files (not in any section), keyed by file name, exposingid,content, andcontent_base64.Previously, custom root-level fields were silently ignored — the only workarounds were moving fields into a named section (to use
section_map) or repurposing standard fields likeurlordatabase.Resolves
Closes #337
Test plan
field_map(TestAccItemDataSourceRootFieldMap) andfile_map(TestAccItemDataSourceRootFileMap) ininternal/provider/onepassword_item_data_source_test.gofield_map(TestAccItemDataSourceRootFieldMap) verified against real 1Password with desktop app integrationfile_map(TestAccItemDataSourceRootFileMap) referencing the pre-existing Test Document item, consistent with the existingTestAccItemDataSourcepatterngo generateNotes
field_mapintentionally includes standard fields (username, password, etc.) as well as custom ones — they are already accessible via their dedicated top-level attributes, but including them infield_maptoo gives users a single place to access any root-level field by label