WIP

Author: sbernauer

modules/contributor/pages/adr/ADR028-discovery-revision.adoc

@@ -23,6 +23,10 @@ Currently we expose some of these information, but not all (e.g. which ca cert t
 On the other hand it could be the case that users have external services which Stackable services should use, e.g.
 an non-stackable HDFS where an Stackable Trino should connect to.
 
+Question 1: How do we want to make products discoverable?
+Question 2: How should clients verify the identity of the server (e.g. the cert of the server)
+Question 3: How should clients authenticate themselves against the server?
+
 == Decision Drivers
 We have some common use-cases that we need to express via the discovery mechanism:
 
@@ -148,8 +152,8 @@ kind: HdfsClusterDiscovery
 metadata:
   name: simple-hdfs
 spec:
+  productVersion: 3.3.4 # *could* be put in common struct and #[serde(flattened)]
   hdfsSitesConfigMap: hdfs-simple-hdfs
-  productVersion: 3.3.4
   httpProtocol:
     http: {}
     # OR
@@ -169,6 +173,26 @@ spec:
     core-site.xml: <xml>
 ----
 
+[source,yaml]
+----
+apiVersion: trino.stackable.tech/v1alpha1
+kind: TrinoClusterDiscovery
+metadata:
+  name: simple-trino
+spec:
+  productVersion: "414"
+  coordinatorEndpoint:
+    host: trino-coordinator.ns.svc.cluster.local
+    port: 8443
+    protocol:
+      http: {}
+      # OR
+      https:
+        caCertSecretClass: tls
+  authentication: <whatever>, see below
+# No CM needed
+----
+
 ==== Pros
 
 * Fixes mount problem from `Discovery Object: Use dedicated CRD object for every product`
@@ -228,32 +252,33 @@ Instead of writing discovery information to dedicated objects - such as CM or cu
   Currently we have the freedom of either connection to a Zookeeper root dir or a ZNode transparently.
 
 === [2] TLS: Discovery config contains SecretClass
-The discovery includes the SecretClass used to obtain the *server* certificate
+The discovery includes the SecretClass used to obtain the ca.crt used to validate the *server* certificate
 
 Trino discovery:
 [source,yaml]
 ----
+apiVersion: trino.stackable.tech/v1alpha1
+kind: TrinoClusterDiscovery
 metadata:
-  name: my-trino
-coordinatorEndpoint:
-  host: trino-coordinator.ns.svc.cluster.local
-  port: 8443
-  protocol:
-    http: {}
-    # OR
-    https:
-      caCertSecretClass: tls
+  name: simple-trino
+spec:
+  coordinatorEndpoint:
+    host: trino-coordinator.ns.svc.cluster.local
+    port: 8443
+    protocol:
+      http: {}
+      # OR
+      https:
+        caCertSecretClass: tls # gives ca.crt used to verify the server cert
 ---
 # superset config
 security:
-  tls:
+  tls: # server tls cert
     secretClassName: tls
-  kerberos:
-    secretClassName: kerberos
 backends: # Don't look at the Superset CRD structure, we are only interested in the tls stuff here
   - name: my-trino
     trino:
-      discoveryConfigName: my-trino
+      discovery: my-trino
 ----
 
 ==== Pros
@@ -269,20 +294,26 @@ For usability reasons it can be omitted and defaults to the SecretClass the clie
 Trino discovery:
 [source,yaml]
 ----
+apiVersion: trino.stackable.tech/v1alpha1
+kind: TrinoClusterDiscovery
 metadata:
-  name: my-trino
-coordinatorEndpoint: https://trino-coordinator.ns.svc.cluster.local:8443
----
+  name: simple-trino
+spec:
+  coordinatorEndpoint:
+    host: trino-coordinator.ns.svc.cluster.local
+    port: 8443
+    protocol:
+      http: {}
+      # OR
+      https: {} # NO! cert information
 # superset config
 security:
   tls:
     secretClassName: tls
-  kerberos:
-    secretClassName: kerberos
 backends: # Don't look at the Superset CRD structure, we are only interested in the tls stuff here
   - name: my-trino
     trino:
-      discoveryConfigName: my-trino
+      discovery: my-trino
       # override tls from the global config, OPTIONALLY
       tlsSecretClass: my-second-pki
 ----
@@ -295,6 +326,7 @@ backends: # Don't look at the Superset CRD structure, we are only interested in
 ==== Cons
 
 * The client has to know what pki/secretClass the server is using.
+* Superset TrinoConnection could not only say "Connect this Superset and this Trino", but need to say "using this ca.crt to validate the Trino server"
 
 === [2] TLS: Include caCert in Discovery config
 
@@ -318,7 +350,6 @@ endpoint:
 
 ==== Pros
 
-* Assuming DiscoveryConfig is located within a CM, the operator can simply mount the discovery CM to get the ca.crt.  Operator does not need to read/look at the DiscoveryConfig.
 * Easier for external clients to use as they don't need to know the concept of SecretClasses and don't even need to run withing k8s.
 * The client has to *not* know what pki/secretClass the server is using.
 
@@ -339,11 +370,13 @@ authentication:
   authenticationClass: my-class
 ----
 
+==== Pros
+* IMPORTANT: This is the only thing the server can know (how he is verifying client identities). He can not recommend an SecretClass used to obtain the client credentials. E.g. he uses an LDAP AuthenticationClass, there is no way it can now what SecretClass provides credentials accepted by LDAP. (Most cases it will be a user logging into a WebUI and the LDAP credentials of the user are not even stored anywhere but just remembered by the user)
+
 ==== Cons
 
 * Operator has to read the AuthenticationClass to determine its type (pw/tls/keytab) and set up the needed volumes and commands.
-* The AuthenticationClass is meant to describe "how should a server verify connecting clients" and re-purpose it to mean "how a client should authenticate itself".
-
+// * The AuthenticationClass is meant to describe "how should a server verify connecting clients" and re-purpose it to mean "how a client should authenticate itself". Image a user creates a Secret `trino-users` with *only* a ca.crt and a SecretClass `trino-users` on top. The connecting client than has no way of knowing how to get a client cert.
 
 === [3] Authentication: Add SecretClass to Discovery Config
 
@@ -359,7 +392,7 @@ authentication:
 ==== Cons
 
 * Operator has to read the SecretClass to determine its type (pw/tls/keytab) and set up the needed volumes and commands.
-
+* Image then SecretClass is of type `k8sSearch`. The connection client (e.g. controlled via superset-operator) than has no idea if he should expect a tls.crd or a keytab when mounting the SecretClass.
 
 === [3] Authentication: Add needed details
 
@@ -372,11 +405,9 @@ authentication:
   none: {}
   password: {}
   tls:
-    secretClass: client-tls # Use this SecretClass to obtain a *client* cert
+    secretClass: client-tls # Use this SecretClass to obtain a *client* cert tls.crt
   kerberos:
     secretClass: client-kerberos # Use this SecretClass to obtain a keytab
-  oauth:
-    secretClass: client-oauth # Use this SecretClass to obtain whatever it needs
 ----
 
 ==== Pros
@@ -385,13 +416,19 @@ authentication:
 
 ==== Cons
 
-* Operator has read the Discovery CM it wants to connect to
+=== [3] Authentication: Don't add information how to authenticate
+
+Trino discovery does not provide any information on how to authenticate
+
+==== Cons
+
+* Not viable, as users need to know how to connect, and are not expected to try 50 different auth methods. We need to give them a AuthenticationClass, that says them e.g. what LDAP or PKI is used.
 
 == Decision Outcome
 
-[1] Discovery Object: TODO
-[2] TLS: TODO
-[3] Authentication: TODO
+[1] Discovery Object: `Discovery Object: Use dedicated CRD object for every product - in combination with ConfigMap`
+[2] Server tls cert: TODO
+[3] Authentication: `Authentication: Add AuthenticationClass to Discovery Config`
 
 === Appendix A
 Let's model a kerberos secured HDFS with the Options "TLS: Include caCert in Discovery config" and "Authentication: Add needed details"