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.

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

Clarify identityRef and identity fields in AzureManagedControlPlaneClassSpec #5202

Open
djryanj opened this issue Oct 23, 2024 · 2 comments
Open

Comments

@djryanj
Copy link

djryanj commented Oct 23, 2024

When deploying a managed control plane thus:

apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: AzureManagedControlPlane
metadata:
  name: poc
spec:
  location: canadacentral
  resourceGroupName: rg
  # sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""}
  subscriptionID: 00000000-0000-0000-0000-000000000000 # fake uuid
  version: v1.30.0
  networkPolicy: azure
  networkPlugin: azure
  nodeResourceGroupName: rg2
  sku:
    tier: Free # or Standard
  identity:
    type: UserAssigned
    userAssignedIdentityResourceID: /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/uai
  identityRef:
    kind: AzureClusterIdentity
    name: cluster-identity
    namespace: to-be-patched
---
apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: AzureClusterIdentity
metadata:
  name: cluster-identity
spec:
  type: WorkloadIdentity
  allowedNamespaces:
    list:
    - <cluster-namespace>
  tenantID: <your-tenant-id>
  clientID: <your-client-id>

If you don't provide identity for a user assigned identity, it won't deploy with the user UAI, even though identityRef is provided (and the intention is to use that reference). In my case, both identity and identityRef point to the same identity. I realize that may not be the intention, but I can't find any descriptive documentation on what each really does to be able to understand each one.

From https://capz.sigs.k8s.io/reference/v1beta1-api#infrastructure.cluster.x-k8s.io/v1beta1.AzureManagedControlPlaneClassSpec:

identityRef
Kubernetes core/v1.ObjectReference
IdentityRef is a reference to a AzureClusterIdentity to be used when reconciling this cluster

And:

identity
Identity
(Optional) Identity configuration used by the AKS control plane.

Given the names, I would assume that they do the same thing, where one is a reference and the other is a directly encoded value. However, based on the CRD documentation, it seems like one is used by CAPI to reconcile, and the other is used by the cluster itself.

Any clarity would be appreciated.

@nojnhuh
Copy link
Contributor

nojnhuh commented Oct 24, 2024

Your interpretation of the CRD docs is correct. identityRef represents the credentials used when invoking Azure to create the AKS cluster. identity maps to the same-named field in the AKS API which assigns that identity to the control plane of the AKS cluster.

This is an unfortunate intersection between our tendency to try to align CAPZ field names with AKS API field names and the prior decision to call that field identityRef in all the other CAPZ types that use it. The good news is that the new ASO-based API doesn't use AzureClusterIdentity and therefore doesn't have an identityRef field to confuse with the identity AKS cluster property.

Overall, I think your correct read of the docs is a sign that we're doing about as well as we can here without changing the API. Is there some other specific change you'd like to see?

@djryanj
Copy link
Author

djryanj commented Oct 24, 2024

Hi @nojnhuh , thanks for the reply.

Given that the interpretation is correct, I can think of a few possible ideas.

  1. Clearer documentation on the AzureManagedControlPlane CRD itself and in other places in the book. As a suggestion:
    • identityRef: A reference to an AzureClusterIdentity that is used by Cluster API to create and manage the cluster via the Azure API.
    • identity: (Optional) Identity to be used by the control plane of the created AKS cluster, i.e., for use with Azure Workload Identity
  2. Along with the previous point, on the AzureClusterIdentity CRD, perhaps some clarity on what it's used for:
    • AzureClusterIdentity is used by Cluster API to create and manage clusters via the Azure API.
  3. Clearer documentation on the generated templates and e.g., https://capz.sigs.k8s.io/topics/workload-identity (or wherever else this intersection and happen).

I was about to suggest modifications to the documentation around moving away from v1 API towards ASO as potential v2, but I see that you have that in there already and since it's not set in stone, nothing to do there.

As a general statement, I think the documentation could be much improved. While I completely understand "only" providing an API reference, the lack of rendered YAML templates as documentation is frustrating to work with (but this is due in part to my general aversion to needing to install clusterctl alongside the plethora of other CLI tools when it's entirely unneeded--but that's beside the point). In a big improvement over the base cluster API book, CAPZ includes plenty of legitimate full YAMLs in the docs, but I think a full section on them would be a huge usability improvement. That said, if the project is open to it, I may open a PR or two with that goal in mind based on my recent experiences with CAPZ.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

2 participants