This repository was archived by the owner on Apr 19, 2026. It is now read-only.
refactor aegislab - #71
Merged
Merged
Conversation
Major reorganization from flat service/producer/consumer layers to a
clean module-based architecture with clear separation of concerns:
- New `src/app/` module: application lifecycle and mode orchestration
- New `src/infra/` module: infrastructure concerns (k8s, redis, db, etcd, harbor, helm, loki, tracing, buildkit, chaos)
- New `src/interface/` module: HTTP controllers, workers, receivers
- New `src/module/` packages: auth, chaossystem, container, dataset, evaluation, execution, group, injection, label, metric, notification, project, rbac, sdk, system, systemmetric, task, team, trace, user
- New `src/router/` module: route registration and grouping
- Delete deprecated DTOs: analyzer, audit, auth, debug, group, metrics, redis, resource, role, system
- Delete deprecated handlers: debug, system/*, v2/{auth,permissions,projects,resources,roles}
- Delete deprecated clients: etcd, harbor, helm, k8s, redis
- Delete deprecated repositories and service/producer files
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…th gRPC - introduce six dedicated service entry points: api-gateway, iam-service, resource-service, orchestrator-service, runtime-worker-service, system-service - add gateway layer in src/app/gateway/ with typed service clients for each boundary - add internal gRPC clients in src/internalclient/* for synchronous inter-service calls - add proto definitions and generated code for iam, orchestrator, resource, runtime, system services - add src/cmd/*/main.go entry points for each dedicated service mode - add handler_service.go to each module for clean handler-to-service wiring - add runtime injection matrix to README documenting mode vs service tradeoffs - update helm templates and docker-compose for multi-service deployment - move module-owned repositories into src/module/*/repository.go per service boundary - add Redis gateway.go for distributed rate limiting and session state - remove legacy spec docs replaced by runtime injection matrix documentation
- add api_key_scope.go with scope-to-target matching logic (e.g., "project:*" matches "project:123:containers") - introduce RequireAPIKeyScopesAny middleware for endpoint-level API key permission checks - add RequireHumanUserAuth to reject service tokens on self-service user endpoints - refactor permission middleware to extract and propagate API key auth type and scopes - rename /access-keys endpoint to /api-keys with human-user-only access guard
…th gRPC - flatten nested project routes into resource-oriented REST API structure - rename grpciam -> grpc/iam and align all grpc interface paths - consolidate OpenAPI generator templates under typescript/ directory - add Apifox integration for portal/sdk/admin target uploads - remove ProducerCompatibilityOptions in favor of explicit module composition Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors the codebase toward an Fx-driven microservices architecture (“aegislab” split) while updating SDK/auth tooling and pruning legacy handlers/DTOs/infra code.
Changes:
- Introduces dedicated service entrypoints (
api-gateway,iam-service,resource-service,orchestrator-service,runtime-worker-service,system-service) and new Fx option stacks/modules for composition and startup validation. - Adds gateway “remote-aware” handler-service decorators and a config-required-targets guard to enforce internal client target configuration.
- Updates SDK/tooling: docs audience validation tests, new API-key header signing flow in
aegisctl, new Python runtime client, and revamped Swagger/SDK generation scripts + settings.
Reviewed changes
Copilot reviewed 133 out of 511 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/handlers/system/monitor.go | Removes legacy system monitor HTTP handlers (likely replaced by microservice/gateway flow). |
| src/handlers/system/audit.go | Removes legacy system audit HTTP handlers. |
| src/handlers/debug.go | Removes legacy debug HTTP handlers. |
| src/go.mod | Adds Fx + sqlmock dependencies to support new wiring/tests. |
| src/dto/trace.go | Removes multiple trace DTOs/validators/builders from this file (refactor/relocation). |
| src/dto/task.go | Switches DTO conversions to aegis/model and removes many task DTOs/validators from this file. |
| src/dto/system.go | Removes legacy system DTOs (likely moved under modules). |
| src/dto/role.go | Removes legacy role DTOs (likely moved under module/rbac). |
| src/dto/resource.go | Removes legacy resource DTOs (likely moved under module/rbac / resource service). |
| src/dto/redis.go | Removes a legacy Redis message DTO. |
| src/dto/project.go | Keeps only ProjectStatistics; removes CRUD DTOs/builders (likely moved under module/project). |
| src/dto/metrics.go | Removes legacy metrics DTOs (likely moved under module/metric). |
| src/dto/log.go | Removes WS log message DTO. |
| src/dto/label.go | Leaves label condition helper; removes CRUD DTOs/validators (likely moved under module/label). |
| src/dto/group.go | Removes legacy group stream DTOs/validators (likely moved under module/group). |
| src/dto/debug.go | Removes legacy debug DTOs. |
| src/dto/common.go | Removes validation helper functions from common DTOs (relocation/refactor). |
| src/dto/auth.go | Removes legacy auth DTOs (likely moved under module/auth). |
| src/dto/audit.go | Removes legacy audit DTOs (likely moved under system service/module). |
| src/dto/analyzer.go | Removes legacy analyzer DTOs (likely moved under analyzer module). |
| src/docs/docs_test.go | Adds tests asserting generated OpenAPI docs contain core paths and audience-filtering matches extensions. |
| src/database/view.go | Removes detector view models + view-creation logic from legacy database package. |
| src/database/scope.go | Removes legacy GORM scope helpers (likely moved/refactored). |
| src/database/database.go | Removes legacy DB init/migrate logic from old package (replaced by infra/db module). |
| src/consts/consts.go | Adds DatapackState.UnmarshalJSON. |
| src/config.dev.toml | Adds internal client targets + gRPC addr sections for microservices local config. |
| src/cmd/system-service/main.go | Adds dedicated system service entrypoint (Fx). |
| src/cmd/runtime-worker-service/main.go | Adds dedicated runtime worker service entrypoint (Fx). |
| src/cmd/resource-service/main.go | Adds dedicated resource service entrypoint (Fx). |
| src/cmd/orchestrator-service/main.go | Adds dedicated orchestrator service entrypoint (Fx). |
| src/cmd/iam-service/main.go | Adds dedicated IAM service entrypoint (Fx). |
| src/cmd/api-gateway/main.go | Adds dedicated API gateway entrypoint (Fx) with configurable port. |
| src/cmd/aegisctl/config/config.go | Extends CLI context config with API-key auth fields and custom YAML unmarshalling. |
| src/cmd/aegisctl/cmd/wait.go | Removes an unused minimal response struct (cleanup). |
| src/cmd/aegisctl/cmd/root.go | Updates CLI quick start to API-key login and documents new env vars; message wording tweak. |
| src/cmd/aegisctl/cmd/inject.go | Wraps Close() calls to avoid ignoring errors (explicitly discarded). |
| src/cmd/aegisctl/client/ws.go | Makes websocket close calls explicitly error-ignored and uses deferred close wrapper. |
| src/cmd/aegisctl/client/sse.go | Wraps response body close with explicit discard. |
| src/cmd/aegisctl/client/client.go | Adds header injection support + PostWithHeaders helper for signed API-key token exchange. |
| src/cmd/aegisctl/client/auth_test.go | Adds tests for canonical signing string, header construction, and PostWithHeaders. |
| src/cmd/aegisctl/client/auth.go | Implements API-key signature flow for token exchange and exposes debug materialization helpers. |
| src/client/redis_client.go | Removes legacy Redis singleton + helpers (replaced by infra/redis). |
| src/client/k8s/k8s_test.go | Removes legacy k8s integration tests. |
| src/client/k8s/client.go | Removes legacy k8s client singletons (replaced by infra/k8s). |
| src/client/helm_test.go | Removes legacy Helm client tests. |
| src/client/harbor_client.go | Removes legacy Harbor singleton client (replaced by infra/harbor). |
| src/client/etcd_client.go | Removes legacy etcd singleton + helpers (replaced by infra/etcd). |
| src/client/debug/status_registry.go | Removes legacy debug registry implementation. |
| src/app/system/options.go | Adds Fx options for system service wiring + required remote target guard. |
| src/app/startup_validate_test.go | Adds Fx graph validation tests for producer/consumer/both option sets. |
| src/app/runtime_stack.go | Adds runtime worker stack module composition (infra + worker/controller/receiver). |
| src/app/runtime/options.go | Adds Fx options for runtime-worker-service including required orchestrator target. |
| src/app/resource/options.go | Adds Fx options for resource-service including required orchestrator target and remote query options. |
| src/app/remote_require_test.go | Adds tests for required-target discovery logic w/ primary and legacy config keys. |
| src/app/remote_require.go | Adds RequireConfiguredTargets Fx hook + missing-target detector. |
| src/app/producer.go | Adds producer (HTTP) Fx option sets + startup initializer wiring. |
| src/app/orchestrator/options.go | Adds Fx options for orchestrator service wiring. |
| src/app/options.go | Adds normalizeAddr helper for port handling. |
| src/app/iam/options.go | Adds Fx options for IAM service wiring + required resource target. |
| src/app/http_modules.go | Adds HTTP module bundle helpers (producer HTTP modules + owner bundles). |
| src/app/gateway/user_services.go | Adds gateway remote-aware user service proxying to IAM. |
| src/app/gateway/team_services_test.go | Adds tests for remote-aware team service missing dependency + happy path. |
| src/app/gateway/team_services.go | Adds gateway remote-aware team service proxying to IAM. |
| src/app/gateway/system_services.go | Adds gateway remote-aware system/systemmetric services proxying to system-service. |
| src/app/gateway/resource_services_test.go | Adds tests for remote-aware label/chaos-system services via resource-service. |
| src/app/gateway/remote_required.go | Adds consistent “missing remote dependency” error helper. |
| src/app/gateway/rbac_services.go | Adds gateway remote-aware RBAC service proxying to IAM. |
| src/app/gateway/options.go | Adds Fx options for api-gateway, required targets, and decorators for remote-aware handler services. |
| src/app/gateway/middleware_service.go | Adds gateway remote-aware middleware token/permission checks proxying to IAM. |
| src/app/gateway/metric_services_test.go | Adds tests for remote-aware metric service algorithms aggregation. |
| src/app/gateway/metric_services.go | Adds gateway remote-aware metric service with orchestrator+resource aggregation logic. |
| src/app/gateway/auth_services.go | Adds gateway remote-aware auth/API-key handler proxying to IAM. |
| src/app/consumer.go | Adds consumer Fx option set. |
| src/app/both.go | Adds combined Fx option set for local “both” mode. |
| src/app/app.go | Adds base/common Fx option stacks mapping to new infra/* modules. |
| skaffold.yaml | Updates regression hook command to new just target. |
| sdk/python/src/rcabench/client/runtime_client.py | Adds runtime-only Python client that authenticates from env service token. |
| sdk/python/src/rcabench/client/base.py | Adds shared authenticated-client/session lifecycle base for handwritten Python clients. |
| sdk/python/src/rcabench/client/init.py | Exports the new runtime client. |
| sdk/python/src/rcabench/init.py | Bumps SDK version and exports new client types in package top-level. |
| sdk/python/pyproject.toml | Bumps Python package version to 1.2.1. |
| sdk/python/README.md | Updates Python SDK docs: auth model + usage for both public and runtime clients. |
| scripts/start.sh | Parameterizes external URLs/proxy settings via env overrides. |
| scripts/command/src/test.py | Reads cert-manager/helm repo URLs from centralized settings. |
| scripts/command/src/swagger/typescript.py | Refactors TypeScript generator into separate portal/admin SDK packages with settings-driven config overrides. |
| scripts/command/src/swagger/python.py | Refactors Python generator to use settings-driven generator volume root and git metadata. |
| scripts/command/src/swagger/common.py | Simplifies swagger common types and expands RunMode (sdk/runtime/portal/admin). |
| scripts/command/src/swagger/apifox.py | Adds Apifox upload utility for converted OpenAPI docs with env-driven configuration. |
| scripts/command/src/swagger/init.py | Updates exports; removes old Generator registry wiring. |
| scripts/command/src/formatter/python.py | Adds ruff binary resolution (PATH/venv) and skips formatting when missing. |
| scripts/command/src/cli/swagger.py | Simplifies swagger CLI to init + optional Apifox upload targets. |
| scripts/command/src/cli/sdk.py | Adds explicit SDK generation CLI for TypeScript (portal/admin) and Python. |
| scripts/command/src/cli/rcabench_.py | Renames version command and updates local deploy message. |
| scripts/command/src/cli/main.py | Registers new sdk subcommand. |
| scripts/command/src/backup/mysql.py | Moves mysql apt config URL into settings. |
| scripts/command/settings.toml | Adds settings for generator volume root, Apifox, SDK metadata, and command URLs/repo URLs. |
| project-index.yaml | Updates requirement references/notes for new swagger/sdk tooling and sources. |
| manifests/microservices/README.md | Adds microservices Kubernetes skeleton README describing intent and assumptions. |
| justfile | Renames multiple targets and adds new SDK generation & swagger init commands. |
| helm/values.yaml | Switches port to 8082 and introduces microservices-specific ports/replica config. |
| helm/templates/service.yaml | Refactors services into api-gateway + per-microservice gRPC services; adjusts staging behavior. |
| helm/templates/configmap.yaml | Updates config to point at api-gateway, adds clients/grpc sections, and renames otlp receiver config key. |
| docs/report-index.md | Adds a consolidated architecture/SDK/auth/report index doc. |
| docs/frontend-redesign.md | Updates frontend SDK generation guidance to new audience/TypeScript SDK workflow. |
| docker-compose.microservices.yaml | Adds docker-compose skeleton to run all 6 services locally. |
| config.dev.toml | Adds internal client targets + gRPC addr sections for root config file. |
| .openapi-generator/typescript/config.json | Updates default npm package name/description for TypeScript generator baseline. |
Comment on lines
+190
to
+197
| func (ds *DatapackState) UnmarshalJSON(data []byte) error { | ||
| var stateName string | ||
| if err := json.Unmarshal(data, &stateName); err != nil { | ||
| return err | ||
| } | ||
| *ds = *GetDatapackStateByName(stateName) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
UnmarshalJSON dereferences the result of GetDatapackStateByName(stateName) without checking for an unknown/invalid state name. If GetDatapackStateByName can return nil (or otherwise signal 'not found'), this will panic on malformed input. Prefer returning a descriptive error for unknown names (e.g., check the lookup result and return fmt.Errorf(\"unknown datapack state: %q\", stateName)), or change the lookup helper to return (DatapackState, bool) to make this impossible to misuse.
| def _run_ruff_format(self, category: str, files: list[str]) -> bool: | ||
| """Run ruff format on files.""" | ||
| cmd = ["ruff", "format"] + files | ||
| cmd = [self.ruff_binary or "ruff", "format"] + files |
There was a problem hiding this comment.
_run_ruff_format appends files twice (+ files and then cmd.extend(files)), which results in duplicate CLI args and redundant work. Drop one of the two additions (e.g., initialize with just [binary, \"format\"] and then extend(files)).
Suggested change
| cmd = [self.ruff_binary or "ruff", "format"] + files | |
| cmd = [self.ruff_binary or "ruff", "format"] |
Comment on lines
+26
to
+27
| assert actual_base_url is not None, "base_url or RCABENCH_BASE_URL is not set" | ||
| assert actual_service_token is not None, "RCABENCH_SERVICE_TOKEN is not set" |
There was a problem hiding this comment.
Using assert for runtime input validation in a library is risky because assertions can be stripped with Python's -O optimization, and failures surface as AssertionError rather than a user-friendly exception type. Prefer raising ValueError/RuntimeError with the same message so missing environment variables are always detected.
Comment on lines
+13
to
+14
| TEST_HTTP_PROXY=${TEST_HTTP_PROXY:-"http://crash:crash@172.18.0.1:7890"} | ||
| TEST_HTTPS_PROXY=${TEST_HTTPS_PROXY:-"http://crash:crash@172.18.0.1:7890"} |
There was a problem hiding this comment.
The script now hardcodes proxy credentials (crash:crash@...) as defaults. This risks leaking credentials in logs and encourages insecure defaults. Consider defaulting these to empty (unset) and documenting how to set them explicitly, or reading them from a local non-committed env file when needed.
Suggested change
| TEST_HTTP_PROXY=${TEST_HTTP_PROXY:-"http://crash:crash@172.18.0.1:7890"} | |
| TEST_HTTPS_PROXY=${TEST_HTTPS_PROXY:-"http://crash:crash@172.18.0.1:7890"} | |
| # Proxy settings are intentionally unset by default. If an authenticated proxy is | |
| # required, export TEST_HTTP_PROXY / TEST_HTTPS_PROXY explicitly before running. | |
| TEST_HTTP_PROXY=${TEST_HTTP_PROXY:-""} | |
| TEST_HTTPS_PROXY=${TEST_HTTPS_PROXY:-""} |
Comment on lines
+141
to
+173
| func PrepareAPIKeyTokenDebug(keyID, keySecret string, now time.Time, nonce string) (*APIKeyTokenDebug, error) { | ||
| keyID = strings.TrimSpace(keyID) | ||
| keySecret = strings.TrimSpace(keySecret) | ||
| nonce = strings.TrimSpace(nonce) | ||
| if keyID == "" { | ||
| return nil, fmt.Errorf("key id is required") | ||
| } | ||
| if keySecret == "" { | ||
| return nil, fmt.Errorf("key secret is required") | ||
| } | ||
| var err error | ||
| if nonce == "" { | ||
| nonce, err = newAPIKeyNonce() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
|
|
||
| timestamp := strconv.FormatInt(now.Unix(), 10) | ||
| bodySHA256 := sha256Hex("") | ||
| canonical := canonicalAPIKeyString("POST", apiKeyTokenPath, timestamp, nonce, bodySHA256) | ||
|
|
||
| return &APIKeyTokenDebug{ | ||
| Method: "POST", | ||
| Path: apiKeyTokenPath, | ||
| KeyID: keyID, | ||
| Timestamp: timestamp, | ||
| Nonce: nonce, | ||
| BodySHA256: bodySHA256, | ||
| CanonicalString: canonical, | ||
| Signature: signAPIKeyRequest(keySecret, canonical), | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
PrepareAPIKeyTokenDebug and prepareAPIKeyDebug duplicate most of the same normalization + canonical/signature assembly logic. This duplication increases drift risk if the signing scheme changes (e.g., body hashing or headers). Consider implementing a single helper that takes (method, path, body) (or (path, bodySHA256)), and have both exported and internal helpers call it.
Comment on lines
+183
to
+215
| func prepareAPIKeyDebug(keyID, keySecret string, now time.Time, path, nonce string) (*APIKeyTokenDebug, error) { | ||
| keyID = strings.TrimSpace(keyID) | ||
| keySecret = strings.TrimSpace(keySecret) | ||
| nonce = strings.TrimSpace(nonce) | ||
| if keyID == "" { | ||
| return nil, fmt.Errorf("key id is required") | ||
| } | ||
| if keySecret == "" { | ||
| return nil, fmt.Errorf("key secret is required") | ||
| } | ||
| var err error | ||
| if nonce == "" { | ||
| nonce, err = newAPIKeyNonce() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
|
|
||
| timestamp := strconv.FormatInt(now.Unix(), 10) | ||
| bodySHA256 := sha256Hex("") | ||
| canonical := canonicalAPIKeyString("POST", path, timestamp, nonce, bodySHA256) | ||
|
|
||
| return &APIKeyTokenDebug{ | ||
| Method: "POST", | ||
| Path: path, | ||
| KeyID: keyID, | ||
| Timestamp: timestamp, | ||
| Nonce: nonce, | ||
| BodySHA256: bodySHA256, | ||
| CanonicalString: canonical, | ||
| Signature: signAPIKeyRequest(keySecret, canonical), | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
PrepareAPIKeyTokenDebug and prepareAPIKeyDebug duplicate most of the same normalization + canonical/signature assembly logic. This duplication increases drift risk if the signing scheme changes (e.g., body hashing or headers). Consider implementing a single helper that takes (method, path, body) (or (path, bodySHA256)), and have both exported and internal helpers call it.
| } | ||
|
|
||
| primaryValue := strings.TrimSpace(config.GetString(target.PrimaryKey)) | ||
| legacyValue := strings.TrimSpace(config.GetString(target.LegacyKey)) |
There was a problem hiding this comment.
missingRequiredTargets reads config.GetString(target.LegacyKey) even when LegacyKey is empty. While current call sites appear to always provide a legacy key, guarding this read (only fetch legacy when LegacyKey != \"\") avoids accidental lookups for the empty key and makes the helper safer for future use.
Suggested change
| legacyValue := strings.TrimSpace(config.GetString(target.LegacyKey)) | |
| legacyValue := "" | |
| if target.LegacyKey != "" { | |
| legacyValue = strings.TrimSpace(config.GetString(target.LegacyKey)) | |
| } |
Integrate 37 commits from main (guided-CLI pipeline, rate-limiter,
pedestal helm, cluster preflight, chaos-experiment v1.0.1 API drift)
into the six-service modular architecture.
Conflict resolution summary:
- handlers/v2/* deleted — logic lives under module/<domain>/handler.go
- service/producer/* deleted — moved into module/<domain>
- repository/{container,task,v2}.go deleted — moved into module repositories
- main's src/service/producer/rate_limiter* replaced by module/ratelimiter/
- main's src/service/producer/spec_convert* kept at module/injection/
- Adopted main's FaultSpec.Params (was Extra), guided-CLI submit shape,
NodeToStruct/GetGroundtruth(ctx) API drift, and container SQL ambiguity fix
Build/vet/short-test gates all clean on the merged tree.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.