Skip to content

feature: add ingress verification #687

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

Anmol1696
Copy link
Collaborator

No description provided.

@Anmol1696 Anmol1696 requested a review from Copilot May 19, 2025 13:27
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds ingress verification support by extending URL construction utilities, updating existing verifiers to accept configuration and handle ingress, and integrating new ingress checks into the main verify flow.

  • Introduce getServiceUrl helper for consistent base URL and path resolution (including ingress)
  • Extend all existing verifiers to accept StarshipConfig and use error handling helper
  • Add ingress.ts with per-service ingress health checks and integrate into verify
  • Update config schema and tests to enable and cover ingress scenarios

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
clients/js/packages/client/src/verifiers/utils.ts Add getServiceUrl util and ServiceUrl interface
clients/js/packages/client/src/verifiers/types.ts Extend verifier function types to accept config; add error helper
clients/js/packages/client/src/verifiers/relayer.ts Update relayer verifiers to use getServiceUrl and accept config
clients/js/packages/client/src/verifiers/registry.ts Update registry verifier to use getServiceUrl and improved error paths
clients/js/packages/client/src/verifiers/explorer.ts Update explorer verifier to accept config and use getServiceUrl
clients/js/packages/client/src/verifiers/chain.ts Update chain verifiers to use getServiceUrl, accept config, and granular error handling
clients/js/packages/client/src/verifiers/ingress.ts New file: implements ingress-specific health checks for all services
clients/js/packages/client/src/verifiers/index.ts Integrate verifyIngress into the main verification pipeline
clients/js/packages/client/src/config.ts Make Ingress.host required and tighten schema
clients/js/packages/client/tests/client.verify.test.ts Add tests for ingress success and failure scenarios
Comments suppressed due to low confidence (2)

clients/js/packages/client/src/verifiers/explorer.ts:12

  • [nitpick] Using 'http' as the endpoint identifier for the explorer verifier deviates from the 'rest' convention used by other services. Consider aligning this to 'rest' or documenting the distinction to avoid confusion.
endpoint: 'http',

clients/js/packages/client/tests/client.verify.test.ts:241

  • The ingress tests mock chain REST, RPC, registry, and explorer URLs but omit mocks for chain faucet and exposer. Without them, unhandled URLs will throw errors. Add mocks for those endpoints to fully cover ingress verification.
mockedAxios.get.mockImplementation((url) => {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant