Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A way to extend provider-alibaba #63

Closed
zzxwill opened this issue Mar 17, 2021 · 9 comments
Closed

A way to extend provider-alibaba #63

zzxwill opened this issue Mar 17, 2021 · 9 comments
Labels
enhancement New feature or request

Comments

@zzxwill
Copy link
Collaborator

zzxwill commented Mar 17, 2021

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

  • crossplane/provider-alibaba

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,
	})}

	...
}
  • internal provider-alibaba

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

  • crossplane/provider-alibaba

OSS managed resource doesn't need PublishConnection.

  • internal provider-alibaba

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.

@zzxwill zzxwill added the enhancement New feature or request label Mar 17, 2021
@wonderflow
Copy link
Contributor

ping @negz @hasheddan

@negz
Copy link
Member

negz commented Mar 23, 2021

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
Copy link
Collaborator Author

zzxwill commented Apr 7, 2021

@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
Copy link
Member

negz commented Apr 12, 2021

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
Copy link
Collaborator Author

zzxwill commented Apr 13, 2021

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

  • When new releases of community provider-alibaba come out, like Bumping Crossplane Runtime to v0.13.0, upgrade has to be made in internal provider-alibaba.
  • When SLS/OSS are refactored, the internal one needs to merge the code.
  • When there are new managed resources supported, @lowkeyrd might hesitate whether to cheery-pick the feature into the internal one.
  • As the internal one won't use Provider, ProviderConfig, these controllers won't be started up in the internal provider. So he will be careful not to merge those controller and api code.

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
Copy link
Collaborator Author

zzxwill commented Apr 23, 2021

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
Copy link
Member

negz commented Apr 23, 2021

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
Copy link
Member

negz commented Apr 23, 2021

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
Copy link
Collaborator Author

zzxwill commented Jun 23, 2021

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

@zzxwill zzxwill closed this as completed Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants