-
Notifications
You must be signed in to change notification settings - Fork 2
Recover old state pre force-push #43
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
Conversation
Co-authored-by: Jake Cahill <45230295+JakeSCahill@users.noreply.github.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThese changes significantly expand the platform's governance, access control, and cloud infrastructure capabilities. The control-plane introduces ACL-based filtering primitives, role-based access control (RBAC) with full CRUD endpoints for roles and permissions, enhanced authentication support (SASL/SCRAM), Prometheus credential retrieval for serverless clusters, Shadow Link v2 models with granular topic synchronization options, and new TLS configuration types. The data-plane introduces MCP (Managed Cloud Platform) server schemas and management endpoints, replacing prior pipeline-oriented constructs with MCP-centric request/response types, alongside ACL filtering, authentication, and shadow-link configuration enhancements. The region manifest adds two new Azure regions with corresponding zones and throughput tiers. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
|
ℹ️ API content change detected: No structural change, nothing to display.
|
|
ℹ️ API content change detected: No structural change, nothing to display.
|
|
🤖 API structural change detected: Added (9)
|
|
ℹ️ API content change detected: No structural change, nothing to display.
|
|
🤖 API structural change detected: Added (7)
Modified (15)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
cloud-controlplane/cloud-controlplane.yaml (6)
4934-4953: Fix response schema: GetNetworkPeering returns Request type.GET /v1/network/{network_id}/network-peerings/{id} should return GetNetworkPeeringResponse, not GetNetworkPeeringRequest.
Apply:
- schema: - $ref: '#/components/schemas/GetNetworkPeeringRequest' + schema: + $ref: '#/components/schemas/GetNetworkPeeringResponse'
4692-4708: Wrong 202 schema for Update Cluster.PATCH /v1/clusters/{cluster.id} returns an Operation; schema currently points to ClusterUpdate.
Use UpdateClusterOperation:
- schema: - $ref: '#/components/schemas/ClusterUpdate' + schema: + $ref: '#/components/schemas/UpdateClusterOperation'
6547-6560: Wrong 202 schema for Update Serverless Cluster.PATCH /v1/serverless/clusters/{id} should return UpdateServerlessClusterOperation, not ServerlessClusterUpdate.
Apply:
- schema: - $ref: '#/components/schemas/ServerlessClusterUpdate' + schema: + $ref: '#/components/schemas/UpdateServerlessClusterOperation'
2287-2293: Network.required references non-existent property.Network.required lists “type”, but the schema defines “cluster_type”.
Apply:
- required: - - name - - resource_group_id - - cloud_provider - - region - - type + required: + - name + - resource_group_id + - cloud_provider + - region + - cluster_type
321-323: Typos and text nits (fix before publishing).
- Line 321: “dns_a_dns_a_recordrecord…” → “dns_a_record…”.
- Lines 1890-1892: “GetAvalableOrganizationsResponse / GetAvalableOrganizations” → “GetAvailable…”.
- Line 2908: “Azure Resoure Groups” → “Azure Resource Groups”.
- Line 4119, 4164, 1286 (elsewhere): “Resoures” → “Resources”.
- Lines 4284-4287: “setn by this cluster” → “sent by this cluster”.
- Lines 3591-3602: “does not effect the state” → “does not affect the state”.
Also applies to: 1890-1892, 2908-2908, 4119-4119, 4164-4164, 4284-4287, 3591-3602
5315-5546: In example: Update operation response uses CreateClusterResponse.For clarity, use UpdateClusterResponse in the “Update done” example to match OperationResponse alternatives.
Suggested change in example only.
🧹 Nitpick comments (15)
cloud-dataplane/cloud-dataplane.yaml (6)
5580-5593: Map query param should declare proper serialization, nottype: string.Expose the tags map with deepObject (or form+explode) so SDKs serialize
filter.tags[key]=valuecorrectly.- - description: Filter MCP servers by tags. MCP servers that match all the provided tags are returned. The query format is "filter.tags[key]=value". - in: query - name: filter.tags[string] - schema: - type: string + - description: Filter MCP servers by tags. MCP servers that match all provided tags are returned. + in: query + name: filter.tags + style: deepObject + explode: true + schema: + type: object + additionalProperties: + type: string
3085-3103: TLS PEM material should be treated as sensitive.Mark
keywrite‑only; consider same forcertif you never return it.TLSPEMSettings: properties: cert: title: The cert - type: string + type: string + writeOnly: true key: title: |- Key and Cert are optional but if one is provided, then both must be The key - type: string + type: string + format: password + writeOnly: true
3064-3077: Doc nit: clarify “both must be” sentence and consider sensitivity for file paths.The title for
key_pathis a run‑on and unclear. Also, confirm whether paths can safely be echoed back. If not, mark readOnly or avoid returning them.
3033-3035: Port should be int32 with range constraints.Ports fit in 16 bits. Tighten the type to avoid surprising overflows in clients.
Source: properties: port: - format: int64 - type: integer + format: int32 + minimum: 0 + maximum: 65535 + type: integerWould you like a quick script to scan for other port fields to align constraints?
665-670: Intervals/durations are plain strings; add a format or pattern.Unformatted strings hinder client validation. Consider a duration format or regex.
- interval: + interval: title: | Sync interval If 0 provided, defaults to 30 seconds - type: string + type: string + pattern: '^[0-9]+(ms|s|m|h)$'Apply similarly across other interval fields.
Also applies to: 2771-2775, 3218-3222
2846-2950: Consistency: many “effective_*” fields mirror configured values.Good addition. Consider marking them
readOnly: trueconsistently (some already are) and add explicit units (ms/bytes) in descriptions for every field to prevent ambiguity.cloud-controlplane/cloud-controlplane.yaml (9)
731-733: Inconsistent global-access property name.Cluster uses gcp_global_access_enabled; ClusterCreate uses gcp_enable_global_access. Pick one name and deprecate the other to avoid client confusion.
Also applies to: 490-494
6510-6514: Response shape inconsistency (wrapper vs resource).
- GET /v1/clusters/{id} returns GetClusterResponse (wrapper).
- GET /v1/serverless/clusters/{id} returns ServerlessCluster (raw).
Prefer a consistent pattern across APIs. Either wrap both or return raw resources for both.
Also applies to: 4778-4838
258-265: "oneof" mentioned but not enforced.AuthenticationConfiguration text says “oneof” but schema doesn’t use oneOf/anyOf. Add oneOf for future extensibility or reword.
43-45: Cosmetic: stray slash in titles.Titles read “/ The …”. Remove the leading slash for ACLOperation, ACLPattern, ACLPermissionType, ACLResource.
Also applies to: 57-59, 65-66, 85-86
3527-3560: TLS settings: clarify mutual exclusivity and validation.Consider a oneOf between tls_file_settings and tls_pem_settings, and add dependentRequired (if key then cert). Helps SDKs enforce correct usage.
Also applies to: 3957-3970
656-666: Prometheus descriptor consistency.Cluster.Prometheus and ServerlessCluster.Prometheus are readOnly objects with urls. Ensure examples and earlier public docs are updated to the nested shape.
Also applies to: 3170-3184
4392-4560: HTTP codes and bodies: confirm consistency.For list/create/update/delete across resources:
- 201 vs 202 usage is good; ensure examples’ schemas match the declared schemas (several mismatches above).
- Double-check all 202 responses point to the corresponding *Operation schema.
Also applies to: 4605-4724, 6456-6536, 6582-6625
1-20: ACL filters: consider required fields.As modeled, empty ACLFilter is valid. If at least one of access_filter/resource_filter is required, add minProperties/anyOf.
Also applies to: 88-101
256-265: Shadow Link sync options: enforce intervals and booleans.Add constraints:
- interval: pattern for durations or integer seconds
- effective_interval: readOnly is good; keep it non-required
- paused defaults? Consider default: false
Also applies to: 3049-3072, 986-1010, 3588-3669
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
cloud-controlplane/cloud-controlplane.yaml(41 hunks)cloud-controlplane/x-topics/cloud-regions.md(2 hunks)cloud-dataplane/cloud-dataplane.yaml(46 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-26T19:18:28.591Z
Learnt from: kbatuigas
Repo: redpanda-data/api-docs PR: 40
File: cloud-controlplane/cloud-controlplane.yaml:1733-1738
Timestamp: 2025-11-26T19:18:28.591Z
Learning: For the redpanda-data/api-docs repository, the OpenAPI specification files in cloud-controlplane/ and cloud-dataplane/ are auto-generated from proto source files. Changes to these specs should be made in the upstream proto sources rather than directly editing the generated OpenAPI YAML files.
Applied to files:
cloud-dataplane/cloud-dataplane.yaml
🔇 Additional comments (4)
cloud-controlplane/x-topics/cloud-regions.md (3)
833-845: Verify this change against the auto-generation configuration.The file header (lines 2-11) explicitly states that this is auto-generated content and should not be edited manually. Adding
germanywestcentralmanually risks the change being lost on the next regeneration.Ensure that the region data source (referenced in the
doc-tools generate cloud-regionscommand) has been updated with this region definition, or document why this is a temporary manual addition.
872-884: Verify this change against the auto-generation configuration.The file header (lines 2-11) explicitly states that this is auto-generated content and should not be edited manually. Adding
swedencentralmanually risks the change being lost on the next regeneration.Ensure that the region data source (referenced in the
doc-tools generate cloud-regionscommand) has been updated with this region definition, or document why this is a temporary manual addition.
833-845: No action required—Azure region and zone naming are correct.The region identifiers
germanywestcentralandswedencentralare valid Azure regions. The zone naming convention used (germanywestcentral-az1,germanywestcentral-az2,germanywestcentral-az3) matches Azure's physical availability zone identifier format.cloud-dataplane/cloud-dataplane.yaml (1)
8025-8028: Tag taxonomy looks clear.New “Remote MCP” and “Monitoring” tags help navigation. LGTM.
Also applies to: 5634-5635, 5716-5717, 5857-5858, 5901-5903
| properties: | ||
| description: | ||
| description: The description of the role. | ||
| example: Billing Admins have access to billing information. | ||
| type: string | ||
| name: | ||
| description: The unique name of the role. | ||
| example: billing_admin | ||
| type: string | ||
| permissions: | ||
| items: | ||
| type: string | ||
| type: array | ||
| type: object | ||
| RoleUpdate: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RoleCreate/RoleUpdate should require name.
Without required fields, empty bodies pass validation. Mark name as required; consider validating permissions exist.
Apply:
RoleCreate:
properties:
@@
type: object
+ required:
+ - name
RoleUpdate:
properties:
@@
type: object
+ required:
+ - nameAlso applies to: 2934-2948
🤖 Prompt for AI Agents
In cloud-controlplane/cloud-controlplane.yaml around lines 2919-2933 (and also
apply the same change to lines 2934-2948), the RoleCreate/RoleUpdate schemas do
not mark the name field as required which allows empty request bodies to pass
validation; update both RoleCreate and RoleUpdate schemas to include a required
array that contains "name" (i.e., add required: ["name"]) and, where permissions
are present, add a validation note to ensure permissions reference valid
permission identifiers (e.g., enum or external ref) so callers cannot submit
non-existent permissions.
| password: | ||
| title: Password | ||
| type: string | ||
| password_set: | ||
| readOnly: true | ||
| title: Indicates that the password has been set | ||
| type: boolean | ||
| password_set_at: | ||
| format: date-time | ||
| readOnly: true | ||
| title: |- | ||
| Timestamp of when the password was last set - only valid if password_set | ||
| is true | ||
| type: string | ||
| scram_mechanism: | ||
| $ref: '#/components/schemas/ScramMechanism' | ||
| username: | ||
| title: SCRAM username | ||
| type: string | ||
| title: SCRAM settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mark secrets with password format and writeOnly where applicable.
- ScramConfig.password should be format: password and writeOnly: true.
- PrometheusCredentials.password should be format: password (writeOnly false is fine since it’s a read API).
- ServiceAccountCredentials.client_secret should be format: password; consider writeOnly unless returned by the API.
Apply:
# ScramConfig
- password:
- title: Password
- type: string
+ password:
+ title: Password
+ type: string
+ format: password
+ writeOnly: true # PrometheusCredentials
- password:
- type: string
+ password:
+ type: string
+ format: password # ServiceAccountCredentials
- client_secret:
- description: The client secret
- example: dYz_FZqNM6E3_O1lKUwONKNQ4TKf7i
- nullable: true
- type: string
+ client_secret:
+ description: The client secret
+ example: dYz_FZqNM6E3_O1lKUwONKNQ4TKf7i
+ nullable: true
+ type: string
+ format: passwordAlso applies to: 2695-2700, 3321-3325
| properties: | ||
| high_watermark: | ||
| format: int64 | ||
| title: Shadowed partition's HWM | ||
| type: string | ||
| partition_id: | ||
| format: int64 | ||
| title: Partition ID | ||
| type: string | ||
| source_high_watermark: | ||
| format: int64 | ||
| title: Source partition's HWM | ||
| type: string | ||
| source_last_stable_offset: | ||
| format: int64 | ||
| title: Source partition's LSO | ||
| type: string | ||
| source_last_updated_timestamp: | ||
| format: date-time | ||
| title: Timestamp of the last time the source partition information was updated | ||
| type: string | ||
| title: Topic partition information | ||
| type: object | ||
| UAISpec: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and its size
wc -l cloud-controlplane/cloud-controlplane.yamlRepository: redpanda-data/api-docs
Length of output: 113
🏁 Script executed:
# Search for the pattern: format: int64 combined with type: string
# We'll look for these fields that are likely consecutive in YAML
rg -A 2 "format: int64" cloud-controlplane/cloud-controlplane.yaml | grep -B 2 "type: string"Repository: redpanda-data/api-docs
Length of output: 539
🏁 Script executed:
# Get line numbers and context for all occurrences
rg -n -B 3 -A 1 "format: int64" cloud-controlplane/cloud-controlplane.yaml | grep -B 3 -A 1 "type: string"Repository: redpanda-data/api-docs
Length of output: 696
🏁 Script executed:
# Get more comprehensive view with better formatting
rg -n "format: int64" cloud-controlplane/cloud-controlplane.yaml | head -20Repository: redpanda-data/api-docs
Length of output: 239
🏁 Script executed:
# Check context for the two additional occurrences at lines 2763 and 2811
sed -n '2760,2815p' cloud-controlplane/cloud-controlplane.yamlRepository: redpanda-data/api-docs
Length of output: 2486
🏁 Script executed:
# Get context for line 2763 to identify the field name
sed -n '2755,2775p' cloud-controlplane/cloud-controlplane.yamlRepository: redpanda-data/api-docs
Length of output: 1140
🏁 Script executed:
# Get more context to find the field name for line 2763
sed -n '2745,2770p' cloud-controlplane/cloud-controlplane.yamlRepository: redpanda-data/api-docs
Length of output: 1267
🏁 Script executed:
# Check the source_last_updated_timestamp field (shown in the code snippet)
sed -n '3685,3695p' cloud-controlplane/cloud-controlplane.yamlRepository: redpanda-data/api-docs
Length of output: 453
int64 fields modeled as strings throughout the file.
Multiple fields incorrectly combine format: int64 with type: string. In OAS 3, int64 implies type: integer. Identified issues:
- TopicPartitionInformation (lines 3673, 3677, 3681, 3685): high_watermark, partition_id, source_high_watermark, source_last_stable_offset
- QuotaFailure (lines 2763, 2811): future_quota_value, quota_value
All should use type: integer with format: int64:
- high_watermark:
- format: int64
- title: Shadowed partition's HWM
- type: string
+ high_watermark:
+ type: integer
+ format: int64
+ title: Shadowed partition's HWM
@@
- partition_id:
- format: int64
- title: Partition ID
- type: string
+ partition_id:
+ type: integer
+ format: int64
+ title: Partition ID
@@
- source_high_watermark:
- format: int64
- title: Source partition's HWM
- type: string
+ source_high_watermark:
+ type: integer
+ format: int64
+ title: Source partition's HWM
@@
- source_last_stable_offset:
- format: int64
- title: Source partition's LSO
- type: string
+ source_last_stable_offset:
+ type: integer
+ format: int64
+ title: Source partition's LSO
@@
- future_quota_value:
- format: int64
- nullable: true
- type: string
+ future_quota_value:
+ type: integer
+ format: int64
+ nullable: true
@@
- quota_value:
- format: int64
- type: string
+ quota_value:
+ type: integer
+ format: int64Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In cloud-controlplane/cloud-controlplane.yaml around lines 3671-3694 and also
update QuotaFailure entries around lines 2763 and 2811, several int64 fields are
declared with type: string; change those fields (TopicPartitionInformation:
high_watermark at ~3673, partition_id ~3677, source_high_watermark ~3681,
source_last_stable_offset ~3685, and QuotaFailure: future_quota_value ~2763,
quota_value ~2811) to use type: integer and keep format: int64 so they conform
to OAS3 integer/int64 semantics; ensure no other attributes are altered and run
schema validation after edits.
| "200": | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/PrometheusCredentials' | ||
| description: OK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the defined wrapper for Prometheus credentials.
You defined GetPrometheusCredentialsResponse but the endpoint returns PrometheusCredentials directly. Align to the wrapper or remove the wrapper.
Suggested fix:
- schema:
- $ref: '#/components/schemas/PrometheusCredentials'
+ schema:
+ $ref: '#/components/schemas/GetPrometheusCredentialsResponse'Also applies to: 1731-1735
🤖 Prompt for AI Agents
In cloud-controlplane/cloud-controlplane.yaml around lines 6592-6597 (and
similarly at lines 1731-1735), the operation response references
PrometheusCredentials directly but you have a defined wrapper schema
GetPrometheusCredentialsResponse; update the response schema to $ref the wrapper
(e.g. '#/components/schemas/GetPrometheusCredentialsResponse') or remove the
wrapper definition if you intend to return the raw PrometheusCredentials, and
ensure both locations are changed consistently so the OpenAPI spec uses the
wrapper schema where expected.
| ACLAccessFilter: | ||
| properties: | ||
| host: | ||
| title: |- | ||
| The host to match. If not set, will default to match all hosts | ||
| with the specified `operation` and `permission_type`. Note that | ||
| the asterisk `*` is literal and matches hosts that are set to `*` | ||
| type: string | ||
| operation: | ||
| $ref: '#/components/schemas/ACLOperation' | ||
| permission_type: | ||
| $ref: '#/components/schemas/ACLPermissionType' | ||
| principal: | ||
| title: |- | ||
| The name of the principal, if not set will default to match | ||
| all principals with the specified `operation` and `permission_type` | ||
| type: string | ||
| title: Filter an ACL based on its access | ||
| type: object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Heads‑up: generated file — make changes in proto, not YAML.
This spec is auto‑generated from protos. Please apply fixes in upstream proto definitions and regenerate, not by hand here. Based on learnings, ...
🤖 Prompt for AI Agents
In cloud-dataplane/cloud-dataplane.yaml lines 21–39: this file is generated from
proto sources and the reviewer warns not to edit the YAML directly; instead
update the upstream proto that defines the ACLAccessFilter message (adjust the
field comments/descriptions for host and principal to reflect default behavior
and note that '*' is literal), then run the generation/regeneration pipeline to
produce the corrected YAML; do not hand-edit this YAML file.
| ListMCPServersRequest.Filter: | ||
| properties: | ||
| display_name_contains: | ||
| description: Filter MCP servers by display name. Any MCP servers that partially match this name are returned. | ||
| type: string | ||
| secret_id: | ||
| description: Filter MCP servers by secret ID. | ||
| type: string | ||
| tags[string]: | ||
| additionalProperties: | ||
| type: string | ||
| description: Filter MCP servers by tags. MCP servers that match all the provided tags are returned. The query format is "filter.tags[key]=value". | ||
| type: object | ||
| type: object | ||
| ListMCPServersResponse: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Invalid schema property name: tags[string] will break codegen.
Within a JSON schema object, property keys must be literal field names. Use tags with additionalProperties instead; keep the bracketed form only in query param names if you must.
- ListMCPServersRequest.Filter:
+ ListMCPServersRequest.Filter:
properties:
- tags[string]:
+ tags:
additionalProperties:
type: string
description: Filter MCP servers by tags. MCP servers that match all the provided tags are returned. The query format is "filter.tags[key]=value".
type: object📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ListMCPServersRequest.Filter: | |
| properties: | |
| display_name_contains: | |
| description: Filter MCP servers by display name. Any MCP servers that partially match this name are returned. | |
| type: string | |
| secret_id: | |
| description: Filter MCP servers by secret ID. | |
| type: string | |
| tags[string]: | |
| additionalProperties: | |
| type: string | |
| description: Filter MCP servers by tags. MCP servers that match all the provided tags are returned. The query format is "filter.tags[key]=value". | |
| type: object | |
| type: object | |
| ListMCPServersResponse: | |
| ListMCPServersRequest.Filter: | |
| properties: | |
| display_name_contains: | |
| description: Filter MCP servers by display name. Any MCP servers that partially match this name are returned. | |
| type: string | |
| secret_id: | |
| description: Filter MCP servers by secret ID. | |
| type: string | |
| tags: | |
| additionalProperties: | |
| type: string | |
| description: Filter MCP servers by tags. MCP servers that match all the provided tags are returned. The query format is "filter.tags[key]=value". | |
| type: object | |
| type: object | |
| ListMCPServersResponse: |
🤖 Prompt for AI Agents
In cloud-dataplane/cloud-dataplane.yaml around lines 1628 to 1642 the JSON
schema uses an invalid property key `tags[string]` which will break codegen;
replace that property name with a literal `tags` property and move the
array/object shape into its value (keep `additionalProperties: type: string` and
description), so the schema uses `tags` as the property key and retains
`additionalProperties` to represent arbitrary tag key/value pairs; update the
description to note the query form "filter.tags[key]=value" if needed.
| MCPServer: | ||
| description: Defines the MCP server resource. | ||
| properties: | ||
| description: | ||
| description: Optional MCP server description. | ||
| type: string | ||
| display_name: | ||
| description: User-friendly MCP server name. | ||
| type: string | ||
| id: | ||
| description: MCP Server ID. | ||
| type: string | ||
| resources: | ||
| $ref: '#/components/schemas/Resources' | ||
| service_account: | ||
| $ref: '#/components/schemas/MCPServer.ServiceAccount' | ||
| state: | ||
| $ref: '#/components/schemas/MCPServer.State' | ||
| status: | ||
| $ref: '#/components/schemas/MCPServer.Status' | ||
| tags: | ||
| additionalProperties: | ||
| type: string | ||
| description: |- | ||
| Tags are key-value pairs that can be assigned to a MCP server resource. | ||
| They help organize MCP servers and enable filtering when listing them. | ||
| type: object | ||
| tools: | ||
| additionalProperties: | ||
| $ref: '#/components/schemas/Tool' | ||
| description: The MCP server configuration. | ||
| type: object | ||
| url: | ||
| readOnly: true | ||
| title: URL to connect to the MCP server | ||
| type: string | ||
| required: | ||
| - id | ||
| - display_name | ||
| - tools | ||
| - service_account | ||
| type: object | ||
| MCPServer.ServiceAccount: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Secret exposure risk in MCP server read model.
MCPServer.service_account includes client_secret and appears in GET responses. Mark secrets write‑only (and preferably split request/response models) to prevent accidental leakage.
MCPServer.ServiceAccount:
properties:
client_id:
description: The service account client ID for authenticating with Redpanda Cloud.
type: string
client_secret:
description: The service account client secret for authenticating with Redpanda Cloud.
- type: string
+ type: string
+ format: password
+ writeOnly: trueOptionally, exclude the entire service_account from the read model and keep it only in MCPServerCreate/MCPServerUpdate. Apply upstream.
Also applies to: 1975-1986
🤖 Prompt for AI Agents
In cloud-dataplane/cloud-dataplane.yaml around lines 1932-1974 (and also apply
the same change to 1975-1986), the MCPServer.service_account currently exposes
secrets (e.g., client_secret) in the read model; update the OpenAPI schemas so
secrets are not returned: either mark secret properties (client_secret and any
other credentials) with writeOnly: true in the MCPServer.ServiceAccount schema
or split into separate request/response schemas (e.g.,
MCPServerCreate/MCPServerUpdate include the secret, MCPServer (response) omits
it). Ensure the MCPServer properties reference the sanitized response schema (no
secret fields) and keep secrets only in create/update request schemas, and apply
the same change for the repeated blocks at lines ~1975-1986.
| properties: | ||
| client_id: | ||
| type: string | ||
| client_secret: | ||
| type: string | ||
| type: object | ||
| Pipeline.State: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pipeline service account also contains secrets; make them write‑only.
Same issue for pipelines. Mark client_secret writeOnly (and consider separate request/response DTOs).
Pipeline.ServiceAccount:
properties:
client_id:
type: string
client_secret:
- type: string
+ type: string
+ format: password
+ writeOnly: trueAlso applies to: 2320-2320, 2343-2344
🤖 Prompt for AI Agents
In cloud-dataplane/cloud-dataplane.yaml around lines 2274-2280 (and also at
lines 2320 and 2343-2344), the Pipeline service account schema exposes
client_secret; make the secret write-only by adding writeOnly: true to the
client_secret property in the schema(s). Update the OpenAPI objects for
request/response or create separate request DTOs that include client_secret
while response DTOs omit or mark it writeOnly, ensuring generated clients and
docs treat the secret as write-only.
| properties: | ||
| password: | ||
| title: Password | ||
| type: string | ||
| password_set: | ||
| readOnly: true | ||
| title: Indicates that the password has been set | ||
| type: boolean | ||
| password_set_at: | ||
| format: date-time | ||
| readOnly: true | ||
| title: |- | ||
| Timestamp of when the password was last set - only valid if password_set | ||
| is true | ||
| type: string | ||
| username: | ||
| title: PLAIN username | ||
| type: string | ||
| title: PLAIN settings | ||
| type: object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passwords should be write‑only and typed as secrets.
Mark these credentials accordingly to keep them out of responses and enable better client validation.
PlainConfig:
properties:
password:
title: Password
- type: string
+ type: string
+ format: password
+ writeOnly: true
...
ScramConfig:
properties:
password:
title: Password
- type: string
+ type: string
+ format: password
+ writeOnly: trueAlso applies to: 2710-2741
🤖 Prompt for AI Agents
In cloud-dataplane/cloud-dataplane.yaml around lines 2365-2384 (and also apply
the same change to 2710-2741), the password property is currently defined as a
plain string and may be exposed in responses; make credentials write-only and
typed as secrets by adding writeOnly: true and setting format: password (or a
project-specific secret format) to the password property, ensure any response
schemas do not include the password field, and keep password_set and
password_set_at as readOnly booleans/timestamps as-is.
| readOnly: true | ||
| title: |- | ||
| The Client ID for the Kafka RPC requests setn by this cluster to the | ||
| source cluster | ||
| type: string | ||
| port: | ||
| format: int64 | ||
| connection_timeout_ms: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: “setn” → “sent”.
- The Client ID for the Kafka RPC requests setn by this cluster to the
+ The Client ID for the Kafka RPC requests sent by this cluster to the🤖 Prompt for AI Agents
In cloud-dataplane/cloud-dataplane.yaml around lines 2855 to 2860, fix the typo
in the title field by changing “setn” to “sent” so the title reads: "The Client
ID for the Kafka RPC requests sent by this cluster to the source cluster".
micheleRP
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Merge commits for Cloud APIs before force push (#40)