Merge remote-tracking branch 'origin/develop'

Author: xiaomi7732

docs/configure-rbac-permissions.md

@@ -1,70 +1,76 @@
 # Configure RBAC permissions
 
-`Microsoft.ApplicationInsights.Kubernetes` uses the service account to query kubernetes information to enhance telemetries. It is important to have proper permissions configured for kubernetes related information like node, pod and so on to be fetched correctly.
+`Microsoft.ApplicationInsights.Kubernetes` uses the service account to query Kubernetes information to enhance telemetries. It is important to have proper permissions configured for Kubernetes-related resources like Node, Pod, and so on to be fetched correctly.
 
-In this post, we will start by describe a method to correctly configure the permissions for an RBAC enabled cluster. And then share a guidance for troubleshooting.
+In this post, we will start by describing a method to correctly configure the permissions for an RBAC-enabled cluster. And then share troubleshooting guidance.
 
 ## Assumptions
 
 In this demo, we will have the following assumptions. Please change the related values accordingly:
 
-* The application will be deployed to namespace of `ai-k8s-demo`.
+* The application will be deployed to namespace `ai-k8s-demo`.
 * The application will leverage the `default` service account.
 
-## Configure ClusterRole and ClusterRoleBinding for the service account
-
-* Create a yaml file, [sa-role.yaml](./sa-role.yaml) for example. We will deploy it when it is ready.
-
-    * Write spec to define a cluster role, name it `appinsights-k8s-property-reader` for example:
- 
-        ```yaml
-         kind: ClusterRole
-         apiVersion: rbac.authorization.k8s.io/v1
-         metadata:
-           # "namespace" omitted since ClusterRoles are not namespaced
-           name: appinsights-k8s-property-reader
-         rules:
-         - apiGroups: ["", "apps"]
-           resources: ["pods", "nodes", "replicasets", "deployments"]
-           verbs: ["get", "list"]
-        ```
-        That spec defines the name of the role, and what permission does the role has, for example, list pods.
-
-        You don't have to use the exact name, but you will need to making sure the name is referenced correctly in the following steps.
-
-    * Append a Cluster role binding spec:
-
-        ```yaml
-        ---
-        # actual binding to the role
-        kind: ClusterRoleBinding
-        apiVersion: rbac.authorization.k8s.io/v1
-        metadata:
-          name: appinsights-k8s-property-reader-binding
-        subjects:
-        - kind: ServiceAccount
-          name: default
-          namespace: ai-k8s-demo
-        roleRef:
-          kind: ClusterRole
-          name: appinsights-k8s-property-reader
-          apiGroup: rbac.authorization.k8s.io
-        ```
-
-    That is to grant the role of `appinsights-k8s-property-reader` to the default service account in namespace of `ai-k8s-demo`.
-    
+## Setup the permissions for the service account
+
+Depending on various considerations, there could be different strategies to set up the permissions for your service account. Here we list 2 common possibilities, as examples.
+
+* If you want to get the Node information along with other resource info like Pod, Deployment, and so on, a ClusterRole and a ClusterRoleBinding are required, and here's how to do it:
+
+  * Create a yaml file, [sa-role.yaml](./sa-role.yaml) for example. We will deploy it when it is ready.
+
+      * Write spec to define a cluster role, name it `appinsights-k8s-property-reader` for example:
+   
+          ```yaml
+           kind: ClusterRole
+           apiVersion: rbac.authorization.k8s.io/v1
+           metadata:
+             # "namespace" omitted since ClusterRoles are not namespaced
+             name: appinsights-k8s-property-reader
+           rules:
+           - apiGroups: ["", "apps"]
+             resources: ["pods", "nodes", "replicasets", "deployments"]
+             verbs: ["get", "list"]
+          ```
+          That spec defines the name of the role, and what permission does the role has, for example, list pods.
+
+          You don't have to use the exact name, but you will need to making sure the name is referenced correctly in the following steps.
+
+      * Append a Cluster role binding spec:
+
+          ```yaml
+          ---
+          # actual binding to the role
+          kind: ClusterRoleBinding
+          apiVersion: rbac.authorization.k8s.io/v1
+          metadata:
+            name: appinsights-k8s-property-reader-binding
+          subjects:
+          - kind: ServiceAccount
+            name: default
+            namespace: ai-k8s-demo
+          roleRef:
+            kind: ClusterRole
+            name: appinsights-k8s-property-reader
+            apiGroup: rbac.authorization.k8s.io
+          ```
+
+      That is to grant the role of `appinsights-k8s-property-reader` to the default service account in the namespace of `ai-k8s-demo`.
+
+  * If you don't want to create a Cluster Role, it is also possible to use Role and RoleBinding starting with Application Insights for Kubernetes 2.0.6+. Follow the example in [sa-role-none-cluster.yaml](./sa-role-none-cluster.yaml). In that case, you will not have node info on the telemetries.
+
 * Now you can deploy it:
 
     ```shell
     kubectl create -f `sa-role.yaml`
     ```
     See [sa-role.yaml](sa-role.yaml) for a full example.
 
