Thoughts on Unreadable fields

[jcanseco]

Creating this issue to start/consolidate discussions on Unreadable fields and how they should be treated by the AIPs.

Unreadable fields are defined as fields whose current state cannot be determined via the API (i.e. via GET requests). Example: the password field of Cloud SQL User.

Problem

Unreadable fields are problematic for declarative clients such as Config Connector and Terraform (but more so for Config Connector):

1. Detecting "drift" on unreadable fields is very difficult (impossible?)

   For example, suppose a user creates a Cloud SQL User with a particular password using Config Connector (or Terraform). If another user changes the password using another tool (e.g. gcloud), Config Connector is expected to change the password back to its original desired state. However, since Config Connector is not able to read the current password from the API, it is not able to detect that the password had been changed.

2. Unreadable fields complicate "import" tools.

   Import tools are used by users who want to start managing existing GCP resources using Config Connector (or Terraform). Import tools work by reading a resource's current state and converting that to the appropriate declarative client configuration.

   Import tools are unable to determine a value for unreadable fields, so they just leave the fields unspecified in the generated configs. The resulting behavior depends on the declarative client, but either way, the unreadable fields end up being "imported" differently from other fields. This inconsistent behavior often causes confusion / surprises for users.

Therefore, we are hoping to strongly discourage use of Unreadable fields in APIs (also known as INPUT_ONLY fields in AIP-203), perhaps even outright banning them if possible.

That said, there are cases where APIs simply would not want to return a value, notably if the field is sensitive (AIP-147).

We are starting this discussion to explore options on how to potentially reduce / discourage / ban use of Unreadable fields.

Ideas so far

Ideas that have been brought up in prior discussions on this topic:

1. From @rileykarson: Require APIs to accompany unreadable fields with an output-only checksum field that can be used for drift detection purposes (e.g. if a resource must have an unreadable field password, make sure resource also has output-only field password_checksum.

2. From @bgrant0607: Require APIs to not store sensitive values in resources, and allow sensitive fields to reference another resource that contains the secret instead (e.g. Secret Manager Secret). Note: This is the approach taken by the Kubernetes API.

At a glance, the second idea sounds like it could resolve both the drift detection and import issues.

Thoughts?

[noahdietz]

Personally, I really like the second option and it is something that I propose as an option in API Design Reviews when I see a potentially sensitive field. I'm not sure if this approach can be used for non-Cloud APIs (say, requiring Google Ads API users to also use Cloud Secret Manager), but maybe this guidance only has to pertain to Cloud anyways.

The first option is also interesting, and might be a nice alternative when option 2 isn't feasible, but Declarative Resources are still desired by the API.

[bgrant0607]

It's specifically relevant to resource operations. Bounding it to cloud is fine.

Anything that needs to either reproduce the current state of a resource or check whether the current state matches the desired state needs to be able to actually read the state.

Special cases like this are death to horizontal integrations that need to work with every resource type across a large API surface. 1000 lines of bespoke, artisanal, handcrafted code times 1000 resource types is 1M LOC.

Option 1 is still custom code unless it can be standardized to the degree that it can be 100% automated for arbitrary resource types. It may also be vulnerable to dictionary attacks. Passwords are short.

[jcanseco]

Great. Since there seems to be agreement, I'll go ahead and create the appropriate issues to track the amendment of the AIPs and creation of API linter rule.

And yes I agree with bounding this to Cloud only.

[jcanseco]

@noahdietz I plan to create issues for each of the following. Can you let me know what you think?

1. (In AIP repo) Ban Cloud APIs from storing sensitive values in resources; suggest referencing separate Secret resource instead
2. (In API linter repo) Sensitive fields in Cloud APIs should be references to SecretManager Secrets

Is point 2 even feasible?

[noahdietz]

1. ✅
2. ❌ Yeah this isn't really feasible in the external repo. There may be an internal rule we could write that would key off of some other annotations that tend to be used when marking sensitive data. But for now we can punt until 1. is resolved.

[jcanseco]

Ok sure, sounds great. Thanks @noahdietz. Let us aim to resolve point 1 for now (created new issue: #884).

Eventually, I hope to have systems in place to more effectively discourage use of unreadable fields moving forward.