Identity Plugin - #288
Conversation
|
@atpugtihsrah this is interesting. I did some work in the istio/isovalent ecosystem for SPIRE. Lmk if you need any help |
11f5595 to
ff81182
Compare
0748a95 to
e355b5e
Compare
|
7b49520 to
ec86027
Compare
|
good meeting this morning @atpugtihsrah! Note to other reviewers contributor is going to clean up the vendor issues and boost the plugin getting started guide. |
cadcd8d to
c3a1707
Compare
|
heads up needs rebase :-) we've been busy getting ready for a release |
b46ddf1 to
3358c45
Compare
|
Ran the test instructions to success using kind... We can add this later: |
mikebrow
left a comment
There was a problem hiding this comment.
LGTM very nice...
would like to get this in and iterate
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds a WIP “Identity Plugin” (identity-injector) to fetch X.509 SVIDs via SPIRE’s Delegated Identity API and inject them into workload containers, along with kustomize manifests and a local-cluster setup guide.
Changes:
- Introduces the identity-injector plugin implementation (gRPC streaming watchers, file writes, annotation parsing) and a unit test for annotation parsing.
- Adds kustomize base/overlay resources to deploy the plugin as a DaemonSet.
- Adds a detailed setup guide for SPIRE/SPIFFE + local build/test flow, and wires the plugin tests into the main Makefile.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/identity-injector/identity-injector.go | Implements the identity-injector plugin, certificate/bundle streaming watchers, and annotation parsing. |
| plugins/identity-injector/identity-injector_test.go | Adds unit tests for parsing identity annotations. |
| plugins/identity-injector/go.mod | Defines the plugin module and dependencies (SPIFFE/SPIRE SDK, gRPC, yaml, etc.). |
| plugins/identity-injector/go.sum | Locks dependency checksums for the plugin module. |
| contrib/kustomize/identity-injector/kustomization.yaml | Adds a kustomize entry point for the identity-injector deployment. |
| contrib/kustomize/identity-injector/base/kustomization.yaml | Defines base resources, image override, and common labels for deployment. |
| contrib/kustomize/identity-injector/base/daemonset.yaml | Adds the DaemonSet manifest to run the plugin and mount required host paths/sockets. |
| plugins/identity-injector/setup.md | Documents end-to-end local setup for SPIRE + plugin + test workload. |
| Makefile | Adds a test target for the new identity-injector plugin. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| } | ||
|
|
||
| // TODO create test cases for processDelegatedIdentityUpdate() |
| sudo mkdir /tmp/spire-data | ||
|
|
||
| sudo chmod 777 /tmp/spire-data |
| sudo mkdir /var/run/spiffe/ | ||
| sudo mkdir /var/run/spiffe/secrets/ | ||
| sudo chmod 777 /var/run/spiffe/secrets/ |
ca3b6ac to
189f5a6
Compare
mikebrow
left a comment
There was a problem hiding this comment.
maybe a little more work around the map initialization part to ensure it's never used when not properly initialized
acc660c to
d488f96
Compare
Signed-off-by: Harshit Gupta <hg2t4e@gmail.com>
d488f96 to
7f3b152
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Request-lifetime cancellation, unsafe filename handling, startup races, and missing recovery paths currently prevent reliable and secure identity delivery.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
plugins/identity-injector/identity-injector.go:372
- The bundle stream has the same permanent-failure path: any transient
Recverror returns, cancels the shared context, and stops SVID rotation too. Re-establish this subscription with bounded backoff while the container watcher remains active.
resp, err := stream.Recv()
if err != nil {
log.Errorf("%s: bundle stream error: %v", containerName(pod, ctr), err)
return
- Files reviewed: 8/9 changed files
- Comments generated: 9
- Review effort level: Balanced
| - "--verbose" | ||
| - "true" |
| if err := p.startCertificateWatcher(ctx, pod, container, int32(container.Pid), hostDir, config); err != nil { | ||
| return fmt.Errorf("failed to start certificate watcher: %w", err) | ||
| } | ||
|
|
||
| return nil |
| p.watchersMu.RUnlock() | ||
|
|
||
| // Create cancellable context for this watcher | ||
| watcherCtx, cancel := context.WithCancel(ctx) |
| if config.CertFileName == "" { | ||
| config.CertFileName = defaultCertFileName | ||
| } | ||
| if config.KeyFileName == "" { | ||
| config.KeyFileName = defaultKeyFileName | ||
| } | ||
| if config.BundleFileName == "" { | ||
| config.BundleFileName = defaultBundleFileName | ||
| } | ||
|
|
||
| return &config, nil |
| # | ||
|
|
||
| test-gopkgs: go-generate test-main test-ulimits test-rdt test-hook-injector test-writable-cgroups | ||
| test-gopkgs: go-generate test-main test-ulimits test-rdt test-hook-injector test-writable-cgroups test-identity-injector |
| resp, err := stream.Recv() | ||
| if err != nil { | ||
| log.Errorf("%s: bundle stream error: %v", containerName(pod, ctr), err) | ||
| return |
| // TODO implement using hint to select relevant svid in case response has multiple svids | ||
| // Get the default SVID | ||
| svidWithKey := x509Svids[0] |
| p := &plugin{ | ||
| watchers: make(map[string]*containerWatcher), |
|
|
||
| ## Step 2.2: Deploy the NRI Identity Plugin | ||
|
|
||
| **Why:** The kustomize overlay in `contrib/kustomize/identity-injector/` contains all of the Kubernetes resources needed to run the NRI Identity Plugin as a DaemonSet in the `kube-system` namespace: the DaemonSet definition (which mounts the SPIRE admin socket and the SVID secrets directory), the NRI registration ConfigMap (so that the Containerd NRI runtime knows to invoke the plugin), and the necessary RBAC resources. Applying it as a single `kubectl apply -k` command ensures all resources are created in the correct order and with the correct labels. |
Identity Plugin WIP
RFC PR: #245
Relates to #240
PR Assisted by IBM Bob.