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 PATCH for connections #168

Closed
rhauch opened this issue Nov 15, 2024 · 1 comment
Closed

Add PATCH for connections #168

rhauch opened this issue Nov 15, 2024 · 1 comment
Assignees

Comments

@rhauch
Copy link
Contributor

rhauch commented Nov 15, 2024

We've added basic and API key credentials (#124) for connections, and any secrets in the credentials are write-only. For example, the following can be provided in the request payload to create (POST) a new connection resource:

{
    "name": "Dev 1",
    "type": "DIRECT",
    "kafka_cluster": {
        "id": "kafka-1",
        "bootstrap_servers": "broker_host:9092",
        "credentials": {
            "username": "my-username",
            "password": "secret-password"
        }
    },
    "schema_registry": {
        "id": "sr-1",
        "uri": "https://schema-registry-host:8081",
        "credentials": {
            "username": "my-sr-username",
            "password": "secret-sr-password"
        }
    }
}

The response for that POST request will have the secrets redacted/masked:

{
  "api_version": "gateway/v1",
  "kind": "ConnectionsList",
  "metadata": {
    "self": "http://localhost:26636/gateway/v1/connections",
    "next": null,
    "total_size": 1
  },
  "data": [
    {
      "api_version": "gateway/v1",
      "kind": "Connection",
      "id": "7329dc3c-2f89-4357-a823-76c98f871d7c",
      "metadata": {
        "self": "http://localhost:26636/gateway/v1/connections/7329dc3c-2f89-4357-a823-76c98f871d7c",
        "resource_name": null
      },
      "spec": {
        "id": "7329dc3c-2f89-4357-a823-76c98f871d7c",
        "name": "Dev 1",
        "type": "DIRECT",
        "kafka_cluster": {
          "id": "kafka-1",
          "bootstrap_servers": "broker_host:9092",
          "credentials": {
            "username": "my-username",
            "password": "********"
          }
        },
        "schema_registry": {
          "id": "sr-1",
          "uri": "https://schema-registry-host:8081",
          "credentials": {
            "username": "my-sr-username",
            "password": "********"
          }
        }
      },
      "status": {
        "authentication": {
          "status": "NO_TOKEN"
        },
        "kafka_cluster": {
          "state": "SUCCESS"
        },
        "schema_registry": {
          "state": "SUCCESS"
        }
      }
    }
  ]
}

If we want to modify the connection with PUT, we'd have to provide the whole spec object again -- including the correct secret values. We can't take the result of a GET and just modify it, unless we also replace the masked values with the actual values.

We should implement PATCH so that a client can modify parts a connection spec without having to specify everything. There are two options for the payload to the PATCH operation:

  1. JSON-Patch -- uses a JSON file containing an array of patch operations: “add”, “remove”, “replace”, “move”, “copy” and “test”. The operations are applied in same order they appear in the file: if any of them fail then the whole patch operation should abort.
  2. JSON Merge Patch -- describes changes to be made to a target JSON document using a syntax that closely mimics the document being modified.

Ideally we'd support both (especially if Quarkus makes this easy), but JSON-Patch is preferred since it is generally more flexible and less ambiguous than JSON Merge Patch, which depends upon the implementation for consistently handling null values to distinguish between "don't change" and "remove".

It looks like Quarkus supports JSON Merge Patch (RFC 7396), though there is an open issue that reports that JSON-Patch works with an imperative endpoint but does not work with a reactive endpoint.

@Cerchie Cerchie self-assigned this Dec 18, 2024
@rhauch rhauch removed their assignment Dec 22, 2024
@Cerchie Cerchie mentioned this issue Dec 27, 2024
5 tasks
@Cerchie
Copy link
Contributor

Cerchie commented Jan 15, 2025

closed via #264

@Cerchie Cerchie closed this as completed Jan 15, 2025
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