Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use sufficient identity roles for conformance tests #5060

Merged
merged 6 commits into from
Aug 9, 2024
Merged

use sufficient identity roles for conformance tests #5060

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fe2d1dd
use sufficient identity roles for conformance tests
jackfrancis Aug 7, 2024
f332505
test original solution
jackfrancis Aug 7, 2024
85d3282
remove storage account contrib
jackfrancis Aug 8, 2024
7b754b1
try Reader-only
jackfrancis Aug 8, 2024
014eb7e
try Contributor instead of Owner
jackfrancis Aug 9, 2024
5d736e8
add RBAC role
jackfrancis Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions scripts/kind-with-registry.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,10 @@ EOF
AZURE_IDENTITY_ID_PRINCIPAL_ID=$(az identity show -n "${USER_IDENTITY}" -g "${AZWI_RESOURCE_GROUP}" --query principalId -o tsv)

echo "${AZURE_IDENTITY_ID}" > "${AZURE_IDENTITY_ID_FILEPATH}"
until az role assignment create --assignee-object-id "${AZURE_IDENTITY_ID_PRINCIPAL_ID}" --role "Owner" --scope "/subscriptions/${AZURE_SUBSCRIPTION_ID}" --assignee-principal-type ServicePrincipal; do
until az role assignment create --assignee-object-id "${AZURE_IDENTITY_ID_PRINCIPAL_ID}" --role "Contributor" --scope "/subscriptions/${AZURE_SUBSCRIPTION_ID}" --assignee-principal-type ServicePrincipal; do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cluster-autoscaler e2e tests need permission to create role assignments (for workload identity auth from CAS) which Contributor doesn't have, but I think that gap can be filled with the "Role Based Access Control Administrator" role.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just verified cluster-autoscaler tests using an identity with Contributor + RBAC Admin passes for me locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK I couldn't remember why we might need the identity to have role assignment create perms, but now I do.

I think adding an add'l role assignment (Contributor + RBAC) is the bigger downside compared to the slight add'l privileges that Owner vs Contributor carries given that we actually need the most interesting part of the delta (the ability to create new role assignments).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I think Contributor + RBAC Admin has slightly fewer perms than Owner, i.e. it would not have the notActions here (aside from the ones granted by RBAC Admin):

{
  "assignableScopes": [
    "/"
  ],
  "description": "Grants full access to manage all resources, but does not allow you to assign roles in Azure RBAC, manage assignments in Azure Blueprints, or share image galleries.",
  "id": "/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c",
  "name": "b24988ac-6180-42a0-ab88-20f7382dd24c",
  "permissions": [
    {
      "actions": [
        "*"
      ],
      "notActions": [
        "Microsoft.Authorization/*/Delete",
        "Microsoft.Authorization/*/Write",
        "Microsoft.Authorization/elevateAccess/Action",
        "Microsoft.Blueprint/blueprintAssignments/write",
        "Microsoft.Blueprint/blueprintAssignments/delete",
        "Microsoft.Compute/galleries/share/action",
        "Microsoft.Purview/consents/write",
        "Microsoft.Purview/consents/delete"
      ],
      "dataActions": [],
      "notDataActions": []
    }
  ],
  "roleName": "Contributor",
  "roleType": "BuiltInRole",
  "type": "Microsoft.Authorization/roleDefinitions"
}

The elevateAccess one in particular might be worth avoiding if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair, testing our new trifecta now

sleep 5
done
until az role assignment create --assignee-object-id "${AZURE_IDENTITY_ID_PRINCIPAL_ID}" --role "Storage Account Contributor" --scope "/subscriptions/${AZURE_SUBSCRIPTION_ID}" --assignee-principal-type ServicePrincipal; do
sleep 5
done
until az role assignment create --assignee-object-id "${AZURE_IDENTITY_ID_PRINCIPAL_ID}" --role "Storage Blob Data Owner" --scope "/subscriptions/${AZURE_SUBSCRIPTION_ID}" --assignee-principal-type ServicePrincipal; do
until az role assignment create --assignee-object-id "${AZURE_IDENTITY_ID_PRINCIPAL_ID}" --role "Storage Blob Data Reader" --scope "/subscriptions/${AZURE_SUBSCRIPTION_ID}" --assignee-principal-type ServicePrincipal; do
sleep 5
done

Expand Down