Skip to content

Commit c41d406

Browse files
zachcasperCopilot
andauthored
Apply suggestions from code review
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Zach Casper <zachcasper@microsoft.com>
1 parent 37ae535 commit c41d406

5 files changed

Lines changed: 8 additions & 8 deletions

File tree

‎specs/003-external-k8s-deploy/plan.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ Enhance Radius to deploy recipe workloads (Terraform and Bicep) to external AKS
1818
**Testing**: `go test` (unit), functional tests in `test/`
1919
**Target Platform**: Kubernetes (in-cluster), Linux containers
2020
**Project Type**: Control plane service (Go), API definitions (TypeSpec)
21-
**Performance Goals**: Kubeconfig acquisition adds <5s overhead per recipe execution (single API call per cloud provider)
21+
**Performance Goals**: Kubeconfig acquisition adds <5s overhead per recipe execution (a small number of API calls per cloud provider)
2222
**Constraints**: Token lifetime ~15 min (EKS), variable for AKS Entra ID tokens. Fresh token per execution, no caching.
2323
**Scale/Scope**: Affects 2 recipe engines (Terraform, Bicep), 1 API type (environments), 1 API version
2424

‎specs/003-external-k8s-deploy/quickstart.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77

88
- A running Radius installation on a Kubernetes cluster
99
- `rad` CLI configured and connected to the Radius installation
10-
- For EKS: AWS credentials registered (`rad credential register aws`) with `eks:DescribeCluster` permission
11-
- For AKS: Azure credentials registered (`rad credential register azure`) with `Azure Kubernetes Service Cluster User Role` and a Kubernetes RBAC role on the target cluster
10+
- For EKS: AWS credentials registered using `rad credential register aws access-key` or `rad credential register aws irsa`, with `eks:DescribeCluster` permission
11+
- For AKS: Azure credentials registered using `rad credential register azure sp` or `rad credential register azure wi`, with `Azure Kubernetes Service Cluster User Role` and a Kubernetes RBAC role on the target cluster
1212
- An external EKS or AKS cluster accessible from the Radius cluster's network
1313

1414
## Deploy to an External EKS Cluster

