Conversion from tfjson to consume schema from cli output

[turkenh]

Description of your changes

Fixes #175 in the scope of #131

I have:

• Read and followed Crossplane's contribution process.
• Run make reviewable to ensure this PR is ready for review.
• Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

See the provider-gcp schema generated using the conversion here: crossplane-contrib/provider-jet-gcp#38

Tested with provider-github with the updated jet template in this PR: crossplane-contrib/provider-jet-template#13

[muvaf]

Thanks @turkenh ! Do you have data about whether we see CRD changes with this in bigger providers, like jet-aws or jet-gcp with full set?

[muvaf]

I'm not sure how long we'd like to keep the backward compatibility, especially if there is no CRD API change, but the next step could be to make builder work with CLI schema directly and do the sdk -> cli schema conversion using this referenced tool. You may consider doing that in this PR, too, if the conversions we have here are too similar to that file anyway.

[turkenh]

As mentioned in the next comment lines, I agree that ultimately our builder should work with cli schema. However, I still see value to experiment current approach with new providers for a while.

I liked the idea of working with CLI schema in type builder by doing the conversion from sdk -> cli schema, but I wouldn't prefer to do in this PR because:

• Making type builder working with CLI would require a good amount of work and I would not prefer to delay this feature for new providers since there are some people struggling with the current approach, i.e. importing tf provider go package.
• Since the cli schema is a more limited representation than the sdk one, we would still lose some information, e.g. int/float distinction or lack of required attribute for a deeply nested object field, etc. So, at this point (having one conversion already implemented), I don't see much benefit of spending time the other way.

Stating this, I believe we can prioritize to work on a follow-up PR updating type builder to work with cli schema and speed up the experimenting process.

[turkenh]

Thanks @turkenh ! Do you have data about whether we see CRD changes with this in bigger providers, like jet-aws or jet-gcp with full set?

@muvaf, not the full set but there is this PR generated with the default resource set in provider-jet-gcp.

Basically, there are two types of schema changes that I observed:

• Int64 to Float64 changes in API types and format: int64, type: integer to type: number in corresponding CRD schema
• Some nested resources lose some information like description or required because of a more limited representation.

I haven't dug further since we first want to experiment with new providers first.

[turkenh]

@muvaf I've resolved/responded to your comments and this is now ready for another pass.

[muvaf]

Thanks @turkenh ! LGTM!

I think the only risk with this change is the usage of float64 which upstream folks have some concerns about it. Though I'm not sure how IntOrString type would work in our runtime tools. Maybe it's no different than others since we use fieldpath?

Another thing regarding float is how Terraform works with it, i.e. if we print 0.0 instead of 0 in main.tf.json, would TF CLI and TF provider be OK with that?

[turkenh]

Ok, I did some testing and writing here for future reference.

I've selected google google_compute_autoscaler resource and it autoscaling_policy.cpu_utilization.target as it expects a float value.

Code Generation:

Here is how it looks like in provider schema tf output:

"target": {
    "type": "number",
    "description": "The target CPU utilization that the autoscaler should maintain.\nMust be a float value in the range (0, 1]. If not specified, the\ndefault is 0.6.\n\nIf the CPU level is below the target utilization, the autoscaler\nscales down the number of instances until it reaches the minimum\nnumber of instances you specified or until the average CPU of\nyour instances reaches the target utilization.\n\nIf the average CPU is above the target utilization, the autoscaler\nscales up until it reaches the maximum number of instances you\nspecified or until the average utilization reaches the target\nutilization.",
    "description_kind": "plain",
    "required": true
}

In the generated schema:

Target *float64 `json:"target" tf:"target,omitempty"`

In CRD Spec:

target:
  description: "The target CPU utilization ...."
  type: number

Runtime:

Created following CR:

apiVersion: compute.gcp.jet.crossplane.io/v1alpha1
kind: Autoscaler
metadata:
  name: numbertest
spec:
  forProvider:
    zone: us-central1-a
    target: projects/<redacted>/zones/us-central1-a/instanceGroupManagers/hasan-another
    autoscalingPolicy:
      - minReplicas: 1
        maxReplicas: 4
        cooldownPeriod: 59
        cpuUtilization:
          - target: 0.43

Generated main.tf by Terrajet:

{
  "resource": {
    "google_compute_autoscaler": {
      "numbertest": {
        "autoscaling_policy": [
          {
            "cooldown_period": 59,
            "cpu_utilization": [
              {
                "target": 0.43
              }
            ],
            "max_replicas": 4,
            "min_replicas": 1
          }
        ]
        ...
      }
    }
  },
}

Resource successfully created and reported Synced & Ready as true. And here is how it looks like in GCP console:

Then I updated target to 1 as below:

    autoscalingPolicy:
        ...
        cpuUtilization:
          - target: 1

With the generated main.tf:

            "cpu_utilization": [
              {
                "target": 1
              }
            ],

It updated and synced as below:

And setting this as 1.0 or 1.00 in CR spec does not change anything as k8s API (or kubectl?) returns unchanged with kubectl apply.

[turkenh]

Thanks @turkenh ! LGTM!

I think the only risk with this change is the usage of float64 which upstream folks have some concerns about it. Though I'm not sure how IntOrString type would work in our runtime tools. Maybe it's no different than others since we use fieldpath?

Another thing regarding float is how Terraform works with it, i.e. if we print 0.0 instead of 0 in main.tf.json, would TF CLI and TF provider be OK with that?

@muvaf in the above experiment, I've validated that we can work with numbers and can both provide integers and/or floats for such fields. With that, I am feeling good with float64.

[muvaf]

@turkenh Thanks for that extensive test! I think this could warrant a Terrajet release.

[muvaf]

Actually let's get #195 as well included in the release.