crossplane-runtime
crossplane-runtime copied to clipboard
Remove unpublish connection details step?
What problem are you facing?
Crossplane managed reconcilers currently orchestrate the following interface to publish connection details.
// A ManagedConnectionPublisher manages the supplied ConnectionDetails for the
// supplied Managed resource. ManagedPublishers must handle the case in which
// the supplied ConnectionDetails are empty.
type ManagedConnectionPublisher interface {
// PublishConnection details for the supplied Managed resource. Publishing
// must be additive; i.e. if details (a, b, c) are published, subsequently
// publicing details (b, c, d) should update (b, c) but not remove a.
PublishConnection(ctx context.Context, mg Managed, c ConnectionDetails) error
// UnpublishConnection details for the supplied Managed resource.
UnpublishConnection(ctx context.Context, mg Managed, c ConnectionDetails) error
}
In practice the only implementation of this interface publishes connection details as Kubernetes secrets. In this implementation UnpublishConnection is a no-op - we rely on Kubernetes garbage collection to remove the secrets when their owner (the managed resource) is deleted.
So, we have no real implementation of this functionality today but the concept is compelling. You might imagine a ManagedConnectionPublisher that posted to a webhook, or sent connection details to Hashicorp Vault, etc.
One catch with the current implementation relates to the signature - the caller is expected to supply the set of ConnectionDetails (a type of map[string][]byte) to unpublish. In practice the managed resource reconciler calls Unpublish with the ConnectionDetails returned during observation of the managed resource, which is frequently not the full set of connection details. Credentials, for example, are frequently only returned at creation time.
How could Crossplane help solve your problem?
Given we can't explicitly pass all of the credentials we want to unpublish, I think we should either update the signature of UnpublishConnection to omit the ConnectionDetails, and assume that if called all previously recorded connection details will be deleted, or just remove the functionality until we have a need for it.
CC @muvaf
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.