Add `buf-plugin-required-fields` plugin #2

areina · 2025-04-02T14:01:24Z

Add a plugin that checks that:

entity-related messages (e.g: Cluster) define a known set of common fields: [id, name, account_id, created_at].
Request messages (e.g: ListClusters) define a known set of common fields: [account_id].

The list of entity required fields can be configured using the plugin options.

areina · 2025-04-02T14:04:12Z

@Robert-Stam @mikheillomidzeq executing it against the current public api, I get these linting issues:

proto/qdrant/cloud/auth/v2/auth.proto:88:1:"ManagementKey" is missing required fields: [name] (buf-plugin-required-fields)
proto/qdrant/cloud/booking/v2/booking.proto:83:1:"Package" is missing required fields: [account_id created_at] (buf-plugin-required-fields)
proto/qdrant/cloud/cluster/auth/v2/database_api_key.proto:88:1:"DatabaseApiKey" is missing required fields: [name] (buf-plugin-required-fields)
proto/qdrant/cloud/cluster/backup/v1/backup.proto:294:1:"BackupSchedule" is missing required fields: [name] (buf-plugin-required-fields)
proto/qdrant/cloud/cluster/v2/cluster.proto:540:1:"QdrantRelease" is missing required fields: [id name account_id created_at] (buf-plugin-required-fields)
proto/qdrant/cloud/platform/v1/platform.proto:61:1:"CloudProvider" is missing required fields: [account_id created_at] (buf-plugin-required-fields)
proto/qdrant/cloud/platform/v1/platform.proto:71:1:"CloudProviderRegion" is missing required fields: [name account_id created_at] (buf-plugin-required-fields)

There are three options:

Update the public api adding the fields (I'm not sure that makes sense for all them, it would be good to review it anyway)
Relax the list of required fields (the only one that we might remove is name)
Add comments where necessary in the proto files to ignore the linting rule

Add a plugin that checks that: - entity-related messages (e.g: Cluster) define a known set of common fields for the Qdrant Cloud API: `[id, name, account_id, created_at]`. - Request messages (e.g: ListClusters) define a known set of common fields for the Qdrant Cloud API: `[account_id]`.

cmd/buf-plugin-required-fields/main.go

Robert-Stam

Small comments, further LGTM.

what about adding an annotation (extension) to proto which we can use to 'override' the default names per message, we we can configure the linter if we like?

areina · 2025-04-02T15:00:21Z

Small comments, further LGTM.

what about adding an annotation (extension) to proto which we can use to 'override' the default names per message, we we can configure the linter if we like?

We could do that. Another option if we don't want to clutter the proto definitions is to enable that kind of configuration using the plugins option. Could be something like (for some reason they don't support a map and we would need a little hack using |):

 - plugin: buf-plugin-required-fields
    options:
      required_entity_fields:
        - ManagementKey|id,account_id,created_at

I think for the current linting issues we have we could just add a comment in the proto file to ignore them and in the future we can implement one of the two options (proto extension or plugin config).

Example ignoring linting in proto definition:

// buf:lint:ignore QDRANT_CLOUD_REQUIRED_ENTITY_FIELDS
message ManagementKey {
   ...
}

Robert-Stam · 2025-04-02T16:50:09Z

// buf:lint:ignore QDRANT_CLOUD_REQUIRED_ENTITY_FIELDS

Fine with // buf:lint:ignore QDRANT_CLOUD_REQUIRED_ENTITY_FIELDS

Robert-Stam · 2025-04-02T16:51:29Z

cmd/buf-plugin-required-fields/main.go

+// Package main implements a plugin that checks that:
+// - entity-related messages (e.g: Cluster) define a known set of common fields
+// for the Qdrant Cloud API. Default values: id, name, account_id, created_at
+// - Request messages (e.g: ListClusters) define a known set of common fields


ListClusters --> ListClustersRequest

Robert-Stam

one remark, further LGTM

areina self-assigned this Apr 2, 2025

areina requested a review from a team as a code owner April 2, 2025 14:01

areina force-pushed the feat/toni/add-required-fields-plugin branch from 8488e4b to 092c8ef Compare April 2, 2025 14:08