Extend resource configuration API to include resource metadata #19

ulucinar · 2022-06-20T14:40:43Z

Description of your changes

Fixes #6

This PR extends the existing upjet resource configuration API to allow configuration & overrides related to the resource metadata (probably scraped from Terraform registry). Some possible configuration scenarios covered as of now:

Fixing syntax errors in the scraped examples from the Terraform registry, e.g., fix azurerm_key_vault_access_policy.example-user's key_permissions attribute.
Replacing/injecting attributes, if desired, in the extracted example manifest (resource specific or provider-wide), e.g., injecting a region attribute for all/select resources in provider-aws
Manipulating/replacing reference fields, e.g., replace location references in provider-azure
Marking resources that require globally unique names, such as the Azure Key Vaults or AWS S3 buckets.

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

A resource specific metadata override example:

p.AddResourceConfigurator("azurerm_disk_encryption_set", func(r *config.Resource) {
                ...
		if err := r.MetaResource.Examples[0].SetPathValue("location", "the middle of nowhere"); err != nil {
			panic(err)
		}
	})

Or, as expected, a provider-wide metadata configuration can be performed as follows (in for example, config/provider.go):

        ...
	for _, r := range pc.Resources {
		if err := r.MetaResource.Examples[0].SetPathValue("region", "Test Region"); err != nil {
			panic(err)
		}
	}

An example manifest generated using these configuration overrides on top of scraped metadata:

apiVersion: compute.azure.upbound.io/v1beta1
kind: DiskEncryptionSet
metadata:
  name: example
spec:
  forProvider:
    identity:
    - type: SystemAssigned
    keyVaultKeyId: ${azurerm_key_vault_key.example.id}
    location: the middle of nowhere
    region: Test Region
    resourceGroupNameRef:
      name: example
  providerConfigRef:
    name: default

To configure a unique name to be generated by the test runner, one may use:

p.AddResourceConfigurator("azurerm_key_vault", func(r *config.Resource) {
    ...
    r.MetaResource.ExternalName = meta.RandRFC1123Subdomain

The generated example manifest now contains:

apiVersion: keyvault.azure.upbound.io/v1beta1
kind: Vault
metadata:
  annotations:
    crossplane.io/external-name: '{{ .Rand.RFC1123Subdomain }}'
...

which is to be substituted by the test runner at runtime with a random RFC1123 subdomain string.

muvaf · 2022-06-20T15:17:47Z

pkg/meta/resource.go

+}
+
+// NewProviderMetadataFromFile loads metadata from the specified YAML-formatted file
+func NewProviderMetadataFromFile(path string) (*ProviderMetadata, error) {


If we're planning on keeping this as single file, would it make sense to use go:embed similar to schema so we don't need to worry about reading files in Upjet?

Looks like that if we use go:embed, we don't need to add RootDir and ProviderMetadataPath fields to config.Provider, which is preferable.

I think at some point it's better to split the monolithic provider-metadata.yaml file. My proposal would be to do the per-resource splitting in further rounds, but we may also do so as part of the ongoing https://github.com/upbound/official-providers/issues/19 issue.

Honestly, I can be convinced either way regarding single or multiple files. But if we do end up going with a single file, I'd prefer to go:embed it so that we do not deal with file reading at all in Upjet.

Done. We now go:embed the monolithic provider metadata file. We may later split the provider metadata per resource in the future as discussed.

pkg/meta/resource.go

ulucinar · 2022-06-23T17:35:39Z

Thanks @muvaf for the comments!

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

- Rename Resource.TitleName as Title - Add field documentation for Resource Signed-off-by: Alper Rifat Ulucinar <[email protected]>

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

muvaf

Thank you @ulucinar ! It's all coming together!!

muvaf · 2022-06-29T17:34:54Z

pkg/config/provider.go

-	ProviderMetadataPath string
+	// ProviderMetadata is the scraped provider metadata
+	// from the corresponding Terraform registry.
+	ProviderMetadata string


Since we're using this only in NewProvider, would it make sense to not expose it in Provider struct?

muvaf · 2022-06-29T17:40:15Z

pkg/pipeline/example.go

 		"spec": map[string]interface{}{
 			"forProvider": exampleParams,
 			"providerConfigRef": map[string]interface{}{
-				"name": "example",
+				"name": "default",


nit: we can omit providerConfigRef altogether since it defaults to default.

muvaf · 2022-06-29T17:44:11Z

pkg/registry/resource.go

+// NewProviderMetadataFromFile loads metadata from the specified YAML-formatted document
+func NewProviderMetadataFromFile(providerMetadata string) (*ProviderMetadata, error) {
+	metadata := &ProviderMetadata{}
+	if err := yaml.Unmarshal([]byte(providerMetadata), metadata); err != nil {


Since we're converting to []byte anyway, we might consider accepting providerMetadata as []byte in NewProvider. go:embed supports it.

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

pull-request-size bot added the size/L label Jun 20, 2022

ulucinar marked this pull request as draft June 21, 2022 07:03

ulucinar force-pushed the fix-6 branch 3 times, most recently from 594fdae to d5053ea Compare June 21, 2022 16:59

ulucinar marked this pull request as ready for review June 21, 2022 16:59

muvaf reviewed Jun 22, 2022

View reviewed changes

ulucinar force-pushed the fix-6 branch from 0751a88 to 81bfd3e Compare June 23, 2022 16:49

ulucinar requested a review from muvaf June 23, 2022 17:35

Extend resource configuration API to include resource metadata

5316ee8

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

ulucinar force-pushed the fix-6 branch from 81bfd3e to 27ab0f5 Compare June 28, 2022 12:28

Rename pkg/meta module as pkg/registry

57bd981

- Rename Resource.TitleName as Title - Add field documentation for Resource Signed-off-by: Alper Rifat Ulucinar <[email protected]>

ulucinar force-pushed the fix-6 branch from 27ab0f5 to 57bd981 Compare June 28, 2022 13:22

ulucinar mentioned this pull request Jun 29, 2022

Add metadata scraper #27

Merged

3 tasks

Use go:embed to read the provider metadata file

1fbf5ad

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

ulucinar mentioned this pull request Jun 29, 2022

Example Manifests formatted as JSON in the scraped provider metadata #29

Closed

muvaf approved these changes Jun 29, 2022

View reviewed changes

muvaf reviewed Jun 29, 2022

View reviewed changes

ulucinar added 2 commits June 30, 2022 10:31

Remove config.Provider.ProviderMetadata

5cdbb90

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

Remove providerConfigRef from the generated examples

a6974ba

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

ulucinar merged commit 17e0597 into crossplane:main Jun 30, 2022

ulucinar deleted the fix-6 branch June 30, 2022 08:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend resource configuration API to include resource metadata #19

Extend resource configuration API to include resource metadata #19

ulucinar commented Jun 20, 2022 •

edited

Loading

muvaf Jun 20, 2022

muvaf Jun 20, 2022

ulucinar Jun 23, 2022

muvaf Jun 24, 2022

ulucinar Jun 29, 2022

ulucinar commented Jun 23, 2022

muvaf left a comment

muvaf Jun 29, 2022

ulucinar Jun 30, 2022

muvaf Jun 29, 2022

ulucinar Jun 30, 2022

muvaf Jun 29, 2022

ulucinar Jun 30, 2022

Extend resource configuration API to include resource metadata #19

Extend resource configuration API to include resource metadata #19

Conversation

ulucinar commented Jun 20, 2022 • edited Loading

Description of your changes

How has this code been tested

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulucinar commented Jun 23, 2022

muvaf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulucinar commented Jun 20, 2022 •

edited

Loading