crossplane-contrib / provider-alibaba

Crossplane provider for Alibaba Cloud
Apache License 2.0
50 stars 18 forks source link

A way to extend provider-alibaba #63

Closed zzxwill closed 3 years ago

zzxwill commented 3 years ago

What problem are you facing?

@lowkeyrd , from Alibaba internal team, would like to extend crossplane/provider-alibaba in his own codebase and make some customizations on api structures, controller setup, PublishConnection and so on.

Forking this community provider is not an option as it will take into so many troubles of merging future new releases.

Here are some examples on how the integration would take place.

1. custom Setup of managed resource controller

For example, in OSS managed resource controller, SetupBucket is as below. There is only one managed.ReconcilerOption.

// SetupBucket adds a controller that reconciles Bucket.
func SetupBucket(mgr ctrl.Manager, l logging.Logger) error {
    options := []managed.ReconcilerOption{managed.WithExternalConnecter(&Connector{
        Client:      mgr.GetClient(),
        Usage:       resource.NewProviderConfigUsageTracker(mgr.GetClient(), &aliv1alpha1.ProviderConfigUsage{}),
        NewClientFn: ossclient.NewClient,
    })}

    ...
}

In internal provider, there are two more managed.ReconcilerOption, one of which is to setup account initialization related reconciler option, the other is for managed.WithConnectionPublishers.

func SetupBucket(mgr ctrl.Manager, l logging.Logger) error {
  options := []managed.ReconcilerOption{
        managed.WithExternalConnecter(&connector{
            baseConnector: crossplaneossctl.Connector{
                ...
            },
            xxxAccountClient: ...
        }),
        managed.WithConnectionPublishers(
            ...
    }

2. custom PublishConnection

OSS managed resource doesn't need PublishConnection.

The internal custom provider needed PublishConnection.


func (p *CloudAccountPublisher) PublishConnection(ctx context.Context, mg resource.Managed, c managed.ConnectionDetails) error {
    ...
}

func (p *CloudAccountPublisher) UnpublishConnection(ctx context.Context, mg resource.Managed, c managed.ConnectionDetails) error {
    ...
}

3. same Observe, Create, Update, Delete logics

Both in crossplane/provider-alibaba, and internal provier-alibaba, in a managed resource controller, functions Observe, Create, Update, Delete are the same.

func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.ExternalObservation, error) {
    ...
}

func (e *external) Create(ctx context.Context, mg resource.Managed) (managed.ExternalCreation, error) {
    ...
}

func (e *external) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) {
    ...
}

func (e *external) Delete(ctx context.Context, mg resource.Managed) error {
    ...
}

How could Crossplane help solve your problem?

Whether it's possible, in the aspect of Crossplane Runtime, to support more friendly extension.

For example, like supporting not copying Observe, Create, Update and Delete functions. If the runtime found they are missing, it will redirect to call the Basexxx one in the same managed resource controller.

Please let me know if I didn't have this feature understood. Thanks.

wonderflow commented 3 years ago

ping @negz @hasheddan

negz commented 3 years ago

I'm afraid I don't quite follow how @lowkeyrd would use these Base<T> functions - perhaps an example would help?

I wonder if instead it would it be possible to define a regular ExternalClient implementation in this provider, which @lowkeyrd could then wrap in a higher level ExternalClient implementation (or some other type of his choosing) to extend its functionality? I would prefer that approach because consistent, idiomatic provider implementations make it easier for contributors to help maintain the provider.

zzxwill commented 3 years ago

@negz I have updated examples based on the original feature description.

As for ExternalClient, ExternalConnector, yes, are needed. For example, ExternalConnector will include the base connector from OSS controller of the community provider, and need more clients.

type connector struct {
    baseConnector      crossplaneossctl.Connector
    xxxAccountClient xxx
}

Could you please take a look at again whether I have made it clear this time. Thanks.

negz commented 3 years ago

Hi @zzxwill, thanks for your reply. As far as I can tell from what you've proposed here, everything that @lowkeyrd needs to do should be possible without needing to add the BaseFoo layer of abstractions. It sounds like they can use the regular ExternalClient implementation but just need to plumb in a few more options (e.g. a different connection publisher).

zzxwill commented 3 years ago

@negz Your solution works. But there might be some drawbacks on code mergeing:

This is why we need the BaseFoo and

Forking this community provider is not an option as it will take into so many troubles of merging future new releases.

Here is a brainstorm: In the internal provider-alibaba, no BaseFoo is needed and when observing a managed resource, Crossplane runtime will load the Observe function in the community provider-alibaba to take the role to observe.

zzxwill commented 3 years ago

Or is there a mechanism which can support something like Terraform module? So others can consume managed resource from provider-alibaba directly, without needing to copy api/pkg code of the resource, and can make some customization.

    module "rds" {
      source = "terraform-alicloud-modules/rds/alicloud"
      engine = "MySQL"
      engine_version = "8.0"
      instance_type = "rds.mysql.c1.large"
      instance_storage = "20"
      instance_name = var.instance_name
      account_name = var.account_name
      password = var.password
    }

    output "DB_NAME" {
      value = module.rds.this_db_instance_name
    }
    output "DB_USER" {
      value = module.rds.this_db_database_account
    }
    output "DB_PORT" {
      value = module.rds.this_db_instance_port
    }
    output "DB_HOST" {
      value = module.rds.this_db_instance_connection_string
    }
    output "DB_PASSWORD" {
      value = var.password
    }

    variable "instance_name" {
      description = "RDS instance name"
      type = string
      default = "poc"
    }

    variable "account_name" {
      description = "RDS instance user account name"
      type = "string"
      default = "oam"
    }

    variable "password" {
      description = "RDS instance account password"
      type = "string"
      default = "xxx"
    }
negz commented 3 years ago

We just had a quick call to discuss our options. I'm hopeful that it will be possible to solve this problem while also maintaining typical patterns in the provider.

I'm thinking the internal Alibaba provider, which is a superset of the OSS Alibaba provider, could do something like this:


type InternalAlibabaConnector struct {
    kube: client.Client
}

func (c *InternalAlibabaConnector) Connect(ctx context.Context, mg resource.Managed) (managed.ExternalClient, error) {
    ossConnector := alibabaoss.Connector{client: c.kube}

    // Create the regular OSS ExternalClient
    ossClient, err := ossConnector.Connect(ctx, mg)

    // Return the special internal Alibaba ExternalClient that wraps the OSS ExternalClient.
    return &InternalAlibabaClient{oss: ossClient}, err
}

// The special, internal Alibaba provider.
type InternalAlibabaClient struct {
    // This implementation wraps the regular OSS ExternalClient implementation.
    oss managed.ExternalClient
}

func (e *InternalAlibabaClient) Observe(ctx context.Context, mg resource.Managed) (managed.ExternalObservation, error) {
    // Special, internal Alibaba logic can be implemented here before the OSS Observe method is called.

    // Call the OSS Observe method.
    observation, err := e.oss.Observe(ctx, mg)

    // Special, internal Alibaba logic can be implemented here, after the OSS Observe method was called.
    // This could include modifying the returned observation.

    return observation, err
}
negz commented 3 years ago

Or is there a mechanism which can support something like Terraform module?

Great question. There is actually. Take a look at https://crossplane.io/docs/v1.1/concepts/composition.html - a Crossplane "Composite Resource (XR)" is very similar to a Terraform module. Maybe you could build only the functionality you need internally at Alibaba as a completely separate provider, then make composite resources that mixed OSS managed resources with your internal managed resources?

zzxwill commented 3 years ago

Thanks, @negz @ryanzhang-oss . The solution has been applied to the project in the internal team.