docs(ccv-cell): remove brownfield operator friction by aelmanaa · Pull Request #53 · smartcontractkit/chainlink-ccv-starter-kit

aelmanaa · 2026-09-25T17:09:14Z

No description provided.

github-actions · 2026-09-25T17:09:27Z

👋 aelmanaa, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

Angelin01

I believe that LLM assistance was used to improve the runbook here, based on a deployment on GKE? LLMs, unfortunately, have a tendency to over-produce (that is how these companies make money after all).

I fear a lot of the changes ended up reducing the DRY-ness factor of the runbook by quite a bit, some pieces of information are repeated up to 4 times!

I do think a lot of the additions are nice, but need some rewording or simplification. For most operators, a lot of the information is immediately obvious: which secret operator to use, how to expose a public endpoint, and so on and so forth.

Angelin01 · 2026-09-25T18:02:52Z

+
+{{/*
+Fail loudly when keystore settings are supplied on a secret path that cannot use them.
+On externalSecret the chart assembles secrets.toml and honours keystoreBackend / kms.*. On every other
+type it mounts the operator's file verbatim and cannot inject into it, so those keys were silently
+ignored: a cell would come up holding no keystore configuration at all, with nothing in the render or
+the logs to say why. Fail instead.
+include "ccv-cell.assertKeystoreUsable" (dict "root" . "component" "verifier" "subComponent" "bootstrap")
+*/}}
+{{- define "ccv-cell.assertKeystoreUsable" -}}
+{{- $secret := index .root.Values .component "secrets" .subComponent -}}
+{{- if ne $secret.type "externalSecret" -}}
+  {{- $set := list -}}
+  {{- if and (hasKey $secret "keystoreBackend") (ne (toString $secret.keystoreBackend) "postgres") -}}
+    {{- $set = append $set "keystoreBackend" -}}
+  {{- end -}}
+  {{- if $secret.kms -}}
+    {{- if or $secret.kms.provider $secret.kms.ecdsaKeyId $secret.kms.ed25519KeyId -}}
+      {{- $set = append $set "kms.*" -}}
+    {{- end -}}
+  {{- end -}}
+  {{- if $set -}}
+    {{- fail (printf "%s.secrets.%s sets %s but type is %q. Those keys are only read on type: externalSecret, where the chart assembles secrets.toml for you. On %q you author secrets.toml yourself, so put the [keystore] and [keystore.kms] blocks inside the secret you supply and remove these keys." .component .subComponent (join " and " $set) $secret.type $secret.type) -}}
+  {{- end -}}
+{{- end -}}
+{{- end -}}


As tempting as this is, I'm not 100% sure it's a good idea.

The more checks like these you are, the more you couple the templating to application behavior, meaning that if you update the application, you need to change the code here.
While it's nice to error on helm template, do consider the maintenance burden this adds to developers.

IMO, the better option would be to have the APPLICATION fail loudly, quickly. Most operators would catch an error like this on first deploy and then never have it happen again. A simple line on the README or values documentation informing that this is a misconfiguration would probably prevent 80% of these problems already, and I believe we already have that!

Angelin01 · 2026-09-25T18:06:22Z

+  chart's [Requirements](charts/ccv-cell/README.md#requirements). Note the database count depends on your
+  keystore: three logical databases on `keystoreBackend: postgres`, but only **two** (`verifier`, `aggregator`)
+  on the `kms` backend recommended for production, because the signing key lives in your cloud KMS and the
+  `bootstrap` database is then unused. Do not provision a `bootstrap` database you will never connect to.


This feels like you are just repeating what the requirements say already. In the interest of keeping things DRY, I'd revert.

Angelin01 · 2026-09-25T18:10:59Z

