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

Terraform Registry API Response Differs from Actual Implementation #27

Closed
skorfmann opened this issue Apr 26, 2020 · 3 comments
Closed
Labels
bug Something isn't working waiting-for-3rd-party-fix

Comments

@skorfmann
Copy link
Contributor

For this VPC module, there's a discrepancy for the actual implementation of the inputs on Github and the API response of the Terraform registry.

Terraform Registry response:

{
  "id": "terraform-aws-modules/vpc/aws/2.33.0",
  "owner": "antonbabenko",
  "namespace": "terraform-aws-modules",
  "name": "vpc",
  "version": "2.33.0",
  "provider": "aws",
  "...": "...",
  "inputs": [
    {
      "...": "..."
    },
    {
      "name": "redshift_subnet_assign_ipv6_address_on_creation",
      "type": "bool",
      "description": "Assign IPv6 address on redshift subnet, must be disabled to change IPv6 CIDRs. This is the IPv6 equivalent of map_public_ip_on_launch",
      "default": "",
      "required": true
    },
    {
      "...": "..."
    }
  ]
}

Actual implementation

variable "redshift_subnet_assign_ipv6_address_on_creation" {
  description = "Assign IPv6 address on redshift subnet, must be disabled to change IPv6 CIDRs. This is the IPv6 equivalent of map_public_ip_on_launch"
  type        = bool
  default     = null
}

According to this blog post, the input should be optional when the default is set to null. Likely a bug in the Terraform Registry?

This affects all inputs where the default is null (12 in this module). The effect is, that those variables are rendered as required attributes, and the Typescript compiler expects them to be set.

I wonder if it's safe to assume that a default of "" (empty string) makes a variable always optional and we could ignore the required flag in those cases. Thoughts?

skorfmann added a commit that referenced this issue Apr 30, 2020
This provides a fix for #12 and includes some refactoring around the resource parsing / emitting.

The primary goal of the refactoring was, to split the parsing from the emitting to make it easier to understand. I'm still not quite happy with the result (in particular around the models, and that some logic is spread across multiple places). I think it needs another iteration, but for alpha it should do.

Right now it's in the "it's working" state, and "jsii" will compile the "AWS" provider without an error. I haven't done a full sanity check of the generated resources, but for the most part it should be usable.

In regards to the complex computed types, I'd see it as a first stab at the problem. It's not flexible and serves a very specific use case only. The goal:

- Make complex computed types accessible
- Provide type information for the computed properties of those types
- Keep it within the constraints of jsii, namely no generics and no proxies (see #12)

A few issues were created as a follow up - see #24 #25 #26 #27 #28 #29 #39
anubhavmishra pushed a commit that referenced this issue May 5, 2020
This provides a fix for #12 and includes some refactoring around the resource parsing / emitting.

The primary goal of the refactoring was, to split the parsing from the emitting to make it easier to understand. I'm still not quite happy with the result (in particular around the models, and that some logic is spread across multiple places). I think it needs another iteration, but for alpha it should do.

Right now it's in the "it's working" state, and "jsii" will compile the "AWS" provider without an error. I haven't done a full sanity check of the generated resources, but for the most part it should be usable.

In regards to the complex computed types, I'd see it as a first stab at the problem. It's not flexible and serves a very specific use case only. The goal:

- Make complex computed types accessible
- Provide type information for the computed properties of those types
- Keep it within the constraints of jsii, namely no generics and no proxies (see #12)

A few issues were created as a follow up - see #24 #25 #26 #27 #28 #29 #39
@joatmon08 joatmon08 added the bug Something isn't working label May 11, 2020
@joatmon08
Copy link
Contributor

Related issue reported with Terraform. hashicorp/terraform/issues/21417

@skorfmann
Copy link
Contributor Author

Since we don't rely on the module registry anymore, this can be closed.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2022

I'm going to lock this issue 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 similar to this, 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 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working waiting-for-3rd-party-fix
Projects
None yet
Development

No branches or pull requests

3 participants
@skorfmann @joatmon08 and others