docs/adr/0013_sovereign_cloud_reference_scenario.md (2)

206-221: Add language identifiers to fenced code blocks to satisfy markdown lint.

markdownlint MD040 is triggered here. Please annotate these fences (for example, text for tree/layout blocks and bash for commands) to keep docs CI clean.

Proposed fix

-```
+```text
 acme.org/sovereign/product (meta-component)
 ...
-```
+```

-```
+```bash
 openssl genpkey -algorithm RSA -out acme-private.pem -pkeyopt rsa_keygen_bits:4096
 openssl rsa -pubout -in acme-private.pem -out acme-public.pem
-```
+```

-```
+```text
 conformance/scenarios/sovereign/
 ...
-```
+```

Also applies to: 261-264, 497-532

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/adr/0013_sovereign_cloud_reference_scenario.md` around lines 206 - 221,
Several fenced code blocks in
docs/adr/0013_sovereign_cloud_reference_scenario.md are missing language
identifiers (triggering markdownlint MD040); update each triple-backtick fence
shown in the diff (the tree/layout blocks such as the component/resource trees
and the conformance/scenarios listing) to use ```text and update any command
blocks (openssl commands) to use ```bash; ensure you apply the same fixes to the
other affected ranges mentioned (lines 261-264 and 497-532) so all fenced blocks
include appropriate language identifiers.

---

455-455: Polish phrasing for readability on Line 455.

“...how to fully self-manage an upgrade” reads a bit awkwardly; consider “...how to self-manage an upgrade fully” or simpler wording.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/adr/0013_sovereign_cloud_reference_scenario.md` at line 455, The
sentence "It also shows how to fully self-manage an upgrade." is awkwardly
phrased; replace it with clearer wording such as "It also shows how to
self-manage an upgrade" or "It also shows how to manage an upgrade fully"
(update the exact fragment "how to fully self-manage an upgrade" on Line 455 to
one of these alternatives for improved readability).

conformance/scenarios/sovereign/Taskfile.yml (2)

272-278: ⚠️ Potential issue | 🟠 Major

cluster:install:flux claims readiness waiting but doesn’t actually wait.

Line 274 says “wait for readiness”, but Line 277 only runs flux install. This can still race cluster:bootstrap when Flux deployments are not Ready yet.

Proposed fix

   cluster:install:flux:
     silent: true
     desc: Install Flux and wait for readiness
     cmd: |
       echo "Installing flux..."
       flux install --components=source-controller,helm-controller
+      flux check --components=source-controller,helm-controller
+      kubectl -n flux-system rollout status deploy/source-controller --timeout=300s
+      kubectl -n flux-system rollout status deploy/helm-controller --timeout=300s

#!/bin/bash
set -euo pipefail

# Verify current task content
rg -n "cluster:install:flux|flux install|flux check|rollout status" conformance/scenarios/sovereign/Taskfile.yml -A4 -B2

# Verify Flux commands/options are available in your environment
flux install --help | sed -n '1,120p'
flux check --help | sed -n '1,120p'

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@conformance/scenarios/sovereign/Taskfile.yml` around lines 272 - 278, The
Taskfile task cluster:install:flux currently runs flux install but doesn’t
actually wait for Flux to become ready; update the task to (after running flux
install) invoke Flux readiness checks such as running flux check (or flux check
--pre) and then wait for the controller deployments in the flux-system namespace
to be Available (e.g., use kubectl wait --for=condition=available
deployment/source-controller,deployment/helm-controller -n flux-system
--timeout=...) so cluster:bootstrap won’t race with incomplete Flux deployments.

---

355-376: ⚠️ Potential issue | 🟠 Major

Missing Docker config path exits before generating OCM_CREDENTIALS_CONFIG.

At Line 355, the early exit 0 skips Lines 368-376, but Line 168 always points OCM_CONFIG to {{ .OCM_CREDENTIALS_CONFIG }}. This can fail runs on machines without ~/.docker/config.json.

Proposed fix

-          [ -f "$SRC" ] || { echo "No Docker config, skipping"; exit 0; }
+          if [ ! -f "$SRC" ]; then
+            echo "No Docker config found; generating empty auth config"
+            echo '{"auths":{}}' > "$DST"
+          fi

           HELPER="docker-credential-$(jq -r '.credsStore // empty' "$SRC")"

-          if command -v "$HELPER" >/dev/null 2>&1; then
+          if [ -f "$SRC" ] && [ -n "$HELPER" ] && command -v "$HELPER" >/dev/null 2>&1; then
             "$HELPER" list 2>/dev/null | jq -r 'keys[]' | while IFS= read -r reg; do
               echo "$reg" | "$HELPER" get 2>/dev/null
             done | jq -s '{auths: ([.[] | select(.Username and .Secret) |
               {(.ServerURL): {auth: ((.Username + ":" + .Secret) | `@base64`)}}] | add // {})}' > "$DST"
-          else
+          elif [ -f "$SRC" ]; then
             cp "$SRC" "$DST"
           fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@conformance/scenarios/sovereign/Taskfile.yml` around lines 355 - 376, The
