Generate example manifests with dependencies

[ulucinar]

Description of your changes

Fixes #14

This PR depends on #12. With this PR, we now generate example manifests together with their dependencies and any auxiliary dependencies.

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

Here is a sample for the azurerm_disk_encryption_set resource:

apiVersion: compute.azure.upbound.io/v1beta1
kind: DiskEncryptionSet
metadata:
  annotations:
    testing.upbound.io/test-target: "true"
  labels:
    testing.upbound.io/example-name: example5
  name: example5
spec:
  forProvider:
    identity:
    - type: SystemAssigned
    keyVaultKeyIdSelector:
      matchLabels:
        testing.upbound.io/example-name: example3
    location: West Europe
    resourceGroupNameSelector:
      matchLabels:
        testing.upbound.io/example-name: example
  providerConfigRef:
    name: default

---

apiVersion: keyvault.azure.upbound.io/v1beta1
kind: Vault
metadata:
  labels:
    testing.upbound.io/example-name: example2
  name: example2
spec:
  forProvider:
    enabledForDiskEncryption: true
    location: West Europe
    purgeProtectionEnabled: true
    resourceGroupNameSelector:
      matchLabels:
        testing.upbound.io/example-name: example
    skuName: premium
    tenantId: ${data.azurerm_client_config.current.tenant_id}
  providerConfigRef:
    name: default

---

apiVersion: keyvault.azure.upbound.io/v1beta1
kind: AccessPolicy
metadata:
  labels:
    testing.upbound.io/example-name: example-disk
  name: example-disk
spec:
  forProvider:
    keyPermissions:
    - Get
    - WrapKey
    - UnwrapKey
    keyVaultIdSelector:
      matchLabels:
        testing.upbound.io/example-name: example2
    objectId: ${azurerm_disk_encryption_set.example5.identity.0.principal_id}
    tenantId: ${azurerm_disk_encryption_set.example5.identity.0.tenant_id}
  providerConfigRef:
    name: default

---

apiVersion: keyvault.azure.upbound.io/v1beta1
kind: AccessPolicy
metadata:
  labels:
    testing.upbound.io/example-name: example-user
  name: example-user
spec:
  forProvider:
    keyPermissions:
    - get
    - create
    - delete
    keyVaultIdSelector:
      matchLabels:
        testing.upbound.io/example-name: example2
    objectId: ${data.azurerm_client_config.current.object_id}
    tenantId: ${data.azurerm_client_config.current.tenant_id}
  providerConfigRef:
    name: default

---

apiVersion: keyvault.azure.upbound.io/v1beta1
kind: Key
metadata:
  labels:
    testing.upbound.io/example-name: example3
  name: example3
spec:
  forProvider:
    keyOpts:
    - decrypt
    - encrypt
    - sign
    - unwrapKey
    - verify
    - wrapKey
    keySize: 2048
    keyType: RSA
    keyVaultIdSelector:
      matchLabels:
        testing.upbound.io/example-name: example2
  providerConfigRef:
    name: default

---

apiVersion: azure.upbound.io/v1beta1
kind: ResourceGroup
metadata:
  labels:
    testing.upbound.io/example-name: example
  name: example
spec:
  forProvider:
    location: West Europe
  providerConfigRef:
    name: default

---

[muvaf]

What would be the advantage of this separate chain over using the existing configurators to manipulate the example like done here?

[ulucinar]

In the configuration use-cases we've met so far, we observed that we may need the config.Resource associated with a manifest which we want to modify. An example is injecting region field to non-IAM resources. Currently, when we load the metadata we only have the config.Resource object available to the root resource and not the dependencies. That's why I went with a modifier function that we can call with a config.Resource for both the root resource and any associated dependencies.

One alternative I considered was to just introduce a SetPathValue function for dependencies and require the caller to make any checks solely based on the Terraform resource name (such as if the resource name starts with aws_iam, do not inject a region field). But this API allows us to write shorter & cleaner configuration overrides compared to this approach.

