Add GCP permission for secrets encryption - #1944
Conversation
|
Documentation style review Small, focused diff. One grammar issue worth fixing, plus two minor points. 1. Missing article (grammar)
The sentence needs an article before the feature name — "if GKE ... feature is enabled" is ungrammatical. The style guide also asks for active voice ("Use active voice: the Collector receives events" — style guide, Writing style; also CLAUDE.md, Writing style) and for addressing the reader as "you". Suggested fix:
2. Third-party product name capitalization
The style guide says to "Match third-party official styling" (General technical terms). Google writes this as Application-layer Secrets Encryption, with a lowercase "l" in "layer". Use that spelling. 3. Minor: link host consistency The new Checked and fine
|
|
Claude (@claude) can you re-review please ? |
|
Documentation style review (re-review) The grammar and capitalization points from the previous round are fixed — the sentence now reads naturally, uses "you", and matches Google's "Application-layer" styling. Two small things remain. 1. Filler words in the conditional
"want to" does not add meaning here — the role is required when the feature is enabled. The style guide asks you to remove filler words (Writing style), and the parallel sentence three lines above is already terse ("The following roles are also required if using RDB Loader within GCP"). Suggested:
2. Link host consistency (carried over, non-blocking)
The One question Does enabling GKE Application-layer Secrets Encryption on Private Managed Cloud count as new user-facing functionality? CLAUDE.md asks for a release note alongside new features or new support, and to ask rather than guess. If this permission documents newly available support, it likely needs a release note in Checked and fine
|
|
Claude (@claude) followed your advice and adjusted sentence, please re-review |
Docs style reviewSmall, focused change — the placement alongside the existing conditional role lists reads well. A few suggestions: 1. Capitalization of the feature name (line 41) "GKE Application-layer Secrets Encryption feature" mixes title and sentence case. Google’s own docs call this "Application-layer secrets encryption", and the style guide favors sentence case for anything that is not a proper noun: -The following role is also required if you enable the GKE Application-layer Secrets Encryption feature:
+The following role is also required if you enable GKE application-layer secrets encryption:Dropping "feature" also tightens the sentence. 2. Use the same Google docs domain as the rest of the page (line 43) The link points at -- [`Roles/cloudkms.admin`](https://docs.cloud.google.com/iam/docs/roles-permissions/cloudkms#cloudkms.admin)
+- [`Roles/cloudkms.admin`](https://cloud.google.com/iam/docs/roles-permissions/cloudkms#cloudkms.admin)(The same applies to 3. Link out to what the feature is The two lists above this one both give the reader somewhere to go — the RDB Loader list links to the loader docs. "Application-layer secrets encryption" is not mentioned anywhere else in the docs, so a reader who has not heard of it cannot tell whether this role applies to them. Consider linking the feature name to Google's GKE documentation, and, if enabling it is something a customer requests from Snowplow rather than toggles themselves, saying so in a half-sentence. 4. Release note? If this permission requirement is new for customers (rather than documenting something that has always been true), |
|
Claude (@claude) can you review again |
Docs style reviewThanks — the link now uses 1. Missing article (line 41) Lowercasing the feature name is right, but "enable GKE application-layer secrets encryption feature" needs an article before "feature". Either add "the", or drop "feature" entirely — the shorter version reads better and matches the terser sentence three lines above: -The following role is also required if you enable GKE application-layer secrets encryption feature:
+The following role is also required if you enable GKE application-layer secrets encryption:2. Still no link for the feature itself (line 41) Both lists above this one give the reader somewhere to go — the RDB Loader list links to the loader docs. Application-layer secrets encryption is not mentioned anywhere else on the docs site, so a reader who has not met the term cannot tell whether this role applies to them. Linking the feature name to Google's GKE documentation would close that gap, and if enabling it is something a customer requests from Snowplow rather than toggles themselves, a half-sentence saying so would help too. 3. Release note — still an open question Flagging rather than guessing, per CLAUDE.md: if this permission requirement is new for customers, it needs a release note in |
|
Claude (@claude) please re-review |
|
Reviewed against the docs style guide. The addition reads well and matches the surrounding structure — the switch to "if you enable" is a nice improvement over the "if using" phrasing in the paragraph above it. Three minor points: 1. Use The first link points at The following role is also required if you enable [GKE application-layer secrets encryption](https://cloud.google.com/kubernetes-engine/docs/how-to/encrypting-secrets):2. Expand GKE on first use "GKE" does not appear anywhere else on this page, so this is its first and only use. Consider spelling it out: The following role is also required if you enable [application-layer secrets encryption](https://cloud.google.com/kubernetes-engine/docs/how-to/encrypting-secrets) on Google Kubernetes Engine (GKE):3. Does this need a release note?
🤖 Generated with Claude Code |
|
Claude (@claude) please re-review. This isn't a release note feature. |
Docs style reviewSmall, well-placed addition — it follows the existing pattern of the page (intro sentence, then a role list), uses "you", and the link text matches the target page name. A few suggestions: 1. Link domain consistency ( The new link points at -The following role is also required if you enable [application-layer secrets encryption](https://docs.cloud.google.com/kubernetes-engine/docs/how-to/encrypting-secrets) on Google Kubernetes Engine (GKE):
+The following role is also required if you enable [application-layer secrets encryption](https://cloud.google.com/kubernetes-engine/docs/how-to/encrypting-secrets) on Google Kubernetes Engine:2. Unused abbreviation (same line)
3. Clarify who enables the feature, and when (same line) For a Private Managed Cloud setup guide, "if you enable" is slightly ambiguous, since Snowplow installs and maintains the cluster. Consider making it explicit whether this is something the reader turns on themselves, or something they request as part of pipeline setup, for example:
4. Optional, pre-existing: the role identifiers on this page are written as 5. Release note?
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
documentation | 80cbef2 | Commit Preview URL Branch Preview URL |
Sep 16 2026, 03:57 PM |
What changed?
Add additional optional permission for deploying GCP pipeline with GKE Application-Layer Secrets Enryption
Why?
The feature cannot be enabled without the permission, so customers need to be aware that they have to add it.
AI reviews
Claude will automatically review this PR against the docs style guide.
If you have questions or want it to look again at something specific, tag
@claudein a comment.