review feedback

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>

Author: aramase

keps/sig-auth/4412-projected-service-account-tokens-for-kubelet-image-credential-providers/README.md

@@ -316,6 +316,7 @@ the design is written to meet the following criteria:
 - No changes to any registry
 - Minimal changes to Kubelet
 - Minimal changes to Kubelet credential providers
+- Minimal changes to Kube API Server
 
 ## Design Details
 
@@ -333,9 +334,10 @@ Adding a new field `TokenAttributes` to the `CredentialProvider` struct in the k
 1. `ServiceAccountTokenAudience` is the intended audience for the credentials. If set, the kubelet will generate a service account token for the audience and pass it to the plugin.
    1. Only a single audience can be configured.
    2. The credential provider can be set up multiple times for each audience.
+      1. The name in the credential provider config must match the name of the provider executable as seen by the kubelet. To configure multiple instances of the same provider with different audiences, the name of the provider executable must be different and this can be done by creating a symlink to the same executable with a different name.
    3. By setting this field, the credential provider is opting into using service account tokens for image pull.
 2. `ServiceAccountAnnotationKeys` is the list of annotation keys that the plugin is interested in. The keys defined in this list will be extracted from the corresponding service account
-   and passed to the plugin as part of `CredentialProviderRequest`. Plugins can use to extract additional information required to fetch credentials. The plugin is responsible for validating the existence of annotations and their values.
+   and passed to the plugin as part of `CredentialProviderRequest`. Plugins can use to extract additional information required to fetch credentials. The plugin is responsible for validating the existence of annotations and their values. There are existing integrations like Azure and GCP that use the extra metadata to link the KSA name to a cloud identity, and this field will allow them to continue to do so. None of the existing integrations use this metadata as any form of security decision, it is simply to aid in doing token exchanges with the KSA token.
    1. Service account annotations are significantly more stable (and map naturally to the service account being the partioning dimension).
    2. This field makes the provider opt into getting the annotations it wants, avoids sending arbitrary annotation contents down to the plugin (including stuff like client-side apply annotations) and shrinks the set of annotations that could invalidate the cache.
    3. This field can't be set without setting the `ServiceAccountTokenAudience` field.
@@ -357,7 +359,7 @@ The configuration will not support a plugin to function in 2 modes simultaneousl
 +       //
 +       // The service account metadata and token attributes will be used as a dimension to cache
 +       // the credentials in kubelet. The cache key is generated by combining the service account metadata
-+       // (namespace, name, UID, and annotations map with keys defined in ServiceAccountTokenAttribute.serviceAccountAnnotationKeys).
++       // (namespace, name, UID, and annotations key+value for the keys defined in ServiceAccountTokenAttribute.serviceAccountAnnotationKeys).
 +       TokenAttributes *ServiceAccountTokenAttributes
 +}
 +
@@ -397,6 +399,7 @@ providers:
       serviceAccountAnnotationKeys:
       - domain.io/identity-id
       - domain.io/identity-type
+      - domain.io/annotation-that-does-not-exist
 ```
 
 ##### Kubernetes API Server (KAS) Changes
@@ -406,6 +409,9 @@ Add a new flag `--allowed-kubelet-audiences` to KAS to allow configuring the lis
 The node audience restriction will be enforced by the [NodeRestriction admission controller](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#noderestriction)
 and kubelet will only be allowed to generate a service account token for the audience configured in the credential provider configuration if it is present in the list of audiences configured in KAS.
 
+Today we only verify the SA in KAS and allow the kubelet to generate a token for any audience. We want to start verifying the audience as well, so we need to be explicit here about what audiences are allowed.
+The other sources of audiences for the SA token can be observed via the Kubernetes API but this one can't, so we're adding the flag.
+
 #### Credential Provider Request API
 
 Add `ServiceAccountToken` and `ServiceAccountAnnotations` fields to `CredentialProviderRequest`.
@@ -460,6 +466,10 @@ Example credential provider request:
   "serviceAccountAnnotations": {
     "domain.io/identity-id": "12345",
     "domain.io/identity-type": "user"
+    // In the tokenAttributes.serviceAccountAnnotationKeys field in the credential provider configuration above we have configured
+    // domain.io/annotation-that-does-not-exist which is not present in the service account annotations. Because of this, this annotation
+    // will not be passed to the plugin. The plugin is responsible for validating the existence of annotations and their values that's required
+    // to fetch credentials.
   }
 }
 ```
@@ -955,6 +965,7 @@ Focusing mostly on:
 -->
 
 Yes. Kubelet will request a KSA token from KAS for pod + KSA for every provider (audience) that is configured to use service account tokens for image pull.
+The load is small because the token generation happens only once during pod startup.
 
 ###### Will enabling / using this feature result in introducing new API types?
 
@@ -975,7 +986,10 @@ Describe them, providing:
   - Estimated increase:
 -->
 
-No.
+This depends on the credential provider plugin implementation on how it fetches the credentials using the service account token.
+The plugin will now be called for every unique KSA + (image/registry) combination that is being pulled if not already cached unlike
+today where the cache is based on the image/registry URL. This could result in more calls to the cloud provider from the plugin to
+exchange the KSA token for the credentials.
 
 ###### Will enabling / using this feature result in increasing size or count of the existing API objects?
 
@@ -999,7 +1013,9 @@ Think about adding additional work or introducing new steps in between
 [existing SLIs/SLOs]: https://git.k8s.io/community/sig-scalability/slos/slos.md#kubernetes-slisslos
 -->
 
-No.
+SLIs/SLOs for image pull times might be impacted with the credential plugin using KSA token for fetching credentials. The credentials are
+granular in scope (per service account) like image pull secrets, but the plugin must dynamically fetch the credentials using the KSA token
+and these credentials could have shorter TTLs resulting in more frequent fetching of credentials.
 
 ###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components?
 

keps/sig-auth/4412-projected-service-account-tokens-for-kubelet-image-credential-providers/kep.yaml

@@ -1,5 +1,5 @@
 title: Projected Service Account Tokens for Kubelet Image Credential Providers
-kep-number: "4412"
+kep-number: 4412
 authors:
 - "@aramase"
 - "@mainred"
@@ -12,6 +12,8 @@ reviewers:
 approvers:
 - "@enj"
 - "@liggitt"
+see-also:
+- "/keps/sig-node/2535-ensure-secret-pulled-images"
 creation-date: "2024-09-09"
 status: implementable
 stage: alpha