Skip to content

Commit

Permalink
azure: add Contributor privileges to new Service Principals
Browse files Browse the repository at this point in the history
When installing Cilium in an AKS cluster, the Cilium Operator requires
an Azure Service Principal with sufficient privileges to the Azure API
for the IPAM allocator to be able to work.

The CLI allows users to provide a Service Principal themselves via
`install` command flags, and otherwise tries to automatically create a
new Service Principal with minimal privileges using the `az ad sp
create-for-rbac` command (documentation:
https://docs.microsoft.com/en-us/cli/azure/ad/sp?view=azure-cli-latest#az-ad-sp-create-for-rbac).

AKS CLI installations with automatically created Service Principals are
now unsuccessful with the operator failing due to permissions errors:

```
level=warning msg="Unable to synchronize Azure virtualnetworks list" error="network.VirtualNetworksClient#ListAll: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code=\"AuthorizationFailed\" Message=\"The client 'd09fb531-793a-40fc-b934-7af73ca60e32' with object id 'd09fb531-793a-40fc-b934-7af73ca60e32' does not have authorization to perform action 'Microsoft.Network/virtualNetworks/read' over scope '/subscriptions/22716d91-fb67-4a07-ac5f-d36ea49d6167' or the scope is invalid. If access was recently granted, please refresh your credentials.\"" subsys=azure
level=fatal msg="Unable to start azure allocator" error="Initial synchronization with instances API failed" subsys=cilium-operator-azure
```

This is due to an Azure-side API change, with the `az ad sp
create-for-rbac` previously defaulting to assigning the `Contributor`
role to new Service Principals when none was provided via the optional
`--role` flag, whereas it now does not assign any role at all. This of
course breaks IPAM allocation due to insufficient permissions.

We fix the issue by manually specifying new Service Principals be
assigned the `Contributor` role.

This commit also fixes the AKS CI to assign the `Contributor` role to
the manually created Service Principal we pass to the CLI.

Signed-off-by: Nicolas Busseneau <[email protected]>
  • Loading branch information
nbusseneau authored and tklauser committed Feb 22, 2022
1 parent e5249c4 commit ad8d0d8
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/aks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ jobs:
AZURE_NODE_RESOURCE_GROUP=$(az aks show --resource-group ${{ env.name }} --name ${{ env.name }} --query "nodeResourceGroup" --output tsv)
# Create Service Principal with minimal privileges over the AKS node resource group
AZURE_SERVICE_PRINCIPAL=$(az ad sp create-for-rbac --scopes /subscriptions/${AZURE_SUBSCRIPTION_ID}/resourceGroups/${AZURE_NODE_RESOURCE_GROUP} --output json --only-show-errors)
AZURE_SERVICE_PRINCIPAL=$(az ad sp create-for-rbac --scopes /subscriptions/${AZURE_SUBSCRIPTION_ID}/resourceGroups/${AZURE_NODE_RESOURCE_GROUP} --role Contributor --output json --only-show-errors)
TENANT_ID=$(echo ${AZURE_SERVICE_PRINCIPAL} | jq -r '.tenant')
CLIENT_ID=$(echo ${AZURE_SERVICE_PRINCIPAL} | jq -r '.appId')
CLIENT_SECRET=$(echo ${AZURE_SERVICE_PRINCIPAL} | jq -r '.password')
Expand Down
7 changes: 4 additions & 3 deletions install/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,9 @@ func (k *K8sInstaller) azureRetrieveAKSNodeResourceGroup(ctx context.Context) er
return nil
}

// Use Service Principal provided by user if available, otherwise automatically create a new Service Principal
// and restrict its scope to the strict minimum (i.e. the AKS node resource group in which Cilium will be installed).
// Use Service Principal provided by user if available, otherwise automatically create a new Service Principal with
// Contributor privileges (required for IPAM within the Cilium Operator) and restrict its scope to the strict minimum
// (i.e. the AKS node resource group in which Cilium will be installed).
//
// We create a new Service Principal for each installation by design:
// - Having dedicated SPs with minimal privileges over their own AKS clusters is more secure.
Expand All @@ -146,7 +147,7 @@ func (k *K8sInstaller) azureRetrieveAKSNodeResourceGroup(ctx context.Context) er
func (k *K8sInstaller) azureSetupServicePrincipal(ctx context.Context) error {
if k.params.Azure.TenantID == "" && k.params.Azure.ClientID == "" && k.params.Azure.ClientSecret == "" {
k.Log("🚀 Creating Azure Service Principal for Cilium operator...")
bytes, err := k.azExec("ad", "sp", "create-for-rbac", "--scopes", "/subscriptions/"+k.params.Azure.SubscriptionID+"/resourceGroups/"+k.params.Azure.AKSNodeResourceGroup)
bytes, err := k.azExec("ad", "sp", "create-for-rbac", "--scopes", "/subscriptions/"+k.params.Azure.SubscriptionID+"/resourceGroups/"+k.params.Azure.AKSNodeResourceGroup, "--role", "Contributor")
if err != nil {
return err
}
Expand Down

0 comments on commit ad8d0d8

Please sign in to comment.