feat: move some config options out of configmap#7164
Conversation
|
kamiyaa
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7164 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 1 1
Lines 14 14
=====================================
Misses 14 14
🚀 New features to boost your workflow:
|
kamiyaa
requested review from
Mo-Hussain,
Xaroz,
ameten,
antigremlin,
ltyu,
paulbalaji,
xeno097,
yjamin and
yorhodes
as code owners
📝 WalkthroughWalkthroughAdds app-contexts config handling alongside existing agent config: new path helpers, script writes a separate app-contexts JSON, relayer config/types split to isolate metricAppContexts, Helm includes the new JSON at runtime, Docker image now ships app-contexts directory, and testnet4 image tags updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Dev
participant Script as update-agent-config.ts
participant Helper as RelayerConfigHelper
participant FS as Filesystem
Dev->>Script: run main()
Script->>Helper: buildConfig(multiProvider, env)
Helper-->>Script: RelayerConfig (includes metricAppContexts)
Script->>FS: writeAgentConfig -> relayer-config.json
Script->>FS: writeAgentAppContexts -> app-contexts/<env>_config.json
Note over FS: Two JSON files produced side-by-side
sequenceDiagram
autonumber
participant K8s as Kubernetes
participant Pod as Relayer Pod
participant Ctr as Container (/app)
participant Relayer as Relayer Process
K8s->>Pod: Start StatefulSet
Pod->>Ctr: Mount configmap relayer-config.json
Pod->>Ctr: Use bundled /app/app-contexts/<env>_config.json
Ctr->>Relayer: Start with CONFIG_FILES=relayer-config.json, app-contexts.json
Relayer-->>Relayer: Load both configs and merge at startup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
typescript/sdk/src/metadata/agentConfig.ts (1)
428-430: Consider aligning other comma-separated fields for consistency.Now that
addressBlacklistuses an array, you've got a bit of a mixed bag—relayChains(line 406) andskipTransactionGasLimitFor(line 434) still use comma-separated strings viaCommaSeparatedChainListandCommaSeparatedDomainList.For consistency and better developer experience, you might want to migrate those fields to arrays as well in a future update. Arrays are easier to manipulate and more idiomatic in TypeScript.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
rust/main/config/mainnet_config.json(114 hunks)typescript/infra/scripts/agent-utils.ts(1 hunks)typescript/infra/scripts/agents/update-agent-config.ts(5 hunks)typescript/infra/src/agents/index.ts(2 hunks)typescript/infra/src/config/agent/relayer.ts(3 hunks)typescript/sdk/src/metadata/agentConfig.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
typescript/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Keep TypeScript SDK source under typescript/sdk/src/ using the documented abstractions (MultiProvider, HyperlaneCore, MultiProtocolCore)
Files:
typescript/sdk/src/metadata/agentConfig.ts
rust/main/config/**
📄 CodeRabbit inference engine (CLAUDE.md)
Store all chain configuration files under rust/main/config so agents can auto-discover them
Files:
rust/main/config/mainnet_config.json
🧬 Code graph analysis (1)
typescript/infra/scripts/agents/update-agent-config.ts (4)
typescript/infra/src/agents/index.ts (6)
environment(73-75)RelayerHelmManager(190-313)helmValues(81-135)helmValues(199-248)helmValues(326-336)helmValues(356-384)typescript/infra/scripts/agent-utils.ts (2)
getAgentConfig(489-503)getAgentConfigMapJsonPath(730-732)typescript/infra/src/utils/utils.ts (2)
readJSONAtPath(220-222)writeAndFormatJsonAtPath(171-174)typescript/utils/src/objects.ts (1)
objMerge(116-151)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-matrix (evm)
🔇 Additional comments (1)
rust/main/config/mainnet_config.json (1)
8488-8517: CoinGecko slug confirmedSlug’s solid—
galactica-comreturns a USD price, so we’re all set.
kamiyaa
force-pushed
the
jeff/configmap-2
branch
from
31e3a92 to
c45da18
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
typescript/infra/scripts/agents/update-agent-config.ts (1)
228-257: Remove unused whitelist field from agentConfigMap
whitelistisn't present in any existing environment configs and wasn’t mentioned in the PR objectives—drop it unless you plan to support it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
typescript/infra/scripts/agents/update-agent-config.ts(5 hunks)typescript/infra/src/agents/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
typescript/infra/scripts/agents/update-agent-config.ts (5)
typescript/infra/src/config/environment.ts (2)
DeployEnvironment(31-31)envNameToAgentEnv(40-44)typescript/infra/scripts/agent-utils.ts (2)
getAgentConfig(489-503)getAgentConfigMapJsonPath(730-732)typescript/infra/src/config/agent/relayer.ts (2)
RelayerConfigHelper(132-280)RelayerConfigMapConfig(86-94)typescript/infra/src/utils/utils.ts (2)
readJSONAtPath(220-222)writeAndFormatJsonAtPath(171-174)typescript/utils/src/objects.ts (1)
objMerge(116-151)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (52)
- GitHub Check: infra-test
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-2)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-1)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: env-test-matrix (mainnet3, inevm, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: env-test-matrix (mainnet3, inevm, igp)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-install-test-run
- GitHub Check: yarn-test-run
- GitHub Check: build-and-push-to-gcr
- GitHub Check: lint-rs
- GitHub Check: lander-coverage
- GitHub Check: test-rs
- GitHub Check: build-and-push-to-gcr
- GitHub Check: e2e-matrix (sealevel)
- GitHub Check: e2e-matrix (cosmwasm)
- GitHub Check: e2e-matrix (cosmosnative)
- GitHub Check: e2e-matrix (radix)
- GitHub Check: e2e-matrix (starknet)
- GitHub Check: e2e-matrix (evm)
🔇 Additional comments (5)
typescript/infra/scripts/agents/update-agent-config.ts (4)
30-33: Well, look at that - direct path to the config helper.The import changes properly address the past review feedback. Using
RelayerConfigHelperdirectly instead of going throughRelayerHelmManager.helmValues()keeps things straightforward and avoids unnecessary AWS credential dependencies.Also applies to: 51-51
61-61: Config map generation flows naturally after agent config.Good sequencing - generating the agent config map right after the agent config keeps the workflow clean.
207-207: Filepath logging adds helpful context.Nice touch - knowing where configs are being written makes debugging easier.
233-235: Heads up - this makes network calls to fetch sanctioned addresses.The
buildConfig()method fetches sanctioned address lists from external GitHub URLs. The script will need network connectivity and will fail if those endpoints are unreachable. Based on learnings, this is expected behavior for address blacklist population.typescript/infra/src/agents/index.ts (1)
9-9: Import looks good for what's visible.The
ProtocolTypeimport is properly retained since it's used later in the file (lines 100 and 110). The annotated code only shows this one line as changed, so can't verify the other import/export removals mentioned in the AI summary.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
typescript/infra/config/environments/testnet4/agent/configmap.json(1 hunks)typescript/infra/scripts/agent-utils.ts(1 hunks)typescript/infra/scripts/agents/update-agent-config.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
typescript/infra/scripts/agent-utils.ts (1)
typescript/infra/src/config/environment.ts (1)
DeployEnvironment(31-31)
typescript/infra/scripts/agents/update-agent-config.ts (4)
typescript/infra/scripts/agent-utils.ts (2)
getAgentConfig(489-503)getAgentConfigMapJsonPath(730-732)typescript/infra/src/config/agent/relayer.ts (2)
RelayerConfigHelper(132-280)RelayerConfigMapConfig(86-94)typescript/infra/src/utils/utils.ts (2)
readJSONAtPath(220-222)writeAndFormatJsonAtPath(171-174)typescript/utils/src/objects.ts (1)
objMerge(116-151)
🔇 Additional comments (1)
typescript/infra/scripts/agents/update-agent-config.ts (1)
228-257: Implementation looks solid.The function properly uses
RelayerConfigHelperto build the config and extract just the fields needed for the configmap, avoiding the heavyweighthelmValues()path that was flagged in previous reviews. The merge logic with existing config follows the same pattern aswriteAgentConfig, which maintains consistency.One thing worth noting: the
whitelistfield is included in the config map extraction (line 240), but the staticconfigmap.jsondoesn't define it. This is fine sinceobjMergehandles undefined fields gracefully, but ifwhitelistis actually used, you might want to add it to the static config for clarity.
|
I'm questioning whether or not this has to be part of the version control. What stopping us from building a second docker image thats only for our infra, where we generate all the configs in the build step and include them in the image? This would keep the repo leaner and we wouldn't have another dependency that might get out of sync, like agents config & registry rn. |
|
I'm also curios if configuring the agent with the deploy script via the HTTP API is a valid option? I think this would make deploys take very long, but maybe I'm mistaken. |
Yeah, that's also an option; to build a second docker image. Might need more opinions from others though |
Also possible, but now we will need to put behind an |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
typescript/infra/scripts/agents/update-agent-config.ts (1)
207-207: Keep the logging consistent across the swamp.You've got
rootLoggerhumming along nicely in the rest of the file (see lines 155, 180), but here you're usingconsole.log. Might want to stick withrootLogger.info()so everything stays in harmony. Same goes for line 244 down below.Apply this diff:
- console.log(`Writing config to ${filepath}`); + rootLogger.info(`Writing agent config to ${filepath}`);And for line 244:
- console.log(`Writing config to ${filepath}`); + rootLogger.info(`Writing app contexts config to ${filepath}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
rust/main/helm/hyperlane-agent/templates/relayer-statefulset.yaml(1 hunks)typescript/infra/scripts/agent-utils.ts(1 hunks)typescript/infra/scripts/agents/update-agent-config.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- rust/main/helm/hyperlane-agent/templates/relayer-statefulset.yaml
🧰 Additional context used
🧬 Code graph analysis (1)
typescript/infra/scripts/agents/update-agent-config.ts (6)
typescript/infra/src/agents/index.ts (1)
environment(75-77)typescript/infra/src/config/environment.ts (2)
DeployEnvironment(31-31)envNameToAgentEnv(40-44)typescript/infra/scripts/agent-utils.ts (2)
getAgentConfig(496-510)getAgentAppContextConfigJsonPath(741-748)typescript/infra/src/config/agent/relayer.ts (2)
RelayerConfigHelper(130-280)RelayerAppContextConfig(92-92)typescript/infra/src/utils/utils.ts (2)
readJSONAtPath(239-241)writeAndFormatJsonAtPath(171-174)typescript/utils/src/objects.ts (1)
objMerge(116-151)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (48)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-2)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-1)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: env-test-matrix (mainnet3, inevm, igp)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, inevm, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: build-and-push-to-gcr
- GitHub Check: build-and-push-to-gcr
- GitHub Check: lint-rs
- GitHub Check: lander-coverage
- GitHub Check: test-rs
- GitHub Check: e2e-matrix (starknet)
- GitHub Check: e2e-matrix (sealevel)
- GitHub Check: e2e-matrix (radix)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: e2e-matrix (cosmwasm)
- GitHub Check: e2e-matrix (cosmosnative)
🔇 Additional comments (2)
typescript/infra/scripts/agent-utils.ts (1)
737-748: Clean path helpers, nicely done!These follow the same pattern as the existing agent config helpers and address the previous review feedback. The directory structure mirrors the agent config layout, which keeps things consistent.
typescript/infra/scripts/agents/update-agent-config.ts (1)
228-254: Solid implementation, this one.The function properly:
- Uses
RelayerConfigHelperdirectly (avoiding theHelmManagerdetour mentioned in past reviews)- Handles file existence with merge logic matching
writeAgentConfig- Uses type-safe
RelayerAppContextConfigto extract only what's neededOne thing to be aware of:
buildConfig()fetches sanctioned addresses from GitHub even though we only needmetricAppContexts. That's a bit of extra work, but since the config building is a single cohesive process, it's probably not worth splitting up just to save a network call.
Co-authored-by: pbio <10051819+paulbalaji@users.noreply.github.com>
4 participants
Description
metricAppContextsnot a string. Makes it easier to diff and read for humanCONFIG_FILESenv var to read from these filesupdate-agent-config.tsto write config file withmetricAppContextsSummary by CodeRabbit
New Features
Refactor
Chores