ESO-494:Adds .md files for External-Secrets Operator

[siddhibhor-56]

Summary by CodeRabbit

• Documentation
  • Added new end-to-end repository guides covering operator architecture, Kubernetes API contract conventions (including validation and merge semantics), standardized reconciliation error-handling, and performance/reconciliation best practices.
  • Introduced comprehensive contribution materials, including safe generated-asset update/verification workflows and recommended local development commands.
  • Published testing and integration guidelines, plus consolidated security requirements for hardened deployments and secure metrics/webhook/RBAC behavior.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: siddhibhor-56
Once this PR has been reviewed and has the lgtm label, please assign swghosh for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Walkthrough

This PR adds documentation for repository workflows, operator architecture, API contracts, integration patterns, performance conventions, security requirements, and testing practices.

Changes

Documentation guideline suite

Layer / File(s)	Summary
Repository workflow docs AGENTS.md, CLAUDE.md, CONTRIBUTING.md	Documents repository structure, build and verification commands, style and naming conventions, contribution workflow, and runtime environment variables.
Architecture and API contracts ARCHITECTURE.md, docs/api-contracts-guidelines.md	Documents operator architecture, API package rules, validation markers, CEL constraints, status patterns, generated-artifact workflow, and API field authoring guidance.
Error handling and performance docs/error-handling-guidelines.md, docs/performance-guidelines.md	Documents reconcile error categories, retry behavior, cache and watch patterns, concurrency rules, drift detection, and polling settings.
Integration guidance docs/integration-guidelines.md	Documents singleton precedence, bindata reconciliation, conditional resource installation, platform integrations, client selection, validation rules, and plugin extension steps.
Security guidance docs/security-guidelines.md	Documents container hardening, metrics and RBAC protections, reserved names, TLS handling, network policies, webhook security, trusted CA handling, and auth failure classification.
Testing guidance docs/testing-guidelines.md	Documents unit, API integration, and E2E testing conventions, coverage generation, and CI test configuration.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Suggested reviewers: mytreya-rh

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	PR only adds markdown docs; no Go/test files or Ginkgo titles were changed, so stable test-name rules are not implicated.
Test Structure And Quality	✅ Passed	PR only adds markdown docs; no *_test.go or Ginkgo files changed, so the Ginkgo test-structure check is not applicable.
Microshift Test Compatibility	✅ Passed	Only markdown docs were added; direct diff vs origin/main shows no Go/e2e test files or Ginkgo constructs, so MicroShift compatibility is not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	Reviewed the added e2e tests; they only exercise operator resources and Bitwarden/CA workflows, with no multi-node or SNO-sensitive assumptions.
Topology-Aware Scheduling Compatibility	✅ Passed	PASS: Changed manifests/code add no topology-specific scheduling; scans found no master/control-plane/worker/arbiter selectors, anti-affinity, spread constraints, or PDBs. Manager affinity is arch/...
Ote Binary Stdout Contract	✅ Passed	Only five .md docs changed; no Go/suite setup code was modified, so no stdout-writing process code was introduced.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PR adds only markdown docs; no new Ginkgo e2e test files are introduced, so the IPv6/disconnected-network check is not applicable.
No-Weak-Crypto	✅ Passed	PR only adds Markdown docs; diff vs main has no code and no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom-crypto, or secret-compare patterns.
Container-Privileges	✅ Passed	Checked 330 changed YAML manifests in HEAD; none contained privileged/hostPID/hostNetwork/hostIPC/CAP_SYS_ADMIN/allowPrivilegeEscalation=true/root flags.
No-Sensitive-Data-In-Logs	✅ Passed	Changed files are docs-only and contain no literal passwords/tokens/PII or log lines that would expose sensitive values.
Title check	✅ Passed	The title matches the change set, which adds multiple documentation .md files for the External Secrets Operator.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (5)

docs/testing-guidelines.md (1)

17-113: 💤 Low value

Consider adding blank lines around headings for Markdown compliance.

Multiple headings violate MD022 (blanks-around-headings). While this is a minor formatting issue, adding blank lines improves readability and passes linting. Given the "Chill" review preference, this is optional.

Example:

 ## Unit Tests (`pkg/`)
 
 ### Conventions
+
 - Files live alongside source in the same package (e.g., `deployments.go` / `deployments_test.go`).

