Proper error reporting and eventing

[turkenh]

What problem are you facing?

I am trying to create a VPC with provider-tf-aws but it is not getting ready, when I describe it, I see the following output:

Name:         sample-vpc
Namespace:
Labels:       <none>
Annotations:  <none>
API Version:  vpc.aws.tf.crossplane.io/v1alpha1
Kind:         Vpc
Metadata:
  Creation Timestamp:  2021-09-01T09:41:19Z
  Finalizers:
    finalizer.managedresource.crossplane.io
  Generation:  1
  # omit ManagedFields
  Resource Version:  2817
  UID:               578e6a58-636b-4e2f-82bd-5bf1c691e9fc
Spec:
  Deletion Policy:  Delete
  For Provider:
    Cidr Block:  10.0.0.0/16
    Region:      us-east-2
    Tags:
      Name:  DemoVpc
    Tags All:
      Name:  DemoVpc
  Provider Config Ref:
    Name:  example
Status:
  At Provider:
    Arn:
    Default Network Acl Id:
    Default Route Table Id:
    Default Security Group Id:
    Dhcp Options Id:
    ipv6AssociationId:
    ipv6CidrBlock:
    Main Route Table Id:
    Owner Id:
  Conditions:
    Last Transition Time:  2021-09-01T09:41:25Z
    Reason:                Creating
    Status:                False
    Type:                  Ready
    Last Transition Time:  2021-09-01T09:42:54Z
    Reason:                ReconcileSuccess
    Status:                True
    Type:                  Synced