+- A **publicly trusted TLS certificate** on the hostname you will give the aggregator, from a CA whose root is
+  in the standard trust stores (Let's Encrypt, ACM, Google-managed, or a commercial CA). Terminating TLS is not
+  enough on its own: the callers are your committee peers and the CCIP indexer, and they will not carry your
+  private CA. A self-signed or internal-CA certificate passes every check you can run against your own cell and
+  still leaves you unreachable to everyone else, which is the hardest failure in this runbook to spot from the
+  inside. If you use cert-manager, that means a real ACME issuer rather than a self-signed or CA issuer.
+- The hostname must also **resolve publicly**. Peers and the indexer look it up from outside your network, so a
+  name that only resolves in your private zone fails for them while working for you.


This entire snippet says the same as the requirement sections part about publicly accessible:

Some kind of Ingress or Gateway controller that supports HTTP/2 and gRPC with TLS termination. The Aggregator exposes
a public gRPC endpoint that must be accessible from the other cells, and the indexer.

If you want to mention it here, I'd summarize it as succinctly as possible:

Suggested change

- A **publicly trusted TLS certificate** on the hostname you will give the aggregator, from a CA whose root is

in the standard trust stores (Let's Encrypt, ACM, Google-managed, or a commercial CA). Terminating TLS is not

enough on its own: the callers are your committee peers and the CCIP indexer, and they will not carry your

private CA. A self-signed or internal-CA certificate passes every check you can run against your own cell and

still leaves you unreachable to everyone else, which is the hardest failure in this runbook to spot from the

inside. If you use cert-manager, that means a real ACME issuer rather than a self-signed or CA issuer.

- The hostname must also **resolve publicly**. Peers and the indexer look it up from outside your network, so a

name that only resolves in your private zone fails for them while working for you.

- A publicly/internet accessible, through HTTP/2 and gRPC with TLS, endpoint. Private CAs are not usable when onboarding with Chainlink's indexer.

Angelin01 · 2026-09-25T18:14:51Z

+- **Which route and secret CRDs your cluster actually has.** Both choices below fork on this, and it is faster
+  to look than to assume:
+  ```bash
+  kubectl get crd | grep -E 'grpcroutes|httproutes|externalsecrets|secretproviderclass'
+  ```
+  Read the route rows like this. Both present, or only `grpcroutes`: use `grpcRoute`. Only `httproutes`: use
+  `httpRoute`, which is the case for GKE's own managed Gateway. Neither, meaning you have no Gateway API
+  controller at all and are on something like nginx-ingress: use `aggregator.ingress` instead, and see
+  [Ingress](#ingress) for the annotations a gRPC backend needs there. No `externalsecrets` means External
+  Secrets Operator is not installed, and the chart cannot assemble your `secrets.toml` for you.


A lot of these are summarized as "contact your cluster admin to answer these for you". Any cluster admin will have an answer immediately. If you prefer, I'd summarize this as:

Suggested change

- **Which route and secret CRDs your cluster actually has.** Both choices below fork on this, and it is faster

to look than to assume:

```bash

kubectl get crd | grep -E 'grpcroutes|httproutes|externalsecrets|secretproviderclass'

```

Read the route rows like this. Both present, or only `grpcroutes`: use `grpcRoute`. Only `httproutes`: use

`httpRoute`, which is the case for GKE's own managed Gateway. Neither, meaning you have no Gateway API

controller at all and are on something like nginx-ingress: use `aggregator.ingress` instead, and see

[Ingress](#ingress) for the annotations a gRPC backend needs there. No `externalsecrets` means External

Secrets Operator is not installed, and the chart cannot assemble your `secrets.toml` for you.

- Prefer a GRPCRoute over HTTPRoute, and that over Ingress to expose the aggregator, depending on availability in you cluster.

Note that it's already mentioned in the values.yaml comments/docs:

grpcRoute: # -- Enable a Gateway API GRPCRoute for the aggregator gRPC endpoint. **Preferred over `httpRoute`** when your # gateway controller supports it. Mutually independent of `ingress.enabled` and `httpRoute.enabled`.

So even this is a bit redundant.

On the secrets, I wouldn't even mention too much, since we have no control over what the user uses.

Angelin01 · 2026-09-25T18:15:49Z

+Before the details, one decision shapes most of the work: **your secrets backend decides who writes the
+configuration files.** On `externalSecret` the chart assembles `secrets.toml` from individual values you supply,
+which is the least work but requires External Secrets Operator in the cluster. On `gcpSecretStore`,
+`awsSecretStore`, `azureKeyVault` and `existingSecret` the chart only mounts what you give it, so **you author each complete
+`secrets.toml` yourself** and put it in your secrets manager. That is four files for a full cell: aggregator
+app, verifier app, verifier bootstrap, and the verifier EVM config if you keep it secret. It is not harder, but
+it is a different job, and picking the backend without knowing this is the most common way to get stuck here.
+
+If you are hand-authoring those files, work from the examples in [`local/config/`](local/config). That directory
+is the docker-compose stack's configuration, so every file in it is a complete, working example of the exact
+shape the chart mounts. Read it before you write your first one rather than reconstructing the shape from the
+field reference.


Much of this information is repeated from the values docs/comments introduced in this PR. In the interests of DRY, pick one, and link the other.

Angelin01 · 2026-09-25T18:29:37Z

+To read the public key yourself, so you can derive the signer address before deploying, you need
+`roles/cloudkms.publicKeyViewer` on the key. `roles/cloudkms.viewer` does not include
+`cloudkms.cryptoKeyVersions.viewPublicKey` and will not do it. Grant yourself the narrow role rather than
+reaching for `roles/cloudkms.signerVerifier`, which would also let you sign with a committee key.
+
 With the `kms` backend the bootstrap secret needs no `[db]` and no `[keystore] password`, so a cell needs
-two logical databases (`verifier`, `aggregator`) rather than three. Note that the chart's `externalSecret`
-path writes a `[db] url` into the bootstrap secret on every backend, so on that path supply one regardless.
+two logical databases (`verifier`, `aggregator`) rather than three. This holds on every secrets backend,
+`externalSecret` included: that path writes the `[db] url` block only when `keystoreBackend` is `postgres`,
+so on `kms` there is no bootstrap database to supply and none to provision.


I know a lot of this was added in a previous PR, but I'm getting the feeling at this point that I have read that that path writes the [db] urlblock only whenkeystoreBackendispostgres`, information in at least 3 or 4 places now. Do we really need to have it here?

Angelin01 · 2026-09-25T18:31:45Z

+> verifier to start! Errors and warnings in the logs during that window are expected.
+>
+> Expect real volume, not a couple of lines. Two measured first starts logged 38 errors over 27 seconds and 24
+> errors over 16 seconds, mostly the verifier failing to reach an aggregator that has not finished coming up.
+> The text of those errors is identical to what a permanently misconfigured cell logs, so the only thing that
+> distinguishes startup noise from a real problem is that it stops. Give it a couple of minutes, then judge it
+> by whether new errors are still arriving, not by whether any appeared.


How about instead simply defining the time window above?

Instead of during that window are expected we say during a window of 1 to 2 minutes of first deploy are expected. If they continue, do investigate further.?

It says pretty much the same thing in many less words.

Angelin01 · 2026-09-25T18:34:19Z

+  - Avoid putting `ccv-cell` in the release name. Helm's naming convention drops the chart name when the
+    release name already contains it, so a release called `ccv-cell-0` produces `ccv-cell-0-aggregator` while
+    a release called `cell-0` produces `cell-0-ccv-cell-aggregator`. Both work, but the names differ, and the
+    ones that change include the ServiceAccounts your cloud IAM bindings are keyed on. Render the chart and
+    read the real names rather than predicting them, as in
+    [Workload Identity](#workload-identity-for-gcpsecretstore) above.


This is actually a bad recommendation. It is common practice to use the chart name in the release name, but you must use the FULL chart name. This is why the name logic drops repeats, and it's embedded in Helm's base template (run helm create locally and the see templates/_helpers.tpl file).

Your example, probably because you tried to name it simply cell-0, caused the repetition because it didn't contain the full name.

A good name would have been ccv-cell-0, or ccv-cell-use1, like above.

Angelin01 · 2026-09-25T18:34:54Z

+Reachable with no API key and no IP allowlist, over TLS with a **publicly trusted certificate** on a
+**publicly resolvable hostname**. The indexer and your committee peers connect from outside your network with
+the standard trust stores, so a private CA or an internal-only DNS name fails for them even though your own
+tests against the cell pass. Standard WAF / DDoS protection in front is fine as long as it does not block
+legitimate public reads.
+
+Confirm it from outside your network rather than from a pod or a workstation that trusts your internal CA. A
+plain `openssl s_client -connect <host>:443` from an unrelated machine, with no custom CA bundle, is enough:
+if it reports a verified chain there, peers will get one too.


The information added here was also duplicated from the requirements at the beginning of the doc.

Angelin01 · 2026-09-25T18:37:22Z

+   **Expect minutes, not seconds.** The verifier waits for
+   source-chain finality before it attests, and `finality_depth` defaults to `0`, which means the chain's
+   finality tag rather than a block count. On Ethereum Sepolia that is roughly 10 to 15 minutes. A measured run
+   had no attestation at 463 seconds and a complete one at 557 seconds. Throughout that window the verifier logs
+   nothing but `Healthy` heartbeats, which looks exactly like a broken cell. Lower
+   `evm.config.chains.<selector>.nodes[].finality_depth` if you want a faster signal while validating, and
+   understand that trades finality safety for latency.


The language here feels hard to follow. Couldn't we just say "at finality_depth = 1, expect the process to take upwards of 15 minutes"?

docs(ccv-cell): remove brownfield operator friction

ddc5fc6

aelmanaa requested review from a team, Angelin01 and khalifaa55 as code owners September 25, 2026 17:09

Angelin01 reviewed Sep 25, 2026

View reviewed changes

Navigation Menu

Repository navigation

docs(ccv-cell): remove brownfield operator friction - #53

docs(ccv-cell): remove brownfield operator friction#53
aelmanaa wants to merge 1 commit into
mainfrom
fix/brownfield-operator-friction

aelmanaa commented Sep 25, 2026

Uh oh!

github-actions Bot commented Sep 25, 2026

Uh oh!

Angelin01 left a comment

Uh oh!

Angelin01 Sep 25, 2026

Uh oh!

Angelin01 Sep 25, 2026

Uh oh!

Angelin01 Sep 25, 2026

Uh oh!

Angelin01 Sep 25, 2026

Uh oh!

Angelin01 Sep 25, 2026

Uh oh!

Angelin01 Sep 25, 2026

Uh oh!

Angelin01 Sep 25, 2026

Uh oh!

Angelin01 Sep 25, 2026

Uh oh!

Angelin01 Sep 25, 2026

Uh oh!

Angelin01 Sep 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

aelmanaa commented Sep 25, 2026

Uh oh!

github-actions Bot commented Sep 25, 2026

Uh oh!

Angelin01 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants