-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Better segregation of permissions when using managed identities #2656
Comments
That seems like a doable thing - Are you open to contributing this? I would use the following approach: podIdentity:
provider: azure # Optional. Default: false
identityId: <id> # Optional. Allows end-users to specific a user-assigned identity different from the default KEDA identity. |
Exactly :). That would be great! |
I will check more in detail to see if I can contribute :) |
Any thoughts on this @zroubalik @JorTurFer? Don't think there should be an issue but just double-checking. |
From high level seems doable, and it could help to reduce the amount of permissions under the same identity. Anyway, we should maintain also current behavior, something like: if other identity is set, use it, if not, use the current approach. |
I started looking into this and I validated the approach through a quick hack just to make sure it would work and yes, it seems doable. I have crafted the high level architecture & config to get it working. I'll dive into it over the weekend. |
Hello @tomkerkhove @JorTurFer I got it working in my environment (AKS 1.21.7) but I did touch a serious number of files....and I face one limitation with Azure Service Bus because of the TokenProvider interface which is forcing GetToken to pass a target audience only, so no way to pass a client id on top of it. However, Azure Service Bus works as usual with its default support of TriggerAuthentication. Before making the proposal, I did not realize that only Azure Storage & Service Bus did support TriggerAuthentication and with the SB limitation, this proposal would only be suitable for Azure Storage (at this stage), so I'm not sure it would add much value. Overall, here is what I did: where all the operator-related identities must reference a single selector in their AzureIdentityBinding CRD, which must match the aadpodidbinding label defined in the operator (.-set podIdentity.activeDirectory.identity=autoscaler-aad-identity when installing KEDA). Thanks to this, the operator is able to leverage any identity associated to the selector. If nothing is specified in the TriggerAuthentication CRD, the call to NMI does not include any clientid, which makes NMI fallback on the first identity available in ascending order... This means that this is only usable if workload-related identities have a name that is "bigger" than the default operator identity. Any "lower" name would be picked by NMI if not explicit identity is specified in the TriggerAuthentication resource. Let me know your thoughts. |
This is only for now, though, as we should end up supporting all of the Azure scalers with managed identity (and I thought we already did?). Feel free to open issues for them
Do you have any sample YAMLs of what it looks like? Overall I think this looks OK. |
Hi @tomkerkhove Sorry for my late reply but I've been rather busy :). The below YAML corresponds to the following scenario:
I have deleted all the resources so no issue to "reveal" sensitive information here, I have just obfuscated my subscription ID. The above YAML represents. Here is a diagram that illustrates the above YAML: To make things simple, I did not include a separate triggerauthentication block without the explicit MI to be used but it works fine (fallback scenario on the diagram). If we go further, I'll post a complete example, including the infrastructure as code bits to deploy the corresponding Azure Components. Best Regards |
This looks good to me, what do you think @JorTurFer? |
Are you willing to contribute this @stephaneey? |
Hi @tomkerkhove Yes of course but I was waiting for @JorTurFer 's feedback. Do you want me to start the pull request process? Thanks |
I think it looks pretty accurate, let's start the feature and doc PR as I don't expect big changes - Sounds good? |
Ok, I'll try to move on with this over the weekend |
No rush, next week or later is fine as I was checking who would pick it up |
Sorry for the delay. This issue was missing, maybe I marked as readed by error... 😔 |
Signed-off-by: Stephane Eyskens stephaneey@hotmail.com
The initial work was done by @stephaneey and is available in the @raorugan @v-shenoy Is this something your team can pick up? This is currently just for Pod Identity (if I'm not mistaken) but would be nice if we could support this for workload identity as well. |
I have been lurking on this issue and PR, but it would be great to see this merged. I have been waiting to add KEDA scalers, but I am reluctant to add an AAD identity that has permissions for every app's resources on our cluster. |
It's definitely coming but we just need somebody to complete the work so it's a matter of time and should definitely make it in to 2.8. |
I will try to take this up after #2907 gets merged. |
Thanks! |
Any update on when this will be picked up @v-shenoy? |
I will start working on this, and will make sure it's a part of the next release. Not sure about the release date. @kedacore/keda-core-contributors can help you there. |
The next release will be at early August so we have time: |
@wsugarman This has been implemented. |
@v-shenoy That's wonderful news! Thank you very much |
Relates to kedacore/keda#2656 Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>
Relates to kedacore/keda#2656 Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com> Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>
I know this is closed. But I am banging my head trying to get this to work. |
Could you share the steps that you performed, and any logs with the errors? |
Please, share also the AAD pod identity logs, this case usually is produced because something sin't correctly assigned in pod identity side and in that case, those pods are who have the errors in the logs |
We are discussing this offline and will open a discussion so we can move the conversation there. The main problem is that owner/manage permissions where lacking for the identity used by KEDA. |
so, do we need to update the sample? Could you open an issue for that? |
No, I missed the Big obvious Note on the documentation which states the permission needed (hangs head), I am still working out now how I think I want this to work, but will open a discussion as per Toms advice (much appreciated). One clarification I would like if we could though: |
This value is optional and will override the value set during helm installation. This is explained in the Authentication Provider section for AAD Pod Identity |
But I think @v-shenoy can explain this better "as the father" of the feature :) |
Yes, this appears to be the part that isn't working for me. I have read this and it's just not clear to me. I wanted to make sure I was using the correct value here, before I raise the issue. |
Can we please move this to a discussion given this is unrelated to this issue please? :)
|
I have created #3776 |
Proposal
Today, when using TriggerAuthentication with podIdentity, Keda is using the operator's associated managed identity to authenticate against the monitored stores (ie: Storage Account, Message Brokers, etc.). When using Keda at scale, this results is granting way too many permissions to a single identity. Keda is used by multiple workloads of different nature with different data confidentiality levels. The current implementation is not in line with the least privilege principle.
What about modifying the current TriggerAuthentication CRD and add a client_id field to let users specify a workload-specific identity to be used. To take a concrete example, in azure_aad_podidentity.go, Azure's MSI endpoint is declared as follows:
What about adding the optional client_id parameter to the query string (as documented here https://docs.microsoft.com/en-us/azure/active-directory/managed-identities-azure-resources/how-to-use-vm-token). This could work the following way:
I might be over simplifying and this may not work with all Cloud providers but this would allow us to better segregate permissions.
Use-Case
Achieve the same level of granularity as with the secret/config map approach but using managed identities.
Anything else?
No response
The text was updated successfully, but these errors were encountered: