Repository navigation
ate-api-server: Create one unified configuration file - #2021
Conversation
Maya Wang (mayawang)
added
kind/feature
| type APIConfig struct { | ||
| state protoimpl.MessageState `protogen:"open.v1"` | ||
| // Authentication configuration for users calling the Substrate APIs. | ||
| Authentication *APIClientAuthentication `protobuf:"bytes,1,opt,name=authentication,proto3" json:"authentication,omitempty"` |
There was a problem hiding this comment.
Is this synced to the proto definition? I only see logging in the proto.
There was a problem hiding this comment.
Synced
|
|
||
| config := &ateconfigpb.APIConfig{} | ||
| switch ext := filepath.Ext(l.configFile); ext { | ||
| case "pb": |
There was a problem hiding this comment.
Why do we need support for binary encoding? Also, I don't think people are familiar with textproto format in general. Why not just stick to one format? protojson?
There was a problem hiding this comment.
Protojson can't have comments, which seem important for a config file unless you are always completely generating it. Textproto seems to fit the bill --- it can have comments, it's a little more compact than JSON, while not having all the scary misfeatures of YAML (references, anchors).
Binary format: Similar to Envoy, I was supposing that you would be able to use different encodings of the same protobuf data model to actually do your config programming. I don't think this is actually that important, but if we don't support multiple formats then I think textproto should be the format.
There was a problem hiding this comment.
So keep just textproto? I dont see a use case for binary encoding really. At least for now.
There was a problem hiding this comment.
On the file location. Are we sure we want this file here? Do we expect consumers of the substrate Go module to import this proto? It sounds like we don't? Should we move it under cmd/ateapi/internal/ ?
There was a problem hiding this comment.
I would assume that external projects would be using these protos? (For example, the setup-gke scripts when they are fully broken out to another repo).
There was a problem hiding this comment.
How about we keep it internal until the time comes for people to consume? We can move it and it should all work. Nobody will check the package in serialized representations (e.g. textproto).
…bstrate#1965) Working on agent-substrate#1563 Fixes agent-substrate#1697, agent-substrate#1701 ## Summary Adds `AccessPolicy` singleton subresources for the Global (`global:root`) and `Atespace` scopes, stored in PostgreSQL and enforced through OpenFGA. ### Changes - **API (`Control` service)** - Global: `CreateGlobalAccessPolicy`, `GetGlobalAccessPolicy`, `UpdateGlobalAccessPolicy` (no Delete; an update with empty bindings clears all grants). - Atespace: `CreateAtespaceAccessPolicy`, `GetAtespaceAccessPolicy`, `UpdateAtespaceAccessPolicy`, `DeleteAtespaceAccessPolicy`. - `Binding.members` has set semantics. - **Always-wired authz**: the OpenFGA server, `Authorizer` and `PolicyManager` are always created. `--experimental-enable-authz` only controls whether the interceptor enforces checks on regular RPCs. Policy tuples therefore stay in sync with the store in every mode, and deleting an atespace always removes its tuples. - **Governance RPCs are always enforced**: AccessPolicy RPCs are marked `alwaysEnforce` in the permission registry and are checked even when enforcement is off, so nobody can grant themselves access before it is turned on. A test fails if an AccessPolicy RPC is added without that marker. - **Bootstrap owners**: `--authz-bootstrap-owners` lists server-configured owners (required when enforcing). They are never written to the store: `Authorizer.Check` adds a contextual `owner global:root` tuple for the calling user only, so access follows the server config and is dropped on restart without them. A bootstrap owner creates the global policy. - **Storage**: migration `000003` adds `global_access_policy` and `atespace_access_policies`. Deleting an atespace cascades to its policy row and removes its OpenFGA tuples in the same transaction. - **Consistency**: `PolicyManager` takes an explicit `pgx.Tx`, locks the metadata row (`FOR UPDATE`) and reconciles OpenFGA tuples in the same transaction. Optimistic concurrency via `uid` / `version`. - **Hardening**: members with control characters are rejected, as are blank, `*`, or control-character bootstrap owner entries; non-canonical OpenFGA error codes are mapped to `Internal`. ### Testing - Interceptor tests for both modes: with enforcement off, regular RPCs skip the check while governance RPCs are still checked. - Functional CRUD tests for both AccessPolicy scopes (`functionaltest/access_policy_test.go`). - PostgreSQL test that a policy update and its tuple writes commit or roll back together, including under concurrent updates. - Governance tests: creating the policy as a bootstrap owner, a bootstrap owner also stored as owner, and a restart without a bootstrap owner. ### Notes - ateapi now needs OpenFGA available at startup even with enforcement off, and atespace deletes always write OpenFGA tuples. - TODO: move the authz settings into the hot-reloadable config proto (agent-substrate#2021) once it lands.
71694c8 to
beff0c3
Compare
f987966 to
fc14607
Compare
fc14607 to
462c634
Compare
462c634 to
1681feb
Compare
| const defaultAPILogLevel = "info" | ||
|
|
||
| // CreateAPIConfig writes the ate-api-server config file. ate-api-server | ||
| // re-reads it while running, so a changed log level takes effect without a |
There was a problem hiding this comment.
I don't see this change yet in main.go. It still only reads the config file once at startup right? May be change this to a TODO.
There was a problem hiding this comment.
Oops, I used to have this functionality, but took it out. Updated the comment.
There was a problem hiding this comment.
Unused variable fix needed.
There was a problem hiding this comment.
Done.
Initially, it hosts nothing but logging config.
A follow-on PR will move the authentication config into the unified config.