Support initProvider

[lsviben]

Description of your changes

Adds support for the new initProvider alpha feature, which is a part of the management
policies feature (design)

Runtime changes

As Upjet uses Terraform, it needs to create the main.tf file and workspace to run commands against it. It does it in the Connect step of the managed reconciler. The creation of the workspace already fills up the main.tf with the resource in question. This means that in the Create and Update steps the passed resource is not actually the one which we Create/Update, as it has already been set in the Connect. This means that at the point of creating the main.tf file, we dont know if it will be used for Update or Create. So to fulfill the purpose of initProvider which is to have fields that are only used during Create, we need to use available TF tools - ignore_changes lifecycle hook, to mark the fields which should only be used for creation.

So if the management policies are enabled, when creating the FileProducer that holds the parameters of the main.tf file:

• we calculate the fields which should be in the ignore_changes by going through the spec.initProvider and spec.forProvider , and looking for fields set in spec.initProvider but not in spec.forProvider
• we merge the spec.initProvider to spec.forProvider

TODO:
currently spec.initProvider fields overwrite the spec.forProvider fields if both are set (slices and maps merge), due to how mergo works. In practice, this will be a problem if the user tries to set a field/map key in both. For our current use cases we dont have this situation, and it can be resolved easily by removing the initProvider conflicting key if the user wants to use forProvider for it. But we should take a look and fix that.
Issue: #240

Code generation

As mentioned in the design, added a new field in the resource spec , spec.initProvider. It will be used to set some fields of the spec.forProvider which should be used just in Create. We are not copying spec.forProvider 1:1 because some field types are not suited to be replaced during Create.

The code generation recognizes a few different field types, based on resource config or terraform API information:

• Identifier fields, which are used to identify resources in addition to names. Good example is AWS region. Configured by provider options
• Reference fields, which allow to resolve cross resource references into some fields. The resolving of references takes place in the beginning of the managed reconciler loop. Configured by provider options. Example is:

	// ARN of the certificate authority.
	// +crossplane:generate:reference:type=CertificateAuthority
	// +kubebuilder:validation:Optional
	CertificateAuthorityArn *string `json:"certificateAuthorityArn,omitempty" tf:"certificate_authority_arn,omitempty"`

	// Reference to a CertificateAuthority to populate certificateAuthorityArn.
	// +kubebuilder:validation:Optional
	CertificateAuthorityArnRef *v1.Reference `json:"certificateAuthorityArnRef,omitempty" tf:"-"`

• Sensitive fields, are those marked in terraform to contain sensitive information, such as passwords. We are replacing them with secret references to avoid having sensitive information in resource yaml's.
• Regular fields, are all other fields for the resource, generated through parsing the fields of the terraform resource schema.

For initProvider we are just interested in the Regular fields. Because:

• Identifiers specify the resource, we cannot create the resource in region: us-west-1 and expect to later managed it in us-east-3.
• References because they are references to other resources. Not sure if there is an use case where a resource depends on one resource during creation, but during lifetime another. It can easily be changed in the forProvider if needed.
• Sensitive fields similarly to above, as they are in the end also references to secrets.
• fields which have a tf: "-" tag. Its the fields above, but added it for precaution, as we are merging initProvider and forProvider and calculating ignore_changes based on the terraform fields.

We could in theory add reference and sensitive fields to the initProvider but as there is not use case for that and their usage could incur bugs, I would rather leave them out unless they are needed.

CEL rules

All required spec.forProvider fields, which are also spec.initProvider fields are now optional, because they can be replaced with initProvider fields. We already have a functionality to mark the top non-identifier required fields as optional and set CEL rules to check if they are set, or if ObserveOnly managementPolicy is active. This only affects the top level fields, as it can easily mark required nested structs.

// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies)",message="%s is a required parameter"

Now because of initProvider, we can also check if the field is in spec.initProvider

// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies || has(self.forProvider.%s))",message="%s is a required parameter"

But because now we merge required nested structs from spec.initProvider to spec.forProvider we need to expand the CEL rules to also mark the nested fields as required either in spec.initProvider or spec.forProvider. This is something still TODO, issue: #239.

Fixes #225

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

The generation code has been tested with generating provider aws

Tested with provider-aws(PR), creating a Node Group from the design example.
with some added fields to test the ignore_changes generation:

apiVersion: eks.aws.upbound.io/v1beta1
kind: NodeGroup
metadata:
  name: sample-eks-ng
spec:
  managementPolicies: ["Observe", "Create", "Update", "Delete"]
  initProvider:
    tags:
      someTag: tag
    forceUpdateVersion: false
    scalingConfig:
      - desiredSize: 1
  forProvider:
    clusterNameRef:
      name: sample-eks-cluster
    nodeRoleArnRef:
      name: sample-node-role
    region: us-west-1
    scalingConfig:
      - maxSize: 4
        minSize: 1
    subnetIdRefs:
      - name: sample-subnet1
      - name: sample-subnet2

Resulting main.tf.json:

