From 1e4133e11aed7ea0d1c8c44392c9f7aa19dacd51 Mon Sep 17 00:00:00 2001 From: Jon Chery Date: Wed, 22 Jul 2026 22:08:23 +0000 Subject: [PATCH] fix(P30): temp dir isolation + forge-agnostic APIs + static-key override (P1-8, P1-9, S1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ---ci--- project: acdl phase: 30 milestone: v1.8 status: execute ---/ci--- P1-8: run_platform.sh now emits adapter output to $WORK/tf (per-run temp dir), not the committed terraform/spike/ directory. The committed terraform/spike/*.tf files are removed — they were scratch artifacts. Deploy workflow artifact upload path updated to /tmp/acdl_platform_run_v18/tf/. P1-9: contract_ingestor.py now reads GITHUB_API_BASE env for forge-agnostic API URLs. _forge_type() detects GitHub vs Gitea. Search URL is branched (GitHub uses /search/issues, Gitea uses /repos/{owner}/{repo}/issues). S1: Deploy workflow configure-aws-credentials step restructured as a single conditional step. OIDC when no static key (role-to-assume), static-key when ACDL_AWS_ACCESS_KEY_ID present (access-key-id/secret-access-key inputs). Both deploy workflows remain byte-identical. Tests: +8 (292 -> 300). All pass. run_platform.sh --check-only green. --- .gitea/workflows/deploy.yml | 11 ++- .github/workflows/deploy.yml | 11 ++- core/lambda/contract_ingestor.py | 49 ++++++++++++-- scripts/run_platform.sh | 27 ++++---- terraform/spike/main.tf | 111 ------------------------------- terraform/spike/providers.tf | 3 - terraform/spike/terraform.tf | 14 ---- tests/test_contract_ingestor.py | 38 ++++++++++- tests/test_pipeline_contract.py | 19 +++++- 9 files changed, 121 insertions(+), 162 deletions(-) delete mode 100644 terraform/spike/main.tf delete mode 100644 terraform/spike/providers.tf delete mode 100644 terraform/spike/terraform.tf diff --git a/.gitea/workflows/deploy.yml b/.gitea/workflows/deploy.yml index ea40873..5511fd0 100644 --- a/.gitea/workflows/deploy.yml +++ b/.gitea/workflows/deploy.yml @@ -91,14 +91,13 @@ jobs: echo "deb [signed-by=/usr/share/keyrings/hashicorp.gpg] https://apt.releases.hashicorp.com $(lsb_release -cs) main" | sudo tee /etc/apt/sources.list.d/hashicorp.list sudo apt-get update && sudo apt-get install -y terraform=1.9.* - - name: Configure AWS credentials (OIDC default) + - name: Configure AWS credentials (OIDC default + static-key override) uses: aws-actions/configure-aws-credentials@v4 with: - role-to-assume: arn:aws:iam::${{ secrets.ACDL_AWS_ACCOUNT_ID }}:role/acdl-deploy-${{ github.repository_id }} + role-to-assume: ${{ secrets.ACDL_AWS_ACCESS_KEY_ID == '' && format('arn:aws:iam::{0}:role/acdl-deploy-{1}', secrets.ACDL_AWS_ACCOUNT_ID, github.repository_id) || '' }} aws-region: us-east-1 - env: - ACDL_AWS_ACCESS_KEY_ID: ${{ secrets.ACDL_AWS_ACCESS_KEY_ID }} - ACDL_AWS_SECRET_ACCESS_KEY: ${{ secrets.ACDL_AWS_SECRET_ACCESS_KEY }} + access-key-id: ${{ secrets.ACDL_AWS_ACCESS_KEY_ID }} + secret-access-key: ${{ secrets.ACDL_AWS_SECRET_ACCESS_KEY }} - name: Run the platform pipeline working-directory: ${{ github.workspace }} @@ -136,7 +135,7 @@ jobs: uses: actions/upload-artifact@v4 with: name: acdl-terraform - path: platform/terraform/spike/*.tf + path: /tmp/acdl_platform_run_v18/tf/*.tf if-no-files-found: warn - name: Upload platform log diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index ea40873..5511fd0 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -91,14 +91,13 @@ jobs: echo "deb [signed-by=/usr/share/keyrings/hashicorp.gpg] https://apt.releases.hashicorp.com $(lsb_release -cs) main" | sudo tee /etc/apt/sources.list.d/hashicorp.list sudo apt-get update && sudo apt-get install -y terraform=1.9.* - - name: Configure AWS credentials (OIDC default) + - name: Configure AWS credentials (OIDC default + static-key override) uses: aws-actions/configure-aws-credentials@v4 with: - role-to-assume: arn:aws:iam::${{ secrets.ACDL_AWS_ACCOUNT_ID }}:role/acdl-deploy-${{ github.repository_id }} + role-to-assume: ${{ secrets.ACDL_AWS_ACCESS_KEY_ID == '' && format('arn:aws:iam::{0}:role/acdl-deploy-{1}', secrets.ACDL_AWS_ACCOUNT_ID, github.repository_id) || '' }} aws-region: us-east-1 - env: - ACDL_AWS_ACCESS_KEY_ID: ${{ secrets.ACDL_AWS_ACCESS_KEY_ID }} - ACDL_AWS_SECRET_ACCESS_KEY: ${{ secrets.ACDL_AWS_SECRET_ACCESS_KEY }} + access-key-id: ${{ secrets.ACDL_AWS_ACCESS_KEY_ID }} + secret-access-key: ${{ secrets.ACDL_AWS_SECRET_ACCESS_KEY }} - name: Run the platform pipeline working-directory: ${{ github.workspace }} @@ -136,7 +135,7 @@ jobs: uses: actions/upload-artifact@v4 with: name: acdl-terraform - path: platform/terraform/spike/*.tf + path: /tmp/acdl_platform_run_v18/tf/*.tf if-no-files-found: warn - name: Upload platform log diff --git a/core/lambda/contract_ingestor.py b/core/lambda/contract_ingestor.py index 37d4de7..00edc4e 100644 --- a/core/lambda/contract_ingestor.py +++ b/core/lambda/contract_ingestor.py @@ -24,6 +24,9 @@ import boto3 TABLE_NAME = os.environ.get("CONTRACTS_TABLE", "acdl-contracts") GITHUB_TOKEN_SECRET_ID = os.environ.get("GITHUB_TOKEN_SECRET_ID", "acdl/github-token") PLATFORM_REPO = os.environ.get("PLATFORM_REPO", "acdl/acdl") +# P1-9: Forge-agnostic API base URL. Defaults to GitHub; set GITHUB_API_BASE +# to a Gitea API root (e.g. https://git.cloudinit.dev/api/v1) for Gitea. +GITHUB_API_BASE = os.environ.get("GITHUB_API_BASE", "https://api.github.com") _dynamodb = None _secrets_client = None @@ -47,6 +50,43 @@ def _iso8601_now(): return datetime.datetime.now(datetime.timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") +def _forge_type(): + """P1-9: Detect whether the API base is GitHub or Gitea. + + Gitea API roots contain '/api/v1'; GitHub's is 'api.github.com'. + """ + if "/api/v1" in GITHUB_API_BASE: + return "gitea" + return "github" + + +def _issues_search_url(owner, repo, encoded_query): + """P1-9: Build the issue search URL based on forge type. + + GitHub uses /search/issues?q=...; Gitea uses /repos/{owner}/{repo}/issues?... + with query params (no /search/issues endpoint). + """ + if _forge_type() == "gitea": + return ( + f"{GITHUB_API_BASE}/repos/{owner}/{repo}/issues" + f"?state=open&type=issues&q={encoded_query}" + ) + return ( + f"{GITHUB_API_BASE}/search/issues?q=repo:{owner}/{repo}" + f"+is:issue+is:open+in:title+%22{encoded_query}%22" + ) + + +def _issues_create_url(owner, repo): + """URL for creating an issue (same pattern for both GitHub + Gitea).""" + return f"{GITHUB_API_BASE}/repos/{owner}/{repo}/issues" + + +def _issue_comments_url(owner, repo, issue_number): + """URL for posting a comment on an issue (same for both forges).""" + return f"{GITHUB_API_BASE}/repos/{owner}/{repo}/issues/{issue_number}/comments" + + def _submit_contract(payload): consumer_repo = payload["consumerRepo"] contract_id = payload["contractId"] @@ -105,10 +145,7 @@ def _report_error(payload): # Check for an existing open issue with the same title (idempotency) # URL-encode the contract_id to prevent search-query injection (P1-1). encoded_contract_id = urllib.parse.quote(contract_id, safe="") - search_url = ( - f"https://api.github.com/search/issues?q=repo:{owner}/{repo}" - f"+is:issue+is:open+in:title+%22{encoded_contract_id}%22" - ) + search_url = _issues_search_url(owner, repo, encoded_contract_id) req = urllib.request.Request(search_url) req.add_header("Authorization", f"token {github_token}") req.add_header("Accept", "application/vnd.github+json") @@ -146,7 +183,7 @@ _This issue was auto-created by the ACDL platform Lambda (D-055). The consumer's if existing: # Comment on the existing issue issue_number = existing[0]["number"] - url = f"https://api.github.com/repos/{owner}/{repo}/issues/{issue_number}/comments" + url = _issue_comments_url(owner, repo, issue_number) data = json.dumps({"body": body}).encode() req = urllib.request.Request(url, data=data, method="POST") req.add_header("Authorization", f"token {github_token}") @@ -160,7 +197,7 @@ _This issue was auto-created by the ACDL platform Lambda (D-055). The consumer's } else: # Create a new issue - url = f"https://api.github.com/repos/{owner}/{repo}/issues" + url = _issues_create_url(owner, repo) data = json.dumps({ "title": title, "body": body, diff --git a/scripts/run_platform.sh b/scripts/run_platform.sh index a8cf0dc..3ed98be 100755 --- a/scripts/run_platform.sh +++ b/scripts/run_platform.sh @@ -86,8 +86,9 @@ stream() { } CONTRACT_ID="11111111-1111-1111-1111-111111111111" # spike fixed UUID -WORK="/tmp/acdl_platform_run_v17" -rm -rf "$WORK"; mkdir -p "$WORK" +WORK="/tmp/acdl_platform_run_v18" +TF_DIR="$WORK/tf" +rm -rf "$WORK"; mkdir -p "$TF_DIR" echo "=== Step 0: environment onboarding check ===" if [ -f "$CONTRACT" ]; then @@ -118,14 +119,14 @@ python3 core/contract_resolver.py "$CONTRACT" "$WORK/stack.json" || fail "resolv python3 -c "import json; d=json.load(open('$WORK/stack.json')); print(f'stack: {d[\"stack\"][\"name\"]} {d[\"stack\"][\"kind\"]} {len(d[\"resources\"])} resource(s)')" echo "" -echo "=== Step 3: adapter compiles stack -> terraform/spike/*.tf ===" -python3 adapters/terraform/adapter.py "$WORK/stack.json" terraform/spike || fail "adapter failed" -echo "adapter: emitted terraform/spike/{main.tf,terraform.tf,providers.tf}" +echo "=== Step 3: adapter compiles stack -> $TF_DIR/*.tf ===" +python3 adapters/terraform/adapter.py "$WORK/stack.json" "$TF_DIR" || fail "adapter failed" +echo "adapter: emitted $TF_DIR/{main.tf,terraform.tf,providers.tf}" if [ "$QUIET" = "0" ]; then echo "" - echo "--- emitted terraform/spike/main.tf ---" - cat terraform/spike/main.tf + echo "--- emitted $TF_DIR/main.tf ---" + cat "$TF_DIR/main.tf" echo "--- end main.tf ---" fi @@ -137,7 +138,7 @@ import json, os d = json.load(open('$WORK/stack.json')) assert d['stack']['name'], 'stack name missing' assert len(d['resources']) >= 1, 'expected at least 1 resource' -tf_dir = 'terraform/spike' +tf_dir = '$TF_DIR' for f in ('main.tf', 'terraform.tf', 'providers.tf'): assert os.path.isfile(os.path.join(tf_dir, f)), f'{f} missing' main = open(os.path.join(tf_dir, 'main.tf')).read() @@ -166,7 +167,7 @@ export AWS_SECRET_ACCESS_KEY="$ACDL_AWS_SECRET_ACCESS_KEY" export AWS_DEFAULT_REGION="$AWS_DEFAULT_REGION" echo "=== Step 4: terraform init + validate + plan -lock=false (real AWS) ===" -cd terraform/spike +cd "$TF_DIR" echo "" echo "--- terraform init ---" @@ -191,11 +192,11 @@ if [ "$PLAN_ONLY" = "1" ]; then fi echo "" -echo "=== Step 5: run Checkov on terraform/spike/main.tf ===" +echo "=== Step 5: run Checkov on $TF_DIR/main.tf ===" if [ "$QUIET" = "0" ]; then - checkov -f terraform/spike/main.tf --framework terraform -o json --soft-fail --external-checks-dir adapters/terraform/policy/custom_rules/ 2>&1 | tee "$WORK/checkov.json" + checkov -f "$TF_DIR/main.tf" --framework terraform -o json --soft-fail --external-checks-dir adapters/terraform/policy/custom_rules/ 2>&1 | tee "$WORK/checkov.json" else - checkov -f terraform/spike/main.tf --framework terraform -o json --soft-fail --external-checks-dir adapters/terraform/policy/custom_rules/ > "$WORK/checkov.json" 2> "$WORK/checkov.err" + checkov -f "$TF_DIR/main.tf" --framework terraform -o json --soft-fail --external-checks-dir adapters/terraform/policy/custom_rules/ > "$WORK/checkov.json" 2> "$WORK/checkov.err" fi [ -s "$WORK/checkov.json" ] || fail "checkov produced no output" echo "" @@ -266,7 +267,7 @@ echo "=== Step 9: publish outputs to SSM + GitHub PR comment ===" # Read terraform outputs (if apply ran) and publish to SSM + format a PR comment. # In --check-only mode, skip (no terraform apply runs). if [ "$CHECK_ONLY" = "0" ]; then - cd terraform/spike + cd "$TF_DIR" TF_OUTPUTS=$(terraform output -json 2>/dev/null || echo "{}") cd "$ROOT" python3 < "$WORK/outputs_step.json" 2>/dev/null || true diff --git a/terraform/spike/main.tf b/terraform/spike/main.tf deleted file mode 100644 index 56b62e9..0000000 --- a/terraform/spike/main.tf +++ /dev/null @@ -1,111 +0,0 @@ -resource "aws_s3_bucket" "s3" { - bucket = "acdl-spike-bucket" - versioning { - enabled = true - } -} - -output "bucket_arn" { - value = aws_s3_bucket.s3.arn -} - -output "bucket_name" { - value = aws_s3_bucket.s3.id -} - -output "bucket_regional_domain_name" { - value = aws_s3_bucket.s3.bucket_regional_domain_name -} - -resource "aws_cloudfront_distribution" "cloudfront-distribution" { - origin { - domain_name = aws_s3_bucket.s3.bucket_regional_domain_name - origin_access_control = aws_cloudfront_origin_access_control.cloudfront-originaccesscontrol.id - s3_origin_config {} - } - enabled = true - default_cache_behavior { - viewer_protocol_policy = "redirect-to-https" - target_origin_id = "cloudfront-distribution" - min_ttl = 0 - default_ttl = 3600 - max_ttl = 86400 - allowed_methods = ["GET", "HEAD"] - cached_methods = ["GET", "HEAD"] - } - price_class = "PriceClass_100" - restrictions { - geo_restriction { - restriction_type = "none" - } - } - viewer_certificate { - cloudfront_default_certificate = true - } - web_acl_id = aws_wafv2_web_acl.waf.arn -} - -output "distribution_arn" { - value = aws_cloudfront_distribution.cloudfront-distribution.arn -} - -output "distribution_domain_name" { - value = aws_cloudfront_distribution.cloudfront-distribution.domain_name -} - -resource "aws_cloudfront_origin_access_control" "cloudfront-originaccesscontrol" { - name = "acdl-oac" - origin_access_control_origin_type = "s3" - origin_access_control_signing_behavior = "always" -} - -output "oac_id" { - value = aws_cloudfront_origin_access_control.cloudfront-originaccesscontrol.id -} - -resource "aws_wafv2_web_acl" "waf" { - name = "acdl-waf" - scope = "cloudfront" - default_action { - allow {} - } - visibility_config { - cloudwatch_metrics_enabled = true - metric_name = "acdl-waf-metrics" - sampled_requests_enabled = true - } - rules { - name = "aws-managed-rules" - priority = 0 - override_action { - none {} - } - statement { - managed_rule_group_statement { - name = "AWSManagedRulesCommonRuleSet" - vendor_name = "AWS" - } - } - visibility_config { - cloudwatch_metrics_enabled = true - metric_name = "aws-managed-rules-metrics" - sampled_requests_enabled = true - } - } -} - -output "web_acl_arn" { - value = aws_wafv2_web_acl.waf.arn -} - -output "distribution_domain_name" { - value = aws_cloudfront_distribution.cloudfront-distribution.domain_name -} - -output "bucket_arn" { - value = aws_s3_bucket.s3.arn -} - -output "web_acl_arn" { - value = aws_wafv2_web_acl.waf.arn -} diff --git a/terraform/spike/providers.tf b/terraform/spike/providers.tf deleted file mode 100644 index c125940..0000000 --- a/terraform/spike/providers.tf +++ /dev/null @@ -1,3 +0,0 @@ -provider "aws" { - region = "us-east-1" -} diff --git a/terraform/spike/terraform.tf b/terraform/spike/terraform.tf deleted file mode 100644 index 552d586..0000000 --- a/terraform/spike/terraform.tf +++ /dev/null @@ -1,14 +0,0 @@ -terraform { - required_version = ">= 1.9, < 1.10" - required_providers { - aws = { - source = "hashicorp/aws" - version = "~> 5.0" - } - } - backend "s3" { - bucket = "acdl-tfstate-581513795199-us-east-1" - key = "spike/static-assets/terraform.tfstate" - region = "us-east-1" - } -} diff --git a/tests/test_contract_ingestor.py b/tests/test_contract_ingestor.py index c8155e5..17e303d 100644 --- a/tests/test_contract_ingestor.py +++ b/tests/test_contract_ingestor.py @@ -381,4 +381,40 @@ class TestCallerIdentityValidation: "requestContext": {"identity": {"userArn": "arn:aws:sts::000:assumed-role/acdl-deploy/acdl-consumer-a"}}, } resp = ingestor.lambda_handler(event, None) - assert resp["statusCode"] == 200 \ No newline at end of file + assert resp["statusCode"] == 200 + + +class TestForgeAgnosticApiUrls: + """P1-9: contract_ingestor uses GITHUB_API_BASE for forge-agnostic URLs.""" + + def test_default_api_base_is_github(self): + assert ingestor.GITHUB_API_BASE == "https://api.github.com" + + def test_forge_type_detects_gitea(self, monkeypatch): + monkeypatch.setattr(ingestor, "GITHUB_API_BASE", "https://git.cloudinit.dev/api/v1") + assert ingestor._forge_type() == "gitea" + + def test_forge_type_detects_github(self): + assert ingestor._forge_type() == "github" + + def test_gitea_search_url_uses_repos_endpoint(self, monkeypatch): + monkeypatch.setattr(ingestor, "GITHUB_API_BASE", "https://git.cloudinit.dev/api/v1") + url = ingestor._issues_search_url("acdl", "acdl", "contract-123") + assert "git.cloudinit.dev/api/v1" in url + assert "/repos/acdl/acdl/issues" in url + assert "/search/issues" not in url + + def test_github_search_url_uses_search_endpoint(self): + url = ingestor._issues_search_url("acdl", "acdl", "contract-123") + assert "api.github.com/search/issues" in url + assert "repo:acdl/acdl" in url + + def test_create_url_uses_api_base(self, monkeypatch): + monkeypatch.setattr(ingestor, "GITHUB_API_BASE", "https://git.cloudinit.dev/api/v1") + url = ingestor._issues_create_url("acdl", "acdl") + assert url == "https://git.cloudinit.dev/api/v1/repos/acdl/acdl/issues" + + def test_comments_url_uses_api_base(self, monkeypatch): + monkeypatch.setattr(ingestor, "GITHUB_API_BASE", "https://git.cloudinit.dev/api/v1") + url = ingestor._issue_comments_url("acdl", "acdl", 42) + assert url == "https://git.cloudinit.dev/api/v1/repos/acdl/acdl/issues/42/comments" \ No newline at end of file diff --git a/tests/test_pipeline_contract.py b/tests/test_pipeline_contract.py index f9d96cc..2b09b2d 100644 --- a/tests/test_pipeline_contract.py +++ b/tests/test_pipeline_contract.py @@ -225,7 +225,8 @@ class TestRunPlatformStreaming: ) assert result.returncode == 0 assert "PLATFORM CHECK OK" in result.stdout - assert "--- emitted terraform/spike/main.tf ---" in result.stdout + assert "--- emitted" in result.stdout + assert "main.tf" in result.stdout assert "aws_s3_bucket" in result.stdout def test_check_only_quiet_suppresses_terraform(self): @@ -236,7 +237,7 @@ class TestRunPlatformStreaming: ) assert result.returncode == 0 assert "PLATFORM CHECK OK" in result.stdout - assert "--- emitted terraform/spike/main.tf ---" not in result.stdout + assert "--- emitted" not in result.stdout class TestDeployPipelineSchema: @@ -353,6 +354,20 @@ class TestDeployWorkflowConformance: assert wf["permissions"]["id-token"] == "write" assert wf["permissions"]["contents"] == "read" + def test_deploy_workflow_static_key_override_wired(self): + """S1: the static-key override must be wired to configure-aws-credentials + inputs (access-key-id/secret-access-key), not inert env vars.""" + wf = _load_workflow(".gitea/workflows/deploy.yml") + deploy_job = wf["jobs"]["deploy"] + creds_step = next( + s for s in deploy_job["steps"] + if "configure-aws-credentials" in s.get("uses", "") + ) + with_block = creds_step.get("with", {}) + assert "access-key-id" in with_block, "S1: access-key-id input must be wired" + assert "secret-access-key" in with_block, "S1: secret-access-key input must be wired" + assert "role-to-assume" in with_block, "S1: role-to-assume must still be present (conditional)" + class TestSampleContractVersioning: def test_sample_contract_uses_versioned_tag(self):