‎specs/003-external-k8s-deploy/research.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ The core token generation logic is ~15 lines using `sts.NewPresignClient` and `P
3636
Use `ListClusterUserCredentials` with exec format to extract cluster endpoint and CA cert, then acquire an Entra ID access token using the registered Azure service principal or workload identity credentials. Do not use `ListClusterAdminCredentials` since local accounts are disabled by default on AKS.
3737

3838
### Rationale
39-
Admin credentials (`listClusterAdminCredential`) fail on AKS clusters with local accounts disabled (the default). The user credential + Entra ID token flow is the standard production approach equivalent to `az aks get-credentials` + `kubelogin convert-kubeconfig --login spn`.
39+
Admin credentials (`ListClusterAdminCredentials`) fail on AKS clusters with local accounts disabled (the default). The user credential + Entra ID token flow is the standard production approach equivalent to `az aks get-credentials` + `kubelogin convert-kubeconfig --login spn`.
4040

4141
### Alternatives Considered
4242
- **Admin credentials**: Simpler but fails on default AKS configurations. Rejected.

‎specs/003-external-k8s-deploy/spec.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ When `providers.kubernetes.target` is omitted or set to `current`, Radius behave
7575

7676
### Session 2026-04-14
7777

78-
- Q: Should direct Kubernetes resource management by Radius (e.g., Applications.Core/containers) also target the external cluster, or only recipe execution? → A: Recipes only — direct resource management stays on the local cluster.
78+
- Q: Should direct Kubernetes resource management by Radius (e.g., Applications.Core/containers) also target the external cluster, or only recipe execution? → A: External-cluster targeting is for recipes and recipe-owned resource lifecycle operations only; direct management by non-recipe Radius resource providers (for example, `Applications.Core/*`) stays on the local cluster.
7979
- Q: Should Radius cache the dynamically-obtained kubeconfig or obtain a fresh one per recipe execution? → A: Fresh kubeconfig per recipe execution (no caching).
8080
- Q: Should the external kubeconfig be passed to Terraform via a temp file (`config_path`) or inline credentials (`host`, `token`, `cluster_ca_certificate`)? → A: Inline credentials in the Terraform provider block.
81-
- Q: Should Radius use AKS admin credentials or user credentials to obtain the kubeconfig? → A: User credentials (`listClusterUserCredential`) with Entra ID (AAD) authentication using the registered Azure service principal or workload identity. Local admin accounts are disabled by default on AKS, so admin credentials are not reliable.
81+
- Q: Should Radius use AKS admin credentials or user credentials to obtain the kubeconfig? → A: User credentials (`ListClusterUserCredentials`) with Entra ID (AAD) authentication using the registered Azure service principal or workload identity. Local admin accounts are disabled by default on AKS, so admin credentials are not reliable.
8282

8383
## Requirements *(mandatory)*
8484

@@ -89,7 +89,7 @@ When `providers.kubernetes.target` is omitted or set to `current`, Radius behave
8989
- **FR-003**: The `providers.kubernetes` model MUST support a new `clusterName` property of type string. This property MUST be required when `clusterType` is `aks` or `eks`.
9090
- **FR-004**: When `target` is `current` or omitted, Radius MUST use the existing kubeconfig resolution logic (in-cluster config or local kubeconfig) with no behavioral change.
9191
- **FR-005**: When `clusterType` is `eks`, Radius MUST use the registered AWS credentials and `providers.aws.region` to dynamically obtain a kubeconfig for the named EKS cluster before recipe execution.
92-
- **FR-006**: When `clusterType` is `aks`, Radius MUST use the registered Azure credentials and `providers.azure.resourceGroupName` to obtain user credentials (`listClusterUserCredential`) for the named AKS cluster, then authenticate via Entra ID (AAD) using the registered service principal or workload identity to obtain an access token. The resulting `host`, `token`, and `cluster_ca_certificate` are used inline.
92+
- **FR-006**: When `clusterType` is `aks`, Radius MUST use the registered Azure credentials and `providers.azure.resourceGroupName` to obtain user credentials (`ListClusterUserCredentials`) for the named AKS cluster, then authenticate via Entra ID (AAD) using the registered service principal or workload identity to obtain an access token. The resulting `host`, `token`, and `cluster_ca_certificate` are used inline.
9393
- **FR-007**: The dynamically-obtained kubeconfig MUST be passed to the Terraform Kubernetes provider as inline credentials (`host`, `token`, `cluster_ca_certificate`) rather than written to a temporary file. No kubeconfig files are written to disk.
9494
- **FR-008**: Terraform state MUST continue to be stored on the local Radius cluster. The Terraform Kubernetes backend configuration MUST NOT use the external cluster's kubeconfig; only the Terraform Kubernetes provider (for deploying resources) uses the external kubeconfig.
9595
- **FR-009**: The dynamically-obtained kubeconfig MUST be used by the Bicep/UCP deployment engine when creating Kubernetes resources.

‎specs/003-external-k8s-deploy/tasks.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
### CLI Changes
4141

4242
- [ ] T013 Add flag constants `KubernetesTargetFlag`, `KubernetesClusterTypeFlag`, `KubernetesClusterNameFlag` to pkg/cli/cmd/commonflags/flags.go and register them in `AddKubernetesScopeFlags()`
43-
- [ ] T014 Update `rad env create` preview command in pkg/cli/cmd/env/create/preview/create.go to accept all provider flags (`--namespace`, `--aws-account-id`, `--aws-region`, `--azure-subscription-id`, `--azure-resource-group`, `--kubernetes-target`, `--kubernetes-cluster-type`, `--kubernetes-cluster-name`) and map them to `EnvironmentProperties.Providers` fields, following the pattern in `rad env update`
43+
- [ ] T014 Update `rad env create` preview command in pkg/cli/cmd/env/create/preview/create.go to accept all provider flags (`--kubernetes-namespace`, `--aws-account-id`, `--aws-region`, `--azure-subscription-id`, `--azure-resource-group`, `--kubernetes-target`, `--kubernetes-cluster-type`, `--kubernetes-cluster-name`) and map them to `EnvironmentProperties.Providers` fields, following the pattern in `rad env update`
4444
- [ ] T015 [P] Update `rad env update` preview command in pkg/cli/cmd/env/update/preview/update.go to accept and extract the new kubernetes flags, following the existing Azure/AWS flag extraction pattern
4545
- [ ] T016 [P] Update `formatKubernetesProperties()` in pkg/cli/cmd/env/show/preview/envproviders.go to display `target`, `clusterType`, `clusterName` fields when present
4646
- [ ] T017 Add unit tests for `rad env create` with all provider flags (AWS, Azure, Kubernetes) in pkg/cli/cmd/env/create/preview/create_test.go

0 commit comments

Comments
 (0)