aad-pod-identity icon indicating copy to clipboard operation
aad-pod-identity copied to clipboard

Use a selector for AzureIdentityBinding

Open ritazh opened this issue 5 years ago • 5 comments

Describe the request Use a selector for AzureIdentityBinding instead of match pod on a static label

Explain why AAD Pod Identity needs it Use selector semantics to provide more flexibility

Describe the solution you'd like As a user, I would like to use standard label selector semantics in AzureIdentityBinding to get matching pods.

For example:

apiVersion: "aadpodidentity.k8s.io/v1"
kind: AzureIdentityBinding
metadata:
  name: test-azure-id-binding
spec: 
  AzureIdentity: "test-azure-identity"
  selector: 
    aadpodidbinding: select_it

Describe alternatives you've considered

Additional context

cc @kkmsft @aramase

ritazh avatar Jun 10 '19 15:06 ritazh

I'll be helping with this issue :)

Are there any E2E tests in place that need to be changed or run to verify the fix?

The following fixes need to be made:

  • [ ] Edit existing YAML files
  • Helm Chart
  • Samples
  • [ ] Edit Go files
  • types.go
  • mic
  • [ ] Update tests
  • add tests to e2e?
  • crd/mic - update existing tests with selector code
  • [ ] Update existing documentation
  • [ ] Test backwards compatibility

Let me know if there's anything I missed. I'll update as I make progress or come across anything else.

cnadolny avatar Jun 10 '19 23:06 cnadolny

@cnadolny The e2e tests are in here - https://github.com/Azure/aad-pod-identity/tree/master/test/e2e along with instructions on how to run them.

Let us know if you have any questions. Looking forward to your PR :)

aramase avatar Jun 10 '19 23:06 aramase

@cnadolny - Thank you for looking into this. One important item with regard to this is backward capability - older installations should work with the new code.

kkmsft avatar Jun 11 '19 01:06 kkmsft

@cnadolny - Thank you for looking into this. One important item with regard to this is backward capability - older installations should work with the new code.

To clarify with backwards compatibility - already existing pods bound to identities should continue to function, but should AzureIdentityBinding be expected to accept both static labels AND label selectors to work with older versions YAML files?

Also, for the label selector - is it only accepting keys labeled with aadpodidbinding? Or are other label keys or multiple keys allowed to be accepted as well?

cnadolny avatar Jun 12 '19 05:06 cnadolny

Hi! I have a use case that this change would make possible-- I see that https://github.com/Azure/aad-pod-identity/pull/244 was closed, but was hoping someone could give any suggestions for a workaround :)

Essentially we have a StatefulSet where each pod will be using a particular HSM in Key Vault for signing. In the initial implementation, each pod in the statefulset is using the same AzureIdentityBinding that uses an identity with access to a single Key Vault with all the keys. Reducing the compromised surface area if an attack were to happen is really important for us, so I'm hoping to separate each HSM into its own Key Vault and have each pod in the statefulset only have access to the Key Vault/HSM they will be using.

I can't find a way to make this design work with the current design of aad-pod-identity-- I'm unable to use the ordinal index of a pod in the value of the aadpodidbinding label. If there was a selector for AzureIdentityBinding I could just use statefulset.kubernetes.io/pod-name, but I'm having trouble finding a good workaround.

Ideally I'd like to keep using a statefulset because it's pretty well suited for the use case and is very easy to work with. Afaik there's no way to restrict access in a key vault on a per-key basis either :(

Any advice is greatly appreciated! :)

tkporter avatar Apr 09 '20 18:04 tkporter