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

DefaultWorkloadApiClient does not work with unix sockets on Mac #76

Open
alwaysastudent opened this issue Aug 4, 2021 · 7 comments
Open

Comments

@alwaysastudent
Copy link

Seems like io.spiffe.workloadapi.internal.GrpcManagedChannelFactory only supports Linux OS for unix schemes. Naive question perhaps, why is it not supporting unix sockets on Mac?

@maxlambrecht
Copy link
Member

Hi @alwaysastudent, it works on Mac with unix sockets, you need to add the dependency grpc-netty-macos.

Using gradle:

runtimeOnly group: 'io.spiffe', name: 'grpc-netty-macos', version: '0.6.3'

@alwaysastudent
Copy link
Author

Thank you so much @maxlambrecht. Do you know if I need to exclude the transitive dependency grpc-netty-linux from the java-spiffe-provider as well?

@alwaysastudent
Copy link
Author

I wish we didn't have to toggle on these mutually exclusive dependencies as consumers of this project, as this makes it difficult to write wrapper extension libraries around this project and distribute internally within our org.

Any reason why can't we unify this logic into the core library itself instead and switch on 2 different GrpcManagedChannelFactory?

@maxlambrecht
Copy link
Member

maxlambrecht commented Aug 5, 2021

Thank you so much @maxlambrecht. Do you know if I need to exclude the transitive dependency grpc-netty-linux from the java-spiffe-provider as well?

You don't need to exclude the grpc-netty-linux, just add the grpc-netty-macos dependency as runtime when running on Macos.

You could use 'com.google.osdetector' to import the dependency for Macos conditionally:

    if (osdetector.os.is('osx') ) {
        runtimeOnly group: 'io.spiffe', name: 'grpc-netty-macos', version: '0.6.3'
    }  

I wish we didn't have to toggle on these mutually exclusive dependencies as consumers of this project, as this makes it difficult to write wrapper extension libraries around this project and distribute internally within our org.

Any reason why can't we unify this logic into the core library itself instead and switch on 2 different GrpcManagedChannelFactory?

The reason is that we decided to use the grpc-netty-shaded dependency, and it only includes support for Epoll events, so as Macos doesn't support Epoll we needed to use a different netty dependency, having 2 different netty dependencies forces us to have 2 different GrpcManagedChannelFactory. The 2 GrpcManagedChannelFactory classes import different packages.
The Macos submodule uses the netty-transport-native-kqueue dependency.

@alwaysastudent
Copy link
Author

hi @maxlambrecht - I am using maven and I had to make sure that the grpc-netty-macos is in front of the grpc-netty-linux, in the declaration order to establish the classpath precedence. Gradle does the same internally and orders grpc-netty-macos is in front of the grpc-netty-linux. When we are developing abstract extension wrappers over this project, we may have to either carry over this same dependency management design which makes it a bit inconvenient.

The reason is that we decided to use the grpc-netty-shaded dependency, and it only includes support for Epoll events, so as Macos doesn't support Epoll we needed to use a different netty dependency, having 2 different netty dependencies forces us to have 2 different GrpcManagedChannelFactory. The 2 GrpcManagedChannelFactory classes import different packages.
The Macos submodule uses the netty-transport-native-kqueue dependency.

Wouldn't it be easier to have 2 different OS-specific GrpcManagedChannelFactory classes that can switch on strategies for creating OS-specific io.grpc.ManagedChannel? Maybe making ManagedChannelWrapper an interface with 2 different implementations would simplify the changes as well.

https://github.com/spiffe/java-spiffe/blob/v0.6.3/java-spiffe-core/src/main/java/io/spiffe/workloadapi/DefaultWorkloadApiClient.java#L133

@maxlambrecht
Copy link
Member

hi @maxlambrecht - I am using maven and I had to make sure that the grpc-netty-macos is in front of the grpc-netty-linux, in the declaration order to establish the classpath precedence. Gradle does the same internally and orders grpc-netty-macos is in front of the grpc-netty-linux. When we are developing abstract extension wrappers over this project, we may have to either carry over this same dependency management design which makes it a bit inconvenient.

The reason is that we decided to use the grpc-netty-shaded dependency, and it only includes support for Epoll events, so as Macos doesn't support Epoll we needed to use a different netty dependency, having 2 different netty dependencies forces us to have 2 different GrpcManagedChannelFactory. The 2 GrpcManagedChannelFactory classes import different packages.
The Macos submodule uses the netty-transport-native-kqueue dependency.

Wouldn't it be easier to have 2 different OS-specific GrpcManagedChannelFactory classes that can switch on strategies for creating OS-specific io.grpc.ManagedChannel? Maybe making ManagedChannelWrapper an interface with 2 different implementations would simplify the changes as well.

https://github.com/spiffe/java-spiffe/blob/v0.6.3/java-spiffe-core/src/main/java/io/spiffe/workloadapi/DefaultWorkloadApiClient.java#L133

Yes, we could make ManagedChannelWrapper an interface with 2 implementations and have a single GrpcManagedChannelFactory that creates that appropriate object that implements the ManagedChannel based on whether it's Linux or Mac. The only caveat is that we would need to include all the grpc/nettty dependencies:

    implementation group: 'io.grpc', name: 'grpc-netty-shaded', version: "${grpcVersion}". // for Linux

    implementation group: 'io.grpc', name: 'grpc-netty', version: "${grpcVersion}" // for Macos
    implementation group: 'io.netty', name: 'netty-transport-native-kqueue', version: '4.1.63.Final', classifier: 'osx-x86_64'. //for Macos

The Linux related implementation would use classes from the shaded dependency and the Macos version would use classes from the other two dependencies. The user of the library will need to exclude the dependencies that are not used.

Any thoughts @rturner3 ?

@alwaysastudent
Copy link
Author

alwaysastudent commented Aug 5, 2021

Yes, we could make ManagedChannelWrapper an interface with 2 implementations and have a single GrpcManagedChannelFactory that creates that appropriate object that implements the ManagedChannel based on whether it's Linux or Mac. The only caveat is that we would need to include all the grpc/nettty dependencies:

    implementation group: 'io.grpc', name: 'grpc-netty-shaded', version: "${grpcVersion}". // for Linux

    implementation group: 'io.grpc', name: 'grpc-netty', version: "${grpcVersion}" // for Macos
    implementation group: 'io.netty', name: 'netty-transport-native-kqueue', version: '4.1.63.Final', classifier: 'osx-x86_64'. //for Macos

The Linux related implementation would use classes from the shaded dependency and the Macos version would use classes from the other two dependencies. The user of the library will need to exclude the dependencies that are not used.

Any thoughts @rturner3 ?

@maxlambrecht - another approach may be to use grpc-netty everywhere in this java-spiffe project and shade them and relocate into spiffe core jars itself, getting all control to yourself and removing the transitives dependency for consumers.

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