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

feat(lib): Terraform Attributes (token system replacement) #706

Closed
wants to merge 32 commits into from

Conversation

jsteinich
Copy link
Collaborator

@jsteinich jsteinich commented May 15, 2021

Closes #525
Fixes #518
Fixes #374
Fixes #434
Fixes #424
Partially implements #25
Fixes #172
Fixes #663
Fixes #679

Copied from issue comment

Current problems

  • Only supports strings, numbers, and lists of strings
  • Doesn't support accessing a single element from a list of strings
  • Confusing output when printing
  • Inconvenient to reference real values

Other desired expansions

  • Support Terraform functions/expressions
  • Provide access to complex computed types

Solution preface

  • Not planning to implement all expansions here, but would like to set up a system which will support them.
  • I think there are some pieces that would only be solvable with Making cdktf dynamic #435 so this will knowingly not solve all issues.

Proposed solution

  • Remove the token system as it currently exists
    • It only works for part of what is needed
    • Not very flexible for desired additions for what it does work with
  • Create a TerraformAttribute class
    • Similar idea to Tokens, but is more explicit about it
    • Allows for keeping track of references
    • Can retrieve raw value if set
    • Can add built-in terraform functions as methods
  • Create subclasses for core types
    • TerraformStringAttribute, TerraformNumberAttribute, TerraformBooleanAttribute, TerraformDynamicAttribute, TerraformListAttribute, TerraformSetAttribute, TerraformMapAttribute, TerraformObjectAttribute
    • This will give the user a more limited set of options for types attributes are
    • Can scope built-in terraform functions that are only designed to work on say collections to appropriate type
    • Possibly do code gen for specific object / collection types to workaround not having generics in jsii
  • Collection types can have additional methods to support accessing elements / iterating in a terraform friendly way
    • The alternative idea of building custom collections into jsii would work well in Java and C# (need to switch to list from array), but I don't believe would work in Typescript (I don't believe it's possible to extend the Array type) (I'm not sure about python)
  • Generate type aliases to allow for using either "primitive" types or TerraformAttributes
    • Still want it to be easy to initialize values
    • Also want to be able to easily reference other resources / data sources
    • Internally initing/setting would always result in a TerraformAttribute (or subtype) being created if not already using one
    • The getter will be a bit deceptive since you'll never get a primitive value from it
    • feat(jsii): support aliasing type unions aws/jsii#1807 is a prerequisite in my mind; else C# will be unusable
    • Provide accessors to nested computed attributes #480 complicates things since the union types will need to match else there will be quite a bit of friction
  • Would be nice to have some language specific libraries to make easier to use
    • One example is having implicit operators in C# to make going between "union type" and "primitive type" easier (partially destructive operation)
  • Still a case where the current token system (or something similar) is necessary
    • When using resources in string interpolation
    • Seems common enough to need support
    • Token would retain reference information to attribute
    • Can make a nicer representation that looks basically like a terraform reference

Updates

  • Some example work at https://github.com/jsteinich/cdktf-ecs-fargate-tokens
  • Manually creating collection types for main element types
  • Definitely want to do code generation for custom types
  • Removing setters on resources so that getters can return an accurate type. Replacing with putX methods

Scope of work

Initial work

  • Create core attribute types
  • Fix/workaround jsii issues with Java generation (Java List compile error: incompatible types aws/jsii#2839)
  • Update core terraform types to work with attributes
  • Update code generation to use attributes
  • Add generation of custom attribute types
  • Handle naming conflicts
  • Implement string interpolation support
  • Add tests
  • Handle transition (any removal / deprecation / bridging of old token system / methods)
  • Update/add documentation

Immediate follow-up

  • Build better jsii support for type aliases

Longer term follow-up

  • Build jsii support for sets
  • Add support for complex collection types (list<list> etc) that are currently used in providers
  • Add in Terraform functions and expressions
  • Add foreach support for collection attributes

@skorfmann
Copy link
Contributor

Cool - thanks for pushing on the topic 🎉 Gonna dig into this a bit deeper over the next few days

Comment on lines +46 to +48
this._operations.forEach(operation => {
reference = operation(reference);
});
Copy link
Member

Choose a reason for hiding this comment

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

I really like this!

@jsteinich jsteinich changed the title Terraform Attributes (token system replacement) feat(lib): Terraform Attributes (token system replacement) Jun 6, 2021
@skorfmann
Copy link
Contributor

As discussed with @jsteinich, @DanielMSchmidt we won't pursue this proposal. We're going to address the issues we have within the existing token system.

@jsteinich thank you again for the effort you put into this PR and the input during the entire process. It sparked important discussions and helped all of us to build a common understanding of the problem domain.

@skorfmann skorfmann closed this Aug 6, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2022

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.