Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Tagging Convention #198

Merged
merged 2 commits into from
Jan 28, 2022
Merged

Tagging Convention #198

merged 2 commits into from
Jan 28, 2022

Conversation

sergenyalcin
Copy link
Member

@sergenyalcin sergenyalcin commented Jan 13, 2022

Description of your changes

Fixes crossplane-contrib/provider-jet-aws#136

This PR provides support for tagging convention for all providers in a generic way. By this implementation, custom initializers can be set.

A new configuration field, Initializers, was added. The type of this field is []InitializerFn:

// InitializerFn returns the Initializer with a client.
type InitializerFn func(client client.Client) managed.Initializer

The field that is expected to be set has been implemented configurable on the basis of resource.

If the Initializers field is not set in the resource configuration, the initializers will not be generated in controllers in Setup function.

In three providers (aws, azure and gcp), a defaulting is not implemented for tag fields. Some statistics were collected to decide the topic:

AWS: 761 Generated Resource / 398 Resources has not "tags" field / 1 resource's "tags" field has different type (not map[string]*string)

Azure: 645 Generated Resource / 401 Resources has not "tags" field / 2 resources' "tags" field has different type (not map[string]*string)

Google: 436 Generated Resource / 376 Resources has not "labels" field / 2 resources' "labels" field has different type (not map[string]*string)

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

This PR was tested in provider-aws by using the S3-Bucket resource. Firstly, the commit id of this PR was used to consume the latest changes. And a custom ExternalTagsFieldName configuration was added for this resource. Then create, update and delete operations were successfully tested.

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
Copy link
Member

@muvaf muvaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I've left small comments and as we synced offline, having a common implementation here and using it as tag initializer after configuring would be a nice UX and also inline with how we do defaulting in Terrajet in general. Thanks Sergen!

pkg/config/resource.go Outdated Show resolved Hide resolved
pkg/config/resource.go Outdated Show resolved Hide resolved
pkg/pipeline/templates/controller.go.tmpl Outdated Show resolved Hide resolved
Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apply tagging convention
2 participants