script currently exits early when the Docker config ($SRC) is missing, which
prevents generation of the required OCM_CREDENTIALS_CONFIG; update the block so
it does not call "exit 0" but instead produces a valid minimal docker config at
"$DST" and continues to write "{{ .OCM_CREDENTIALS_CONFIG }}". Concretely,
replace the early exit with a fallback that creates an empty auths JSON (e.g.,
'{ "auths": {} }' redirected to "$DST") or copies a minimal template, keep the
HELPER lookup and credential extraction logic intact (using HELPER, SRC, DST),
and ensure the subsequent cat >> "{{ .OCM_CREDENTIALS_CONFIG }}" runs
unconditionally so OCM_CREDENTIALS_CONFIG is always generated.

conformance/scenarios/sovereign/README.md (1)

156-156: ⚠️ Potential issue | 🟡 Minor

Add a language tag to the fenced block.

Line 156 still uses an untyped fenced code block, so markdownlint MD040 will continue to fail. Use a typed fence like ```text.

Proposed fix

-```
+```text
 sovereign-scenario/
 ├── README.md                    # This file
 ├── Taskfile.yml                 # Build and deployment automation
 ├── components/                  # OCM component definitions
 │   ├── notes/                   # Notes application component
 │   ├── postgres/                # PostgreSQL component  
 │   └── product/                 # Meta component
 ├── deploy/                      # OCM controller deployment manifests and bootstrap apis

</details>

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

Verify each finding against the current code and only fix it if needed.

In @conformance/scenarios/sovereign/README.md at line 156, The README.md
contains an untyped fenced code block showing the directory tree (the block
starting with "sovereign-scenario/"); change the opening fence from to a typed fence such astext so markdownlint MD040 passes, and ensure the closing

docs/adr/0013_sovereign_cloud_reference_scenario.md (1)

63-63: Tighten wording in overview sentence.

“value propositioning statements” reads awkwardly; consider “core value proposition”.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/adr/0013_sovereign_cloud_reference_scenario.md` at line 63, The overview
sentence uses the awkward phrase "value propositioning statements"; update that
sentence to use "core value proposition" instead—e.g., change "one of our core
value propositioning statements" to "one of our core value propositions" (or
"core value proposition") in the opening line so it reads naturally; locate the
sentence in docs/adr/0013_sovereign_cloud_reference_scenario.md that begins
"This document designs a reference scenario..." and replace the phrase
accordingly.

conformance/scenarios/sovereign/components/notes/cmd/sovereign-notes/main.go (1)

192-199: Consider masking internal error details in responses.

Line 194 exposes the raw database error to clients. While acceptable for a reference scenario where debugging visibility is valuable, production code should return a generic message and log the actual error server-side.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@conformance/scenarios/sovereign/components/notes/cmd/sovereign-notes/main.go`
around lines 192 - 199, In readinessHandler replace returning the raw DB error
to the client; call db.PingContext(r.Context()) as before but on error respond
with a generic message (e.g. "Database not ready") and a 503, and send the
actual err.Error() to server-side logs instead (use the existing logger if
available, or log.Printf/log.Println) so internal error details are not exposed
to clients; ensure you still return after writing the response and keep the
final 200 "Ready" path unchanged.

conformance/scenarios/sovereign/components/notes/deploy/chart/templates/deployment.yaml (2)

58-69: Parameterize the ConfigMap name instead of hard-coding it.

Lines 58, 63, and 68 pin this chart to sovereign-product-config, which makes reuse harder and can cause name collisions across installs.

♻️ Proposed refactor

-                  name: sovereign-product-config
+                  name: {{ .Values.productConfigMapName | default "sovereign-product-config" }}
...
-                  name: sovereign-product-config
+                  name: {{ .Values.productConfigMapName | default "sovereign-product-config" }}
...
-                  name: sovereign-product-config
+                  name: {{ .Values.productConfigMapName | default "sovereign-product-config" }}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@conformance/scenarios/sovereign/components/notes/deploy/chart/templates/deployment.yaml`
around lines 58 - 69, The ConfigMap name "sovereign-product-config" is
hard-coded for the env vars referencing keys (database.host, DB_PORT, DB_NAME);
change those configMapKeyRef.name fields to use a templated value (e.g., a Helm
value or template helper like configMapName) so the chart is reusable and avoids
collisions, update all occurrences (the entries for database.host, DB_PORT,
DB_NAME) and add a corresponding default key in values.yaml (or the helper) so
installs can override the ConfigMap name.

