-
Notifications
You must be signed in to change notification settings - Fork 2
fix(ipa): Update IPA to support read-only resources #26
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -88,9 +88,20 @@ standard methods: | |
| - [Custom methods](0109.md) help define functionality that does not cleanly | ||
| map to any of the standard methods | ||
|
|
||
| ### Readonly Resources | ||
|
|
||
| - Unsupported operations on readonly resources **should** return | ||
| ### Read-Only Resources | ||
|
|
||
| Read-only resources are resources that cannot be modified by API consumers. | ||
|
|
||
| - Read-only resources **must** have [Get](0104.md) and [List](0105.md) methods | ||
| - Read-only resources **must not** have [Create](0106.md), [Update](0107.md), or | ||
| [Delete](0108.md) methods | ||
| - Read-only resources **may** have [custom methods](0109.md) as appropriate | ||
| - All response schema properties for read-only resources **must** be marked as | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we also consider create-only / non-updatable resources? (resources that can be created and deleted, but not updated), or other resource patterns, or it's ok at the moment just read-only?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may consider create-only resources in the future, but that’s out of scope for this PR. This PR specifically focuses on supporting read-only resources. As a note for the future: would create-only resources be acceptable as declarative-friendly resources? |
||
| read-only | ||
| - In OpenAPI, this means all properties **must** have `readOnly: true` | ||
| - All fields in read-only resources are server-owned. For guidance on | ||
| server-owned fields, see [IPA-111](0111.md#single-owner-fields) | ||
| - Unsupported operations on read-only resources **should** return | ||
| `405 Not Allowed` | ||
| - Unsupported operations **must not** be documented | ||
| - Some declarative-friendly clients require all standard methods to be | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,34 +21,56 @@ Experimental | |
| ### Single Owner Fields | ||
|
|
||
| Fields **must** have a single owner, whether that is the client or the server. | ||
| Server-owned fields must be documented as such. All other types of fields | ||
| **must** be considered to be owned by the client. The server **must** respect | ||
| the value (or lack thereof) of all client-owned fields and not modify them. | ||
|
|
||
| - **Server-owned fields** are fields that are controlled and modified by the | ||
| service | ||
| - Server-owned fields **must** be documented as read-only | ||
| - In OpenAPI, server-owned fields **must** have `readOnly: true` | ||
| - Server-owned fields **must not** be accepted in request bodies for | ||
| [Create](0106.md) or [Update](0107.md) operations | ||
| - **Client-owned fields** are fields that are controlled by the client | ||
| - All fields that are not explicitly documented as server-owned **must** be | ||
| considered client-owned | ||
| - The server **must** respect the value (or lack thereof) of all client-owned | ||
| fields and not modify them | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would add something like ... and always return the same value the client sent (or absence of value)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
| - The server **must** always return the same value the client sent (or absence | ||
| of value) for client-owned fields | ||
|
|
||
| :::note | ||
|
|
||
| For resources where all fields are server-owned, see | ||
| [read-only resources](0101.md#read-only-resources) and | ||
| [read-only singleton resources](0113.md#read-only-singleton-resources). | ||
|
|
||
| ::: | ||
|
|
||
| ### Effective Values | ||
|
|
||
| There are instances where a service will allocate, generate, or calculate a | ||
| value if the client chooses not to specify one. | ||
|
|
||
| **For example:** a client creates a cluster without specifying a version. Such a | ||
| scenario is opting for the default mongodb version. | ||
| value that may differ from what the client specified. | ||
|
|
||
| An attribute with an effective value **must** be expressed as two fields in the | ||
| API: | ||
|
|
||
| - A mutable field that **may** be optionally set by the user and **must not** be | ||
| modified by the service | ||
| - A read-only field that records the effective value decided on by the service | ||
| - Effective values **must** be named by prefixing effective to the mutable | ||
| - A **client-owned** mutable field that **may** be optionally set by the user | ||
AgustinBettati marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| and **must not** be modified by the service | ||
| - A **server-owned** read-only field that records the effective value decided on | ||
| by the service | ||
| - Effective values **must** be named by prefixing `effective` to the mutable | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it think "must" is strong for prefixing with effective, would use "should", e.g. in some scenarios it can be clear without adding effective
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think adding a descriptive prefix and forcing it would be helpful here. Since the fields share the same name, a clear prefix would help distinguish ownership. In the following example, both fields are named
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hear you, just saying that we should use "must" precautiously and I don't think it's so critical here. feel free to keep as it is
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd agree with Leo here on adjusting the keyword to should instead of must, I could see use cases where we might want a different name, and AFAIK we don't validate that it's called "effective" nor rely on the naming for downstream tooling
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m okay with using either must or should. At a minimum, we could ask for a distinctive prefix so users can clearly understand that the field is effective. For context, this wording comes from existing guidance — I didn’t introduce any changes here, and I’m not sure if Gus had a strong opinion when it was written. This is just my perspective. I assume he was referring to the AIP guideline here The reason we don’t validate this is that it’s a semantic concern — we can’t reliably determine programmatically if a field is effective
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I missed this was not an addition! I'm fine to leave it or update, up to you 👍 |
||
| field's name | ||
| - In OpenAPI, the effective value field **must** have `readOnly: true` | ||
|
|
||
| Example | ||
|
|
||
| A cluster's instance size may have a different computed value from the server if | ||
| auto-scaling is enabled. The client specifies their desired instance size, but | ||
| the server may scale it up or down based on load. | ||
|
|
||
| ```json | ||
| // For managing autoscaling of a cluster | ||
| // For managing a cluster with auto-scaling enabled | ||
| { | ||
| "clusterTier": "M10", | ||
| "effectiveClusterTier": "M30" | ||
| "instanceSize": "M10", | ||
| "effectiveInstanceSize": "M30" | ||
| } | ||
| ``` | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,8 +22,9 @@ Adopt | |
| [Delete](0108.md) standard methods | ||
| - The singleton is implicitly created or deleted when its parent is created or | ||
| deleted | ||
| - Singleton resources **should** define the [Get](0104.md) and [Update](0107.md) | ||
| methods | ||
| - Singleton resources **must** define the [Get](0104.md) method | ||
| - Singleton resources **should** define the [Update](0107.md) method, unless the | ||
| resource is [read-only](0113.md#read-only-singleton-resources) | ||
| - Singleton resources **may** define [custom methods](0109.md) as appropriate | ||
|
|
||
| Example | ||
|
|
@@ -33,3 +34,22 @@ GET /groups/${groupId}/settings | |
| ### | ||
| PATCH /groups/${groupId}/settings | ||
| ``` | ||
|
|
||
| ### Read-Only Singleton Resources | ||
|
|
||
| Read-only singleton resources are [singleton resources](0113.md) that cannot be | ||
| modified by API consumers. | ||
|
|
||
| - Read-only singleton resources **must** have only the [Get](0104.md) method | ||
| - Read-only singleton resources **must not** have [Create](0106.md), | ||
| [Update](0107.md), or [Delete](0108.md) methods | ||
| - Read-only singleton resources **may** have [custom methods](0109.md) as | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. custom methods are mostly used to cause some effect, is there some example where custom methods would be useful here that don't change anything?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point! We generally don’t recommend custom methods for declarative-friendly resources |
||
| appropriate, provided they do not modify the resource | ||
| - All response schema properties for read-only singleton resources **must** be | ||
| marked as read-only | ||
| - In OpenAPI, this means all properties **must** have `readOnly: true` | ||
| - All fields in read-only singleton resources are server-owned. For guidance | ||
| on server-owned fields, see [IPA-111](0111.md#single-owner-fields) | ||
| - Unsupported operations on read-only singleton resources **should** return | ||
| `405 Not Allowed` | ||
| - Unsupported operations **must not** be documented | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,8 @@ and support **automation**. | |
| verbs. The `GET` operation is critical for IaC tools to read the current | ||
| state and compare it against the desired state. | ||
| - A resource **must** have `CREATE`, `DELETE`, `GET`, `LIST` and `UPDATE` | ||
| methods. | ||
| methods, except for [read-only resources](#read-only-resources) which **must** | ||
| have only `GET` and `LIST` methods. | ||
| - Response schemas of `CREATE`, `GET` and `UPDATE` **must** be the same | ||
| - Fields defined in `CREATE` and `UPDATE` request schemas **should be** the | ||
| same and **should** be present in response schema | ||
|
|
@@ -53,6 +54,15 @@ and support **automation**. | |
| fully rely on understandable API responses ([IPA-114](0114.md)) | ||
| - A resource field **should** have a single type ([IPA-125](0125.md)) | ||
|
|
||
| ### Read-Only Resources | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Placing Read only resource within declarative friendly resources is slightly confusing to me. By definition a declarative friendly resource allows to In the case of read only resources they are useful for reading the state of a resource, but provides no management over it. Is there a reason we want to mention it here?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, IPA-127 has been updated to clarify that read-only resources are considered declarative-friendly. For more details on the restrictions, readers can refer to the references in the Read-Only Resources section. Are there any concerns regarding the assumption that read-only resources are counted as declarative-friendly?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No significant concern. This might be more related to how I interpret a declarative-friendly resource:
On the other hand read-only resources wont provide any form of management or handling of a desired state. The are only read operations (in terraform world these can be represented as data sources).
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Discussed offline. We have some open questions about whether IPA-127 should define autogeneration safety or cover broader concerns, and whether it would be better to introduce a separate IPA focused specifically on autogeneration safety. For now, we’re proceeding with merging this PR while we continue to look for a better long-term place for this clarification. TL;DR: Read-only resources, including read-only singleton resources, are supported by IaC tooling. |
||
|
|
||
| [Read-only resources](0101.md#read-only-resources) are declarative-friendly when | ||
| they provide observable state that IaC tools need to reference or depend upon. | ||
|
|
||
| For full guidance on read-only resources, see | ||
| [IPA-101](0101.md#read-only-resources). For read-only singleton resources, see | ||
| [IPA-113](0113.md#read-only-singleton-resources). | ||
|
|
||
| ### Motivation and Strategic Goals | ||
|
|
||
| Our engineering teams and customers rely on IaC tools to manage infrastructure | ||
|
|
||
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.
can it be a singleton read-only resource that only has Get but not List?
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.
Yes, I added read-only singleton resources section to IPA-113: Singleton Resources
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.
perfect, thanks