Events:
  Type     Reason                        Age                From                                  Message
  ----     ------                        ----               ----                                  -------
  Normal   CreatedExternalResource       6s (x10 over 95s)  managed/vpc.vpc.aws.tf.crossplane.io  Successfully requested creation of external resource
  Warning  CannotCreateExternalResource  6s (x3 over 82s)   managed/vpc.vpc.aws.tf.crossplane.io  failed to update: cannot update with tf cli: failed to apply Terraform configuration:
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # aws_vpc.sample-vpc will be created
  + resource "aws_vpc" "sample-vpc" {
      + arn                              = (known after apply)
      + assign_generated_ipv6_cidr_block = false
      + cidr_block                       = "10.0.0.0/16"
      + default_network_acl_id           = (known after apply)
      + default_route_table_id           = (known after apply)
      + default_security_group_id        = (known after apply)
      + dhcp_options_id                  = (known after apply)
      + enable_classiclink               = (known after apply)
      + enable_classiclink_dns_support   = (known after apply)
      + enable_dns_hostnames             = (known after apply)
      + enable_dns_support               = true
      + id                               = (known after apply)
      + instance_tenancy                 = "default"
      + ipv6_association_id              = (known after apply)
      + ipv6_cidr_block                  = (known after apply)
      + main_route_table_id              = (known after apply)
      + owner_id                         = (known after apply)
      + tags                             = {
          + "Name" = "DemoVpc"
        }
      + tags_all                         = {
          + "Name" = "DemoVpc"
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.
aws_vpc.sample-vpc: Creating...

However, when I check the controller logs, I can see the actual problem somewhere in a long log message:

2021-09-01T12:43:01.760+0300    INFO    provider-tf-aws Failed to run Terraform CLI     {"tfcli-version": "0.0.0", "args": ["apply", "-auto-approve", "-input=false"], "executable": "terraform", "cwd": "/var/folders/jb/zwwlz42935308fcydsp4h05h0000gn/T/ws-93c57199f76ed373701483fb75b20d8d22840904b04ce4f7a1093882131d1b99", "stderr": "\u001b[31m╷\u001b[0m\u001b[0m\n\u001b[31m│\u001b[0m \u001b[0m\u001b[1m\u001b[31mError: \u001b[0m\u001b[0m\u001b[1mError creating VPC: VpcLimitExceeded: The maximum number of VPCs has been reached.\n\u001b[31m│\u001b[0m \u001b[0m\tstatus code: 400, request id: e26f86cf-8bb1-40f3-a5f8-6c9e36351b52\u001b[0m\n\u001b[31m│\u001b[0m \u001b[0m\n\u001b[31m│\u001b[0m \u001b[0m\u001b[0m  with aws_vpc.sample-vpc,\n\u001b[31m│\u001b[0m \u001b[0m  on main.tf.json line 20, in resource.aws_vpc.sample-vpc.tags_all:\n\u001b[31m│\u001b[0m \u001b[0m  20:             \"sample-vpc\": {\"assign_generated_ipv6_cidr_block\":null,\"cidr_block\":\"10.0.0.0/16\",\"enable_classiclink\":null,\"enable_classiclink_dns_support\":null,\"enable_dns_hostnames\":null,\"enable_dns_support\":null,\"instance_tenancy\":null,\"tags\":{\"Name\":\"DemoVpc\"},\"tags_all\":{\"Name\":\"DemoVpc\"}\u001b[4m}\u001b[0m\u001b[0m\n\u001b[31m│\u001b[0m \u001b[0m\n\u001b[31m╵\u001b[0m\u001b[0m\n", "stdout": "\nTerraform used the selected providers to generate the following execution\nplan. Resource actions are indicated with the following symbols:\n  \u001b[32m+\u001b[0m create\n\u001b[0m\nTerraform will perform the following actions:\n\n\u001b[1m  # aws_vpc.sample-vpc\u001b[0m will be created\u001b[0m\u001b[0m\n\u001b[0m  \u001b[32m+\u001b[0m\u001b[0m resource \"aws_vpc\" \"sample-vpc\" {\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0marn\u001b[0m\u001b[0m                              = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0massign_generated_ipv6_cidr_block\u001b[0m\u001b[0m = false\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mcidr_block\u001b[0m\u001b[0m                       = \"10.0.0.0/16\"\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mdefault_network_acl_id\u001b[0m\u001b[0m           = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mdefault_route_table_id\u001b[0m\u001b[0m           = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mdefault_security_group_id\u001b[0m\u001b[0m        = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mdhcp_options_id\u001b[0m\u001b[0m                  = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0menable_classiclink\u001b[0m\u001b[0m               = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0menable_classiclink_dns_support\u001b[0m\u001b[0m   = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0menable_dns_hostnames\u001b[0m\u001b[0m             = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0menable_dns_support\u001b[0m\u001b[0m               = true\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mid\u001b[0m\u001b[0m                               = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0minstance_tenancy\u001b[0m\u001b[0m                 = \"default\"\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mipv6_association_id\u001b[0m\u001b[0m              = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mipv6_cidr_block\u001b[0m\u001b[0m                  = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mmain_route_table_id\u001b[0m\u001b[0m              = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mowner_id\u001b[0m\u001b[0m                         = (known after apply)\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mtags\u001b[0m\u001b[0m                             = {\n          \u001b[32m+\u001b[0m \u001b[0m\"Name\" = \"DemoVpc\"\n        }\n      \u001b[32m+\u001b[0m \u001b[0m\u001b[1m\u001b[0mtags_all\u001b[0m\u001b[0m                         = {\n          \u001b[32m+\u001b[0m \u001b[0m\"Name\" = \"DemoVpc\"\n        }\n    }\n\n\u001b[0m\u001b[1mPlan:\u001b[0m 1 to add, 0 to change, 0 to destroy.\n\u001b[0m\u001b[0m\u001b[1maws_vpc.sample-vpc: Creating...\u001b[0m\u001b[0m\n", "error": "exit status 1"}

Which is indeed:

Error creating VPC: VpcLimitExceeded: The maximum number of VPCs has been reached.

How could Terrajet help solve your problem?

We need to ensure to return actual errors in the form of events on the managed resource.
Also, similar to other crossplane providers, we should hide (per resource) logs at info level.

[muvaf]

I think this will be addressed as part of #74 @turkenh what do you think?

[turkenh]

I think this will be addressed as part of #74 @turkenh what do you think?

Yeah, this could be taken into account while working on #74 and get resolved.

[muvaf]

We report everything Terraform tells us but it's a JSON object which is a bit hard to read. So, scope of this issue would probably be parsing that JSON and return human-readable error string.

Additionally, we can report diff, too.

[muvaf]

Relabeled it as beta since it's not as critical anymore.

[muvaf]

In #132 , I pass down the provider config map to GetIDFn but that comes with the assumption that all information is stored there. Since the reason for adding Env []string as part of the provider config was to not expose it in the error logs, does it make sense to remove Env []string once we implement error log processing? It'd simplify how we supply creds in several places with a single map to worry about.