docs: add Alauda Build of SPIRE workload security documentation#616
Open
tossmilestone wants to merge 3 commits intomasterfrom
Open
docs: add Alauda Build of SPIRE workload security documentation#616tossmilestone wants to merge 3 commits intomasterfrom
tossmilestone wants to merge 3 commits intomasterfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded two new workload-security docs: Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/en/security/workload_security/index.mdx`:
- Line 1: Rename the docs directory "workload_security" to "workload-security"
and update any references/imports/links that point to workload_security (e.g.,
sidebar entries, internal MDX links, or import paths referencing
workload_security) so they use the new kebab-case name; ensure the MDX file
index.mdx (or any files under the directory) remain reachable and update any
build/config references to the directory name.
In `@docs/en/security/workload_security/spire.mdx`:
- Line 85: Replace the awkward heading phrase "Workload Authenticating" with
"Workload Authentication" in the sentence that mentions the SPIFFE Workload API
(the line containing "/run/spiffe.io/public"); update the wording and
capitalization to match surrounding doc style so the sentence reads "Workload
Authentication: Your application can now use the SPIFFE Workload API at
`/run/spiffe.io/public` to retrieve its identity and certificates." Ensure only
the phrase is changed and punctuation/grammar around it remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8536ca83-1f68-4876-928c-a91f193358f0
📒 Files selected for processing (2)
docs/en/security/workload_security/index.mdxdocs/en/security/workload_security/spire.mdx
Deploying alauda-container-platform with
|
| Latest commit: |
183f235
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0d428676.alauda-container-platform.pages.dev |
| Branch Preview URL: | https://feat-spire-docs.alauda-container-platform.pages.dev |
tossmilestone
force-pushed
the
feat/spire-docs
branch
from
d4d2fa3 to
b866ad5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/en/security/workload_security/spire.mdx (1)
343-356: Consider clarifying the failure scenario steps.The failure scenario mentions modifying the
--verify-uriparameter but doesn't explain how to apply this change (e.g., editing the deployment or redeploying). Adding a brief note about redeploying the client with the modified parameter would improve clarity.📝 Suggested clarification
Consider adding a note like:
To demonstrate proper security enforcement, modify the client deployment's `--verify-uri` parameter to an incorrect URI (e.g., edit line 304 to use `spiffe://example.org/ns/example/sa/server` instead of `spiffe://example.org/ns/example/sa/server-sa`) and reapply the manifest. Then test:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/security/workload_security/spire.mdx` around lines 343 - 356, Clarify the failure scenario by telling the reader to modify the client's --verify-uri parameter in the client deployment manifest (the client-workload/ghostunnel container args) to an incorrect SPIFFE URI (e.g., change --verify-uri from spiffe://example.org/ns/example/sa/server-sa to spiffe://example.org/ns/example/sa/server), save and reapply the manifest (kubectl apply -f <manifest>) or redeploy the client so the new args take effect, and then run the provided test command (kubectl exec -n example deploy/client-workload -c ghostunnel -- curl -s http://127.0.0.1:8080) to observe the expected failure (exit code 56 and unauthorized log).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/en/security/workload_security/spire.mdx`:
- Around line 205-264: The manifest defines a Deployment named server-workload
(container ghostunnel listening on :8443) but no Kubernetes Service, so DNS name
server-workload:8443 will not resolve for the client; add a Service resource
selecting app: server-workload that exposes port 8443 -> targetPort 8443
(ClusterIP) and place it after the server Deployment so the client can reach
server-workload:8443.
- Line 248: The two workload image references are inconsistent: the server
workload uses docker-mirrors.alauda.cn/ghostunnel/ghostunnel:v1.9.0 while the
client workload uses ghostunnel/ghostunnel:v1.9.0; update the client workload
image reference (the client workload block that currently lists
"ghostunnel/ghostunnel:v1.9.0") to match the Alauda mirror
"docker-mirrors.alauda.cn/ghostunnel/ghostunnel:v1.9.0" so both server and
client use the same registry and tag.
---
Nitpick comments:
In `@docs/en/security/workload_security/spire.mdx`:
- Around line 343-356: Clarify the failure scenario by telling the reader to
modify the client's --verify-uri parameter in the client deployment manifest
(the client-workload/ghostunnel container args) to an incorrect SPIFFE URI
(e.g., change --verify-uri from spiffe://example.org/ns/example/sa/server-sa to
spiffe://example.org/ns/example/sa/server), save and reapply the manifest
(kubectl apply -f <manifest>) or redeploy the client so the new args take
effect, and then run the provided test command (kubectl exec -n example
deploy/client-workload -c ghostunnel -- curl -s http://127.0.0.1:8080) to
observe the expected failure (exit code 56 and unauthorized log).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18122903-7e4a-4cab-865e-7a237501d16a
📒 Files selected for processing (3)
.cspell/terms.txtdocs/en/security/workload_security/index.mdxdocs/en/security/workload_security/spire.mdx
✅ Files skipped from review due to trivial changes (2)
- .cspell/terms.txt
- docs/en/security/workload_security/index.mdx
| - 127.0.0.1:8080 | ||
| - --allow-uri | ||
| - spiffe://example.org/ns/example/sa/client-sa | ||
| image: docker-mirrors.alauda.cn/ghostunnel/ghostunnel:v1.9.0 |
Contributor
There was a problem hiding this comment.
Inconsistent image registry references.
The server workload uses docker-mirrors.alauda.cn/ghostunnel/ghostunnel:v1.9.0 (line 248) while the client workload uses ghostunnel/ghostunnel:v1.9.0 (line 305). For consistency and to avoid potential pull failures, both should reference the same registry.
🔄 Standardize to Alauda mirror
Update line 305 to use the Alauda mirror:
- image: ghostunnel/ghostunnel:v1.9.0
+ image: docker-mirrors.alauda.cn/ghostunnel/ghostunnel:v1.9.0Also applies to: 305-305
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/en/security/workload_security/spire.mdx` at line 248, The two workload
image references are inconsistent: the server workload uses
docker-mirrors.alauda.cn/ghostunnel/ghostunnel:v1.9.0 while the client workload
uses ghostunnel/ghostunnel:v1.9.0; update the client workload image reference
(the client workload block that currently lists "ghostunnel/ghostunnel:v1.9.0")
to match the Alauda mirror
"docker-mirrors.alauda.cn/ghostunnel/ghostunnel:v1.9.0" so both server and
client use the same registry and tag.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/en/security/workload_security/spire.mdx (1)
159-218:⚠️ Potential issue | 🔴 CriticalAdd a Service for
server-workloadso client DNS resolves.
client-workloadcallsserver-workload:8443(Line 255), but only a Deployment is defined for the server. Without a Service, this hostname won’t resolve in-cluster.🔧 Suggested doc patch
apiVersion: apps/v1 kind: Deployment metadata: name: server-workload namespace: example @@ volumes: - hostPath: path: /run/spire/agent-sockets type: Directory name: spiffe-socket +--- +apiVersion: v1 +kind: Service +metadata: + name: server-workload + namespace: example +spec: + selector: + app: server-workload + ports: + - protocol: TCP + port: 8443 + targetPort: 8443Also applies to: 255-255
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/security/workload_security/spire.mdx` around lines 159 - 218, The Deployment defines server-workload but no Service, so client-workload's call to server-workload:8443 will not resolve; add a Kubernetes Service manifest named server-workload that selects pods with label app: server-workload (matching the Deployment template) and exposes port 8443 (port 8443 targetPort 8443, protocol TCP) so in-cluster DNS for "server-workload" works; ensure the Service is created in the same namespace (example) and referenced by the existing server-sa/server-workload deployment rather than changing pod labels or ports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/en/security/workload_security/spire.mdx`:
- Around line 131-137: The two SPIRE entry create examples use the same selector
"k8s:ns:example", which permits ambiguous identities; update each spire-server
entry create command (the one with spiffeID
spiffe://example.org/ns/example/sa/client-sa and the corresponding server entry)
to bind to distinct selectors that include the specific service account (e.g.,
k8s:ns:example:k8s:sa:client-sa for the client entry and
k8s:ns:example:k8s:sa:server-sa for the server entry) and optionally add a pod
label selector (e.g., k8s:label:app:client-app) so each registration uniquely
ties to its service account/pod.
- Around line 299-303: The docs tell readers to change the client's --verify-uri
but do not show the actual mutation and redeploy step; update the example to
patch the client deployment's ghostunnel container args to the incorrect URI and
wait for rollout before re-running the curl check. Specifically, add a kubectl
patch/replace targeting the client-workload deployment's container args
(ghostunnel container, e.g., modify args index used for --verify-uri) and then
run kubectl rollout status deploy/client-workload prior to the kubectl exec ...
curl command so the failure is reproducible.
---
Duplicate comments:
In `@docs/en/security/workload_security/spire.mdx`:
- Around line 159-218: The Deployment defines server-workload but no Service, so
client-workload's call to server-workload:8443 will not resolve; add a
Kubernetes Service manifest named server-workload that selects pods with label
app: server-workload (matching the Deployment template) and exposes port 8443
(port 8443 targetPort 8443, protocol TCP) so in-cluster DNS for
"server-workload" works; ensure the Service is created in the same namespace
(example) and referenced by the existing server-sa/server-workload deployment
rather than changing pod labels or ports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c70529d-b82f-44eb-99e6-7fec058698be
📒 Files selected for processing (1)
docs/en/security/workload_security/spire.mdx
Comment on lines
+131
to
+137
| ```bash | ||
| kubectl exec -n spire spire-server-0 -- \ | ||
| /opt/spire/bin/spire-server entry create \ | ||
| -spiffeID spiffe://example.org/ns/example/sa/client-sa \ | ||
| -parentID spiffe://example.org/ns/spire/sa/spire-agent \ | ||
| -selector k8s:ns:example | ||
| ``` |
Contributor
There was a problem hiding this comment.
Use distinct workload selectors for client vs server registrations.
Both SPIRE entries currently use the same selector (k8s:ns:example). That allows ambiguous identity issuance and undermines the mTLS authz example. Bind each entry to its specific service account (and optionally pod label).
🔐 Suggested selector fix
kubectl exec -n spire spire-server-0 -- \
/opt/spire/bin/spire-server entry create \
-spiffeID spiffe://example.org/ns/example/sa/client-sa \
-parentID spiffe://example.org/ns/spire/sa/spire-agent \
- -selector k8s:ns:example
+ -selector k8s:ns:example \
+ -selector k8s:sa:client-sa
kubectl exec -n spire spire-server-0 -- \
/opt/spire/bin/spire-server entry create \
-spiffeID spiffe://example.org/ns/example/sa/server-sa \
-parentID spiffe://example.org/ns/spire/sa/spire-agent \
- -selector k8s:ns:example
+ -selector k8s:ns:example \
+ -selector k8s:sa:server-saAlso applies to: 142-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/en/security/workload_security/spire.mdx` around lines 131 - 137, The two
SPIRE entry create examples use the same selector "k8s:ns:example", which
permits ambiguous identities; update each spire-server entry create command (the
one with spiffeID spiffe://example.org/ns/example/sa/client-sa and the
corresponding server entry) to bind to distinct selectors that include the
specific service account (e.g., k8s:ns:example:k8s:sa:client-sa for the client
entry and k8s:ns:example:k8s:sa:server-sa for the server entry) and optionally
add a pod label selector (e.g., k8s:label:app:client-app) so each registration
uniquely ties to its service account/pod.
Comment on lines
+299
to
+303
| To demonstrate proper security enforcement, modify the client's `--verify-uri` parameter to an incorrect URI (e.g., `spiffe://example.org/ns/example/sa/server` instead of `spiffe://example.org/ns/example/sa/server-sa`): | ||
|
|
||
| ```bash | ||
| kubectl exec -n example deploy/client-workload -c ghostunnel -- curl -s http://127.0.0.1:8080 | ||
| ``` |
Contributor
There was a problem hiding this comment.
Make the failure test reproducible with an explicit mutation step.
Line 299 says to change --verify-uri, but the command shown is unchanged curl invocation. Add a concrete patch/redeploy step before re-running curl.
🧪 Suggested doc addition
To demonstrate proper security enforcement, modify the client's `--verify-uri` parameter to an incorrect URI (e.g., `spiffe://example.org/ns/example/sa/server` instead of `spiffe://example.org/ns/example/sa/server-sa`):
+
+```bash
+kubectl -n example patch deploy client-workload --type='json' -p='[
+ {"op":"replace","path":"/spec/template/spec/containers/0/args/8","value":"spiffe://example.org/ns/example/sa/server"}
+]'
+kubectl -n example rollout status deploy/client-workload
+```
```bash
kubectl exec -n example deploy/client-workload -c ghostunnel -- curl -s http://127.0.0.1:8080</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
To demonstrate proper security enforcement, modify the client's `--verify-uri` parameter to an incorrect URI (e.g., `spiffe://example.org/ns/example/sa/server` instead of `spiffe://example.org/ns/example/sa/server-sa`):
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/en/security/workload_security/spire.mdx` around lines 299 - 303, The
docs tell readers to change the client's --verify-uri but do not show the actual
mutation and redeploy step; update the example to patch the client deployment's
ghostunnel container args to the incorrect URI and wait for rollout before
re-running the curl check. Specifically, add a kubectl patch/replace targeting
the client-workload deployment's container args (ghostunnel container, e.g.,
modify args index used for --verify-uri) and then run kubectl rollout status
deploy/client-workload prior to the kubectl exec ... curl command so the failure
is reproducible.
74eb6c8 to
6fcc5f7
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
docs/en/security/workload_security/spire.mdx (3)
131-136:⚠️ Potential issue | 🔴 CriticalUse distinct selectors for client/server SPIRE entries.
Both registrations use only namespace selector, so either workload can match either entry. Add service-account selectors to bind identities unambiguously.
Suggested fix
kubectl exec -n spire spire-server-0 -- \ /opt/spire/bin/spire-server entry create \ -spiffeID spiffe://example.org/ns/example/sa/client-sa \ -parentID spiffe://example.org/ns/spire/sa/spire-agent \ - -selector k8s:ns:example + -selector k8s:ns:example \ + -selector k8s:sa:client-sa kubectl exec -n spire spire-server-0 -- \ /opt/spire/bin/spire-server entry create \ -spiffeID spiffe://example.org/ns/example/sa/server-sa \ -parentID spiffe://example.org/ns/spire/sa/spire-agent \ - -selector k8s:ns:example + -selector k8s:ns:example \ + -selector k8s:sa:server-saAlso applies to: 142-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/security/workload_security/spire.mdx` around lines 131 - 136, The SPIRE registration entries use only the namespace selector (k8s:ns:example) so client and server entries can both match; update the entry creation commands (the spiffe IDs spiffe://example.org/ns/example/sa/client-sa and spiffe://example.org/ns/spire/sa/spire-agent and their corresponding entry create invocations) to include service-account selectors (e.g., add k8s:sa:client-sa to the client entry and k8s:sa:spire-agent to the server entry) so identities are bound unambiguously—apply the same change to the other entry block referenced (lines 142–146).
165-217:⚠️ Potential issue | 🔴 CriticalAdd a Service for
server-workloadbefore usingserver-workload:8443.Line 254 relies on Kubernetes DNS, but only a Deployment is defined for the server. Add a Service so the client can resolve and connect.
Suggested fix
apiVersion: apps/v1 kind: Deployment metadata: name: server-workload namespace: example ... name: spiffe-socket +--- +apiVersion: v1 +kind: Service +metadata: + name: server-workload + namespace: example +spec: + selector: + app: server-workload + ports: + - protocol: TCP + port: 8443 + targetPort: 8443Also applies to: 254-254
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/security/workload_security/spire.mdx` around lines 165 - 217, The Deployment exposes ghostunnel on port 8443 (server-workload) but no Kubernetes Service is defined, so DNS name server-workload:8443 will not resolve; add a Service that selects pods with label app: server-workload (from the Deployment template) and exposes port 8443 (targetPort 8443, port 8443, protocol TCP) so clients can resolve and connect (use a ClusterIP Service name matching the DNS used, e.g., server-workload).
298-302:⚠️ Potential issue | 🟠 MajorMake the failure case reproducible with an explicit patch + rollout step.
The text says to change
--verify-uri, but no mutation command is provided before re-running curl.Suggested fix
To demonstrate proper security enforcement, modify the client's `--verify-uri` parameter to an incorrect URI (e.g., `spiffe://example.org/ns/example/sa/server` instead of `spiffe://example.org/ns/example/sa/server-sa`): ```bash +kubectl -n example patch deploy client-workload --type='json' -p='[ + {"op":"replace","path":"/spec/template/spec/containers/0/args/8","value":"spiffe://example.org/ns/example/sa/server"} +]' +kubectl -n example rollout status deploy/client-workload kubectl exec -n example deploy/client-workload -c ghostunnel -- curl -s http://127.0.0.1:8080</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/en/security/workload_security/spire.mdxaround lines 298 - 302, Add an
explicit mutation + rollout step that patches the client-workload deployment to
set the ghostunnel container's --verify-uri argument to the incorrect SPIFFE URI
and wait for the rollout to complete before re-running the curl; specifically,
apply a kubectl patch against the deployment "client-workload" to replace the
container args entry (e.g., the relevant index under
/spec/template/spec/containers/0/args or the arg matching --verify-uri) with the
wrong value (spiffe://example.org/ns/example/sa/server) and then run kubectl -n
example rollout status deploy/client-workload prior to executing the existing
kubectl exec ... curl command so the failure is reproducible.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In@docs/en/security/workload_security/spire.mdx:
- Around line 131-136: The SPIRE registration entries use only the namespace
selector (k8s:ns:example) so client and server entries can both match; update
the entry creation commands (the spiffe IDs
spiffe://example.org/ns/example/sa/client-sa and
spiffe://example.org/ns/spire/sa/spire-agent and their corresponding entry
create invocations) to include service-account selectors (e.g., add
k8s:sa:client-sa to the client entry and k8s:sa:spire-agent to the server entry)
so identities are bound unambiguously—apply the same change to the other entry
block referenced (lines 142–146).- Around line 165-217: The Deployment exposes ghostunnel on port 8443
(server-workload) but no Kubernetes Service is defined, so DNS name
server-workload:8443 will not resolve; add a Service that selects pods with
label app: server-workload (from the Deployment template) and exposes port 8443
(targetPort 8443, port 8443, protocol TCP) so clients can resolve and connect
(use a ClusterIP Service name matching the DNS used, e.g., server-workload).- Around line 298-302: Add an explicit mutation + rollout step that patches the
client-workload deployment to set the ghostunnel container's --verify-uri
argument to the incorrect SPIFFE URI and wait for the rollout to complete before
re-running the curl; specifically, apply a kubectl patch against the deployment
"client-workload" to replace the container args entry (e.g., the relevant index
under /spec/template/spec/containers/0/args or the arg matching --verify-uri)
with the wrong value (spiffe://example.org/ns/example/sa/server) and then run
kubectl -n example rollout status deploy/client-workload prior to executing the
existing kubectl exec ... curl command so the failure is reproducible.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `52981c56-cf48-440c-8e54-7d86631e3e02` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 74eb6c8c05482ee87d293b6ea34d9b3333befd86 and 6fcc5f75d56dfd7fc06d0cf832e703c6358500fc. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `docs/en/security/workload_security/spire.mdx` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit