Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

terraform: access list support #916

Merged
merged 1 commit into from
Sep 27, 2023
Merged

Conversation

hugoShaka
Copy link
Contributor

@hugoShaka hugoShaka commented Sep 15, 2023

This PR adds access list support to the Terraform provider. Since we stopped using gogoproto, generating new resources became increasingly complex.

The access list terraform schema was generated from the protobuf types and not the ones in api/types, which causes all kinds of small oddities from a user PoV, and made this PR way more complex.

User-facing limitations:

  • the metadata field is nested under the header one because of the protobuf structure
  • the traits is not a map but a list of structs, each struct with a key and a value attribute

Non user-facing limitations the PR had to work around:

  • the protobuf time is different from go's time, I initially wanted to create ProtoTimeType/ProtoTimeValue but we cannot embbed a protobuf timestamp directly (because it contains a lock, the terraform
    value must be a reference, which is not supported by the protoc generator). The workaround is to use custom types (Timestamp and Duration). Requires: Add custom_types config parameter protoc-gen-terraform#36
  • the provider has to do the conversion between the proto type and the api/type. This required writing a new provider template for new non-gogo resources (gen/plural_data_source_new.go.tpl and gen/plural_resources_new.go.tpl)
  • The time type difference caused conversion issues fixed by convert go's zero time into protobuf zero's time teleport#32135

@hugoShaka hugoShaka requested a review from mdwn September 15, 2023 20:14
@hugoShaka hugoShaka force-pushed the hugo/terraform-access-lists branch 2 times, most recently from 3f56627 to 5d918fb Compare September 19, 2023 21:56
@hugoShaka
Copy link
Contributor Author

hugoShaka commented Sep 19, 2023

Note: linting and tests are all over the place because I changed the teleport and teleport/api references to origin/master. Once gravitational/teleport#32135 is merged and backported, I'll update the replace statements and things should be less red.

@hugoShaka hugoShaka changed the title Hugo/terraform access lists terraform: access list support Sep 19, 2023
@hugoShaka hugoShaka marked this pull request as ready for review September 19, 2023 22:25
hugoShaka added a commit to gravitational/protoc-gen-terraform that referenced this pull request Sep 20, 2023
Adds a custom_type configuration knob used by the PR gravitational/teleport-plugins#916

This allows to override types without specifying "gogoproto.customtype" (we don't use gogoproto anymore for the new types).
Copy link
Contributor

@mdwn mdwn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this LGTM. The new templates look reasonable, but it's a little tough for me to review the very Terraform specific generator code.

terraform/gen/plural_data_source_new.go.tpl Outdated Show resolved Hide resolved
Copy link
Contributor

@marcoandredinis marcoandredinis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

Did you test this on a local cluster?

terraform/tfschema/types_terraform.go Outdated Show resolved Hide resolved
terraform/test/main_test.go Show resolved Hide resolved
terraform/gen/main.go Outdated Show resolved Hide resolved
terraform/reference.mdx Outdated Show resolved Hide resolved
@hugoShaka hugoShaka force-pushed the hugo/terraform-access-lists branch from e483741 to f514c5c Compare September 21, 2023 20:43
@hugoShaka
Copy link
Contributor Author

hugoShaka commented Sep 22, 2023

I hacked a bit, a breaking change in the plugin lib was pushed to branch/v14 and the plugins repo were not updated to deal with the break. I created a ref (hugo/aaaaaa) just before the breaking commit, until we fix the plugins we must not update the teleport and teleport/api versions.

The CI is also using a dev release of teleport as the tests require a change which was not in the latest release 14.0.0. This CI reference must be updated the next time we do an official v14 release.

@marcoandredinis
Copy link
Contributor

a breaking change in the plugin lib was pushed to branch/v14 and the plugins repo were not updated to deal with the break

What change was it?

@hugoShaka
Copy link
Contributor Author

What change was it?

gravitational/teleport@df6dc24#diff-ce5b43206a5487e9c590d632fb4893abd1e76577cdfdd5e01f1a25595216dc7cR47

@marcoandredinis marcoandredinis force-pushed the hugo/terraform-access-lists branch from 650c1fd to df14d38 Compare September 26, 2023 15:25
@marcoandredinis
Copy link
Contributor

@mdwn Can you please take another look?
I ended up removing the new generators and changing the old ones, so things change quite a bit but should still be the equivalent.

@marcoandredinis marcoandredinis force-pushed the hugo/terraform-access-lists branch from 73f17f8 to 12f96de Compare September 27, 2023 07:15
This PR adds access list support to the Terraform provider. Since we
stopped using gogoproto, generating new resources became increasingly
complex.

The access list terraform schema was generated from the protobuf types
and not the ones in `api/types`, which causes all kind of small oddities
from a user PoV, and made this PR way more complex.

User-facing limitations:
- the `metadata` field is nested under the `header` one because of the
  protobuf structure
- the traits is not a map but a list of structs, each struct with a key
  and a value attribute

Non user-facing limitations the PR had to work around:
- the protobuf time is different from go's time, I initially wanted to
  create `ProtoTimeType`/`ProtoTimeValue` but we cannot embbed a
  protobuf timestamp directly (because it contains a lock, the terraform
  value must be a reference, which is not supported by the protoc
  generator). The workaround is to use custom types (`Timestamp` and
  `Duration`).
- the provider has to do the conversion between the proto type and the
  api/type. This required writing a new provider template for new
  non-gogo resources (`gen/plural_data_source_new.go.tpl` and
  `gen/plural_resources_new.go.tpl`)
- The time type difference caused conversion issues fixed by
  gravitational/teleport#32135
@marcoandredinis marcoandredinis force-pushed the hugo/terraform-access-lists branch from 12f96de to 0a5793d Compare September 27, 2023 07:23
@marcoandredinis marcoandredinis merged commit bbc3714 into master Sep 27, 2023
13 checks passed
@marcoandredinis marcoandredinis deleted the hugo/terraform-access-lists branch September 27, 2023 07:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants