OAPE-815: Add Level 2 team context documentation#147
Conversation
Adds team-level context documents under docs/team/ as part of the Track 1 Contextification initiative. These provide discoverable organizational context for both humans and AI agents. Files added: - team_overview.md: team identity, scope, members, org placement - development_standards.md: language, conventions, linting, FIPS - testing_standards.md: philosophy, expectations, frameworks - code_review_standards.md: approval process, Prow commands, checklist - ci_cd_configuration.md: Prow, Konflux, release branches, stage builds - dependency_map.md: upstream/downstream relationships, coordination These reference existing detailed guides in docs/ rather than duplicating them. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@PillaiManish: This pull request references OAPE-815 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: PillaiManish The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughThis PR adds six new documentation files under ChangesTeam Documentation Additions
Estimated code review effort: 1 (Trivial) | ~10 minutes Estimated code review effort: 1 (Trivial) | ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
docs/team/dependency_map.md (1)
21-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the dependency versions instead of using
latest.
latestmakes this map non-actionable and immediately stale. Please replace these entries with the resolved versions from the module graph, or explicitly mark them as intentionally floating.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/team/dependency_map.md` around lines 21 - 28, The dependency map still uses floating versions for github.com/openshift/api and github.com/openshift/build-machinery-go, which makes the list non-actionable. Update the entries in dependency_map.md to pinned versions resolved from the module graph, or clearly label them as intentionally floating if that is the intended policy. Use the existing dependency table entries for these two modules as the place to make the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/team/code_review_standards.md`:
- Around line 38-41: The Turnaround Expectations section is still a placeholder,
so update the code_review_standards doc to either state the agreed review
turnaround SLA or clearly mark it as draft until finalized. Use the existing
“Turnaround Expectations” heading and replace the TODO text with the actual
target timing so the standard is explicit and actionable.
In `@docs/team/dependency_map.md`:
- Around line 43-45: The dependency map text is naming the SPIRE Agent socket
file instead of the mounted directory, which can confuse operators about the
actual mount value. Update the wording in the OSSM/SPIRE integration description
to refer to the mounted path used by the SPIFFE CSI Driver, and use the existing
dependency map phrasing around the integration point to keep the location clear
without calling out the socket filename.
In `@docs/team/development_standards.md`:
- Line 34: Update the wording in the development standards entry so the timeout
description is hyphenated correctly; the phrase in the configuration note for
`.golangci.yml` should read “5-minute timeout” instead of “5 minute timeout.”
Locate the text in the standards document and adjust only that label so it
matches the lint warning’s wording.
In `@docs/team/team_overview.md`:
- Around line 45-57: The team overview still contains placeholder sections for
leadership, org placement, and repositories, so update the
docs/team/team_overview.md content with the actual Tech Lead, Engineering
Manager, Product Manager, and Hybrid Platforms pillar/group details, plus any
additional owned repos. If those values are not confirmed yet, mark the page as
draft in the team_overview document instead of leaving TODO placeholders. Refer
to the existing sections in team_overview to keep the structure consistent.
In `@docs/team/testing_standards.md`:
- Around line 45-46: The testing standards table entry uses an abbreviated
timeout phrase that triggers the spelling/style warning; update the `make
test-e2e` description to use a full hyphenated unit form. Keep the wording in
`docs/team/testing_standards.md` consistent with the surrounding table and
replace the shorthand in that row with “45-minute timeout” so the `make
test-e2e` entry reads cleanly.
- Around line 52-61: The fenced directory listing in the testing standards doc
is missing an explicit language tag, which triggers markdownlint. Update the
fenced block under the section listing the spire-server test files to use a text
language identifier so the block is rendered as plain text and remains
lint-friendly.
- Around line 33-38: The Coverage section in testing_standards.md still contains
a placeholder TODO instead of the actual policy. Update the Coverage section to
state the concrete coverage threshold or no-regression rule, or remove the
section if it is not finalized yet; use the existing Coverage heading and
surrounding coverage-instrumented build note to keep the wording consistent.
---
Nitpick comments:
In `@docs/team/dependency_map.md`:
- Around line 21-28: The dependency map still uses floating versions for
github.com/openshift/api and github.com/openshift/build-machinery-go, which
makes the list non-actionable. Update the entries in dependency_map.md to pinned
versions resolved from the module graph, or clearly label them as intentionally
floating if that is the intended policy. Use the existing dependency table
entries for these two modules as the place to make the change.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d97c1c1d-034c-4c1f-8469-c2abc7d1aa3c
📒 Files selected for processing (6)
docs/team/ci_cd_configuration.mddocs/team/code_review_standards.mddocs/team/dependency_map.mddocs/team/development_standards.mddocs/team/team_overview.mddocs/team/testing_standards.md
| ## Turnaround Expectations | ||
|
|
||
| [TODO: team to fill in — target review turnaround time, e.g., "initial review within 1 business day"] | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- target file excerpt ---\n'
nl -ba docs/team/code_review_standards.md | sed -n '1,120p'
printf '\n--- placeholder search in docs/team ---\n'
rg -n "\[TODO: team to fill in|draft|turnaround|review turnaround" docs/team -S || trueRepository: openshift/zero-trust-workload-identity-manager
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- target file excerpt ---\n'
sed -n '1,140p' docs/team/code_review_standards.md | cat -n
printf '\n--- placeholder search in docs/team ---\n'
rg -n "\[TODO: team to fill in|draft|turnaround|review turnaround" docs/team -S || trueRepository: openshift/zero-trust-workload-identity-manager
Length of output: 3819
Fill in the review turnaround SLA. This section still reads as a placeholder; add the agreed target review turnaround time or mark the doc as draft until it’s finalized.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/team/code_review_standards.md` around lines 38 - 41, The Turnaround
Expectations section is still a placeholder, so update the code_review_standards
doc to either state the agreed review turnaround SLA or clearly mark it as draft
until finalized. Use the existing “Turnaround Expectations” heading and replace
the TODO text with the actual target timing so the standard is explicit and
actionable.
| OSSM consumes the SPIRE Agent's Workload API socket for mTLS identity in the service mesh | ||
| data plane. The integration point is the agent socket path | ||
| (`/run/spire/agent-sockets/spire-agent.sock`) mounted via the SPIFFE CSI Driver. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File size ==\n'
wc -l docs/team/dependency_map.md
printf '\n== Relevant lines ==\n'
cat -n docs/team/dependency_map.md | sed -n '35,55p'
printf '\n== Search for SPIFFE/SPIRE mount path references ==\n'
rg -n "spire-agent\.sock|agent-sockets|Workload API socket|SPIFFE CSI Driver|SPIRE Agent" docs . --glob '!**/.git/**'Repository: openshift/zero-trust-workload-identity-manager
Length of output: 13651
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Any SPIFFE CSI Driver mount-path wording in repo ==\n'
rg -n "mounted via the SPIFFE CSI Driver|mounted path|socket directory|socket file" docs . --glob '!**/.git/**'
printf '\n== If docs mention the directory explicitly elsewhere ==\n'
rg -n "/run/spire/agent-sockets(?:/spire-agent\.sock)?" docs . --glob '!**/.git/**'Repository: openshift/zero-trust-workload-identity-manager
Length of output: 2496
🌐 Web query:
SPIFFE CSI Driver mount socket directory workload API socket path documentation
💡 Result:
The SPIFFE CSI Driver is designed to inject the SPIFFE Workload API socket into workload pods as an ephemeral inline volume, avoiding the need for direct hostPath access in those pods [1][2]. Key architectural and configuration details regarding the socket directories include: 1. Workload API Socket Directory: The SPIFFE CSI Driver and the Workload API implementation (such as the SPIRE Agent) must share the directory containing the Workload API Unix Domain Socket [1]. This is typically achieved using a hostPath volume on the node [1][3]. An emptyDir volume is generally avoided for this purpose because restarting the CSI driver pod would invalidate the mount for existing workloads [1][3]. 2. CSI Driver Configuration: - Workload API Socket Directory: The driver is configured with the flag -workload-api-socket-dir, which specifies the path where it expects to find the Workload API socket provided by the SPIFFE implementation (e.g., /spire-agent-socket) [3][4]. - CSI Socket Path: The path for the communication between the Kubelet and the CSI driver itself is configured via -csi-socket-path, commonly placed in a directory like /var/lib/kubelet/plugins/csi.spiffe.io [3][4]. 3. Workload Usage: Workload pods define the volume using the csi driver name csi.spiffe.io [5]. The driver performs a read-only bind mount of the directory containing the Workload API socket into the workload container at the specified mountPath [1][2]. Once a pod is destroyed, the driver automatically removes this bind mount [2]. For implementation, ensure that the hostPath volume shared between the SPIRE agent and the SPIFFE CSI driver has the appropriate permissions and type (e.g., DirectoryOrCreate) [3][4]. Documentation suggests referencing the official SPIFFE CSI Driver repository for deployment examples and manifest configurations [1][6].
Citations:
- 1: https://github.com/spiffe/spiffe-csi/blob/main/README.md
- 2: https://hub.docker.com/hardened-images/catalog/dhi/spiffe-csi-driver
- 3: spiffe/spiffe-csi@a170c13
- 4: https://github.com/nephio-project/catalog/blob/main/nephio/optional/spire-agent/spiffe-csi.yaml
- 5: https://hub.docker.com/hardened-images/catalog/dhi/spiffe-csi-driver/guides
- 6: https://github.com/spiffe/spiffe-csi/tree/main/example
Describe the mounted directory, not the socket file
/run/spire/agent-sockets is the mounted path; spire-agent.sock lives inside it. Naming the socket file here can mislead operators about the config value.
Suggested wording
- The integration point is the agent socket path (`/run/spire/agent-sockets/spire-agent.sock`) mounted via the SPIFFE CSI Driver.
+ The integration point is the agent socket directory (`/run/spire/agent-sockets`, containing `spire-agent.sock`) mounted via the SPIFFE CSI Driver.📝 Committable suggestion
‼️ 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.
| OSSM consumes the SPIRE Agent's Workload API socket for mTLS identity in the service mesh | |
| data plane. The integration point is the agent socket path | |
| (`/run/spire/agent-sockets/spire-agent.sock`) mounted via the SPIFFE CSI Driver. | |
| OSSM consumes the SPIRE Agent's Workload API socket for mTLS identity in the service mesh | |
| data plane. The integration point is the agent socket directory (`/run/spire/agent-sockets`, containing `spire-agent.sock`) mounted via the SPIFFE CSI Driver. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/team/dependency_map.md` around lines 43 - 45, The dependency map text is
naming the SPIRE Agent socket file instead of the mounted directory, which can
confuse operators about the actual mount value. Update the wording in the
OSSM/SPIRE integration description to refer to the mounted path used by the
SPIFFE CSI Driver, and use the existing dependency map phrasing around the
integration point to keep the location clear without calling out the socket
filename.
| `goimports`, `gosimple`, `govet`, `ineffassign`, `lll`, `misspell`, `nakedret`, `prealloc`, | ||
| `revive`, `staticcheck`, `typecheck`, `unconvert`, `unparam`, `unused` | ||
|
|
||
| Configuration: `.golangci.yml` (5 minute timeout, parallel runners enabled). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Hyphenate the timeout description.
5 minute timeout should be 5-minute timeout; that's the wording the lint warning is pointing at.
Suggested fix
-Configuration: `.golangci.yml` (5 minute timeout, parallel runners enabled).
+Configuration: `.golangci.yml` (5-minute timeout, parallel runners enabled).📝 Committable suggestion
‼️ 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.
| Configuration: `.golangci.yml` (5 minute timeout, parallel runners enabled). | |
| Configuration: `.golangci.yml` (5-minute timeout, parallel runners enabled). |
🧰 Tools
🪛 LanguageTool
[grammar] ~34-~34: Use a hyphen to join words.
Context: ...used Configuration:.golangci.yml` (5 minute timeout, parallel runners enabled...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/team/development_standards.md` at line 34, Update the wording in the
development standards entry so the timeout description is hyphenated correctly;
the phrase in the configuration note for `.golangci.yml` should read “5-minute
timeout” instead of “5 minute timeout.” Locate the text in the standards
document and adjust only that label so it matches the lint warning’s wording.
Source: Linters/SAST tools
| [TODO: team to fill in — Tech Lead, Engineering Manager, Product Manager roles] | ||
|
|
||
| ## Org Placement | ||
|
|
||
| [TODO: team to fill in — which pillar/group this team sits under within Hybrid Platforms] | ||
|
|
||
| ## Repositories | ||
|
|
||
| | Repository | Purpose | | ||
| |------------|---------| | ||
| | `openshift/zero-trust-workload-identity-manager` | Main operator repo | | ||
|
|
||
| [TODO: team to fill in — any other repos the team owns or contributes to] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## File contents\n'
git ls-files docs/team/team_overview.md
wc -l docs/team/team_overview.md
cat -n docs/team/team_overview.md | sed -n '1,220p'
printf '\n## Nearby team docs and references\n'
fd -a 'team_overview.md|team' docs . 2>/dev/null | sed -n '1,200p' || true
rg -n --hidden --no-messages 'Hybrid Platforms|Tech Lead|Engineering Manager|Product Manager|zero-trust-workload-identity-manager|TODO: team to fill in' docs . | sed -n '1,240p' || trueRepository: openshift/zero-trust-workload-identity-manager
Length of output: 35555
Fill in the remaining team details docs/team/team_overview.md:45-57 — the Tech Lead/Engineering Manager/Product Manager, org placement, and owned-repo sections are still placeholders. If those details aren’t final yet, mark this page as draft until they are.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/team/team_overview.md` around lines 45 - 57, The team overview still
contains placeholder sections for leadership, org placement, and repositories,
so update the docs/team/team_overview.md content with the actual Tech Lead,
Engineering Manager, Product Manager, and Hybrid Platforms pillar/group details,
plus any additional owned repos. If those values are not confirmed yet, mark the
page as draft in the team_overview document instead of leaving TODO
placeholders. Refer to the existing sections in team_overview to keep the
structure consistent.
| ## Coverage | ||
|
|
||
| [TODO: team to fill in — coverage thresholds or "no regression" policy] | ||
|
|
||
| Coverage-instrumented builds are available via `make docker-build-coverage` for E2E | ||
| coverage collection. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,120p' docs/team/testing_standards.md && printf '\n---\n' && rg -n "coverage|no regression|threshold" docs -g '*.md'Repository: openshift/zero-trust-workload-identity-manager
Length of output: 2670
Define the coverage policy. The Coverage section still has a TODO, so the doc doesn’t state the actual threshold or no-regression rule. Add the concrete policy or remove the section until it’s finalized.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/team/testing_standards.md` around lines 33 - 38, The Coverage section in
testing_standards.md still contains a placeholder TODO instead of the actual
policy. Update the Coverage section to state the concrete coverage threshold or
no-regression rule, or remove the section if it is not finalized yet; use the
existing Coverage heading and surrounding coverage-instrumented build note to
keep the wording consistent.
| | `make test-e2e` | E2E tests against a live cluster. Requires `KUBECONFIG`. 45 min timeout. | | ||
| | `make verify` | vet + fmt + lint (includes `ginkgolinter` for test style). | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a full timeout unit.
45 min timeout should be 45-minute timeout for clarity and to satisfy the spelling warning.
Suggested fix
-| `make test-e2e` | E2E tests against a live cluster. Requires `KUBECONFIG`. 45 min timeout. |
+| `make test-e2e` | E2E tests against a live cluster. Requires `KUBECONFIG`. 45-minute timeout. |📝 Committable suggestion
‼️ 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.
| | `make test-e2e` | E2E tests against a live cluster. Requires `KUBECONFIG`. 45 min timeout. | | |
| | `make verify` | vet + fmt + lint (includes `ginkgolinter` for test style). | | |
| | `make test-e2e` | E2E tests against a live cluster. Requires `KUBECONFIG`. 45-minute timeout. | | |
| | `make verify` | vet + fmt + lint (includes `ginkgolinter` for test style). | |
🧰 Tools
🪛 LanguageTool
[grammar] ~45-~45: Ensure spelling is correct
Context: ... tests against a live cluster. Requires KUBECONFIG. 45 min timeout. | | make verify | ve...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/team/testing_standards.md` around lines 45 - 46, The testing standards
table entry uses an abbreviated timeout phrase that triggers the spelling/style
warning; update the `make test-e2e` description to use a full hyphenated unit
form. Keep the wording in `docs/team/testing_standards.md` consistent with the
surrounding table and replace the shorthand in that row with “45-minute timeout”
so the `make test-e2e` entry reads cleanly.
Source: Linters/SAST tools
| ``` | ||
| pkg/controller/spire-server/ | ||
| ├── controller_test.go | ||
| ├── service_account_test.go | ||
| ├── service_test.go | ||
| ├── rbac_test.go | ||
| ├── configmaps_test.go | ||
| ├── statefulset_test.go | ||
| └── suite_test.go | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language to this fenced block.
text is enough here and keeps markdownlint happy.
Suggested fix
-```
+```text📝 Committable suggestion
‼️ 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.
| ``` | |
| pkg/controller/spire-server/ | |
| ├── controller_test.go | |
| ├── service_account_test.go | |
| ├── service_test.go | |
| ├── rbac_test.go | |
| ├── configmaps_test.go | |
| ├── statefulset_test.go | |
| └── suite_test.go | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/team/testing_standards.md` around lines 52 - 61, The fenced directory
listing in the testing standards doc is missing an explicit language tag, which
triggers markdownlint. Update the fenced block under the section listing the
spire-server test files to use a text language identifier so the block is
rendered as plain text and remains lint-friendly.
Source: Linters/SAST tools
|
This files would be at a common place along with other components. So this PR would be in draft state. |
Summary
Adds Level 2 team context documentation under
docs/team/as part of the Track 1 Contextification initiative (OAPE-815). These are higher-level organizational context docs that answer "who, what, how" questions -- distinct from the existing technical guidelines indocs/.Files Added
docs/team/team_overview.mddocs/team/development_standards.mddocs/team/testing_standards.mddocs/team/code_review_standards.mddocs/team/ci_cd_configuration.mddocs/team/dependency_map.mdRelationship to Existing Docs
These reference (not duplicate) the existing detailed guides:
docs/testing-guidelines.mddocs/security-guidelines.mddocs/error-handling-guidelines.mddocs/api-contracts-guidelines.mddocs/integration-guidelines.mddocs/performance-guidelines.mdTODOs for Team Discussion
Some sections are marked
[TODO: team to fill in]and need team input:Test Plan
Made with Cursor
Summary by CodeRabbit