---

70-71: Make DB TLS mode configurable instead of forcing sslmode=disable.

Line 71 hard-codes plaintext DB transport and prevents secure-mode rollout in environments requiring encrypted in-cluster traffic. While other database parameters (host, port, credentials) are configurable through Helm values, sslmode is fixed.

🔐 Proposed change

-            - name: DATABASE_URL
-              value: "postgresql://$(DB_USER):$(DB_PASSWORD)@$(DB_HOST):$(DB_PORT)/$(DB_NAME)?sslmode=disable"
+            - name: DATABASE_URL
+              value: "postgresql://$(DB_USER):$(DB_PASSWORD)@$(DB_HOST):$(DB_PORT)/$(DB_NAME)?sslmode={{ .Values.database.sslMode | default "disable" }}"

Add database.sslMode: disable to values.yaml to allow operators to override with require, prefer, or other PostgreSQL-supported modes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@conformance/scenarios/sovereign/components/notes/deploy/chart/templates/deployment.yaml`
around lines 70 - 71, The DATABASE_URL environment variable currently hardcodes
sslmode=disable; make SSL mode configurable by reading a Helm values key (e.g.,
.Values.database.sslMode) and defaulting to "disable" if unset—update the
deployment template where the env var named DATABASE_URL is constructed to use
that values key instead of the literal "sslmode=disable", and add the new
values.yaml key database.sslMode with a default of "disable" so operators can
override with "require", "prefer", etc.

conformance/scenarios/sovereign/README.md (1)

156-156: ⚠️ Potential issue | 🟡 Minor

Add a language identifier to the directory-structure code fence.

Line 156 still uses a plain fenced block, which keeps markdownlint MD040 failing.

🛠️ Proposed fix

-```
+```text
 sovereign-scenario/
 ├── README.md                    # This file
 ├── Taskfile.yml                 # Build and deployment automation
 ├── components/                  # OCM component definitions
 │   ├── notes/                   # Notes application component
 │   ├── postgres/                # PostgreSQL component  
 │   └── product/                 # Meta component
 ├── deploy/                      # OCM controller deployment manifests and bootstrap apis

</details>

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

Verify each finding against the current code and only fix it if needed.

In @conformance/scenarios/sovereign/README.md at line 156, Change the plain
fenced code block in conformance/scenarios/sovereign/README.md (the
directory-structure example around the README's code fence) to include a
language identifier so markdownlint MD040 stops failing; specifically update the
opening fence from totext (or another appropriate language like bash) so the example block begins with text and the closing fence remains ``` to
maintain the directory tree rendering.

</details>

</blockquote></details>
<details>
<summary>conformance/scenarios/sovereign/Taskfile.yml (1)</summary><blockquote>

`355-376`: _⚠️ Potential issue_ | _🟠 Major_

**Always generate `OCM_CREDENTIALS_CONFIG`; current early exit can leave `OCM_CONFIG` dangling.**

Line 355 exits before Lines 368-376 run when Docker config is absent, but Line 168 still exports `OCM_CONFIG` to that file path.

<details>
<summary>🛠️ Proposed fix</summary>

```diff
-          [ -f "$SRC" ] || { echo "No Docker config, skipping"; exit 0; }
-
-          HELPER="docker-credential-$(jq -r '.credsStore // empty' "$SRC")"
-
-          if command -v "$HELPER" >/dev/null 2>&1; then
-            "$HELPER" list 2>/dev/null | jq -r 'keys[]' | while IFS= read -r reg; do
-              echo "$reg" | "$HELPER" get 2>/dev/null
-            done | jq -s '{auths: ([.[] | select(.Username and .Secret) |
-              {(.ServerURL): {auth: ((.Username + ":" + .Secret) | `@base64`)}}] | add // {})}' > "$DST"
-          else
-            cp "$SRC" "$DST"
-          fi
+          if [ -f "$SRC" ]; then
+            HELPER="docker-credential-$(jq -r '.credsStore // empty' "$SRC")"
+            if command -v "$HELPER" >/dev/null 2>&1; then
+              "$HELPER" list 2>/dev/null | jq -r 'keys[]' | while IFS= read -r reg; do
+                echo "$reg" | "$HELPER" get 2>/dev/null
+              done | jq -s '{auths: ([.[] | select(.Username and .Secret) |
+                {(.ServerURL): {auth: ((.Username + ":" + .Secret) | `@base64`)}}] | add // {})}' > "$DST"
+            else
+              cp "$SRC" "$DST"
+            fi
+          else
+            echo "No Docker config found; writing empty auth config"
+            echo '{"auths":{}}' > "$DST"
+          fi
```
</details>

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

