Add support for embedded fields#29
Merged
Merged
Conversation
- update field_build_context to return message path instead of full path when the field is embedded field - unit tests for copyTo copyFrom functions
flyinghermit
requested review from
klizhentas,
r0mant and
russjones
as code owners
Contributor
|
@nklaassen Tagging you as reviewer since you are also reviewing gravitational/teleport-plugins#801 |
marcoandredinis
commented
May 24, 2023
marcoandredinis
commented
May 24, 2023
marcoandredinis
commented
May 24, 2023
marcoandredinis
left a comment
marcoandredinis
left a comment
Contributor
Author
There was a problem hiding this comment.
Looks good (I might be a little biased 🙈)
rosstimothy
reviewed
May 24, 2023
| if len(fs) == 0 { | ||
| return trace.BadParameter("expected at least one field") | ||
| } | ||
| f.MapValueField = fs[0] |
Contributor
There was a problem hiding this comment.
Good question, pinging @marcoandredinis for reply.
Contributor
There was a problem hiding this comment.
It looks like for maps this can only ever be of size 0. When this is an embedded struct, it can return multiple fields. If it's not an embedded struct, then it just returns a single element array.
mdwn
approved these changes
May 24, 2023
mdwn
left a comment
mdwn
left a comment
Contributor
There was a problem hiding this comment.
This is way simpler than my implementation!
This was referenced May 24, 2023
strideynet
approved these changes
May 26, 2023
Contributor
|
Friendly ping for code owner review please @r0mant |
r0mant
approved these changes
May 26, 2023
Co-authored-by: Roman Tkachenko <roman@goteleport.com>
Co-authored-by: Roman Tkachenko <roman@goteleport.com>
Contributor
|
Thanks everyone for review. |
flyinghermit
added a commit
to gravitational/teleport-plugins
that referenced
this pull request
May 29, 2023
Terraform support for device trust. ### Device type definition Currently, there's two Device types implemented for Device Trust: 1. [Device](https://github.com/gravitational/teleport/blob/master/api/proto/teleport/devicetrust/v1/device.proto) proto, which is the main definition used by all Device Trust related services. 2. [DeviceV1](https://github.com/gravitational/teleport/blob/master/api/proto/teleport/legacy/types/device.proto) used by this provider. ### Provider implementation Unlike other existing resources, [DeviceV1](https://github.com/gravitational/teleport/blob/master/api/proto/teleport/legacy/types/device.proto) does not fully utilize the metadata fields such as `label`, `description`, etc. Only `metadata.name` is supported, specifically to be compatible with schema generation for terraform provider. Additionally, the [DeviceV1](https://github.com/gravitational/teleport/blob/master/api/proto/teleport/legacy/types/device.proto) embeds the `ResourceHeader` field. [protoc-gen-terraform](gravitational/protoc-gen-terraform#29) is updated to support such embedded fields. This PR adds a new fields to the template payload: - `UUIDMetadataName` field: used to generate `Metadata.Name` as uuid value. The [`plural_resource.go.tpl`](https://github.com/gravitational/teleport-plugins/blob/a76be035e7498090958e05f356426d2a4084281d/terraform/gen/plural_resource.go.tpl) template is updated to accommodate `UUIDMetadataName` field. ### Supported device trust CRUD fields As for supported fields, only `asset_tag`, `os_type`, and `enroll_status` are supported by this provider. 1. Create: Create device: `asset_tag`, `os_type`, `enroll_status` (optional as this value can be updated outside of Terraform and it won't be a good experience for our users to keep the `.tf` files in sync with the state ) 4. Update: Update `enroll_status`: `enrolled -> not_enrolled` 5. Delete: Delete device.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When there's an embed proto field (extension
(gogoproto.embed) = true) we need to embed the message's fields into the current message.So this:
Should become the following tfschema:
However, we were creating the following schema:
This PR changes this to support embedding those fields.
I did a manual test for the DeviceTrust resource which we are about to add support for in Teleport's Terraform provider.
TODO