GeezerVison is showing github.com — ADRXXX: Service/Product Discovery (Revision) by sbernauer · Pull Request #442 · stackabletech/documentation
Conversation
sbernauer
commented
Aug 24, 2023
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Continued on on-site 2023-08-23
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
netlify
Bot
commented
Aug 24, 2023
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Techassi
changed the title
ADR on Discovery 2.0
ADRXXX: Service/Product Discovery (Revision)
Aug 25, 2023
|
I would disagree with the entire premise of this.
- Discovery option 3: who would still be using the
HdfsClusterDiscovery? They'd still be subject to the same versioning nightmare of 2.
- Discovery options 2/3: moving more types into op-rs adds a lot of overhead when changing them (especially since this still needs to be coordinated with the owning operators).
- TLS/authnclass: Having to read this from the discovery objects adds a lot of complexity and overhead to the consuming operators. Especially when the expected use case is that the whole cluster exists in the same TLS PKI/Kerberos realm. Overriding the secretclasses should be the exceptional case.
- TLS/authn: Negotiating the precise authentication method is largely up to the product in question. I'd expect it to be more along the lines of the client specifying "when using this method, get the credential this way". Knowing that passwords are validated against LDAP is useless for the client, they just want a valid username/password.
- TLS/authn 2: There is no way to exhaustively know the type of credential that a
SecretClass provisions.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@nightkr What do you mean with "I would disagree with the entire premise of this." What's the premise?
I also read it now, and wanted to share some thoughts too. I think the ease of use of the ConfigMap is actually really nice. I think most discovery configmaps contain connect strings or files, and those can then either be mounted as an env var or as files, so it's very easy to use.
The document mentions that cross-namespace is an issue with ConfigMaps, and also name clashes. If these are problems, I'd like to see these aspects mentioned as decision drivers in the earlier section of the ADR. I personally do not think that these two things are very important.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
What's the premise?
The primary driver for this (motivations 2/3) seem to be authentication. I disagree that discovery is the right place to tackle either.
The document mentions that cross-namespace is an issue with ConfigMaps, and also name clashes. If these are problems, I'd like to see these aspects mentioned as decision drivers in the earlier section of the ADR. I personally do not think that these two things are very important.
Agreed that both of these are effectively treated as decision drivers looking at the rest of the document. I'd also disagree with the document about discovery being the right place to tackle either of those either.
Cross-namespace use: Kubernetes namespaces are effectively a tenancy border, we should be very careful blurring that line. To me, cross-namespace use should be something you request explicitly, and which is ideally isolated from other "tenants" (ZookeeperZnode ticks both of these boxes, but isolation is easier done for a storage system like ZK or HDFS than something like Trino).
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Hmm, can you elaborate why the discovery mechanism shouldn't also contain information on the authentication mechanism? It makes sense to me, it's information that a client would need when connecting to the service.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Cross-namespace use: Kubernetes namespaces are effectively a tenancy border, we should be very careful blurring that line. To me, cross-namespace use should be something you request explicitly, and which is ideally isolated from other "tenants" (ZookeeperZnode ticks both of these boxes, but isolation is easier done for a storage system like ZK or HDFS than something like Trino).
The discovery-operator proposal is partly based on the ZNode logic. E.g. for deploying / mounting configmaps across namespaces.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
It sounds to me like this is worth an online debate?
Would everybody be happy for me to organize and prepare a meeting to discuss?
|
maltesander, fhennig, and Techassi reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This branch has not been deployed
No deployments
Uh oh!
There was an error while loading. Please reload this page.
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|