Address review feedback on IAM scope, runner_ip, and validation retries

- CloudWatchAgentServerPolicy already covers the agent's OTLP trace writes, so
  xray:PutSpans is unnecessary. Removing it leaves the AKS role with no inline
  policy at all; the VM inline policy is now validation reads only.
- runner_ip is required in both modules instead of defaulting to "". This removes
  the path where an unset value left the AKS API server reachable from any IP
  while reading as restricted, and drops the dynamic block that worked around it.
- VM log validation now retries on the same schedule as the AKS path, so a slow
  first ingestion no longer reports as a delivery failure.
- Scope AKS metric validation on a test_id datapoint attribute and assert
  cloud.platform=azure_aks. Datapoint attributes are the one surface no resource
  processor rewrites, and cloud.platform is emitted only by the aks detector, so
  the assertion covers detection rather than echoing back the injected payload.
- Match the kubernetes provider constraint already used by eks/daemon/efa.
- Move the load-generator payload rationale into the script, and drop the stale
  USE_DEFAULT_CONFIG references and all-merged prerequisite list from the README.

Author: movence

terraform/azure/aks/main.tf

@@ -24,14 +24,10 @@ resource "azurerm_kubernetes_cluster" "cwagent" {
   oidc_issuer_enabled       = true
   workload_identity_enabled = true
 
-  # Terraform drives the cluster over the public API server, so restrict it to the runner that created
-  # it. The block is omitted entirely when runner_ip is unset rather than emitted with an empty list --
-  # an empty authorized_ip_ranges means "open to all", which would read as restricted while being open.
-  dynamic "api_server_access_profile" {
-    for_each = var.runner_ip != "" ? [var.runner_ip] : []
-    content {
-      authorized_ip_ranges = [api_server_access_profile.value]
-    }
+  # Terraform drives the cluster over the public API server, so restrict it to the runner that created it.
+  # runner_ip is required, so there is no path where this silently ends up open to all.
+  api_server_access_profile {
+    authorized_ip_ranges = [var.runner_ip]
   }
 
   identity {
@@ -114,25 +110,8 @@ resource "aws_iam_role_policy_attachment" "cwagent_server_policy" {
   policy_arn = "arn:aws:iam::aws:policy/CloudWatchAgentServerPolicy"
 }
 
-# Unlike the VM test, the AKS test binary runs on the runner under the runner's own credentials, so this
-# role needs agent writes only -- no validation reads.
-data "aws_iam_policy_document" "cwagent_permissions" {
-  statement {
-    effect = "Allow"
-    actions = [
-      # Agent write omitted from CloudWatchAgentServerPolicy: the X-Ray OTLP endpoint needs PutSpans,
-      # which is a different action from PutTraceSegments.
-      "xray:PutSpans",
-    ]
-    resources = ["*"]
-  }
-}
-
-resource "aws_iam_role_policy" "cwagent" {
-  name   = "cwa-aks-integ-policy-${module.common.testing_id}"
-  role   = aws_iam_role.cwagent.id
-  policy = data.aws_iam_policy_document.cwagent_permissions.json
-}
+# No inline policy: the AKS test binary runs on the runner under its own credentials, so this role needs
+# agent writes only -- and CloudWatchAgentServerPolicy alone covers them, OTLP traces included.
 
 #####################################################################
 # Kubernetes resources: deploy CWA DaemonSet from ECR image
@@ -331,18 +310,14 @@ resource "kubernetes_daemon_set_v1" "cwagent" {
 
   depends_on = [
     kubernetes_cluster_role_binding.cwagent,
-    aws_iam_role_policy.cwagent,
     aws_iam_role_policy_attachment.cwagent_server_policy,
   ]
 }
 
 #####################################################################
 # Load generator: pushes OTLP to localhost:4318 for 3 min via hostNetwork
 #####################################################################
-# The payloads carry k8s.cluster.name and k8s.namespace.name resource attributes so the
-# agent's k8s logs-routing template produces a deterministic per-cluster destination
-# (/aws/cwagent/<cluster>/otlp, stream amazon-cloudwatch/<service>); resourcedetection
-# only overrides keys it detects (e.g. host.id), so these pass through intact.
+# See otlp_load_generator.sh for what the payloads carry and why.
 resource "kubernetes_job_v1" "otlp_load" {
   metadata {
     name      = "otlp-load-generator"

terraform/azure/aks/otlp_load_generator.sh

@@ -7,9 +7,12 @@
 #
 # Rendered by terraform via templatefile(), so a single-dollar brace expansion is a template
 # variable and a double-dollar brace expansion is passed through as a literal shell expansion.
-# Every payload carries k8s.cluster.name and k8s.namespace.name so the agent's k8s logs-routing
-# template produces a deterministic per-cluster destination, and resourcedetection (which only
-# overrides keys it detects, e.g. host.id) leaves them intact.
+# Every payload carries k8s.cluster.name and k8s.namespace.name so the agent's k8s logs-routing template
+# produces a deterministic per-cluster destination (/aws/cwagent/<cluster>/otlp, stream
+# amazon-cloudwatch/<service>); resourcedetection only overrides keys it detects, so these pass through.
+#
+# Metric datapoints carry test_id (value = per-run cluster name) for the Go test to scope on:
+# datapoint attributes are the one surface no resource processor rewrites.
 
 SERVICE_NAME="${service_name}"
 INSTANCE_ID="${instance_id}"
@@ -28,7 +31,7 @@ while [ $(date +%s) -lt $END ]; do
   SPAN_ID=$(printf '%016x' "$NOW_S$SEQ")
 
   curl -sf -X POST "$ENDPOINT/v1/metrics" -H "Content-Type: application/json" \
-    -d "{\"resourceMetrics\":[{\"resource\":{\"attributes\":[{\"key\":\"service.name\",\"value\":{\"stringValue\":\"$SERVICE_NAME\"}},{\"key\":\"host.id\",\"value\":{\"stringValue\":\"$INSTANCE_ID\"}},{\"key\":\"k8s.cluster.name\",\"value\":{\"stringValue\":\"$INSTANCE_ID\"}},{\"key\":\"k8s.namespace.name\",\"value\":{\"stringValue\":\"amazon-cloudwatch\"}}]},\"scopeMetrics\":[{\"scope\":{\"name\":\"aks-otlp-test\"},\"metrics\":[{\"name\":\"aks_otlp_counter\",\"unit\":\"1\",\"sum\":{\"aggregationTemporality\":2,\"isMonotonic\":true,\"dataPoints\":[{\"asInt\":\"$SEQ\",\"startTimeUnixNano\":\"$${START}000000000\",\"timeUnixNano\":\"$NOW_NS\",\"attributes\":[{\"key\":\"ClusterName\",\"value\":{\"stringValue\":\"$INSTANCE_ID\"}}]}]}}]}]}]}" || true
+    -d "{\"resourceMetrics\":[{\"resource\":{\"attributes\":[{\"key\":\"service.name\",\"value\":{\"stringValue\":\"$SERVICE_NAME\"}},{\"key\":\"host.id\",\"value\":{\"stringValue\":\"$INSTANCE_ID\"}},{\"key\":\"k8s.cluster.name\",\"value\":{\"stringValue\":\"$INSTANCE_ID\"}},{\"key\":\"k8s.namespace.name\",\"value\":{\"stringValue\":\"amazon-cloudwatch\"}}]},\"scopeMetrics\":[{\"scope\":{\"name\":\"aks-otlp-test\"},\"metrics\":[{\"name\":\"aks_otlp_counter\",\"unit\":\"1\",\"sum\":{\"aggregationTemporality\":2,\"isMonotonic\":true,\"dataPoints\":[{\"asInt\":\"$SEQ\",\"startTimeUnixNano\":\"$${START}000000000\",\"timeUnixNano\":\"$NOW_NS\",\"attributes\":[{\"key\":\"test_id\",\"value\":{\"stringValue\":\"$INSTANCE_ID\"}}]}]}}]}]}]}" || true
 
   curl -sf -X POST "$ENDPOINT/v1/logs" -H "Content-Type: application/json" \
     -d "{\"resourceLogs\":[{\"resource\":{\"attributes\":[{\"key\":\"service.name\",\"value\":{\"stringValue\":\"$SERVICE_NAME\"}},{\"key\":\"host.id\",\"value\":{\"stringValue\":\"$INSTANCE_ID\"}},{\"key\":\"k8s.cluster.name\",\"value\":{\"stringValue\":\"$INSTANCE_ID\"}},{\"key\":\"k8s.namespace.name\",\"value\":{\"stringValue\":\"amazon-cloudwatch\"}}]},\"scopeLogs\":[{\"scope\":{\"name\":\"aks-otlp-test\"},\"logRecords\":[{\"timeUnixNano\":\"$NOW_NS\",\"severityText\":\"INFO\",\"body\":{\"stringValue\":\"aks_otlp_log_$INSTANCE_ID\"},\"attributes\":[{\"key\":\"ClusterName\",\"value\":{\"stringValue\":\"$INSTANCE_ID\"}}]}]}]}]}" || true

terraform/azure/aks/providers.tf

@@ -1,11 +1,8 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: MIT
 
-# aws is constrained the same way the rest of the repo constrains it. azurerm and kubernetes are
-# major-version pinned because they are new to this repo and both have breaking schema changes past
-# the majors this module targets: unpinning azurerm resolved 5.0.0, which fails with
-# "At least 1 node_provisioning_profile blocks are required" on azurerm_kubernetes_cluster, and
-# kubernetes resolved 3.2.1 across its own 2.x -> 3.x break.
+# aws and kubernetes match terraform/eks/daemon/efa. azurerm has no precedent in this repo and is
+# pinned to 4.x because 5.0.0 requires a node_provisioning_profile block on azurerm_kubernetes_cluster.
 terraform {
   required_providers {
     aws = {
@@ -18,7 +15,7 @@ terraform {
     }
     kubernetes = {
       source  = "hashicorp/kubernetes"
-      version = "~> 2.30"
+      version = "~> 2.0"
     }
     tls = {
       source  = "hashicorp/tls"

terraform/azure/aks/variables.tf

@@ -34,10 +34,10 @@ variable "azure_subnet_name" {
   default = "default"
 }
 
+# Required, not defaulted: the API server is public and this is the only thing scoping it to the runner.
 variable "runner_ip" {
   type        = string
-  description = "Runner public IP, used to scope API server access to the runner. Empty leaves the API server open to all IPs."
-  default     = ""
+  description = "Runner public IP CIDR (e.g. \"1.2.3.4/32\") allowed to reach the AKS API server."
 }
 
 variable "aks_node_vm_size" {

terraform/azure/vm/iam.tf

@@ -57,16 +57,12 @@ resource "aws_iam_role_policy_attachment" "cwagent_server_policy" {
   policy_arn = "arn:aws:iam::aws:policy/CloudWatchAgentServerPolicy"
 }
 
-# Everything CloudWatchAgentServerPolicy does not cover. The test binary runs on the VM under this same
-# role, so the validation reads have to live here too.
+# Validation reads only -- the agent's own writes are fully covered by CloudWatchAgentServerPolicy. The
+# test binary runs on the VM under this same role, so these have to live here.
 data "aws_iam_policy_document" "cwagent_permissions" {
   statement {
     effect = "Allow"
     actions = [
-      # Agent write omitted from CloudWatchAgentServerPolicy: the X-Ray OTLP endpoint needs PutSpans,
-      # which is a different action from PutTraceSegments.
-      "xray:PutSpans",
-      # Reads used to assert delivery.
       "cloudwatch:ListMetrics",
       "cloudwatch:GetMetricData",
       "logs:GetLogEvents",

terraform/azure/vm/providers.tf

@@ -1,9 +1,8 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: MIT
 
-# aws is constrained the same way the rest of the repo constrains it. azurerm is major-version pinned
-# because it is new to this repo and 5.x carries breaking schema changes relative to the 4.x resources
-# used here (unpinning it resolved 5.0.0 and broke the sibling AKS module).
+# aws matches terraform/eks/daemon/efa. azurerm has no precedent in this repo and is pinned to 4.x
+# because 5.0.0 carries breaking schema changes relative to the 4.x resources used here.
 terraform {
   required_providers {
     aws = {

terraform/azure/vm/variables.tf

@@ -77,10 +77,10 @@ variable "admin_username" {
   default = "cwagent"
 }
 
-# CIDR allowed inbound SSH to the VM (the CI runner's public IP, e.g. "1.2.3.4/32"); no default so it must be set.
+# CIDR allowed inbound SSH to the VM (the CI runner's public IP, e.g. "1.2.3.4/32").
+# Required, not defaulted: this is the only thing scoping the NSG's Allow-22 rule.
 variable "runner_ip" {
-  type    = string
-  default = ""
+  type = string
 }
 
 # Ubuntu image the VM boots; matches the Debian-package install path used below.

test/azure/aks/aks_test.go

@@ -47,13 +47,14 @@ func TestMain(m *testing.M) {
 
 func TestAKS(t *testing.T) {
 	t.Run("Metrics", func(t *testing.T) {
-		// host.id is overwritten by resourcedetection with the node's VMSS instance ID,
-		// so scope by the cluster-name resource attribute the load generator sends.
+		// test_id is a datapoint attribute, the one surface no resource processor rewrites, so it
+		// isolates this run. cloud.platform=azure_aks comes only from the aks detector: proves detection ran.
 		group := otlpvalidation.ValidateOtlpMetricsWithLabels(
 			"AKSDefaultOtel", env.Region, []string{"aks_otlp_counter"},
 			map[string]string{
-				"@resource.k8s.cluster.name": env.AKSClusterName,
-				"@resource.cloud.provider":   "azure",
+				"test_id":                  env.AKSClusterName,
+				"@resource.cloud.platform": "azure_aks",
+				"@resource.cloud.provider": "azure",
 			},
 		)
 		for _, r := range group.TestResults {

test/azure/vm/README.md

@@ -1,15 +1,15 @@
 # Azure VM integration test
 
 Validates the CloudWatch Agent on a **real Azure VM** running the default OTel configuration
-(`USE_DEFAULT_CONFIG=otel` / `default:otel`). It exercises the full generated OTel pipeline
+(`default:otel`). It exercises the full generated OTel pipeline
 (Azure IMDS detection → translation → collector startup) and verifies that metrics, logs, and traces
 reach CloudWatch through the Azure → AWS credential chain (`oidctoken` extension → `sigv4auth` web identity).
 
 ## What it checks
 
-The agent is started with no JSON config and `USE_DEFAULT_CONFIG=otel`, so it emits the default OTel
-pipeline (OTLP receivers on `127.0.0.1:4317/4318` → native CloudWatch OTLP exporters). The test pushes OTLP
-and asserts delivery:
+The agent is started with no JSON config, via `amazon-cloudwatch-agent-ctl -a fetch-config -m auto -s -c
+default:otel`, so it emits the default OTel pipeline (OTLP receivers on `127.0.0.1:4317/4318` → native
+CloudWatch OTLP exporters). The test pushes OTLP and asserts delivery:
 
 | Signal  | Destination                                   | Assertion |
 |---------|-----------------------------------------------|-----------|
@@ -21,14 +21,6 @@ Delivery of all three signals is itself the credential-chain assertion: nothing
 the Azure web-identity exchange succeeded. The `cloud.provider=azure` resource label additionally proves the
 telemetry traversed the Azure detection path rather than being attributed to stray AWS credentials.
 
-## Agent prerequisites
-
-- `#2179` Add default OTel config — **merged** (`USE_DEFAULT_CONFIG=otel`)
-- `#2183` Azure platform detection — **merged** (IMDS detection → translation)
-- `#2010` OIDC auth (`oidctoken`) — **merged**
-- `#2197` web-identity for `awscloudwatchlogsprovisioner` — **merged** (log-group creation on Azure VM)
-- `#2211` `set-env` ctl action — **merged** (used to persist `AWS_REGION` / `CWAGENT_ROLE_ARN`)
-
 ## Prerequisites that MUST be satisfied before a live run
 
 These were verified against a real Azure VM and the merged agent translator. Without them a live

test/azure/vm/azurevm_test.go

@@ -122,26 +122,36 @@ func measuredMetrics() []string { return []string{"azurevm_otlp_counter", "azure
 func validateLogs() status.TestResult {
 	testResult := status.TestResult{Name: "AzureVM_Logs", Status: status.FAILED}
 
-	streams := awsservice.GetLogStreams(otlpLogGroup)
-	if len(streams) == 0 {
-		testResult.Reason = fmt.Errorf("no log streams found in %s", otlpLogGroup)
-		return testResult
-	}
-
-	since := time.Now().Add(-loadWindow - time.Minute)
-	until := time.Now()
+	// Retry on the same schedule as the AKS log path: the stream and its events can both lag the
+	// load window, so a single attempt fails on ingestion delay rather than on delivery.
 	marker := fmt.Sprintf("azurevm_otlp_log_%s", env.InstanceId)
-	for _, stream := range streams {
-		err := awsservice.ValidateLogs(
-			otlpLogGroup, *stream.LogStreamName, &since, &until,
-			awsservice.AssertLogsNotEmpty(),
-			awsservice.AssertPerLog(awsservice.AssertLogContainsSubstring(marker)),
-		)
-		if err == nil {
-			testResult.Status = status.SUCCESSFUL
-			return testResult
+	const maxRetries = 4
+	const retryInterval = 30 * time.Second
+	for attempt := 1; attempt <= maxRetries; attempt++ {
+		since := time.Now().Add(-loadWindow - time.Minute)
+		until := time.Now()
+
+		streams := awsservice.GetLogStreams(otlpLogGroup)
+		if len(streams) == 0 {
+			testResult.Reason = fmt.Errorf("attempt %d: no log streams found in %s", attempt, otlpLogGroup)
+		}
+		for _, stream := range streams {
+			log.Printf("[AzureVM_Logs] attempt %d: checking %s/%s", attempt, otlpLogGroup, *stream.LogStreamName)
+			err := awsservice.ValidateLogs(
+				otlpLogGroup, *stream.LogStreamName, &since, &until,
+				awsservice.AssertLogsNotEmpty(),
+				awsservice.AssertPerLog(awsservice.AssertLogContainsSubstring(marker)),
+			)
+			if err == nil {
+				testResult.Status = status.SUCCESSFUL
+				return testResult
+			}
+			testResult.Reason = err
+		}
+		if attempt < maxRetries {
+			log.Printf("[AzureVM_Logs] %v — retrying in %v", testResult.Reason, retryInterval)
+			time.Sleep(retryInterval)
 		}
-		testResult.Reason = err
 	}
 	return testResult
 }