🤖 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/testing-guidelines.md` around lines 17 - 113, The Markdown file has
multiple headings without surrounding blank lines (violating MD022); fix by
inserting a single blank line before and after each heading token (e.g., the
"### Conventions", "### Fake Client Pattern", "### Test Helpers", "### Error
Assertion", "### Adding a New Unit Test", "## API Integration Tests
(`test/apis/`)", "### Data-Driven Test Suites", "### envtest Setup", "###
Running", "### Adding a New API Test", "## E2E Tests (`test/e2e/`)", "### Build
Tag", "### Label-Based Platform Selection", "### E2E Structure", "### Test
Data", "### E2E Test Utilities (`test/utils/`)", "### Adding a New E2E Test") so
each heading is preceded and followed by exactly one blank line to satisfy lint
MD022 and improve readability.

CLAUDE.md (1)

9-51: ⚡ Quick win

Add blank lines around headings and code fences for Markdown compliance.

Multiple sections violate MD022 (blanks-around-headings) and MD031 (blanks-around-fences). While the content renders correctly in most viewers, proper spacing improves readability and passes linting.

📐 Example fix for one section

 
 ### Before Committing
+
 ```bash
 make update && make verify

This regenerates all code, manifests, and bindata, then runs verification checks. CI will reject PRs with stale generated files.