"resource": {
    "aws_eks_node_group": {
      "sample-eks-ng": {
        "cluster_name": "sample-eks-cluster",
        "force_update_version": false,
        "lifecycle": {
          "ignore_changes": [
            "force_update_version",
            "scaling_config[0].desired_size",
            "tags[\"someTag\"]"
          ],
          "prevent_destroy": true
        },
        "node_group_name": "sample-eks-ng",
        "node_role_arn": "arn:aws:iam::609897127049:role/sample-node-role",
        "scaling_config": [
          {
            "desired_size": 1,
            "max_size": 4,
            "min_size": 1
          }
        ],
        "subnet_ids": [
          "subnet-034a2e1a7ffdd3e7f",
          "subnet-044de3d67a78caa77"
        ],
        "tags": {
          "crossplane-kind": "nodegroup.eks.aws.upbound.io",
          "crossplane-name": "sample-eks-ng",
          "crossplane-providerconfig": "default",
          "someTag": "tag"
        }
      }
    }
  },
  "terraform": {
    "required_providers": {
      "aws": {
        "source": "hashicorp/aws",
        "version": "4.56.0"
      }
    }
  }

DynamoDB Table

spec:
  managementPolicies: ["Observe", "Create", "Update", "Delete"]
  initProvider:
    writeCapacity: 20
    readCapacity: 19
  forProvider:
    region: us-west-1
    attribute:
    - name: UserId
      type: S
    - name: GameTitle
      type: S
    - name: TopScore
      type: "N"
    billingMode: PROVISIONED
    globalSecondaryIndex:
    - hashKey: GameTitle
      name: GameTitleIndex
      nonKeyAttributes:
      - UserId
      projectionType: INCLUDE
      rangeKey: TopScore
      readCapacity: 10
      writeCapacity: 10
    hashKey: UserId
    rangeKey: GameTitle

"resource": {
    "aws_dynamodb_table": {
      "example": {
        "attribute": [
          {
            "name": "UserId",
            "type": "S"
          },
          {
            "name": "GameTitle",
            "type": "S"
          },
          {
            "name": "TopScore",
            "type": "N"
          }
        ],
        "billing_mode": "PROVISIONED",
        "global_secondary_index": [
          {
            "hash_key": "GameTitle",
            "name": "GameTitleIndex",
            "non_key_attributes": [
              "UserId"
            ],
            "projection_type": "INCLUDE",
            "range_key": "TopScore",
            "read_capacity": 10,
            "write_capacity": 10
          }
        ],
        "hash_key": "UserId",
        "lifecycle": {
          "ignore_changes": [
            "read_capacity",
            "write_capacity"
          ],
          "prevent_destroy": true
        },
        "name": "example",
        "range_key": "GameTitle",
        "read_capacity": 19,
        "tags": {
          "crossplane-kind": "table.dynamodb.aws.upbound.io",
          "crossplane-name": "example",
          "crossplane-providerconfig": "default"
        },
        "write_capacity": 20
      }
    }
  },

[lsviben]

Checked the size change for the CRDs in providers

provider-aws - ~15%

~/go/provider-aws/package (main) [stash: 1]$ du -sh crds
27M	crds

~/go/provider-aws/package (granular_management_policies) [stash: 1]$ du -sh crds
31M	crds

provider-gcp - ~23%

~/go/provider-gcp/package (main) $ du -sh crds
13M	crds

~/go/provider-gcp/package (gmp) $ du -sh crds
16M	crds

provider-azure - 16%

~/go/provider-azure/package (main) $ du -sh crds
25M	crds

~/go/provider-azure/package (gmp) $ du -sh crds
29M	crds

[turkenh]

Added code generation for spec.initProvider which is based on spec.forProvider with some exceptions:

• does not inclued Identifiers - they should be spec.forProvider specific due to their nature
• does not include references
• does not include sensitive fields
• does not include non-terraform fields

CEL rules are now updated to generate the required check in spec.forProvider.<path?> and spec.initProvider.<path?` if the field is not one of the above.

@lsviben it is getting complicated, and I am a bit lost with when (i.e. managementPolicy) we use which (required/optional vs CEL) validation on where (initProvider vs forProvider) for different parameter types (identifier/required/optional). It would be great if you could provide a summary of what to expect in that regard. I guess another dimension is top-level vs nested 🤦

[lsviben]

Added code generation for spec.initProvider which is based on spec.forProvider with some exceptions:

• does not inclued Identifiers - they should be spec.forProvider specific due to their nature
• does not include references
• does not include sensitive fields
• does not include non-terraform fields

CEL rules are now updated to generate the required check in spec.forProvider.<path?> and spec.initProvider.<path?` if the field is not one of the above.

@lsviben it is getting complicated, and I am a bit lost with when (i.e. managementPolicy) we use which (required/optional vs CEL) validation on where (initProvider vs forProvider) for different parameter types (identifier/required/optional). It would be great if you could provide a summary of what to expect in that regard. I guess another dimension is top-level vs nested facepalm

I updated the description of the issue, I hope it makes more sense now.

[turkenh]

LGTM, except I want to make sure to minimize the diff caused by this PR by putting // +kubebuilder:validation:Optional markers back as I stated here.

[ulucinar]

Thank you @lsviben for working on this feature. Left some comments to capture some of the offline discussions we had and some suggestions for you consider. None is blocking from my perspective.

[ulucinar]

Not for this PR, but should we consider putting the generation of the spec.initProvider API behind a configuration parameter (make the generation of the API optional)?

How do we feel about the following question? Will all the providers generated by upjet need this API on their MRs? From our previous discussions, my understanding is that not every MR (or provider in the higher level) may need this API. If this is the case, then we can consider opening an issue in upjet so that we can parameterize the code generation pipeline.

[lsviben]

I would say that probably because of tags it will be needed for all resources. But as an alpha feature we can see how it is used. We could add a configuration parameter not to generate initProviders for some fields, and just put an empty struct for.