Another approach that I did not want to implement here as I think it would deserve a separate PR was to change the registry.MetaResource structure so that we can associate config.Resource with the dependencies. One challenge here (associated with both the root resource and its dependencies) is that the associated config.Resource may not have been finalized yet as we load the provider metadata. The deferring made possible with the modifier functions is that we make sure we consume the final config.Resource.

An example consumption for this API is as follows:

func setDefaultRegion(object *fieldpath.Paved, r *config.Resource) error {
        if r.ShortGroup == "iam" {
                return nil
        }
        return errors.Wrap(object.SetValue("spec.forProvider.region", "us-west-1"), "cannot set default region")
}

, which is used to set the default region for both root resources and any dependencies encountered.

[muvaf]

Currently, when we load the metadata we only have the config.Resource object available to the root resource and not the dependencies. That's why I went with a modifier function that we can call with a config.Resource for both the root resource and any associated dependencies.

Another approach that I did not want to implement here as I think it would deserve a separate PR was to change the registry.MetaResource structure so that we can associate config.Resource with the dependencies. One challenge here (associated with both the root resource and its dependencies) is that the associated config.Resource may not have been finalized yet as we load the provider metadata. The deferring made possible with the modifier functions is that we make sure we consume the final config.Resource.

What do you mean by dependencies, is it the .MetaResource.Examples[*].Dependencies field or something else? If it's MetaResource field, after a recent change, it's loaded here before any configurator is run, we're able to set the region for all examples via a configuration and all config.Resource objects are finalized before any codegen starts.

If I understand correctly, at the moment, functionally the difference between using a configurator and the example modifier introduced here is when they are called; the latter being called after all operations are done on the examples, the former being called before any codegen starts. I think this touches a little bit to our discussion here regarding finalization of the config/instructions before starting the codegen and I see a similar pattern here with SkipReferencesTo as well where we give instruction to be applied later, independent of other generators.

In general, I think if we can do what these additional config endpoints can do with a configurator, we should do so because:

• It'd keep the whole configuration story cohesive that there is only one place to change the source of truth for all things regarding the resource and you can have any generator be confident that they state of the config they're given is finalized and shared across other generators when they're run.
  • Having examples or schema or MetaResource in some way modified separately strips them off from that assumption, i.e. you might have removed a field from an example that the CRD builder used to derive some information.
    • One recent example is that if we were to give a config to CRD builder regarding move of a field from spec to status instead of manipulating the schema itself, then other generators wouldn't have this information so we'd have to build a dependency graph of builders to keep all of their outputs in sync with each other.
• If we can do everything that can be done in an example modifier using resource configurator, are we really getting value from the additional field? If the function signature is the concern, we can come with common helper functions with a UX similar to the example modifier that can be called inside a configurator.

[ulucinar]

I wanted to have the structured resource.Config available for the dependency manifests as we modify them but now removed the modifier chain. Now we may do something like the following for our configuration case (assuming no IAM resource has some non-IAM dependencies):

func RegionAddition() config.ResourceOption {
	return func(r *config.Resource) {
		if r.ShortGroup == "iam" {
			return
		}
                ...
                for _, ex := range r.MetaResource.Examples {
			if err := ex.SetPathValue("region", "us-west-1"); err != nil {
				panic(err)
			}
			for k := range ex.Dependencies {
				if strings.HasPrefix(k, "aws_iam") {
					continue
				}
				if err := ex.Dependencies.SetPathValue(k, "region", "us-west-1"); err != nil {
					panic(err)
				}
			}
		}

[muvaf]

@ulucinar Thank you for following up!

[ulucinar]

Also removed the Resource.SkipReferencesTo API. Without regex matching it was not powerful enough and as you mentioned above, we would not prefer to invest in it.

[muvaf]

@ulucinar Opened #59 to track implementation of example modifier interface via config.ResourceOption.

[muvaf]

This is so much logic without tests but I can't block this anymore. Opened #47

[muvaf]

LGTM! Thanks @ulucinar ! I think we can cut a new release of Upjet after merging this PR.