Apply similar spacing to all other sections (Development Workflow, Testing, Dependency Management, Claude Code Preferences).
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @CLAUDE.md around lines 9 - 51, Add a blank line before and after each
Markdown heading and before and after each fenced code block in CLAUDE.md (e.g.,
the "Before Committing", "Development Workflow", "Testing", "Dependency
Management", and "Claude Code Preferences" headings and their bash fences)
so the file complies with MD022 and MD031; specifically, ensure there is an
empty line separating the heading from the previous paragraph and an empty line
between the fenced code opening/closing lines and surrounding text throughout
the file.

</details>

<!-- cr-comment:v1:fc5ea020d20ce7d1cb3e1afc -->

</blockquote></details>
<details>
<summary>docs/api-contracts-guidelines.md (1)</summary><blockquote>

`23-23`: _💤 Low value_

**Consider addressing markdown formatting for better consistency.**

The markdownlint tool flagged several minor style issues:
- Line 23: The code block should specify a language (e.g., ````go`) and be surrounded by blank lines
- Several headings (lines 38, 43, 47, 54, 57, 82, 90, 95) should have blank lines before and after them

These don't affect functionality but would improve markdown parsing consistency and potentially enable better syntax highlighting.





Also applies to: 38-38, 43-43, 47-47, 54-54, 57-57, 82-82, 90-90, 95-95

<details>
<summary>🤖 Prompt for AI Agents</summary>

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/api-contracts-guidelines.md at line 23, Update the markdown for
docs/api-contracts-guidelines.md by adding a language specifier to the code
fence at the shown code block (e.g., change togo) and ensure there is a
blank line before and after that fenced code block; also add a blank line before
and after each top-level heading referenced in the review (the headings at the
noted locations) so headings are separated from surrounding paragraphs and lists
for consistent markdownlint-compliant formatting.

</details>

<!-- cr-comment:v1:4182dd4ed530dcbd4d4b986e -->

_Source: Linters/SAST tools_

</blockquote></details>
<details>
<summary>docs/performance-guidelines.md (2)</summary><blockquote>

`1-93`: _⚡ Quick win_

**Add blank lines below section headings for consistency.**

Multiple level-3 headings (`###`) are missing blank lines below them before content begins (lines 5, 12, 19, 24, 38, 43, 51, 54, 59, 62, 65, 70, 77, 88, 91). Markdown best practice requires blank lines around headings for improved readability and consistency.




<details>
<summary>Example fix pattern</summary>

```diff
 ### Manager-Level Label-Filtered Cache
+
 The operator uses a single manager-level cache...
```

Apply this pattern to all flagged headings.
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
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/performance-guidelines.md` around lines 1 - 93, Add a single blank line
after each level-3 Markdown heading that currently runs straight into content so
headings follow the "blank line below headings" convention; specifically insert
a blank line after the headings "### Manager-Level Label-Filtered Cache", "###
Uncached Client Usage", "### Conditional CRD Cache Registration", "### Predicate
Strategy by Resource Type", "### Map Function Convention", "### Error
Classification and Requeue Strategy", "### Condition Update Optimization", "###
CR Annotation Patching", "### UpdateWithRetry Pattern", "### Status Update
Pattern", "### Bootstrap Resource Creation", "### Resettable sync.Once
(`common.Now`)", "### HasObjectChanged Pattern", and "### Asset Decoding" in the
document so each heading is followed by one empty line before its paragraph
content.
```

</details>

<!-- cr-comment:v1:e77c49c434d3ab410a8685d0 -->

_Source: Linters/SAST tools_

---

`5-21`: _⚡ Quick win_

**Consider removing hard-coded line reference.**

Line 8 references "line ~68" in `controller.go`. Hard-coded line numbers in documentation become stale as code evolves. Consider using a more resilient reference like "in `controllerManagedResources` constant" or "in the resource list near the cache builder initialization".

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
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/performance-guidelines.md` around lines 5 - 21, The doc references a
hard-coded line number ("line ~68" in controller.go); replace that brittle
reference with a resilient identifier such as the constant or symbol name (e.g.,
controllerManagedResources) and/or a contextual location (e.g., "the resource
list near NewCacheBuilder() / buildCacheObjectList() in
pkg/controller/external_secrets/controller.go") so readers can find where to
register new resources without relying on line numbers.
```

</details>

<!-- cr-comment:v1:ebe16097cce0cd6e4abe1069 -->

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

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 @AGENTS.md:

• Around line 28-49: The fenced directory-listing block in AGENTS.md is missing
  a language identifier; update the opening triple-backticks that precede the
  directory list (the block containing lines like "api/v1alpha1/ CRD
  type definitions...") to include a plaintext identifier (e.g., text) so the code fence renders and lints correctly; ensure only the first backtick fence is changed and the closing fence remains unchanged.

In @CLAUDE.md:

• Line 3: Replace the nonstandard annotation '@AGENTS.md' in CLAUDE.md with a
  proper Markdown link or an explanatory comment; specifically locate the literal
  '@AGENTS.md' token and either change it to a Markdown link like
  AGENTS.md or, if it is tool-specific, add a short inline comment
  explaining the syntax and its purpose so readers know why it isn't standard
  Markdown.

In @docs/security-guidelines.md:

• Around line 51-55: The markdown fenced code block showing the regex for
  disallowedLabelMatcher (in install_external_secrets.go) needs proper formatting:
  add a blank line before and after the fence and include a language identifier
  (e.g., "regex") on the opening fence so the block renders correctly; update the
  docs/security-guidelines.md block accordingly around the regex snippet for
  disallowedLabelMatcher.

---

Nitpick comments:
In @CLAUDE.md:

• Around line 9-51: Add a blank line before and after each Markdown heading and
  before and after each fenced code block in CLAUDE.md (e.g., the "Before
  Committing", "Development Workflow", "Testing", "Dependency Management", and
  "Claude Code Preferences" headings and their bash fences) so the file
  complies with MD022 and MD031; specifically, ensure there is an empty line
  separating the heading from the previous paragraph and an empty line between the
  fenced code opening/closing lines and surrounding text throughout the file.

In @docs/api-contracts-guidelines.md:

• Line 23: Update the markdown for docs/api-contracts-guidelines.md by adding a
  language specifier to the code fence at the shown code block (e.g., change togo) and ensure there is a blank line before and after that fenced code
  block; also add a blank line before and after each top-level heading referenced
  in the review (the headings at the noted locations) so headings are separated
  from surrounding paragraphs and lists for consistent markdownlint-compliant
  formatting.

In @docs/performance-guidelines.md:

• Around line 1-93: Add a single blank line after each level-3 Markdown heading
  that currently runs straight into content so headings follow the "blank line
  below headings" convention; specifically insert a blank line after the headings
  "### Manager-Level Label-Filtered Cache", "### Uncached Client Usage", "###
  Conditional CRD Cache Registration", "### Predicate Strategy by Resource Type",
  "### Map Function Convention", "### Error Classification and Requeue Strategy",
  "### Condition Update Optimization", "### CR Annotation Patching", "###
  UpdateWithRetry Pattern", "### Status Update Pattern", "### Bootstrap Resource
  Creation", "### Resettable sync.Once (common.Now)", "### HasObjectChanged
  Pattern", and "### Asset Decoding" in the document so each heading is followed
  by one empty line before its paragraph content.
• Around line 5-21: The doc references a hard-coded line number ("line ~68" in
  controller.go); replace that brittle reference with a resilient identifier such
  as the constant or symbol name (e.g., controllerManagedResources) and/or a
  contextual location (e.g., "the resource list near NewCacheBuilder() /
  buildCacheObjectList() in pkg/controller/external_secrets/controller.go") so
  readers can find where to register new resources without relying on line
  numbers.

In @docs/testing-guidelines.md:

• Around line 17-113: The Markdown file has multiple headings without
  surrounding blank lines (violating MD022); fix by inserting a single blank line
  before and after each heading token (e.g., the "### Conventions", "### Fake
  Client Pattern", "### Test Helpers", "### Error Assertion", "### Adding a New
  Unit Test", "## API Integration Tests (test/apis/)", "### Data-Driven Test
  Suites", "### envtest Setup", "### Running", "### Adding a New API Test", "##
  E2E Tests (test/e2e/)", "### Build Tag", "### Label-Based Platform Selection",
  "### E2E Structure", "### Test Data", "### E2E Test Utilities (test/utils/)",
  "### Adding a New E2E Test") so each heading is preceded and followed by exactly
  one blank line to satisfy lint MD022 and improve readability.

</details>

<details>
<summary>🪄 Autofix (Beta)</summary>

Fix all unresolved CodeRabbit comments on this PR:

- [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended)
- [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: Repository: openshift/coderabbit/.coderabbit.yaml

**Review profile**: CHILL

**Plan**: Enterprise

**Run ID**: `088f4b9d-3b92-48d9-a894-246e50996f77`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between e9969c3b3dbb343ccdc9e781d9feffd549b74d71 and 13849aa8957958b3770a70ca98889d060d8ee63a.

</details>

<details>
<summary>📒 Files selected for processing (8)</summary>

* `AGENTS.md`
* `CLAUDE.md`
* `docs/api-contracts-guidelines.md`
* `docs/error-handling-guidelines.md`
* `docs/integration-guidelines.md`
* `docs/performance-guidelines.md`
* `docs/security-guidelines.md`
* `docs/testing-guidelines.md`

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify or remove the @AGENTS.md annotation.

The @AGENTS.md syntax is not standard Markdown. If this is intended as a reference or link, use a proper Markdown link: [AGENTS.md](AGENTS.md). If it's meant for a specific tool or extension, add a comment explaining the syntax.

🔗 Proposed fix

 # CLAUDE.md
 
-@AGENTS.md
+See [AGENTS.md](AGENTS.md) for comprehensive architectural details.

📝 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.

Suggested change

	@AGENTS.md
	# CLAUDE.md
	See [AGENTS.md](AGENTS.md) for comprehensive architectural details.

🤖 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 `@CLAUDE.md` at line 3, Replace the nonstandard annotation '`@AGENTS.md`' in
CLAUDE.md with a proper Markdown link or an explanatory comment; specifically
locate the literal '`@AGENTS.md`' token and either change it to a Markdown link
like [AGENTS.md](AGENTS.md) or, if it is tool-specific, add a short inline
comment explaining the syntax and its purpose so readers know why it isn't
standard Markdown.

[siddhibhor-56]

/retest

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

CLAUDE.md (1)

5-52: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Normalize the heading and fence spacing.

markdownlint still flags MD022/MD031 in the command sections. Add blank lines around the headings and fenced blocks so the doc passes lint cleanly.

🤖 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 `@CLAUDE.md` around lines 5 - 52, The command section markdown in CLAUDE.md
still violates markdownlint MD022/MD031; update the Build and Test Commands and
related headings so they are surrounded by blank lines, and ensure each fenced
command block has the proper blank line spacing before and after it. Keep the
existing content but normalize the spacing around the headings and code fences
so the document passes lint cleanly.

Source: Linters/SAST tools

🤖 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 `@ARCHITECTURE.md`:
- Around line 95-99: The reconciliation-flow markdown block in ARCHITECTURE.md
is missing a language tag on the second fenced block, which trips markdownlint.
Update the relevant fenced section so it opens with text, matching the existing
code fence style used in the document, and keep the change localized to the
reconciliation-flow description.
- Around line 13-43: Add a text language tag to the fenced ASCII diagram so
markdownlint no longer reports MD040; update the diagram fence in
ARCHITECTURE.md to open as text while keeping the existing diagram content
unchanged. Locate the fenced block containing the ExternalSecretsConfig
architecture diagram and adjust only the opening fence.

In `@docs/error-handling-guidelines.md`:
- Around line 91-94: The `CreateDefaultESMResource` retry flow is treating
`TooManyRequests` as a terminal stop condition, which prevents backoff and
retry. Update the `shouldRetryOnError` predicate used by `retry.OnError` in
`externalsecretsmanager.go` to remove `TooManyRequests` from the stop list,
while keeping the other non-retryable errors intact, so 429 responses continue
through retry logic instead of returning success early.

In `@docs/integration-guidelines.md`:
- Around line 80-84: Clarify the label rules in the integration guidelines: the
current “managed resources carry app: external-secrets” requirement conflicts
with the disallowed “app” prefix in the blocklist. Update the wording around the
managed label set and the selector/cache filter contract in this section so it
clearly states whether app is required, exempted, or should be replaced by
another label key, keeping the guidance consistent with the validation rules.

In `@docs/testing-guidelines.md`:
- Around line 3-131: The Markdown in testing-guidelines.md needs spacing
normalization to satisfy markdownlint (MD022/MD031). Update the document so
headings in the Test Architecture, Unit Tests, API Integration Tests, E2E Tests,
Coverage, and CI Notes sections have proper blank lines before/after them, and
ensure any tables or fenced-style blocks are separated by blank lines where
needed. Focus on the formatting around the section headings and the main content
blocks so the file renders cleanly and passes lint.

---

Outside diff comments:
In `@CLAUDE.md`:
- Around line 5-52: The command section markdown in CLAUDE.md still violates
markdownlint MD022/MD031; update the Build and Test Commands and related
headings so they are surrounded by blank lines, and ensure each fenced command
block has the proper blank line spacing before and after it. Keep the existing
content but normalize the spacing around the headings and code fences so the
document passes lint cleanly.

🪄 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: c0edcaf7-3570-40f7-a01d-6a9e32e1ff93

📥 Commits

Reviewing files that changed from the base of the PR and between 13849aa and dfed87b.

📒 Files selected for processing (10)

• AGENTS.md
• ARCHITECTURE.md
• CLAUDE.md
• CONTRIBUTING.md
• docs/api-contracts-guidelines.md
• docs/error-handling-guidelines.md
• docs/integration-guidelines.md
• docs/performance-guidelines.md
• docs/security-guidelines.md
• docs/testing-guidelines.md

✅ Files skipped from review due to trivial changes (1)

• CONTRIBUTING.md

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== doc snippet ==\n'
sed -n '80,105p' docs/error-handling-guidelines.md

printf '\n== controller references ==\n'
rg -n "shouldRetryOnError|TooManyRequests|retry.OnError" pkg/controller/external_secrets_manager -S

printf '\n== file outline ==\n'
ast-grep outline pkg/controller/external_secrets_manager/externalsecretsmanager.go --view expanded

Repository: openshift/external-secrets-operator

Length of output: 2302

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== externalsecretsmanager.go ==\n'
cat -n pkg/controller/external_secrets_manager/externalsecretsmanager.go

printf '\n== call sites ==\n'
rg -n "CreateDefaultESMResource\(" -S .

Repository: openshift/external-secrets-operator

Length of output: 2616

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== setup_manager.go ==\n'
cat -n pkg/operator/setup_manager.go

Repository: openshift/external-secrets-operator

Length of output: 2568

---

Remove TooManyRequests from the stop list. CreateDefaultESMResource treats stop-list errors as success (return nil inside the retry.OnError callback), so a 429 currently skips the create entirely instead of backing off and retrying.

🤖 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/error-handling-guidelines.md` around lines 91 - 94, The
`CreateDefaultESMResource` retry flow is treating `TooManyRequests` as a
terminal stop condition, which prevents backoff and retry. Update the
`shouldRetryOnError` predicate used by `retry.OnError` in
`externalsecretsmanager.go` to remove `TooManyRequests` from the stop list,
while keeping the other non-retryable errors intact, so 429 responses continue
through retry logic instead of returning success early.

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Normalize the Markdown block spacing.

This file still trips MD022/MD031 across most sections. Add blank lines around the headings and fenced blocks so it passes markdownlint.

🧰 Tools

🪛 LanguageTool

[grammar] ~40-~40: Ensure spelling is correct
Context: ...lient{}for the client. 4. Assert witht.Errorf`; avoid external assertion libraries in u...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🪛 markdownlint-cli2 (0.22.1)

[warning] 17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 28-28: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 32-32: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 36-36: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 44-44: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 49-49: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 55-55: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 59-59: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 67-67: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 70-70: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 90-90: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 97-97: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 102-102: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 111-111: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

🤖 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/testing-guidelines.md` around lines 3 - 131, The Markdown in
testing-guidelines.md needs spacing normalization to satisfy markdownlint
(MD022/MD031). Update the document so headings in the Test Architecture, Unit
Tests, API Integration Tests, E2E Tests, Coverage, and CI Notes sections have
proper blank lines before/after them, and ensure any tables or fenced-style
blocks are separated by blank lines where needed. Focus on the formatting around
the section headings and the main content blocks so the file renders cleanly and
passes lint.

Source: Linters/SAST tools

[bharath-b-rh]

Still going through, just a few suggestions for now.

[bharath-b-rh]

Shouldn't we add a section on Godoc too?

The Godoc on any field in the API should be sufficiently explained such that an end user understands the purpose, interactions, limitations, and default behavior.

[siddhibhor-56]

Done

[bharath-b-rh]

Lets add below detail to avoid defining defaults in CRD going forward.

Excerpt from OpenShift API conventions

With configuration APIs, we typically default fields within the controller and not within the API. This means that the platform has the ability to make changes to the defaults over time.

[siddhibhor-56]

Done

[bharath-b-rh]

We should also add a rule not define any functions in API package.

[siddhibhor-56]

Done

[bharath-b-rh]

This section will need an update when we merge the e2e restructuring PR. Mainly around the labeling of test cases.

[siddhibhor-56]

True

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/api-contracts-guidelines.md (1)

5-11: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Clarify the init() exception.

Line 5 bans all functions in api/v1alpha1/, but Line 11 requires SchemeBuilder.Register() inside an init() in the same package. Please either exempt this minimal registration function or reword the rule; as written, the guidance is self-contradictory.

✍️ Suggested wording

-- **No functions in the API package.** The `api/v1alpha1/` package must contain only type definitions, constants, markers, and generated code (deepcopy).
+- **No business-logic functions in the API package.** The `api/v1alpha1/` package must contain only type definitions, constants, markers, generated code (deepcopy), and the minimal `init()` registration needed for `SchemeBuilder`.

🤖 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/api-contracts-guidelines.md` around lines 5 - 11, The API package
guidance is internally inconsistent because it bans all functions in
api/v1alpha1/ while also requiring an init() there for SchemeBuilder.Register().
Reword the “No functions in the API package” rule to explicitly exempt the
minimal init() used only for scheme registration, and keep the exception aligned
with the SchemeBuilder.Register() requirement in the types file.

🤖 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.

Outside diff comments:
In `@docs/api-contracts-guidelines.md`:
- Around line 5-11: The API package guidance is internally inconsistent because
it bans all functions in api/v1alpha1/ while also requiring an init() there for
SchemeBuilder.Register(). Reword the “No functions in the API package” rule to
explicitly exempt the minimal init() used only for scheme registration, and keep
the exception aligned with the SchemeBuilder.Register() requirement in the types
file.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ade6c0d5-07c1-4764-9c39-2c39e609f561

📥 Commits

Reviewing files that changed from the base of the PR and between dfed87b and d220dda.

📒 Files selected for processing (5)

• AGENTS.md
• ARCHITECTURE.md
• docs/api-contracts-guidelines.md
• docs/integration-guidelines.md
• docs/security-guidelines.md

✅ Files skipped from review due to trivial changes (4)

• docs/integration-guidelines.md
• ARCHITECTURE.md
• docs/security-guidelines.md
• AGENTS.md

[openshift-ci]

@siddhibhor-56: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

[openshift-ci-robot]

@siddhibhor-56: This pull request references ESO-494 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 story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary by CodeRabbit

• Documentation
• Added new end-to-end repository guides covering operator architecture, Kubernetes API contract conventions (including validation and merge semantics), standardized reconciliation error-handling, and performance/reconciliation best practices.
• Introduced comprehensive contribution materials, including safe generated-asset update/verification workflows and recommended local development commands.
• Published testing and integration guidelines, plus consolidated security requirements for hardened deployments and secure metrics/webhook/RBAC behavior.

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.