Skip to content

Commit 97dabfe

Browse files
md2119md2119
authored
chore(docs): ACS API guidelines - Part 2: Decoupling (stackrox#7675)
1 parent 098cee3 commit 97dabfe

1 file changed

Lines changed: 132 additions & 7 deletions

File tree

.github/api_best_practices.md renamed to .github/api_guidelines.md

Lines changed: 132 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,132 @@ in slack channel [#forum-acs-api-design](https://redhat-internal.slack.com/archi
88
## Table of Contents
99

1010
- [Naming Guidelines](#naming-guidelines)
11+
- [Decoupling](#decoupling)
1112
- [gRPC Service Name](#grpc-service-name)
1213
- [gRPC Method Name](#grpc-method-name)
1314
- [gRPC Message Name](#grpc-message-name)
1415

16+
## Decoupling
17+
18+
While certain data may exist in the database for internal purposes or other functionalities, it is important to
19+
exercise caution when deciding what data to make accessible through the API. The rule of thumb to follow is that the APIs
20+
**must not** expose data structures defined for internal purposes. Instead, define a data structure for the sole purpose of
21+
API use, generally in the same package where the API is defined. This applies to all APIs including the
22+
inter-component APIs.
23+
- All services defined in the package [/proto/api/v2](https://github.com/stackrox/stackrox/blob/master/proto/api/v2)
24+
**must not** import from the following packages:
25+
- [/proto/storage](https://github.com/stackrox/stackrox/blob/master/proto/storage)
26+
- [/proto/internalapi](https://github.com/stackrox/stackrox/blob/master/proto/internalapi)
27+
- [/proto/api/v1](https://github.com/stackrox/stackrox/blob/master/proto/api/v1)
28+
- All new services and methods defined in the package [/proto/api/v1](https://github.com/stackrox/stackrox/blob/master/proto/api/v1)
29+
**must not** import from the following packages:
30+
- [/proto/storage](https://github.com/stackrox/stackrox/blob/master/proto/storage)
31+
- [/proto/internalapi](https://github.com/stackrox/stackrox/blob/master/proto/internalapi)
32+
- [/proto/api/v2](https://github.com/stackrox/stackrox/blob/master/proto/api/v2)
33+
- All new messages defined in the package [/proto/internalapi](https://github.com/stackrox/stackrox/blob/master/proto/internalapi)
34+
**must not** import from the following packages:
35+
- [/proto/storage](https://github.com/stackrox/stackrox/blob/master/proto/storage)
36+
- [/proto/api/v1](https://github.com/stackrox/stackrox/blob/master/proto/api/v1)
37+
- [/proto/api/v2](https://github.com/stackrox/stackrox/blob/master/proto/api/v2)
38+
- All services of the type `APIServiceWithCustomRoutes` **must not** import from the following packages:
39+
- [/proto/storage](https://github.com/stackrox/stackrox/blob/master/proto/storage)
40+
- [/proto/internalapi](https://github.com/stackrox/stackrox/blob/master/proto/internalapi)
41+
- Structs used by APIs, such as those defined in [/proto/api/](https://github.com/stackrox/stackrox/blob/master/proto/api/),
42+
[/proto/internalapi](https://github.com/stackrox/stackrox/blob/master/proto/internalapi), **must not** be
43+
written to database.
44+
45+
Exposing internal data structures, especially the ones representing and affecting database schema directly at
46+
the API can pose security risks. By carefully selecting and limiting the data exposed via the API, we can prevent
47+
unauthorized access and potential data breaches which can have compliance ramifications.
48+
49+
Furthermore, exposing unnecessary data in the API can lead to overfetching, where the client receives more data
50+
than required. This inefficiency increases network traffic, consumes extra bandwidth, and impacts API performance.
51+
By limiting API data to what is actually needed by clients, we optimize resource usage and improve response times.
52+
53+
Consider the following database/internal structures as an example data structure used in Central.
54+
55+
```
56+
Deployment {
57+
string id
58+
string name
59+
repeated KeyValue labels
60+
repeated KeyValue annotations
61+
repeated Container containers
62+
}
63+
64+
Container {
65+
string name
66+
string image_name
67+
repeated Volume volumes
68+
}
69+
```
70+
71+
Exposing the above data structure in the API will always lead to reading all the `Container` bytes from the database,
72+
even if the user does not need them. Instead, design the API such that users can request the information
73+
they need, as below:
74+
75+
```
76+
// GetDeploymentRequest requests deployment information. Deployment metadata is requested by default.
77+
GetDeploymentRequest {
78+
Options {
79+
bool get_container_spec `json: "getContainerSpec"`
80+
}
81+
}
82+
83+
// Default response
84+
GetDeploymentResponse {
85+
Metadata {
86+
string name `json: "name"`
87+
string id `json: "id"`
88+
}
89+
}
90+
91+
// If `"getContainerSpec": true`
92+
GetDeploymentResponse {
93+
Metadata {
94+
string name `json: "name"`
95+
string id `json: "name"`
96+
...
97+
}
98+
ContainerSpec {
99+
string name `json: "name"`
100+
string image_name `json: "imageName"`
101+
...
102+
}
103+
}
104+
```
105+
106+
As APIs evolve over time, adding or removing data fields becomes inevitable. By limiting the exposure of internal
107+
database structures, we decouple the API from the database schema, allowing us to modify the internal structures
108+
without breaking existing client applications.
109+
110+
Consider the following database/internal structures as an example data structure exposed by APIs.
111+
```
112+
Alert {
113+
string id
114+
repeated Violation violations
115+
string policy_name
116+
string policy_description
117+
string policy_enforcement
118+
}
119+
```
120+
Changing the above `Alert` data structure to the following structure may break the client application.
121+
```
122+
Alert {
123+
string id;
124+
repeated Violation violations
125+
126+
Policy {
127+
string policy_name
128+
string policy_description
129+
string policy_enforcement
130+
}
131+
}
132+
```
133+
134+
Keeping the API focused and concise simplifies maintenance efforts and reduces the chances of introducing bugs.
135+
A clean and manageable codebase improves the overall maintainability and stability of the API.
136+
15137
## Naming Guidelines
16138

17139
### gRPC Service Name
@@ -190,17 +312,18 @@ GetDeploymentRequest {
190312
</td>
191313
<td>
192314

315+
```
316+
GetDeploymentImageScanResponse {
317+
Image image;
318+
}
319+
```
320+
or,
193321
```
194322
GetDeploymentResponse {
195323
Deployment deployment;
196324
Image image;
197325
}
198326
199-
Or,
200-
201-
GetDeploymentImageScanResponse {
202-
Image image;
203-
}
204327
```
205328

206329
</td>
@@ -227,5 +350,7 @@ autogenerated by the proto compiler. By default, field names are converted to ca
227350
| `network_data_start_time` | `networkDataStartTime` |
228351
| `expiry_date` | `expiryDate` |
229352

230-
Be explicit about conveying the specific purpose of fields e.g. instead of `expires_on`
231-
use `expiry_date`(/`timestamp`), instead of `network_data_since` use `network_data_start_time`.
353+
Be explicit about conveying the specific purpose of fields e.g. instead of `expires_on` use `expiry_date`(/`timestamp`)
354+
as it informs users if the field returns the date portion of the timestamp or the full timestamp, and use
355+
`network_data_start_time` instead of `network_data_since` for a similar reason. The fields should convey their purpose without
356+
requiring users to read the documentation.

0 commit comments

Comments
 (0)