Repository navigation
dbaas: add acl list, create, delete and update commands - #923
pierre-emmanuelJ wants to merge 2 commits into
Conversation
Manage the ACLs of Kafka (topic and Schema Registry) and OpenSearch services under `exo dbaas acl`, with the service type detected from the service name. `acl show` now also covers these two types. Kafka entries cannot be updated and ClickHouse ACLs are read-only: the API has no call for either. [sc-104243] AI-assisted: true Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AI-assisted: true Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| // updateOpensearchAclConfig replaces the ACL configuration of an OpenSearch | ||
| // Database Service: the API only exposes it as a whole. | ||
| func updateOpensearchAclConfig(ctx context.Context, client *v3.Client, name string, config v3.DBAASOpensearchAclConfig, message string) error { | ||
| op, err := client.UpdateDBAASOpensearchAclConfig(ctx, name, config) |
There was a problem hiding this comment.
I think we have an omitempty gremlin here.
The API insists on getting acls, but the generated tag drops it when the list is empty. That hits both a fresh config and deleting the final ACL, and the live e2e is currently falling over with Invalid value in [3].acls - should be a Collection.
Would it make sense to mark the field as required in the DBaaS source schema, regenerate egoscale, and turn nil into [] before this PUT?
| case "kafka": | ||
| return c.deleteKafka(ctx, client) | ||
| case "opensearch": | ||
| return c.deleteOpensearch(ctx, client) |
There was a problem hiding this comment.
I think this has a slightly alarming trapdoor.
For OpenSearch, --kafka-topic, --permission, and extra positional arguments are accepted but ignored. That leaves OpensearchIndex empty, which means delete every ACL for this user.
Could we reject those selectors before dispatch, so a Kafka-shaped typo cannot turn into delete-all?
| return errors.New("--opensearch-index is required for an opensearch service") | ||
| } | ||
|
|
||
| config, err := client.GetDBAASOpensearchAclConfig(ctx, c.Name) |
There was a problem hiding this comment.
Is there some API-side revision, ETag, or locking magic here that I'm missing?
Each command reads the full config, tweaks one entry, then sends the whole thing back. Two commands can both succeed, then whichever PUT lands last quietly eats the other change.
If there isn't a CAS guarantee, I think OpenSearch writes need an atomic API operation before we expose them here.
natalie-o-perret
left a comment
natalie-o-perret
left a comment
There was a problem hiding this comment.
These gremlins look blocking to me, especially the invalid empty-ACL payload, the delete-all trapdoor, and the lost-update path. Requesting changes until those are nailed down.
This branch has not been deployed
2 participants
Description
exo dbaas aclonly hadshow, for ClickHouse. This adds the rest of the tree for the services whose ACLs the API lets us manage. [sc-104243]exo dbaas acl list|create|delete NAME ...for Kafka (topic and Schema Registry entries) and OpenSearch (index entries);acl updatefor OpenSearch (ACL toggles, permission of an entry);acl shownow also covers Kafka and OpenSearch. The service is a positional name and its type is detected, as inexo dbaas user.acl showon a ClickHouse service keeps its output, at the cost of one extra call to find the service type; on other service types it now fails with an explicit "unsupported" error instead of an API error.Checklist
(For exoscale contributors)
CHANGELOG.md)Testing
go test ./cmd/dbaas/...),make build,gofmtandgo veton the touched packages, and the--helpof each subcommand.kafka_acl_crud.txtar,opensearch_acl_crud.txtar) but have not been run: they create billed Kafka and OpenSearch services. The commands have not been exercised against the real API yet.Note
AI assistance: code, tests, PR description.
🤖 Generated with Claude Code