-
Notifications
You must be signed in to change notification settings - Fork 2
chore(xtest): extract composite actions from xtest.yml #435
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
base: main
Are you sure you want to change the base?
Changes from all commits
3567fd3
0ea2e5d
7c20d73
cd0aec5
1f02700
9b9c9d7
0c1b428
bf739dc
edb5e3a
8f783c3
1e5fc53
821c02c
2901544
7e99e2d
13385f5
bfe1119
9cfc673
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,223 @@ | ||
| """CI-specific commands for otdf-local. | ||
|
|
||
| These commands adapt the local environment management for GitHub Actions CI, | ||
| where the platform is already started by an external action and we only need | ||
| to start KAS instances as background processes. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import os | ||
| import sys | ||
| from pathlib import Path | ||
| from typing import Annotated | ||
|
|
||
| import typer | ||
|
|
||
| from otdf_local.config.ports import Ports | ||
| from otdf_local.config.settings import Settings | ||
| from otdf_local.health.waits import WaitTimeoutError, wait_for_health | ||
| from otdf_local.services import get_kas_manager | ||
| from otdf_local.utils.console import ( | ||
| print_error, | ||
| print_info, | ||
| print_success, | ||
| print_warning, | ||
| ) | ||
| from otdf_local.utils.yaml import load_yaml, save_yaml, set_nested | ||
|
|
||
| ci_app = typer.Typer( | ||
| name="ci", | ||
| help="CI-specific commands for GitHub Actions workflows.", | ||
| no_args_is_help=True, | ||
| ) | ||
|
|
||
|
|
||
| def _emit_github_output(key: str, value: str) -> None: | ||
| """Write a key=value pair to $GITHUB_OUTPUT if available, else print to stdout.""" | ||
| github_output = os.environ.get("GITHUB_OUTPUT") | ||
| if github_output: | ||
| with open(github_output, "a") as f: | ||
| f.write(f"{key}={value}\n") | ||
| else: | ||
| # Fallback for local testing | ||
| print(f"{key}={value}", file=sys.stdout) | ||
|
|
||
|
|
||
| def _prepare_kas_template( | ||
| settings: Settings, root_key: str | None, ec_tdf_enabled: bool | ||
| ) -> None: | ||
| """Ensure the KAS template config has the right root key and EC TDF settings. | ||
|
|
||
| In CI, the platform config may have a root_key that differs from what | ||
| we want for additional KAS instances. This updates the platform config | ||
| in-place so that KASService._generate_config reads the correct root_key. | ||
| """ | ||
| if root_key: | ||
| config = load_yaml(settings.platform_config) | ||
| set_nested(config, "services.kas.root_key", root_key) | ||
| if ec_tdf_enabled: | ||
| set_nested(config, "services.kas.preview.ec_tdf_enabled", True) | ||
| save_yaml(settings.platform_config, config) | ||
|
|
||
|
|
||
| @ci_app.command("start-kas") | ||
| def start_kas( | ||
| platform_dir: Annotated[ | ||
| Path, | ||
| typer.Option( | ||
| "--platform-dir", | ||
| help="Path to the platform checkout (must contain opentdf-kas-mode.yaml)", | ||
| envvar="OTDF_LOCAL_PLATFORM_DIR", | ||
| ), | ||
| ], | ||
| root_key: Annotated[ | ||
| str | None, | ||
| typer.Option( | ||
| "--root-key", | ||
| help="Root key for KAS instances (overrides platform config value)", | ||
| envvar="OT_ROOT_KEY", | ||
| ), | ||
| ] = None, | ||
| ec_tdf_enabled: Annotated[ | ||
| bool, | ||
| typer.Option( | ||
| "--ec-tdf-enabled/--no-ec-tdf", | ||
| help="Enable EC TDF support", | ||
| ), | ||
| ] = True, | ||
| key_management: Annotated[ | ||
| bool, | ||
| typer.Option( | ||
| "--key-management/--no-key-management", | ||
| help="Enable key management on km1/km2 instances", | ||
| ), | ||
| ] = False, | ||
| log_type: Annotated[ | ||
| str, | ||
| typer.Option( | ||
| "--log-type", | ||
| help="Log format type (json, text)", | ||
| ), | ||
| ] = "json", | ||
| health_timeout: Annotated[ | ||
| int, | ||
| typer.Option( | ||
| "--health-timeout", | ||
| help="Seconds to wait for each KAS instance to become healthy", | ||
| ), | ||
| ] = 60, | ||
|
Comment on lines
+89
to
+109
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Both parameters are declared in the CLI signature but never referenced in the function body (lines 118-223). Per the AI summary, the new At minimum, 🤖 Prompt for AI Agents |
||
| instances: Annotated[ | ||
| str | None, | ||
| typer.Option( | ||
| "--instances", | ||
| help="Comma-separated KAS instance names (default: all)", | ||
| ), | ||
| ] = None, | ||
| ) -> None: | ||
| """Start KAS instances for CI and emit GitHub Actions outputs. | ||
|
|
||
| Expects the platform to already be running (started by start-up-with-containers). | ||
| Starts all 6 KAS instances (alpha, beta, gamma, delta, km1, km2) as background | ||
| processes, waits for each to pass health checks, and emits log file paths as | ||
| GitHub Actions step outputs. | ||
|
|
||
| Output keys (written to $GITHUB_OUTPUT): | ||
| kas-alpha-log-file, kas-beta-log-file, kas-gamma-log-file, | ||
| kas-delta-log-file, kas-km1-log-file, kas-km2-log-file | ||
| """ | ||
| platform_dir = platform_dir.resolve() | ||
| if not platform_dir.is_dir(): | ||
| print_error(f"Platform directory does not exist: {platform_dir}") | ||
| raise typer.Exit(1) | ||
|
|
||
| # Check for required template files | ||
| kas_template = platform_dir / "opentdf-kas-mode.yaml" | ||
| platform_config = platform_dir / "opentdf-dev.yaml" | ||
| if not kas_template.exists(): | ||
| # Fall back to opentdf.yaml if opentdf-kas-mode.yaml doesn't exist | ||
| kas_template_alt = platform_dir / "opentdf.yaml" | ||
| if kas_template_alt.exists(): | ||
| print_info( | ||
| f"Using {kas_template_alt} as KAS template (opentdf-kas-mode.yaml not found)" | ||
| ) | ||
| else: | ||
| print_error( | ||
| f"Neither opentdf-kas-mode.yaml nor opentdf.yaml found in {platform_dir}" | ||
| ) | ||
| raise typer.Exit(1) | ||
|
|
||
| if not platform_config.exists(): | ||
| # Try opentdf.yaml as fallback | ||
| platform_config_alt = platform_dir / "opentdf.yaml" | ||
| if platform_config_alt.exists(): | ||
| platform_config = platform_config_alt | ||
|
Comment on lines
+135
to
+154
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variables
Comment on lines
+134
to
+154
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Template/config fallback logic is effectively dead code. Two issues in this block:
Either wire the resolved paths into 🤖 Prompt for AI Agents |
||
|
|
||
| # Build settings with CI-specific overrides | ||
| # We use a fresh xtest_root derived from this package's location | ||
| settings = Settings( | ||
| platform_dir=platform_dir, | ||
| ) | ||
| settings.ensure_directories() | ||
|
|
||
| # Update root key in platform config if provided | ||
| if root_key: | ||
| _prepare_kas_template(settings, root_key, ec_tdf_enabled) | ||
|
|
||
| # Determine which instances to start | ||
| if instances: | ||
| kas_names = [n.strip() for n in instances.split(",")] | ||
| for name in kas_names: | ||
| if name not in Ports.all_kas_names(): | ||
| print_error(f"Unknown KAS instance: {name}") | ||
| raise typer.Exit(1) | ||
| else: | ||
| kas_names = Ports.all_kas_names() | ||
|
|
||
| # Start KAS instances | ||
| print_info(f"Starting KAS instances: {', '.join(kas_names)}...") | ||
| kas_manager = get_kas_manager(settings) | ||
|
|
||
| failed = [] | ||
| for name in kas_names: | ||
| kas = kas_manager.get(name) | ||
| if kas is None: | ||
| print_error(f"KAS instance {name} not found in manager") | ||
| failed.append(name) | ||
| continue | ||
| if not kas.start(): | ||
| print_error(f"Failed to start KAS {name}") | ||
| failed.append(name) | ||
|
|
||
| if failed: | ||
| print_error(f"Failed to start: {', '.join(failed)}") | ||
| raise typer.Exit(1) | ||
|
|
||
| # Wait for health | ||
| print_info("Waiting for KAS health checks...") | ||
| unhealthy = [] | ||
| for name in kas_names: | ||
| port = Ports.get_kas_port(name) | ||
| try: | ||
| wait_for_health( | ||
| f"http://localhost:{port}/healthz", | ||
| timeout=health_timeout, | ||
| service_name=f"KAS {name}", | ||
| ) | ||
| except WaitTimeoutError as e: | ||
| print_warning(str(e)) | ||
| unhealthy.append(name) | ||
|
|
||
| if unhealthy: | ||
| print_error(f"KAS instances failed health check: {', '.join(unhealthy)}") | ||
| raise typer.Exit(1) | ||
|
|
||
| print_success(f"All {len(kas_names)} KAS instances are healthy") | ||
|
|
||
| # Emit outputs | ||
| for name in kas_names: | ||
| log_path = settings.get_kas_log_path(name) | ||
| output_key = f"kas-{name}-log-file" | ||
| _emit_github_output(output_key, str(log_path)) | ||
|
|
||
| print_success("CI KAS startup complete") | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -93,3 +93,39 @@ def java_fixup( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from otdf_sdk_mgr.java_fixup import post_checkout_java_fixup | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| post_checkout_java_fixup(base_dir) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @app.command("go-fixup") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def go_fixup_cmd( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| platform_dir: Annotated[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Path, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typer.Option("--platform-dir", help="Path to the platform checkout root"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| heads: Annotated[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Optional[str], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typer.Option( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "--heads", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| help="JSON list of head version tags to process (e.g. '[\"main\"]')", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base_dir: Annotated[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Optional[Path], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typer.Argument(help="Base directory for Go source trees"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Bridge Go client go.mod to server shared modules for head builds. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Performs go mod edit -replace + go mod tidy for each head version, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pointing platform module imports at the local platform checkout. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Only needed for standalone otdfctl checkouts. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import json as json_mod | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from otdf_sdk_mgr.go_fixup import go_fixup | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| heads_list = json_mod.loads(heads) if heads else None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| go_fixup(platform_dir, heads=heads_list, base_dir=base_dir) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except (FileNotFoundError, subprocess.CalledProcessError) as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typer.echo(f"Error: {e}", err=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise typer.Exit(1) from e | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+122
to
+131
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle malformed If 🛡️ Proposed fix import json as json_mod
from otdf_sdk_mgr.go_fixup import go_fixup
- heads_list = json_mod.loads(heads) if heads else None
+ heads_list: list[str] | None = None
+ if heads:
+ try:
+ heads_list = json_mod.loads(heads)
+ except json_mod.JSONDecodeError as e:
+ typer.echo(f"Error: --heads must be valid JSON: {e}", err=True)
+ raise typer.Exit(1) from e
+ if not isinstance(heads_list, list) or not all(isinstance(h, str) for h in heads_list):
+ typer.echo("Error: --heads must be a JSON list of strings", err=True)
+ raise typer.Exit(1)
try:
go_fixup(platform_dir, heads=heads_list, base_dir=base_dir)
except (FileNotFoundError, subprocess.CalledProcessError) as e:
typer.echo(f"Error: {e}", err=True)
raise typer.Exit(1) from e📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
ec_tdf_enabledis silently ignored unless--root-keyis provided.The EC TDF toggle is only applied inside the
if root_key:branch of_prepare_kas_template, and_prepare_kas_templateitself is only called whenroot_keyis truthy (line 164). A user passing--ec-tdf-enabled(or relying on the defaultTrue) without--root-keywill get no EC TDF override written to the platform config.Also, the function name
_prepare_kas_templateis misleading — it mutatessettings.platform_config(i.e.,opentdf-dev.yaml), not the KAS template (opentdf-kas-mode.yaml). Consider renaming to_prepare_platform_configand decoupling the two flags:🐛 Proposed fix
Also applies to: 163-165
🤖 Prompt for AI Agents