```
Verify each finding against the current code and only fix it if needed.

In `@conformance/scenarios/sovereign/Taskfile.yml` around lines 355 - 376, The
script currently exits early at the `[ -f "$SRC" ] || { echo "No Docker config,
skipping"; exit 0; }` check which prevents the `{{ .OCM_CREDENTIALS_CONFIG }}`
file from being written; remove the early `exit 0` path and instead ensure DST
is always created: if $SRC exists use the existing logic (use docker helper or
copy $SRC into "$DST"), otherwise create a minimal docker config at "$DST" (e.g.
an empty `{ "auths": {} }` JSON) so the later cat > "{{ .OCM_CREDENTIALS_CONFIG
}}" block always runs; keep references to HELPER, "$HELPER", "$SRC", "$DST", and
"{{ .OCM_CREDENTIALS_CONFIG }}" when making the change.
```

</details>

</blockquote></details>
<details>
<summary>conformance/scenarios/sovereign/components/notes/cmd/sovereign-notes/main.go (2)</summary><blockquote>

`223-230`: _⚠️ Potential issue_ | _🟡 Minor_

**Return `[]` instead of `null` for empty note lists.**

Current nil-slice initialization serializes empty results as JSON `null`, which can break array-typed clients.

  

<details>
<summary>💡 Proposed fix</summary>

```diff
-	var notes []Note
+	notes := make([]Note, 0)
```
</details>

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

```
Verify each finding against the current code and only fix it if needed.

In `@conformance/scenarios/sovereign/components/notes/cmd/sovereign-notes/main.go`
around lines 223 - 230, The handler currently declares var notes []Note which
stays nil when no rows are found and encodes to JSON null; change the
initialization of notes to an empty slice (e.g., notes := make([]Note, 0) or
notes := []Note{}) before iterating, or ensure after the loop you set notes =
[]Note{} if len(notes) == 0, so the JSON encoder emits [] instead of null;
update the code around the notes variable/for rows.Next() loop (and any code
that writes notes to the response) to use the non-nil empty slice.
```

</details>

---

`106-108`: _⚠️ Potential issue_ | _🟠 Major_

**Exit non-zero when server startup fails.**

If the port bind/start fails, this path logs and exits `main()` with status `0`. This misleads orchestrators and CI checks about startup success.

Since this server lacks graceful shutdown logic, simplify the fix to:

<details>
<summary>💡 Proposed fix</summary>

```diff
-	if err := srv.ListenAndServe(); err != nil {
-		log.Println("Server failed to start:", err)
-	}
+	if err := srv.ListenAndServe(); err != nil {
+		log.Fatal("Server failed to start:", err)
+	}
```
</details>

If you add graceful shutdown (signal handlers + `srv.Shutdown()`), extend the check to `err != nil && !errors.Is(err, http.ErrServerClosed)` to allow clean shutdowns.

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

```
Verify each finding against the current code and only fix it if needed.

In `@conformance/scenarios/sovereign/components/notes/cmd/sovereign-notes/main.go`
around lines 106 - 108, The current main() logs srv.ListenAndServe() errors but
then exits with status 0; update the error handling after srv.ListenAndServe()
so the process exits non-zero on startup failure by calling os.Exit(1) when err
!= nil. If you later add graceful shutdown, replace the simple exit with the
conditional check err != nil && !errors.Is(err, http.ErrServerClosed) to only
treat unexpected errors as fatal; refer to srv.ListenAndServe(), main, and
http.ErrServerClosed when making the change.
```

</details>

</blockquote></details>
<details>
<summary>conformance/scenarios/sovereign/components/postgres/deploy/chart/templates/deployment.yaml (1)</summary><blockquote>

`42-43`: _⚠️ Potential issue_ | _🟠 Major_

**Avoid rendering `POSTGRES_PASSWORD` as a literal value.**

This still exposes credentials in rendered manifests and Helm release metadata. Use `valueFrom.secretKeyRef` instead (and back it with a Secret template or existing secret support).  
 

<details>
<summary>Suggested hardening</summary>

```diff
           env:
             - name: POSTGRES_DB
               value: {{ .Values.postgresql.postgresqlDatabase | quote }}
             - name: POSTGRES_USER
               value: {{ .Values.postgresql.postgresqlUsername | quote }}
             - name: POSTGRES_PASSWORD
-              value: {{ .Values.postgresql.postgresqlPassword | quote }}
+              valueFrom:
+                secretKeyRef:
+                  name: {{ include "sovereign-postgres.fullname" . }}
+                  key: postgresql-password
```
</details>

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

```
Verify each finding against the current code and only fix it if needed.

In
`@conformance/scenarios/sovereign/components/postgres/deploy/chart/templates/deployment.yaml`
around lines 42 - 43, The deployment currently sets the POSTGRES_PASSWORD env
var with a literal value from .Values.postgresql.postgresqlPassword which
exposes credentials in rendered manifests; change the Deployment template (the
env entry where name: POSTGRES_PASSWORD is defined) to use
valueFrom.secretKeyRef instead, and add or reference a Secret template that
stores .Values.postgresql.postgresqlPassword (or support an existing secret
name/key) so the Deployment uses secretKeyRef.name and secretKeyRef.key rather
than embedding the password directly.
```

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🧹 Nitpick comments (3)</summary><blockquote>

<details>
<summary>conformance/scenarios/sovereign/components/notes/deploy/chart/templates/deployment.yaml (1)</summary><blockquote>

`55-69`: **Consider parameterizing the ConfigMap name for consistency.**

The secret reference is configurable via `.Values.databaseSecretRef` (lines 48, 53), but the ConfigMap name `sovereign-product-config` is hardcoded. This inconsistency limits flexibility if users need to deploy with a different ConfigMap name.



<details>
<summary>♻️ Proposed fix</summary>

Add a value for the ConfigMap reference (e.g., `databaseConfigRef`) and use it in the template:

```diff
             - name: DB_HOST
               valueFrom:
                 configMapKeyRef:
-                  name: sovereign-product-config
+                  name: {{ .Values.databaseConfigRef }}
                   key: database.host
             - name: DB_PORT
               valueFrom:
                 configMapKeyRef:
-                  name: sovereign-product-config
+                  name: {{ .Values.databaseConfigRef }}
                   key: database.port
             - name: DB_NAME
               valueFrom:
                 configMapKeyRef:
-                  name: sovereign-product-config
+                  name: {{ .Values.databaseConfigRef }}
                   key: database.name
```

Then add the default in `values.yaml`:
```yaml
databaseConfigRef: sovereign-product-config
```
</details>

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

```
Verify each finding against the current code and only fix it if needed.

In
`@conformance/scenarios/sovereign/components/notes/deploy/chart/templates/deployment.yaml`
around lines 55 - 69, The ConfigMap name is hardcoded for the DB env vars
(DB_HOST, DB_PORT, DB_NAME) while the secret uses .Values.databaseSecretRef;
make the ConfigMap name configurable by introducing a new value (e.g.,
.Values.databaseConfigRef) and replace the literal "sovereign-product-config"
references in the deployment template with that value, then add a default key
(databaseConfigRef: sovereign-product-config) to values.yaml so deployments can
override the ConfigMap name consistently with .Values.databaseSecretRef.
```

</details>

</blockquote></details>
<details>
<summary>docs/adr/0013_sovereign_cloud_reference_scenario.md (1)</summary><blockquote>

`428-435`: **Clarify placeholder syntax in bootstrap commands.**

The secret creation command at lines 428-430 is missing the namespace flag (this was already flagged in a previous review). Additionally, lines 431-432 and 434 use ellipsis (`...`) as placeholders for resource names, which may confuse readers attempting to execute these commands literally.



<details>
<summary>📝 Suggested improvements for clarity</summary>

```diff
-kubectl create secret generic acme-signing-key \
+kubectl create secret generic acme-signing-key -n sovereign-product \
   --from-file=default=tmp/keys/acme-public.pem          # Public signing key for verification
 kubectl apply -f deploy/bootstrap.yaml                  # Repository + Component + Resource + Deployer
-kubectl wait --for=condition=Ready deployer/...          # RGD is now installed by the Deployer
-kubectl wait --for=condition=Ready ResourceGraphDefinition/...
+kubectl wait --for=condition=Ready -n sovereign-product deployer/sovereign-product-deployer
+kubectl wait --for=condition=Ready resourcegraphdefinition/sovereign-product
 kubectl apply -f deploy/sample-product-1.0.0.yaml       # Create SovereignProduct instance
