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

Firestore: using listDocuments() requires EmulatorCredentials when running with dev services #554

Closed
jfbenckhuijsen opened this issue Jan 8, 2024 · 10 comments

Comments

@jfbenckhuijsen
Copy link
Contributor

Operations like Firestore.listDocuments() require specific Admin credentials. In the production environment this is ensured by the default GCP credentials, however, when connecting to an emulator (i.e. via Dev Services) specific credentials need to be provided, for which the EmulatorCredentials class can be used.

The current integration logic does not provide this functionality, therefore these operations cannot be used in an emulator environment.

@jfbenckhuijsen
Copy link
Contributor Author

jfbenckhuijsen commented Jan 8, 2024

Example implementation based on the reference docs. Running this example results in an error when dev services are enabled.

package org.acme;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;

import jakarta.inject.Inject;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;

import com.google.api.core.ApiFuture;
import com.google.api.core.ApiFutures;
import com.google.cloud.firestore.CollectionReference;
import com.google.cloud.firestore.Firestore;
import com.google.cloud.firestore.Query;
import com.google.cloud.firestore.QuerySnapshot;
import com.google.cloud.firestore.WriteResult;

@Path("/firestore")
public class FirestoreResource {
    @Inject
    Firestore firestore; // Inject Firestore

    @GET
    @Produces(MediaType.TEXT_PLAIN)
    public String firestore() throws ExecutionException, InterruptedException {
        // Insert 3 persons
        CollectionReference persons = firestore.collection("persons");
        List<ApiFuture<WriteResult>> futures = new ArrayList<>();
        futures.add(persons.document("1").set(new Person(1L, "John", "Doe")));
        futures.add(persons.document("2").set(new Person(2L, "Jane", "Doe")));
        futures.add(persons.document("3").set(new Person(3L, "Charles", "Baudelaire")));
        ApiFutures.allAsList(futures).get();

        return StreamSupport.stream(persons.listDocuments().spliterator(), false)
                .map(r -> {
                    try {
                       return  r.get().get();
                    } catch (InterruptedException | ExecutionException e) {
                        throw new RuntimeException(e);
                    }
                }).map(document -> document.getId() + " - " + document.getString("firstname") + " "
                        + document.getString("lastname") + "\n")
                .collect(Collectors.joining());
//
        // Search for lastname=Doe
//        Query query = persons.whereEqualTo("lastname", "Doe");
//        ApiFuture<QuerySnapshot> querySnapshot = query.get();
//        return querySnapshot.get().getDocuments().stream()
//                .map(document -> document.getId() + " - " + document.getString("firstname") + " "
//                        + document.getString("lastname") + "\n")
//                .collect(Collectors.joining());
    }
}

@jfbenckhuijsen
Copy link
Contributor Author

@jfbenckhuijsen
Copy link
Contributor Author

I see the following issues with the current implementation:

  • A mock for GoogleCredentials can be provided to be injected into the services.
  • The EmulatorCredentials is not a subclass of GoogleCredentials, but it is a subclass of Credentials, which is used in the majority of the services
  • Firebase-admin DOES require a GoogleCredentials, so providing EmulatorCredentials next to the current mock for the GoogleCredentials will cause CDI issues, as CDI will not know without additional hints/annotations which of those two to use for injection into the majority of the services
  • I presume (but haven't tested) that the rest of the services might or might not work with the EmulatorCredentials provided by the Firestore implementation

It might be possible to hard-code (or make it configurable) to inject the EmulatorCredentials in the FirestoreProducer.java code in case a connection to an emulator host is detected (hostOverride.isPresent()). However there might be cases where users want to use a specific credential. Basically, I can use Firestore in two ways:

  • Admin authentication (which the EmulatorCredential provides): the app has full control of the database, can do everything and will handle access control. This is the quarkus.google.cloud.access-token-enabled=false case.
  • User authentication. In this case you often just pass on the GoogleCredential of the user to the calls, so you can use the access control features of Firestore (but in this case you CAN'T use methods like listDocuments().

Given the above, do we want to inject the EmulatorCredentials based on quarkus.google.cloud.access-token-enabled=false && hostOveride.isPresent()?

I think I can probably create a PR for the above, but would first like some discussion on where to take this.

@jfbenckhuijsen
Copy link
Contributor Author

Anyone able to respond to this?

@loicmathieu
Copy link
Collaborator

Hi,

I think we can switch to Credentials everywhere, this has the advantage of making it easy to mock the credentials and stick to the lower level type in the hierarchy that is used almost everywhere.

As you said, this is not the case for FirebaseAdmin, people that uses FirebaseAdmin must provide an object of type GoogleCredentials.

This PR did exactly that: #579

As you said, we should be able, when using emulator, to automatically create such overridden bean, if you're willing to do this (ideally for all supported service that provides emulator configuration) a PR would be highly welcome.

@jfbenckhuijsen
Copy link
Contributor Author

On it!

@jfbenckhuijsen
Copy link
Contributor Author

#602 created

@jfbenckhuijsen
Copy link
Contributor Author

As far as I could tell, only Firestore has specific EmulatorCredentials to use. Spanner has a setEmulatorHost, but that has already been implemented. So AFAIK this should complete this issue

@jfbenckhuijsen
Copy link
Contributor Author

@loicmathieu tested a local SNAPSHOT version of the code vs my own setup, seems to work nicely, so looks like we can close this issue?

@loicmathieu
Copy link
Collaborator

Yes, thanks for reporting!

Fixed by #602

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

No branches or pull requests

2 participants