-
Notifications
You must be signed in to change notification settings - Fork 38
feature: remaining port features #711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements the remaining port features by integrating new functionality for Ethereum, monitoring, and ingress into the generator. Key changes include updating the Manifest type to support Ingress, adding comprehensive monitoring generators (Prometheus and Grafana), and introducing new Ingress and Ethereum chain builders.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/packages/generator/src/types.ts | Updated Manifest union to include Ingress and "any" to cover additional resource types. |
| packages/packages/generator/src/builders/monitoring.ts | Added Prometheus and Grafana generators for enhanced monitoring. |
| packages/packages/generator/src/builders/ingress.ts | Implemented new Ingress resource and cert issuer generators. |
| packages/packages/generator/src/builders/index.ts | Updated BuilderManager to instantiate IngressBuilder and MonitoringBuilder. |
| packages/packages/generator/src/builders/chains/index.ts | Adjusted the chain builder registry to include EthereumBuilder and clarified default behavior. |
| packages/packages/generator/src/builders/chains/ethereum/statefulset.ts | Introduced the Ethereum StatefulSet generator with chain-specific configurations and init containers. |
| packages/packages/generator/src/builders/chains/ethereum/service.ts | Added Ethereum Service generator for exposing Ethereum chain endpoints. |
| packages/packages/generator/src/builders/chains/ethereum/index.ts | Orchestrates Ethereum ConfigMap, Service, and StatefulSet generation. |
| packages/packages/generator/src/builders/chains/ethereum/configmap.ts | Added Ethereum ConfigMap generator for chain configuration. |
| packages/packages/generator/src/builders/chains/cosmos/index.ts | Updated Cosmos chain filtering to exclude Ethereum chains. |
| packages/packages/generator/tests/builder.test.ts | Modified test snapshots and descriptions to reflect new Ethereum generation behavior. |
Comments suppressed due to low confidence (1)
packages/packages/generator/src/builders/chains/ethereum/statefulset.ts:96
- [nitpick] The property name 'numValidator' is ambiguous and may be more clearly expressed as 'numValidators' for consistency. Consider renaming it for improved clarity.
const numValidators = chain.config?.validator?.numValidator || 1;
| | Deployment | ||
| | StatefulSet | ||
| | Ingress | ||
| | any; |
Copilot
AI
Jun 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using 'any' in the Manifest union significantly weakens type safety. Consider replacing 'any' with a more specific type to maintain stricter type checks.
| | any; | |
| | UnknownManifestType; |
| --http.vhosts=* \\ | ||
| --authrpc.vhosts=* \\ | ||
| --authrpc.jwtsecret=/etc/secrets/jwt.hex \\ | ||
| --unlock=0x123463a4B065722E99115D6c222f267d9cABb524 \\ |
Copilot
AI
Jun 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hard-coded Ethereum account address in the unlock flag may pose security and maintainability issues. Consider externalizing this configuration to avoid potential risks in production.
Part of: #695