-kubectl wait --for=condition=Ready sovereignproduct/...  # Wait for full deployment
+kubectl wait --for=condition=Ready -n sovereign-product sovereignproduct/<instance-name>
```

</details>

**Note:** If the exact resource names vary by deployment, consider documenting this explicitly or linking to the bootstrap.yaml file where readers can find the actual names.

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

```
Verify each finding against the current code and only fix it if needed.

In `@docs/adr/0013_sovereign_cloud_reference_scenario.md` around lines 428 - 435,
Update the bootstrap commands to include the missing namespace flag on the
secret creation command (the line with kubectl create secret generic
acme-signing-key --from-file=default=tmp/keys/acme-public.pem) and replace the
literal ellipses used in the wait commands (deployer/...,
ResourceGraphDefinition/..., sovereignproduct/...) with clear placeholders (e.g.
<deployer-name>, <rgd-name>, <sovereignproduct-name>) or explicit instructions
to derive actual names from deploy/bootstrap.yaml and
deploy/sample-product-1.0.0.yaml; ensure the text explicitly states that
namespace and resource names must be set or looked up before running the kubectl
wait and apply commands.
```

</details>

</blockquote></details>
<details>
<summary>conformance/scenarios/sovereign/components/product/deploy/rgd.yaml (1)</summary><blockquote>

`22-23`: **`prefix` is defined but not actually used for generated names.**

Line 22 exposes `spec.prefix`, but key resource names are still hardcoded, so the field is effectively a no-op and can mislead users expecting customizable naming.

<details>
<summary>♻️ Suggested direction</summary>

```diff
 spec:
   schema:
@@
-      prefix: string | default="sovereign-product"
+      prefix: string | default="sovereign-product"
@@
-          name: sovereign-product
+          name: ${schema.spec.prefix}
@@
-          name: sovereign-repo
+          name: ${schema.spec.prefix}-repo
@@
-          name: sovereign-product-component
+          name: ${schema.spec.prefix}-component
```

</details>


Also applies to: 60-61, 71-72, 84-85

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

```
Verify each finding against the current code and only fix it if needed.

