# Phase 1: Critical Fixes Required

## 🔴 CRITICAL ISSUES (Must Fix Before Production)

### 1. Key Vault Access Policy for VM Managed Identities

**Issue**: VMs have Managed Identity enabled but Key Vault has no access policy for them.

**Impact**: VMs cannot access Key Vault to retrieve secrets, keys, or certificates.

**Location**: `terraform/modules/secrets/main.tf`

**Current State**:
```terraform
# Only current user has access
access_policy {
  tenant_id = data.azurerm_client_config.current.tenant_id
  object_id = data.azurerm_client_config.current.object_id
  # ... permissions
}
```

**Fix Required**: Add access policy for each VM's Managed Identity after VMs are created.

**Solution Options**:
1. **Option A**: Add access policy in Key Vault module (requires VM identity IDs)
2. **Option B**: Use separate `azurerm_key_vault_access_policy` resources in phase1-main.tf
3. **Option C**: Migrate to RBAC (recommended long-term)

**Recommended Fix**: Add access policies in phase1-main.tf after VMs are created:

```terraform
# Key Vault access policies for VM Managed Identities
resource "azurerm_key_vault_access_policy" "vm_phase1" {
  for_each = module.vm_phase1
  
  key_vault_id = module.keyvault.key_vault_id
  tenant_id    = data.azurerm_client_config.current.tenant_id
  object_id    = each.value.principal_id  # Need to add principal_id output to VM module
  
  secret_permissions = [
    "Get",
    "List"
  ]
  
  depends_on = [module.vm_phase1]
}
```

**Note**: Requires adding `principal_id` output to VM module.

---

### 2. NSG Rules Too Permissive

**Issue**: All NSG rules allow from `*` (entire internet).

**Impact**: Security vulnerability - entire internet can access:
- SSH (port 22)
- P2P (port 30303)
- RPC (ports 8545, 8546)
- Metrics (port 9545)

**Location**: `terraform/modules/networking-vm/main.tf`

**Fix Required**: Restrict to specific IP ranges/subnets.

**Solution**: Add variables for allowed IPs and update NSG rules:

```terraform
variable "allowed_ssh_ips" {
  description = "List of IP addresses/CIDR blocks allowed for SSH"
  type        = list(string)
  default     = []
}

variable "allowed_rpc_ips" {
  description = "List of IP addresses/CIDR blocks allowed for RPC (Nginx proxy subnet)"
  type        = list(string)
  default     = []
}

# Then in NSG rules:
source_address_prefixes = length(var.allowed_ssh_ips) > 0 ? var.allowed_ssh_ips : ["*"]
```

---

### 3. Address Space Conflicts (If VPN Deployed)

**Issue**: All regions use same address space (10.0.0.0/16).

**Impact**: If VPN/ExpressRoute connects regions, IP conflicts will occur.

**Location**: `terraform/modules/networking-vm/main.tf` line 7

**Fix Required**: Use region-specific address spaces.

**Solution**: Add variable for address space or use region-specific mapping:

```terraform
locals {
  region_address_spaces = {
    eastus     = "10.1.0.0/16"
    westus     = "10.2.0.0/16"
    centralus  = "10.3.0.0/16"
    eastus2    = "10.4.0.0/16"
    westus2    = "10.5.0.0/16"
    westeurope = "10.10.0.0/16"
  }
}

# In networking module:
address_space = [lookup(local.region_address_spaces, var.location, "10.0.0.16")]
```

---

### 4. Key Vault Network ACLs (Production)

**Issue**: Production Key Vault has `default_action = "Deny"` but no IPs/subnets whitelisted.

**Impact**: Key Vault might be inaccessible from Terraform or VMs.

**Location**: `terraform/modules/secrets/main.tf` line 33

**Fix Required**: Whitelist required IPs/subnets.

**Solution**: Add variables for allowed IPs/subnets:

```terraform
variable "key_vault_allowed_ips" {
  description = "List of IP addresses allowed to access Key Vault"
  type        = list(string)
  default     = []
}

variable "key_vault_allowed_subnets" {
  description = "List of subnet IDs allowed to access Key Vault"
  type        = list(string)
  default     = []
}

# In Key Vault resource:
network_acls {
  default_action = var.environment == "prod" ? "Deny" : "Allow"
  bypass         = "AzureServices"
  ip_rules       = var.key_vault_allowed_ips
  virtual_network_subnet_ids = var.key_vault_allowed_subnets
}
```

---

## 🟡 HIGH PRIORITY ISSUES (Should Fix Soon)

### 5. VM Scale Set Public IP Logic

**Issue**: VM Scale Set always creates public IP, even for "besu-node" type.

**Location**: `terraform/modules/vm-deployment/main.tf` line 150

**Fix**: Make public IP conditional on node_type (match individual VM logic).

---

### 6. Nginx Backend Validation

**Issue**: No validation if backend_vms is empty or private_ips is empty.

**Location**: `terraform/modules/nginx-proxy/nginx-cloud-init.yaml` line 63

**Fix**: Add validation or default empty upstream blocks.

---

### 7. Storage Account Name Collision Risk

**Issue**: Boot diagnostics storage name uses MD5 hash which might collide.

**Location**: `terraform/phases/phase1/phase1-main.tf` line 113

**Fix**: Add region index or timestamp for additional uniqueness.

---

## 🟢 MEDIUM PRIORITY ISSUES (Nice to Have)

### 8. Missing Monitoring

- No Log Analytics Workspace
- No Application Insights
- No metrics collection

### 9. Missing Backups

- No Recovery Services Vault
- No automated backup policies

### 10. High Availability

- Single VM per region (no redundancy)
- Single Nginx proxy (no redundancy)

---

## Implementation Priority

1. **🔴 CRITICAL**: Fix Key Vault access for VMs (#1)
2. **🔴 CRITICAL**: Restrict NSG rules (#2)
3. **🔴 CRITICAL**: Fix address spaces if VPN planned (#3)
4. **🔴 CRITICAL**: Configure Key Vault network ACLs (#4)
5. **🟡 HIGH**: Fix VM Scale Set public IP logic (#5)
6. **🟡 HIGH**: Add Nginx backend validation (#6)
7. **🟡 HIGH**: Improve storage account naming (#7)
8. **🟢 MEDIUM**: Add monitoring (#8)
9. **🟢 MEDIUM**: Add backups (#9)
10. **🟢 MEDIUM**: Implement HA (#10)

---

## Quick Fix Checklist

Before production deployment:
- [ ] Add Key Vault access policies for VM Managed Identities
- [ ] Restrict NSG rules to specific IP ranges
- [ ] Fix address spaces if VPN/ExpressRoute will be deployed
- [ ] Configure Key Vault network ACLs with allowed IPs/subnets
- [ ] Test VM access to Key Vault
- [ ] Test network connectivity with restricted NSG rules
- [ ] Verify VPN/ExpressRoute connectivity (if deployed)
- [ ] Test end-to-end functionality