-> :warning: Check back for various permissions needed. Depends on the implementations, it may change in the over time.
+> :warning: Check back for various permissions needed. Depending on the properties we try to fetch, it may change over time.
 
 ## Ad-hoc troubleshooting for permission
 
-Kubectl provides an `auth --can-i` sub command for troubleshooting permissions. It supports impersonate the service account. We can leverage it for permission troubleshooting, for example:
+Kubectl provides an `auth --can-i` subcommand for troubleshooting permissions. It supports impersonating the service account. We can leverage it for permission troubleshooting, for example:
 
 ```shell
 kubectl auth can-i list pod --namespace ai-k8s-demo --as system:serviceaccount:ai-k8s-demo:default
@@ -84,7 +90,7 @@ yes
 
 ## Use SubjectAccessReview
 
-Kubernetes also provides `SubjectAccessReview` to check permission for given user on target resource.
+Kubernetes also provides `SubjectAccessReview` to check permission for a given user on the target resource.
 
 ### Basic usage
 
@@ -147,7 +153,7 @@ Kubernetes also provides `SubjectAccessReview` to check permission for given use
     * [subject-access-review-key.yaml](./subject-access-review-key.yaml): a subset of permissions for probing the RBAC settings.
     * [subject-access-review-full.yaml](./subject-access-review-full.yaml): a full list of all permissions needed for RBAC settings in case any specific permission is missing.
 
-Let us know if there's questions, suggestions by filing [issues](https://github.com/microsoft/ApplicationInsights-Kubernetes/issues).
+Let us know if there are questions or suggestions by filing [issues](https://github.com/microsoft/ApplicationInsights-Kubernetes/issues).
 
 ## References
 

docs/sa-role-none-cluster.yaml

@@ -0,0 +1,24 @@
+kind: Role
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  namespace: ai-k8s-demo
+  name: appinsights-k8s-property-reader-role
+rules:
+- apiGroups: ["", "apps"]
+  resources: ["pods", "replicasets", "deployments"]
+  verbs: ["get", "list"]
+---
+# Actual RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata: 
+  name: appinsights-k8s-property-reader-binding
+  namespace: ai-k8s-demo
+subjects:
+- kind: ServiceAccount
+  name: default
+  namespace: ai-k8s-demo
+roleRef:
+  kind: Role
+  name: appinsights-k8s-property-reader-role
+  apiGroup: rbac.authorization.k8s.io

src/ApplicationInsights.Kubernetes/IK8sQueryClient.cs

@@ -11,7 +11,7 @@ namespace Microsoft.ApplicationInsights.Kubernetes
     internal interface IK8sQueryClient : IDisposable
     {
         Task<IEnumerable<K8sDeployment>> GetDeploymentsAsync(CancellationToken cancellationToken);
-        Task<IEnumerable<K8sNode>> GetNodesAsync(CancellationToken cancellationToken);
+        Task<IEnumerable<K8sNode>> GetNodesAsync(bool ignoreForbiddenException, CancellationToken cancellationToken);
         Task<IEnumerable<K8sPod>> GetPodsAsync(CancellationToken cancellationToken);
         Task<K8sPod?> GetPodAsync(string podName, CancellationToken cancellationToken);
         Task<IEnumerable<K8sReplicaSet>> GetReplicasAsync(CancellationToken cancellationToken);

src/ApplicationInsights.Kubernetes/K8sEnvironmentFactory.cs

@@ -106,14 +106,15 @@ public K8sEnvironmentFactory(
 
                     if (instance.myPod is not null)
                     {
-                        IEnumerable<K8sNode> nodeList = await queryClient.GetNodesAsync(cancellationToken).ConfigureAwait(false);
+                        IEnumerable<K8sNode> nodeList = await queryClient.GetNodesAsync(ignoreForbiddenException: true, cancellationToken: cancellationToken).ConfigureAwait(false);
                         string nodeName = instance.myPod.Spec.NodeName;
-                        if (!string.IsNullOrEmpty(nodeName))
+                        if (!string.IsNullOrEmpty(nodeName) && nodeList.Any())
                         {
                             instance.myNode = nodeList.FirstOrDefault(node => string.Equals(node.Metadata?.Name, nodeName, StringComparison.OrdinalIgnoreCase));
                         }
                     }
                 }
+
                 return instance;
             }
             catch (UnauthorizedAccessException ex)

src/ApplicationInsights.Kubernetes/K8sQueryClient.cs

@@ -91,12 +91,27 @@ public Task<IEnumerable<K8sDeployment>> GetDeploymentsAsync(CancellationToken ca
         #endregion
 
         #region Node
-        public Task<IEnumerable<K8sNode>> GetNodesAsync(CancellationToken cancellationToken)
+        public async Task<IEnumerable<K8sNode>> GetNodesAsync(bool ignoreForbiddenException, CancellationToken cancellationToken)
         {
             EnsureNotDisposed();
 
-            string url = Invariant($"api/v1/nodes");
-            return GetAllItemsAsync<K8sNode>(url, cancellationToken);
+            try
+            {
+                string url = Invariant($"api/v1/nodes");
+                return await GetAllItemsAsync<K8sNode>(url, cancellationToken).ConfigureAwait(false);
+            }
+            catch (UnauthorizedAccessException ex)
+            {
+                // Prefer ignoring the unauthorized access exception
+                if (ignoreForbiddenException)
+                {
+                    _logger.LogDebug(ex.Message);
+                    _logger.LogTrace(ex.ToString());
+                    return Enumerable.Empty<K8sNode>();
+                }
+                // else
+                throw;
+            }
         }
         #endregion
 

src/ApplicationInsights.Kubernetes/TelemetryInitializers/KubernetesTelemetryInitializer.cs

@@ -159,8 +159,8 @@ private void SetCustomDimensions(ISupportProperties telemetry)
             SetCustomDimension(telemetry, Deployment.Name, this._k8sEnvironment.DeploymentName, isValueOptional: true);
 
             // Node
-            SetCustomDimension(telemetry, Node.ID, this._k8sEnvironment.NodeUid);
-            SetCustomDimension(telemetry, Node.Name, this._k8sEnvironment.NodeName);
+            SetCustomDimension(telemetry, Node.ID, this._k8sEnvironment.NodeUid, isValueOptional: true);
+            SetCustomDimension(telemetry, Node.Name, this._k8sEnvironment.NodeName, isValueOptional: true);
         }
 
         private void SetCustomDimension(ISupportProperties telemetry, string key, string value, bool isValueOptional = false)

tests/UnitTests/K8sQueryClientTests.cs

@@ -215,7 +215,7 @@ public async Task GetNodesAsyncShouldHitsTheUri()
             httpClientMock.Setup(httpClient => httpClient.SendAsync(It.IsAny<HttpRequestMessage>(), It.IsAny<CancellationToken>())).Returns(Task.FromResult(response));
             using (K8sQueryClient target = new K8sQueryClient(httpClientMock.Object))
             {
-                await target.GetNodesAsync(cancellationToken: default);
+                await target.GetNodesAsync(ignoreForbiddenException: true, cancellationToken: default);
             }
 
             httpClientMock.Verify(mock => mock.SendAsync(It.Is<HttpRequestMessage>(m => m.RequestUri.AbsoluteUri.Equals("https://baseaddress/api/v1/nodes")), It.IsAny<CancellationToken>()), Times.Once);
@@ -245,7 +245,7 @@ public async Task GetNodesAsyncShouldReturnsMultipleNodes()
 
             using (K8sQueryClient target = new K8sQueryClient(httpClientMock.Object))
             {
-                IEnumerable<K8sNode> result = await target.GetNodesAsync(cancellationToken: default);
+                IEnumerable<K8sNode> result = await target.GetNodesAsync(ignoreForbiddenException: true, cancellationToken: default);
 
                 Assert.NotNull(result);
                 Assert.Equal(2, result.Count());

tests/UnitTests/KubernetesTelemetryInitializerTests.cs

@@ -144,17 +144,18 @@ public void InitializeWithEmptyForRequiredPropertyDoesLogError()
             var envMock = new Mock<IK8sEnvironment>();
             envMock.Setup(env => env.ContainerName).Returns("Hello RoleName");
 
+            // These 2 properties are required.
+            envMock.Setup(env => env.PodID).Returns<string>(null);
+            envMock.Setup(env => env.PodName).Returns<string>(null);
+            
             envMock.Setup(env => env.ContainerID).Returns("Cid");
             envMock.Setup(env => env.ContainerName).Returns("CName");
-            envMock.Setup(env => env.PodID).Returns("Pid");
-            envMock.Setup(env => env.PodName).Returns("PName");
             envMock.Setup(env => env.PodLabels).Returns("PLabels");
             envMock.Setup(env => env.ReplicaSetUid).Returns<string>(null);
             envMock.Setup(env => env.ReplicaSetName).Returns<string>(null);
             envMock.Setup(env => env.DeploymentUid).Returns<string>(null);
             envMock.Setup(env => env.DeploymentName).Returns<string>(null);
             envMock.Setup(env => env.PodNamespace).Returns<string>(null);
-            // These 2 properties are required.
             envMock.Setup(env => env.NodeUid).Returns<string>(null);
             envMock.Setup(env => env.NodeName).Returns<string>(null);