Skip to content

Commit e861393

Browse files
authored
Add an api convention doc (#385)
Signed-off-by: Jian Qiu <[email protected]>
1 parent 779879f commit e861393

File tree

1 file changed

+103
-0
lines changed

1 file changed

+103
-0
lines changed

docs/api-conventions.md

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
# OCM API Conventions
2+
3+
OCM APIs follow the [Kubernetes API conventions](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md)
4+
with some additional guidlines inspired from [OpenShift API Convention](https://github.com/openshift/enhancements/blob/master/dev-guide/api-conventions.md).
5+
6+
## API Guidance
7+
8+
### Configuration vs Workload APIs
9+
10+
A configuration API is one that is typically cluster-scoped, a singleton within the cluster, and managed by a cluster
11+
administrator only. Operator APIs such as `ClusterManager`, `Klusterlet` are configuration APIs.
12+
13+
A workload API typically is namespaced and is used by end users of the cluster. Most of the APIs, such as
14+
`ManagedCluster`, `ManifestWork`, and `Placement` are workload APIs.
15+
16+
#### Defaulting
17+
18+
In workload APIs, we typically default fields on create (or update) when the field isn't set.
19+
This has the effect that changing the default value in the API does not change the value for objects that have
20+
previously been created.
21+
22+
This has the implication that you cannot change the behaviour of a default value once the API is defined as that would
23+
cause the same object to result in different behavior for different versions of the API, which would surprise users and
24+
compromise portability.
25+
26+
To change the default behaviour could constitute a breaking change and disrupt the end user's workload;
27+
the behaviour must remain consistent through the lifetime of the resource.
28+
This also means that defaults cannot be changed without a breaking change to the API.
29+
If a user were to delete their workload API resource and recreate it, the behaviour should remain the same.
30+
31+
With configuration APIs, we typically default fields within the controller and not within the API.
32+
33+
Typically, optional fields on configuration APIs contain a statement within their Godoc to describe
34+
the default behaviour when they are omitted, along with a note that this is subject to change over time.
35+
36+
#### Pointers
37+
38+
In configuration APIs specifically, we advise to avoid making fields pointers unless there is an absolute need to do so.
39+
An absolute need being the need to distinguish between the zero value and a nil value.
40+
41+
Using pointers makes writing code to interact with the API harder and more error prone, and it also harms the
42+
discoverability of the API.
43+
44+
##### Pointers to structs
45+
46+
An exception to this rule is when using a pointer to a struct in combination with an API validation that requires the
47+
field to be unset.
48+
49+
The JSON tag `omitempty` is not compatible with struct references. Meaning any struct will always, when empty, be
50+
marshalled to `{}`. If the struct **must** genuinely be omitted, it must be a pointer.
51+
52+
### Discriminated Unions
53+
54+
A discriminated union is a structure within the API that closely resembles a union type.
55+
A number of fields exist within the structure and we are expecting the user to configure precisely one of
56+
the fields.
57+
58+
In particular, for a discriminated union, an extra field exists which allows the user to declaratively
59+
state which of particular fields they are configuring.
60+
61+
We use discriminated unions in Kubernetes APIs so that we force the user to make a choice.
62+
We do not want our code to guess what the user meant, they should tell us which of the choices they made
63+
using the discriminant.
64+
65+
Important to note:
66+
* All structs within the union **MUST** be pointers
67+
* All structs within the union **MUST** be optional
68+
* The discriminant should be required
69+
* The discriminant **MUST** be a string (or string alias) type
70+
* Discriminant values should be PascalCase and should be equivalent to the camelCase field name (json tag) of one member of the union
71+
* Empty union members (discriminant values without a paired union member) are also permitted
72+
73+
### No annotation-based APIs
74+
75+
Do not use annotations for extending an API. Annotations may seem as a good candidate for introducing experimental/new
76+
API. Nevertheless, migration from annotations to formal schema usually never happens as it requires breaking changes
77+
in customer deployments.
78+
79+
1. Validation does not always come with definition. User set values can be too broad and hard to limit later on.
80+
2. Lack of discoverability. There's no pre-existing schema that can be published.
81+
3. Validation is limited. Certain kinds of validators aren't allowed on annotations, so hooks are more frequently used instead.
82+
4. Hard to extend. An annotation value (a string) can not be extended with additional fields under a parent.
83+
5. Unclear versioning. Annotation keys can omit versioning. Or, there are multiple ways to specify a version.
84+
6. Users can "squat" on annotations by adding an unvalidated annotation value for a key that is used in a future version.
85+
86+
### Use JSON Field Names in Godoc
87+
88+
Ensure that the godoc for a field name matches the JSON name, not the Go name,
89+
in Go definitions for API objects. In particular, this means that the godoc for
90+
field names should use an initial lower-case letter.
91+
92+
### Use Resource Name Rather Than Kind in Object References
93+
94+
Use resource names rather than kinds for object references.
95+
96+
### Do not use Boolean fields
97+
98+
Many ideas start as a Boolean value, e.g. `FooEnabled: true|false`, but often evolve into needing 3, 4, or even more
99+
states at some point during the API's lifetime.
100+
As a Boolean value can only ever have 2 or in some cases 3 values (`true`, `false`, `omitted` when a pointer), we have
101+
seen examples in which API authors have later added additional fields, paired with a Boolean field, that are only
102+
meaningful when the original field has a certain state. This makes it confusing for an end user as they have to be
103+
aware that the field they are trying to use only has an effect in certain circumstances.

0 commit comments

Comments
 (0)