Skip to content

External DependentResource with Updater cannot check if desired and actual match #2128

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

Closed
BramMeerten opened this issue Nov 20, 2023 · 2 comments · Fixed by #2130
Closed
Assignees

Comments

@BramMeerten
Copy link
Contributor

If I have an dependentResource that extends from AbstractExternalDependentResource, but also from Updater, then the update will never be called when the Primary resource is updated. This is caused by a problem with the match method.

For example this dependent resource:

public class ExternalUserDependentResource
        extends PerResourcePollingDependentResource<ExternalUser, MyPrimary>
        implements Creator<ExternalUser, MyPrimary>, 
                             Deleter<MyPrimary>, 
                             Updater<ExternalUser, MyPrimary> {
    // ... 
    @Override
    protected ExternalUser desired(MyPrimary primary, Context<MyPrimary> context) {
        return new ExternalUser(primary.getSpec().username());
    }

    @Override
    public ExternalUser create(ExternalUser desired, MyPrimary primary, Context<Prim> context) {
        externalSystem.create(desired.username());
        return desired;
    }

    @Override
    public ExternalUser update(ExternalUser actual, ExternalUser desired, MyPrimary primary, Context<MyPrimary> context) {
        externalSystem.update(desired.username());
        return desired;
    }

    @Override
    public Set<ExternalUser> fetchResources(MyPrimary primary) {
        return externalSystem
               .findUsers(primary.getSpec().username())
               .map(u -> new ExternalUser(u.username())
               .collect(toSet());
    }
}

This is caused by a method call to match that keeps calling itself:

  1. AbstractDependentResource::match is called:
public Result<R> match(R resource, P primary, Context<P> context) {
    return updater.match(resource, primary, context);
}
  1. It calls the match method on the field updater, but this is the same dependent resource (ExternalUserDependentResource in my example):
protected AbstractDependentResource() {
    creator = creatable ? (Creator<R, P>) this : null;
    updater = updatable ? (Updater<R, P>) this : null;
  1. Stuck in a loop

Workaround:
Override match in the dependent resource and do the check yourself.

@csviri
Copy link
Collaborator

csviri commented Nov 21, 2023

@BramMeerten I added a PR that might make this a bit more user friendly. There is no good solution for this IMO, the intention was that this is indeed overwritten. That is what the matcher method in the Updater should indicate. But this is complicated with the Bulk resources, where the matcher has different footprint and you want that to be defaulted that case. Pls check the PR if that makes sense, it provides a default not recursive implementation.

@BramMeerten
Copy link
Contributor Author

@csviri Looks good to me
Having to override matcher is no problem for me, the main problem that I had was that it wasn't working without any error that indicated what was wrong.
Somehow there was no stackoverflowexception because of infinite recursion, the recursion just stopped after a while.

And this PR fixes the infinite recursion, thx!

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