In `@conformance/scenarios/sovereign/components/product/deploy/rgd.yaml` around
lines 22 - 23, The spec exposes prefix (spec.prefix / prefix: string |
default="sovereign-product") but the resource names remain hardcoded; update all
name templates that currently use the literal "sovereign-product" (and other
hardcoded resource names referenced around the noted areas) to derive their
names from the prefix value (e.g., use prefix + suffix or a
templated/concatenated name) so that spec.prefix actually controls generated
resource names — apply this change to the occurrences around the prefix
declaration and the areas referenced (lines near 60-61, 71-72, 84-85) ensuring
any name-generation logic or templates that create Deployment/Service/Config
names use the prefix variable instead of a fixed string.
```

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

Inline comments:
In
@conformance/scenarios/sovereign/components/notes/cmd/sovereign-notes-v1/main.go:

• Around line 97-99: The server startup error is currently only logged and the
  process continues; change the handling of srv.ListenAndServe() so the program
  exits on failure like the other startup steps: replace the log.Println in the
  error branch for the srv.ListenAndServe() call with a call to log.Fatal or
  log.Fatalf (referencing srv and ListenAndServe) so any error returned from
  srv.ListenAndServe() causes the process to terminate with a non-zero exit code.
• Around line 172-179: The handler currently declares a nil slice with "var
  notes []Note", which JSON-encodes to null when empty; change the initialization
  of the notes slice to an empty slice (e.g., notes := make([]Note, 0) or notes :=
  []Note{}) before iterating (the variable referenced as notes and the struct Note
  in the rows.Next loop and rows.Scan) so that when no rows are found the response
  encodes as [] instead of null.

In
@conformance/scenarios/sovereign/components/postgres/deploy/chart/templates/deployment.yaml:

• Around line 44-45: Replace the POSTGRES_MAX_CONNECTIONS environment variable
  with a command arg for the postgres process: remove the env var entry named
  POSTGRES_MAX_CONNECTIONS and instead set the container's command/args to invoke
  "postgres" with "-c max_connections=..." using the Helm value (e.g. in the
  container spec referenced by this template, add command: ["postgres"] and args:
  ["-c", "max_connections={{ .Values.postgresql.postgresqlMaxConnections }}"]).
  Ensure you update the Deployment template where POSTGRES_MAX_CONNECTIONS is
  declared so the configuration is applied deterministically at start time.

In @conformance/scenarios/sovereign/Taskfile.yml:

• Around line 257-271: The helm install commands in the cluster:install:toolkit,
  cluster:install:kro, and cluster:install:flux tasks reintroduce blocking
  behavior by using "--wait --timeout=5m"; remove the "--wait" and "--timeout=5m"
  flags from the helm install invocations in those task definitions (e.g., the
  helm install lines inside the cluster:install:toolkit, cluster:install:kro, and
  cluster:install:flux tasks) so installations are non-blocking and rely on
  eventual-consistency reconciliation.

---

Duplicate comments:
In
@conformance/scenarios/sovereign/components/notes/cmd/sovereign-notes/main.go:

• Around line 223-230: The handler currently declares var notes []Note which
  stays nil when no rows are found and encodes to JSON null; change the
  initialization of notes to an empty slice (e.g., notes := make([]Note, 0) or
  notes := []Note{}) before iterating, or ensure after the loop you set notes =
  []Note{} if len(notes) == 0, so the JSON encoder emits [] instead of null;
  update the code around the notes variable/for rows.Next() loop (and any code
  that writes notes to the response) to use the non-nil empty slice.
• Around line 106-108: The current main() logs srv.ListenAndServe() errors but
  then exits with status 0; update the error handling after srv.ListenAndServe()
  so the process exits non-zero on startup failure by calling os.Exit(1) when err
  != nil. If you later add graceful shutdown, replace the simple exit with the
  conditional check err != nil && !errors.Is(err, http.ErrServerClosed) to only
  treat unexpected errors as fatal; refer to srv.ListenAndServe(), main, and
  http.ErrServerClosed when making the change.

In
@conformance/scenarios/sovereign/components/postgres/deploy/chart/templates/deployment.yaml:

• Around line 42-43: The deployment currently sets the POSTGRES_PASSWORD env var
  with a literal value from .Values.postgresql.postgresqlPassword which exposes
  credentials in rendered manifests; change the Deployment template (the env entry
  where name: POSTGRES_PASSWORD is defined) to use valueFrom.secretKeyRef instead,
  and add or reference a Secret template that stores
  .Values.postgresql.postgresqlPassword (or support an existing secret name/key)
  so the Deployment uses secretKeyRef.name and secretKeyRef.key rather than
  embedding the password directly.

In @conformance/scenarios/sovereign/README.md:

• Line 156: Change the plain fenced code block in
  conformance/scenarios/sovereign/README.md (the directory-structure example
  around the README's code fence) to include a language identifier so markdownlint
  MD040 stops failing; specifically update the opening fence from totext
  (or another appropriate language like bash) so the example block begins with text and the closing fence remains ``` to maintain the directory tree
  rendering.

In @conformance/scenarios/sovereign/Taskfile.yml:

• Around line 355-376: The script currently exits early at the [ -f "$SRC" ] || { echo "No Docker config, skipping"; exit 0; } check which prevents the {{ .OCM_CREDENTIALS_CONFIG }} file from being written; remove the early exit 0
  path and instead ensure DST is always created: if $SRC exists use the existing
  logic (use docker helper or copy $SRC into "$DST"), otherwise create a minimal
  docker config at "$DST" (e.g. an empty { "auths": {} } JSON) so the later cat

"{{ .OCM_CREDENTIALS_CONFIG }}" block always runs; keep references to HELPER,
"$HELPER", "$SRC", "$DST", and "{{ .OCM_CREDENTIALS_CONFIG }}" when making the
change.

---

Nitpick comments:
In
@conformance/scenarios/sovereign/components/notes/deploy/chart/templates/deployment.yaml:

• Around line 55-69: The ConfigMap name is hardcoded for the DB env vars
  (DB_HOST, DB_PORT, DB_NAME) while the secret uses .Values.databaseSecretRef;
  make the ConfigMap name configurable by introducing a new value (e.g.,
  .Values.databaseConfigRef) and replace the literal "sovereign-product-config"
  references in the deployment template with that value, then add a default key
  (databaseConfigRef: sovereign-product-config) to values.yaml so deployments can
  override the ConfigMap name consistently with .Values.databaseSecretRef.

In @conformance/scenarios/sovereign/components/product/deploy/rgd.yaml:

