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

feat(transfer): add vpcendpoint dns entries to connection secrets #2145

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

anagarlau
Copy link
Collaborator

@anagarlau anagarlau commented Jan 22, 2025

Description of your changes

Fixes #

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Comment on lines 57 to 64
c := &custom{
client: e.client,
kube: e.kube,
external: e.external,
vpcEndpointClient: e.vpcEndpointClient,
}

e.postObserve = c.postObserve
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can avoid a second GET on the providerconfig by implementing a custom connector that returns a custom external with the standard external nested in it. Something like:

type customConnector struct {
	kube client.Client
	opts []option
}

func (c *connector) Connect(ctx context.Context, mg cpresource.Managed) (managed.ExternalClient, error) {
	// the code can either be copied from zz_controller.go or reused and the result casted (whatever feels better)

        cr, ok := mg.(*svcapitypes.Server)
	if !ok {
		return nil, errors.New(errUnexpectedObject)
	}
	sess, err := connectaws.GetConfigV1(ctx, c.kube, mg, cr.Spec.ForProvider.Region)
	if err != nil {
		return nil, errors.Wrap(err, errCreateSession)
	}

	ec2Client := c.newClientFn(*cfg)
	if vpcClient == nil {
		return nil, errors.New("failed to initialize EC2 client")
	}

	return &customExternal{
		ec2: ec2Client
		external: newExternal(c.kube, svcapi.New(sess), c.opts)
	}
}

type customExternal struct {
  *external
  ec2Client ec2.Client
}

func (e *customExternal) postObserve() {
  // endpoint observation + existing code
}

Copy link
Collaborator Author

@anagarlau anagarlau Jan 22, 2025

Choose a reason for hiding this comment

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

Technically this would be the best way to do it but given that the Connect method on the connector is already generated, this approach would require generated code to be removed. I m not sure it can be skipped. However, I could not call the Connect twice but implement the functionality in (c *connector) Connect in the (c *customConnector) Connect and remove any redundancies.
Also, ec2 Client needs the v2 Config while the transfer client uses v1

@anagarlau anagarlau force-pushed the transfer-vpcendp branch 3 times, most recently from 1d0b5db to bab59ff Compare January 22, 2025 16:06
Copy link
Collaborator

@MisterMX MisterMX left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you very much @anagarlau! Since this is purely cosmetic addition and before that transfer was pretty unusable without the endpoint I guess we can backport this to v0.51.

@MisterMX MisterMX merged commit 6f85b24 into crossplane-contrib:master Jan 22, 2025
8 of 9 checks passed
Copy link

Successfully created backport PR #2147 for release-0.51.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants