Upgrade ProviderConfig version to v1beta1 #95
Conversation
By upgrading the version of ProviderConfig to v1beta1 can fix the build issue and simplify the process of managed resource connection. Fixed the issue mentioned in crossplane-contrib#64 (comment) Signed-off-by: zzxwill <[email protected]>
| @@ -73,44 +71,16 @@ type machineGroupBindingConnector struct { | |||
| func (c *machineGroupBindingConnector) Connect(ctx context.Context, mg resource.Managed) (managed.ExternalClient, error) { | |||
| cr, ok := mg.(*aliv1alpha1.MachineGroupBinding) | |||
| if !ok { | |||
| return nil, errors.New(errNotMachineGroupBinding) | |||
| return nil, errors.New(errNotLogtail) | |||
There was a problem hiding this comment.
Is this correct? This appears to be the ExternalConnector for the machine group binding type.
| nn := types.NamespacedName{Namespace: sel.Namespace, Name: sel.Name} | ||
| if err := c.client.Get(ctx, nn, s); err != nil { | ||
| return nil, errors.Wrap(err, errGetConnectionSecret) | ||
| info, err := util.PrepareClient(ctx, mg, cr, c.client, c.usage, cr.Spec.ProviderConfigReference.Name) |
There was a problem hiding this comment.
This was not introduced in this PR, but FWIW packages named util are considered an anti-pattern. See "bad package names" at https://blog.golang.org/package-names.
There was a problem hiding this comment.
Yeah. Fixing this bad package in #96.
| @@ -72,44 +70,16 @@ type machineGroupConnector struct { | |||
| func (c *machineGroupConnector) Connect(ctx context.Context, mg resource.Managed) (managed.ExternalClient, error) { | |||
| cr, ok := mg.(*aliv1alpha1.MachineGroup) | |||
| if !ok { | |||
| return nil, errors.New(errNotMachineGroup) | |||
| return nil, errors.New(errNotLogtail) | |||
There was a problem hiding this comment.
This seems like errNotMachineGroup was correct.
Signed-off-by: zzxwill <[email protected]>
|
@negz Addressed those two comments. I have also checked all the similar locations for all managed resources and they are correct. Thanks for pointing it out. |
|
@negz @hasheddan This PR is targeted to fix the corruption issue for the master branch. And the other two PRs are depended on this one. Thanks. |
By upgrading the version of ProviderConfig to v1beta1 can fix the
build issue and simplify the process of managed resource connection.
Fixed the issue mentioned in #64 (comment)
Description of your changes
Fixes #
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested