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

Add basic credentials/authN to direct connections #124

Closed
rhauch opened this issue Oct 26, 2024 · 0 comments · Fixed by #152
Closed

Add basic credentials/authN to direct connections #124

rhauch opened this issue Oct 26, 2024 · 0 comments · Fixed by #152
Assignees

Comments

@rhauch
Copy link
Contributor

rhauch commented Oct 26, 2024

Continuation of #96, #122 and #123, and followup to #116 that added initial and limited direct connections (with GraphQL support) but did not support credentials and only worked with Kafka clusters and SR clusters that did not require authN.

We want to add basic credentials to direct connections via the kafka_cluster and schema_registry definitions, and use this to connect to downstream resources (e.g., Kafka REST API, SR API endpoints, and Message Consumer API). All secrets in credentials will be write-only: the Connection REST API endpoints must always mask secrets.

We'll track adding support for other credential types in separate issues:

rhauch added a commit that referenced this issue Oct 26, 2024
## Summary of Changes

Resolves #96

This adds the _initial but limited_ support for a new type of connection resources, called `DIRECT` connections, that represent a local or remote Kafka cluster and/or Schema Registry. The extension might create a direct connection to talk to a locally running CP installation, or a remote Apache Kafka cluster with a remote Schema Registry, or even a CCloud cluster with Schema Registry. The user may have to provide credentials to successfully authenticate and connect to each remote service/cluster.

The direction connections are not necessarily the constructs we want to expose. It is likely that near-term versions of the VS Code extension will allow users to connect to local or remote Kafka and SR clusters, but it may also expose to users different kinds of connections that are all represented in the sidecar as direct connections. For example, maybe the VS Code extension allows users to connect to “Confluent Platform”, “Apache Kafka”, and “WarpStream” (in addition to CCloud and local AK+SR), and these might all be customized facades on top of direct connections.

This is the first step in full support for direct connections, and as such only limited functionality is available in this PR:
* CRUDL connection resources with `type=DIRECT`, with validation of fields for each type of connection.
* A direct connection contains zero or one `kafka_cluster` object and zero or one `schema_registry` objects, allowing it to connect to a (remote or local) Kafka cluster and/or a (remote or local) Schema Registry.
* GraphQL support for the Kafka cluster and SR referenced by direct connections.

Once this PR is merged, the following will handled as followups to complete the support for direct connections:
- #123
- #124
- #125 
- #126
- #127

After that, we will have more improvements to better support Confluent Platform: 
- #128

Other improvements may include:
- #129
- #130
- Allow direct connections to connect to Flink cluster and Kafka Connect cluster.

Also, we avoided using `javax` and instead used `jakarta` imports due to the transfer of Java EE to the Eclipse Foundation, where it became Jakarta EE. The older `javax` packages within Java EE have been deprecated. Jakarta EE uses the `jakarta` namespace, which is incompatible with the legacy javax namespace. Mixing the two can cause issues. Several projects and frameworks… have already adopted Jakarta. Additionally, upgrading to Jakarta allows you to keep up with the latest changes and modernize your applications.

https://github.com/jakartaee/platform/blob/main/namespace/mappings.adoc#mapping-javax-to-jakarta


## Pull request checklist

Please check if your PR fulfills the following (if applicable):

- Tests:
    - [x] Added new
    - [x] Updated existing
    - [x] Deleted existing
- [x] Have you validated this change locally against a running instance of the Quarkus dev server?
    ```shell
    make quarkus-dev
    ```
- [x] Have you validated this change against a locally running native executable?
    ```shell
    make mvn-package-native && ./target/ide-sidecar-*-runner
    ```
@rhauch rhauch self-assigned this Oct 26, 2024
rhauch added a commit that referenced this issue Nov 4, 2024
… connections (#141)

Resolves #122

Modified the `ClusterStrategyProcessor` and `ConsumeStrategyProcessor` implementations to support proxying requests to Kafka and SR clusters defined on direct connections.

Limitations: the `status` on direct connections does not yet reflect the ability to connect (see #123), and direct connections do not yet have authentication credentials (see #124).

Also slightly refactors the ITs for these APIs that run against local connections, so that we can easily run these tests against local connections and direct connections.
airlock-confluentinc bot pushed a commit that referenced this issue Nov 8, 2024
…nnections

Resolves #124

Adds basic and API key+secret credentials to direct connections, including validating the credentials in the Connections API and using credentials when connecting to the Kafka cluster and SR defined in the direct connection spec.

This also adds a new `Password` class and `ApiSecret` class that extend a new `Redactable` abstract class representing any literal String value that must be redacted in all API responses and never logged in messages (or output by the sidecar). These are essentially write-only values that prevent reads. It overrides the includes a custom serializer that always writes a _masked_ representation consisting of exactly eight asterisk (`*`) characters _regardless of the actual literal value_. The `toString()` method also outputs the same _masked_ representation, primarily to help prevent sensitive literal values from being included in logs or exception messages. (Note that these behaviors are defined in `Redactable` and marked as final to ensure subclasses do not alter the behavior.)

The `Credentials` interface and `BasicCredentials` and `ApiKeyAndSecret` record types have methods that build the auth-related configuration properties for Kafka clients and SR clients. Each concrete `Credentials` type customizes the logic, though parameters are used to supply information not in the `Credentials` objects.

The `ConnectionState` has helper methods to obtain the `Credentials` for a Kafka cluster with a given ID or a Schema Registry cluster with a given ID. The `DirectConnectionState` subclass always returns the credentials for the one Kafka cluster or one SR cluster. In the future, other `ConnectionState` subclasses (e.g., for CP MDS) might need to maintain a map of credentials by cluster ID for any clusters do not have the same MDS credentials (e.g., the Kafka or SR cluster does not delegate authN functionality to MDS).

And modified the way Kafka admin, consumer and producer clients and the Schema Registry clients are configured, by moving the logic into a new `ClientConfigurator` bean. This encapsulates the logic of building the configuration properties for these clients, by relying on the `Credentials` methods to get only the auth-related config properties and to supply the information needed by those methods from the `KafkaCluster` or `SchemaRegistry` cluster.

The `ClientConfigurator` bean’s methods have a boolean parameter as to whether any redactable values should be redacted rather than use the actual literal secrets. This is so that we can reuse the logic and expose the connection properties to the user, say to allow them to copy the connection properties and use them in their application, or if we use the generated (but redacted) connection configs in the template service.

This builds in support to easily add other types of credentials (see #125, #126, #127) by defining new record types that implement the `Credentials` interface and implementing all methods.
@rhauch rhauch closed this as completed in 0a3f965 Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant