feat(deployment): enable AutomountServiceAccountToken for the log-collector image #319
Conversation
WalkthroughSet PodSpec.AutomountServiceAccountToken based on container image name by adding image-name parsing helpers and using their result in the Deployment builder's Create and Update flows; add unit tests and SDL testdata for the log-collector image. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test
participant Builder as DeploymentBuilder
participant Helper as ImageHelpers
participant K8s as Kubernetes API
Test->>Builder: Create()/Update(ctx)
Builder->>Builder: build container spec (once)
Builder->>Helper: extractImageName(image)
Helper-->>Builder: baseImageName
Builder->>Helper: determineAutomountServiceAccountToken(baseImageName)
Helper-->>Builder: true/false
Builder->>Builder: set PodSpec.Containers and PodSpec.AutomountServiceAccountToken
Builder->>K8s: submit Deployment
K8s-->>Test: Deployment with AutomountServiceAccountToken set
rect rgba(200,230,255,0.15)
note over Helper,Builder: New helpers parse image names and decide automount flag
end
alt image matches log-collector
note right of Builder: Automount = true
else other images
note right of Builder: Automount = false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.2.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/product/migration-guide for migration instructions 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
cluster/kube/builder/deployment.go (1)
106-120: Small maintainability tweak: normalize once and avoid per-iteration case-fold.Lowercase the extracted name once and compare against a lowercase allowlist. Optional, but clearer and slightly cheaper.
-func (b *deployment) determineAutomountServiceAccountToken(image string) bool { - automountImages := []string{ - "ghcr.io/akash-network/log-collector", - } - - imageName := extractImageName(image) - - for _, automountImage := range automountImages { - if strings.EqualFold(imageName, automountImage) { - return true - } - } - return false -} +func (b *deployment) determineAutomountServiceAccountToken(image string) bool { + allow := map[string]struct{}{ + "ghcr.io/akash-network/log-collector": {}, + } + imageName := strings.ToLower(extractImageName(image)) + _, ok := allow[imageName] + return ok +}cluster/kube/builder/deployment_test.go (3)
133-141: Avoid concrete type assertion; use the Deployment interface.Keeps the test decoupled from internal struct types.
- deploymentBuilder := NewDeployment(workload) - - require.NotNil(t, deploymentBuilder) - - deploymentInstance := deploymentBuilder.(*deployment) - - deployment, err := deploymentInstance.Create() + deploymentBuilder := NewDeployment(workload) + require.NotNil(t, deploymentBuilder) + deployment, err := deploymentBuilder.Create()
143-146: Allow nil when automount is not enabled (if preserving default is desired).If we don’t want to force false, assert nil for the “regular image” case; keep true and non-nil for log-collector.
- automountValue := deployment.Spec.Template.Spec.AutomountServiceAccountToken - require.NotNil(t, automountValue) - require.Equal(t, testCase.expectedResult, *automountValue) + automountValue := deployment.Spec.Template.Spec.AutomountServiceAccountToken + if testCase.expectedResult { + require.NotNil(t, automountValue) + require.True(t, *automountValue) + } else { + require.Nil(t, automountValue) + }
108-147: Optional: also cover Update() path.Create once, then call Update and re-assert the field to prevent regressions.
deployment, err := deploymentBuilder.Create() require.NoError(t, err) require.NotNil(t, deployment) + updated, err := deploymentBuilder.Update(deployment) + require.NoError(t, err) + require.NotNil(t, updated) + require.Equal(t, deployment.Spec.Template.Spec.AutomountServiceAccountToken, updated.Spec.Template.Spec.AutomountServiceAccountToken)testdata/deployment/deployment-log-collector.yaml (1)
15-17: Use clearly dummy values and avoid embedded quotes in env examples.To avoid accidental leakage and quote-propagation, prefer unquoted selector and an obviously dummy secret.
- - POD_LABEL_SELECTOR="akash.network/manifest-service=target" - - DD_API_KEY=some-secret-value + - POD_LABEL_SELECTOR=akash.network/manifest-service=target + - DD_API_KEY=dummy-key-for-tests
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
cluster/kube/builder/deployment.go(4 hunks)cluster/kube/builder/deployment_test.go(1 hunks)testdata/deployment/deployment-log-collector.yaml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cluster/kube/builder/deployment_test.go (7)
cluster/kube/builder/settings.go (1)
Settings(15-46)pkg/apis/akash.network/v2beta2/types.go (1)
LeaseID(49-55)pkg/apis/akash.network/v2beta2/manifest.go (4)
Manifest(19-24)SchedulerParams(103-106)NewManifest(137-165)ClusterSettings(108-110)cluster/types/v1beta2/types.go (1)
Group(65-68)cluster/kube/builder/builder.go (1)
ClusterDeployment(83-89)cluster/kube/builder/workload.go (1)
NewWorkloadBuilder(42-71)cluster/kube/builder/deployment.go (1)
NewDeployment(23-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: integration-tests / crd-e2e
- GitHub Check: lint
- GitHub Check: tests
- GitHub Check: release-dry-run
- GitHub Check: build-bins
| container := b.container() | ||
| image := container.Image | ||
| automountServiceAccountToken := b.determineAutomountServiceAccountToken(image) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Don’t force AutomountServiceAccountToken=false for non-allowlisted images — preserve k8s default.
Setting this field to false flips Kubernetes’ default (nil → SA/pod default, commonly true) and may break workloads that expect a token. Recommend setting it only when you need to enable it (log-collector), and leaving it nil otherwise.
Apply:
@@
- container := b.container()
- image := container.Image
- automountServiceAccountToken := b.determineAutomountServiceAccountToken(image)
+ container := b.container()
+ image := container.Image
+ automount := b.determineAutomountServiceAccountToken(image)
+ var automountPtr *bool
+ if automount {
+ v := true
+ automountPtr = &v
+ }
@@
- AutomountServiceAccountToken: &automountServiceAccountToken,
+ AutomountServiceAccountToken: automountPtr,
Containers: []corev1.Container{container},
@@
- image := container.Image
- automountServiceAccountToken := b.determineAutomountServiceAccountToken(image)
- uobj.Spec.Template.Spec.AutomountServiceAccountToken = &automountServiceAccountToken
+ image := container.Image
+ automount := b.determineAutomountServiceAccountToken(image)
+ var automountPtr *bool
+ if automount {
+ v := true
+ automountPtr = &v
+ }
+ uobj.Spec.Template.Spec.AutomountServiceAccountToken = automountPtrAlso applies to: 73-75, 99-102
🏁 Script executed:
#!/bin/bash
set -e
# Locate the determineAutomountServiceAccountToken definition
rg -nP --type=go '\bfunc\s+\(b \*Builder\)\s+determineAutomountServiceAccountToken' -C3 .
# Confirm its return type and usage sites in deployment.go
rg -nP --type=go 'determineAutomountServiceAccountToken' -C3 cluster/kube/builder/deployment.goLength of output: 100
🏁 Script executed:
#!/bin/bash
set -e
# Search for the method across the entire repository without type filters
rg -n 'determineAutomountServiceAccountToken' -C3 .
# Search for any references to AutomountServiceAccountToken to find context
rg -n 'AutomountServiceAccountToken' -C3 .
# List all Go files in cluster/kube/builder to locate definitions
find cluster/kube/builder -maxdepth 1 -type f -name '*.go'Length of output: 5822
Preserve k8s default AutomountServiceAccountToken
- In cluster/kube/builder/deployment.go (lines 41–44, 73–75 & 99–102), only set
.AutomountServiceAccountTokenwhendetermineAutomountServiceAccountToken(image)returns true (i.e. point totrue); leave itnilotherwise so Kubernetes uses its default. - Update cluster/kube/builder/deployment_test.go (lines 143–145) to expect a
nilpointer when automount should be disabled instead of always requiring a non-nilpointer and boolean comparison. - No change needed in statefulset.go – it intentionally disables automount for all StatefulSets.
🤖 Prompt for AI Agents
In cluster/kube/builder/deployment.go around lines 41–44, 73–75 and 99–102, the
code unconditionally sets PodSpec.AutomountServiceAccountToken to a pointer
value derived from determineAutomountServiceAccountToken(image); change this so
you only set the AutomountServiceAccountToken field when
determineAutomountServiceAccountToken(image) returns true (i.e., set to pointer
to true), and leave the field nil when it would be false so Kubernetes uses its
default. Also update cluster/kube/builder/deployment_test.go around lines
143–145 to expect a nil pointer when automount should be disabled instead of
asserting a non-nil boolean value.
ygrishajev
force-pushed
the
feature/log-collector
branch
from
57c4f44 to
7bb5715
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
cluster/kube/builder/deployment.go (1)
41-44: Only set AutomountServiceAccountToken when true; keep nil otherwise to preserve k8s default.Currently you always set a non-nil pointer (false flips default and may break workloads that expect tokens). Set pointer only for the log-collector; leave nil otherwise.
Apply:
@@ - container := b.container() - image := container.Image - automountServiceAccountToken := b.determineAutomountServiceAccountToken(image) + container := b.container() + image := container.Image + automount := b.determineAutomountServiceAccountToken(image) + var automountPtr *bool + if automount { + v := true + automountPtr = &v + } @@ - AutomountServiceAccountToken: &automountServiceAccountToken, - Containers: []corev1.Container{container}, + AutomountServiceAccountToken: automountPtr, + Containers: []corev1.Container{container}, @@ - image := container.Image - automountServiceAccountToken := b.determineAutomountServiceAccountToken(image) - uobj.Spec.Template.Spec.AutomountServiceAccountToken = &automountServiceAccountToken + image := container.Image + automount := b.determineAutomountServiceAccountToken(image) + var automountPtr *bool + if automount { + v := true + automountPtr = &v + } + uobj.Spec.Template.Spec.AutomountServiceAccountToken = automountPtrRun to spot any other places forcing false:
#!/bin/bash rg -nP --type=go 'AutomountServiceAccountToken\s*:\s*&?\s*false' -C2 rg -nP --type=go '\.\s*AutomountServiceAccountToken\s*=' -C2Also applies to: 73-75, 99-102
🧹 Nitpick comments (2)
cluster/kube/builder/deployment.go (2)
106-120: Minor: centralize allowlist for maintainability.Consider promoting the allowlist to a package-level var/const (or map for O(1) lookups) to simplify future additions and reuse.
Example:
+var automountImages = map[string]struct{}{ + "ghcr.io/akash-network/log-collector": {}, +} @@ - for _, automountImage := range automountImages { - if strings.EqualFold(imageName, automountImage) { - return true - } - } + _, ok := automountImages[strings.ToLower(imageName)] + return ok - return false
122-134: Trim whitespace in extractImageName.Adds resilience for inputs with accidental spaces.
func extractImageName(image string) string { - if idxAt := strings.LastIndex(image, "@"); idxAt != -1 { + image = strings.TrimSpace(image) + if idxAt := strings.LastIndex(image, "@"); idxAt != -1 { return image[:idxAt] }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
cluster/kube/builder/deployment.go(4 hunks)cluster/kube/builder/deployment_test.go(1 hunks)testdata/deployment/deployment-log-collector.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cluster/kube/builder/deployment_test.go
- testdata/deployment/deployment-log-collector.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: integration-tests / crd-e2e
- GitHub Check: build-bins
- GitHub Check: release-dry-run
- GitHub Check: tests
- GitHub Check: lint
🔇 Additional comments (1)
cluster/kube/builder/deployment.go (1)
87-87: Good reuse of container construction.Pulling
container := b.container()once and reusing improves clarity and avoids duplication.Also applies to: 95-95
|
|
||
| func (b *deployment) determineAutomountServiceAccountToken(image string) bool { | ||
| automountImages := []string{ | ||
| "ghcr.io/akash-network/log-collector", |
There was a problem hiding this comment.
This should be configurable as any change to the image location would require recompilation.
No description provided.