quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.82k stars 2.69k forks source link

`stork-service-discovery-kubernetes` extension requires some extra configutation that is not documented #24444

Closed pjgg closed 2 years ago

pjgg commented 2 years ago

Describe the bug

stork-service-discovery-kubernetes requires org.bouncycastle:bctls-jdk15on dependency and also some extra configuration on your application.properties:

Example

# application properties should be here
quarkus.security.security-providers=BCJSSE

quarkus.http.ssl.certificate.key-store-file=server-keystore.jks
quarkus.http.ssl.certificate.key-store-password=password
quarkus.http.ssl.certificate.trust-store-file=server-truststore.jks
quarkus.http.ssl.certificate.trust-store-password=password

quarkus.native.additional-build-args=--allow-incomplete-classpath

Also, end-user must be sure that has the following K8s/ocp cluster roles binding (JVM and native mode):

Cluster Role

kind: ClusterRole
apiVersion: rbac.authorization.k8s.io/v1
metadata:
  namespace: "${NAMESPACE}"
  name: endpoints-reader
rules:
  - apiGroups: [""] # "" indicates the core API group
    resources: ["endpoints"]
    verbs: ["get", "watch", "list"]

ClusterRoleBinding

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: fabric8-rbac
subjects:
  - kind: ServiceAccount
    # Reference to upper's `metadata.name`
    name: default
    namespace: "${NAMESPACE}"
roleRef:
  kind: ClusterRole
  name: cluster-admin
  apiGroup: rbac.authorization.k8s.io

I think that these requirements should be documented: DocRef: https://quarkus.io/blog/stork-kubernetes-discovery/

Also would be great to understand why org.bouncycastle:bctls-jdk15on is needed and check if we could do something in order to avoid this extra dependency

Reproducer: https://github.com/quarkus-qe/quarkus-test-suite/pull/572

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

quarkus-bot[bot] commented 2 years ago

/cc @geoand, @iocanel, @michalszynkiewicz

michalszynkiewicz commented 2 years ago

CC @aureamunoz

pjgg commented 2 years ago

Regarding native mode is also possible to make it work by adding the following configuration to your application.properties:

quarkus.native.additional-build-args=--allow-incomplete-classpath, --initialize-at-run-time=io.fabric8.kubernetes.client.internal.CertUtils, --enable-url-protocols=https
iocanel commented 2 years ago

It's not clear to me why we need to setup bouncycastle. Can you please provide some more details?

The rbac configuration is indeed needed (with some minor modifications). However, when the kubernetes-client extension is used along with the kubernetes extension, these are expected to be generated.

I think that the main issue here is that stork-service-discovery-kubernetes is not reusing the kubernetes-client extension.

aureamunoz commented 2 years ago

Yes, the rbac configuration is needed, I'm working in the documentation about it and will create a PR soon.

Regarding the bouncycastle, the problem is a NoClassDefFoundError:

Error: Class initialization of io.fabric8.kubernetes.client.internal.CertUtils$1 failed. Use the option --initialize-at-run-time=io.fabric8.kubernetes.client.internal.CertUtils$1 to explicitly request delayed initialization of this class.
Original exception that caused the problem: java.lang.NoClassDefFoundError: org/bouncycastle/jce/provider/BouncyCastleProvider
        at java.base/jdk.internal.misc.Unsafe.ensureClassInitialized0(Native Method)
        at java.base/jdk.internal.misc.Unsafe.ensureClassInitialized(Unsafe.java:1042)
        at jdk.unsupported/sun.misc.Unsafe.ensureClassInitialized(Unsafe.java:698)
        at

And this seem to be because GraalVM doesn't have them (the JVM has them).

stork-service-discovery-kubernetes does use the kubernetes-client directly, not via Quarkus because Stork is a framework independent on Quarkus. Why, indeed, these dependencies are not added in the fabric8 Kubernetes Client? We can add them to stork-service-discovery-kubernetes module and it should fix the problem but we think that they should be added from the Kubernetes Client side, where they are actually needed. WDYT @iocanel @manusa ?

aureamunoz commented 2 years ago

Documentation PR: https://github.com/smallrye/smallrye-stork/pull/278

manusa commented 2 years ago

Bouncy Castle dependencies are optional and should only be necessary in case ECDSA keys are used. I think these might also be required for FIPS mode.

Is this property quarkus.security.security-providers=BCJSSE something you are adding due to this issue (on top of the BC dependencies)?

michalszynkiewicz commented 2 years ago

A possible solution we discussed for the native problem is to create an conditional dependency on the quarkus-kubernetes-client extension (is this the thing that has all the adjustments to make the client work in native?) that would be "enabled" where the kubernetes service discovery is added to a user's project. I've never used it but it seems it should work: https://quarkus.io/guides/conditional-extension-dependencies

If the client brings too many things (or we need the quarkus-kubernetes extension which generates the deployment yamls), we could pull out some quarkus-kubernetes-common (hidden) extension.

geoand commented 2 years ago

IIUC, the whole problem is that Stork does not use the Kubernetes Client provided by the Kubernetes extension, correct?

cescoffier commented 2 years ago

@geoand Yes, stork is a single "extension". We do not have a stork-kubernetes extension that could depend on the Quarkus Kubernetes extension.

Stork itself should not depend on that extension. It's only when you use the kubernetes service discovery that the problem occurs. Having to use an extension for each service discovery / load balancing strategy seems a bit too much. However, I'm pretty sure we would have the issue with consul too. So, we may have to be smart about it...

geoand commented 2 years ago

Thanks for the description of the problem.

Could Stork not have an SPI that basically allows the Kubernetes Client to be set and then in Quarkus when the Kubernetes Client capability is present have the SPI implemented by pulling the Kubernetes Client from CDI? That would not require a new extension.

Of course, the conditional dependencies could also work (we've used it for various things), but I think the SPI approach would be fine for this use case.

michalszynkiewicz commented 2 years ago

It could be done via QuarkusStorkInfrastructure: https://github.com/quarkusio/quarkus/blob/main/extensions/smallrye-stork/runtime/src/main/java/io/quarkus/stork/QuarkusStorkInfrastructure.java

Right now it's only used to reuse the Quarkus Vertx instance but it's easy to change the Kube service discovery to use it for kube client as well.

How does it help? Would you want the user to add the kubernetes client extension when they add the kube service discovery?

geoand commented 2 years ago

Would you want the user to add the kubernetes client extension when they add the kube service discovery?

Pretty much, yeah. Similarly to how we expect users to add quarkus-kotlin if they plan to do Kotlin. We can also easily warn about the missing extension at build time.

michalszynkiewicz commented 2 years ago

We can require adding kube client without sharing the client between the discovery and the other places. If someone adds it now, it will all work, right @aureamunoz ?

cescoffier commented 2 years ago

I just discussed this issue with @aureamunoz and @michalszynkiewicz

The plan of action is the following:

edeandrea commented 2 years ago

Hey @cescoffier I just filed #25688. This creates a problem when using stork with the various kubernetes extensions if you have created any of your own kubernetes files in src/main/kubernetes. The resulting yamls generated by the combination of the kubernetes-client and the kubernetes extensions aren't deployable.