Aad-pod-identity: Proposed Behavior Change: Identity/Namespaces/Assignment

[khenidak]

Ref: #106 #45 and other

Today, Identities are hardcoded to default namespace. and pods are matched using selector irrespective of pod.namespace. The is causing the following:

1. Users would like to compartmentalize application resources at the namespace level. This includes the identity.
2. This model currently does not allow matching identity.namespace == pod.namespace

Proposed change:

1. Remove the hardcoded selector in https://github.com/Azure/aad-pod-identity/blob/master/pkg/crd/crd.go#L128 (along with any reference to default namespace in listXXX calls)
2. Add annotation to azureIdentity to force matching to current namespace only. i.e true = (pod.namespace == azureIdentity.namespace) When the annotation is not found then we match in any namespace.

@kkmsft @rite2nikhil @ritazh Thoughts?

[kkmsft]

/sgtm

[rite2nikhil]

The default is to match on any namespace, we will be backward compatible /lgtm

[brantb]

Out of curiosity, what is the motivation behind selecting pods based on the label aadpodidbinding instead of allowing arbitrary label selectors as described by the k8s documentation? Typically a namespaced resource will typically reference either (a) other resources in the same namespace, or (b) a non-namespaced resource like a ClusterRole or a PersistentVolume.

I'd like to suggest the following behavior:

1. Allow AzureIdentity and AzureIdentityBinding to be placed in any namespace.
2. [breaking] Change the spec of AzureIdentityBinding to reference pods in the current namespace using an arbitrary selector, e.g.

apiVersion: aadpodidentity.k8s.io/v1
kind: AzureIdentityBinding
metadata:
  name: demo-azure-id-binding
  namespace: my-namespace
spec:
  AzureIdentity: my-azure-identity-name
  selector:
    matchLabels:
      app: myapplication # example
      environment: dev   # example

1. [breaking] When an AzureIdentityBinding references an AzureIdentity by name in spec.AzureIdentity, match only in the same namespace. (Alternate but also breaking idea: Make AzureIdentity a cluster-wide resource so that they do not reside in any namespace)

I completely understand if you don't want to make breaking changes at this time, but this would bring aad-pod-identity more in line with how I've seen many other Kubernetes applications operate.

[pjbgf]

I agree with @brantb's fair suggestions. I am specially interested on points 1 and 2 being implemented, as together they would allow for a more secure approach than what we currently have with default namespace based identities.

Having the ability of referencing identities cross namespaces can be useful to be backwards compatible, however, that will be prone to less secure implementations. If that is implemented, it would be good to have a setting to disable it at cluster level.

[pjbgf]

I am working on a PR for this. To confirm what we are expecting in terms of behaviour, the table below has three columns that tells in which namespace each object may live.

| Azure Identity | Azure Identity Binding | Target Pod | Status |
| --------------- | ------------- | ------------- | ------------- |
| default | default | default | Supported |
| default | default | custom-app1-ns | Supported |
| default | custom-app2-ns | custom-app2-ns | New - To be implemented |
| custom-app3-ns | custom-app3-ns | custom-app3-ns | New - To be implemented |

Below are a few suggestions of scenarios I believe we should not support:

| Azure Identity | Azure Identity Binding | Target Pod |
| --------------- | ------------- | ------------- |
| default | custom-app3-ns | custom-app4-ns |
| custom-app4-ns | custom-app3-ns | custom-app4-ns |
| custom-app2-ns | custom-app3-ns | custom-app4-ns |
| custom-app3-ns | custom-app4-ns | custom-app4-ns |

On a side note, can we also safely agree that AzureAssignedIdentity objects should always be created at the same namespace as the aad-pod-identity pods are deployed into?

Any views on this @rite2nikhil @kkmsft @khenidak @ritazh ?

[khenidak]

close with #140