• Around line 22-23: The spec exposes prefix (spec.prefix / prefix: string |
  default="sovereign-product") but the resource names remain hardcoded; update all
  name templates that currently use the literal "sovereign-product" (and other
  hardcoded resource names referenced around the noted areas) to derive their
  names from the prefix value (e.g., use prefix + suffix or a
  templated/concatenated name) so that spec.prefix actually controls generated
  resource names — apply this change to the occurrences around the prefix
  declaration and the areas referenced (lines near 60-61, 71-72, 84-85) ensuring
  any name-generation logic or templates that create Deployment/Service/Config
  names use the prefix variable instead of a fixed string.

In @docs/adr/0013_sovereign_cloud_reference_scenario.md:

• Around line 428-435: Update the bootstrap commands to include the missing
  namespace flag on the secret creation command (the line with kubectl create
  secret generic acme-signing-key --from-file=default=tmp/keys/acme-public.pem)
  and replace the literal ellipses used in the wait commands (deployer/...,
  ResourceGraphDefinition/..., sovereignproduct/...) with clear placeholders (e.g.
  , , ) or explicit instructions
  to derive actual names from deploy/bootstrap.yaml and
  deploy/sample-product-1.0.0.yaml; ensure the text explicitly states that
  namespace and resource names must be set or looked up before running the kubectl
  wait and apply commands.

</details>

---

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

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

**Configuration used**: defaults

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `5805f829-87f7-4b4c-af2a-c1910048d050`

</details>

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

Reviewing files that changed from the base of the PR and between 54a02926ab32165b5039fddfe304ca79b3c5e2ad and 560b75fc02fcde5f7163471148630f6a188b9f13.

</details>

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

* `.github/workflows/kubernetes-controller.yml`
* `conformance/scenarios/sovereign/README.md`
* `conformance/scenarios/sovereign/Taskfile.yml`
* `conformance/scenarios/sovereign/components/notes/cmd/sovereign-notes-v1/main.go`
* `conformance/scenarios/sovereign/components/notes/cmd/sovereign-notes/main.go`
* `conformance/scenarios/sovereign/components/notes/deploy/chart/templates/deployment.yaml`
* `conformance/scenarios/sovereign/components/postgres/deploy/chart/Chart.yaml`
* `conformance/scenarios/sovereign/components/postgres/deploy/chart/templates/deployment.yaml`
* `conformance/scenarios/sovereign/components/postgres/deploy/chart/values.yaml`
* `conformance/scenarios/sovereign/components/product/deploy/rgd.yaml`
* `conformance/scenarios/sovereign/deploy/bootstrap.yaml`
* `conformance/scenarios/sovereign/deploy/sample-product-1.0.0.yaml`
* `conformance/scenarios/sovereign/deploy/sample-product-1.1.0.yaml`
* `docs/adr/0013_sovereign_cloud_reference_scenario.md`

</details>

<details>
<summary>🚧 Files skipped from review as they are similar to previous changes (3)</summary>

* conformance/scenarios/sovereign/deploy/sample-product-1.0.0.yaml
* conformance/scenarios/sovereign/components/postgres/deploy/chart/values.yaml
* conformance/scenarios/sovereign/deploy/sample-product-1.1.0.yaml

</details>

</details>

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

conformance/scenarios/sovereign/components/notes/deploy/chart/templates/deployment.yaml (1)

55-69: Hardcoded ConfigMap name sovereign-product-config.

The ConfigMap references for DB_HOST, DB_PORT, and DB_NAME use the hardcoded name sovereign-product-config. This is acceptable for a conformance scenario where the product ConfigMap name is known, but reduces chart reusability if deployed independently.

For improved flexibility, consider making this configurable via a value such as .Values.productConfigMapRef.

♻️ Optional: Make ConfigMap name configurable

In values.yaml, add:

productConfigMapRef: sovereign-product-config

Then in deployment.yaml:

            - name: DB_HOST
              valueFrom:
                configMapKeyRef:
-                  name: sovereign-product-config
+                  name: {{ .Values.productConfigMapRef }}
                  key: database.host

Apply similarly for DB_PORT and DB_NAME.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@conformance/scenarios/sovereign/components/notes/deploy/chart/templates/deployment.yaml`
around lines 55 - 69, The DB environment variables DB_HOST, DB_PORT and DB_NAME
in the deployment template are using the hardcoded ConfigMap name
"sovereign-product-config"; change the template to read the ConfigMap name from
a chart value (e.g. .Values.productConfigMapRef) so the reference is
configurable. Add a default productConfigMapRef entry in values.yaml (e.g.
"sovereign-product-config") and update the valueFrom: configMapKeyRef: name:
fields for DB_HOST, DB_PORT and DB_NAME in deployment.yaml to use that value
instead of the literal string.