fix: default artifact streaming to disabled for all image families by comtalyst · Pull Request #1611 · Azure/karpenter-provider-azure

comtalyst · 2026-04-02T01:54:34Z

Description

Aligns NAP/Karpenter artifact streaming default with the AKS AgentPool API: disabled unless the user explicitly opts in via artifactStreaming.enabled: true in the AKSNodeClass spec.

Architecture	`artifactStreaming.enabled`	Result
ARM64	Any	Disabled (unsupported)
AMD64	`true`	Enabled
AMD64	`false`	Disabled
AMD64	Not set (nil)	Disabled (changed from enabled for Ubuntu)

How was this change tested?

(TODO) NAP E2E

Does this change impact docs?

Yes, PR includes docs updates
Yes, issue opened: #
No

Release Note

Artifact streaming now defaults to disabled for all image families, matching AKS AgentPool API behavior. Users must explicitly set artifactStreaming.enabled: true in AKSNodeClass to enable it.

Align NAP/Karpenter artifact streaming default with AKS AgentPool API behavior: disabled unless the user explicitly sets `artifactStreaming.enabled: true` in the AKSNodeClass spec. Previously, artifact streaming defaulted to enabled for Ubuntu image families (AMD64 only), which diverged from the AgentPool API where artifact streaming is always opt-in. This created an inconsistency where NAP-provisioned nodes had artifact streaming enabled by default while traditional agent pool nodes did not. The `IsEnabled` method no longer requires an `imageFamily` parameter since the default is now uniformly `false` regardless of image family. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Address review comments: - Remove IsEnabled() method from ArtifactStreaming struct — callers now check the Enabled field directly with nil-safety - Remove IsArtifactStreamingExplicitlyEnabled() — with default=false, it's identical to IsArtifactStreamingEnabled() - Inline artifact streaming logic in bootstrap (ARM64 check + nil-safe field dereference), matching GetEncryptionAtHost() pattern - Simplify bootstrap tests: remove redundant per-OSSKU default cases since default is now uniform false. Keep: AMD64/ARM64 defaults, explicit enable/disable, ARM64+explicit-enable override, error case Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The refactored isInstanceTypeSupportedByArtifactStreaming used IsArtifactStreamingEnabled(arch) which includes the ARM64 guard — always returning false for ARM64, defeating the filter. The filter needs an arch-independent check: "did the user enable artifact streaming?" Use direct field access instead. Also fix cache key to use the same arch-independent check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Document why the instance type filter exists as a workaround: there's no CEL or webhook validation for artifactStreaming.enabled=true on ARM64 because AKSNodeClass doesn't know the architecture at admission time (it comes from NodePool requirements, a separate resource). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Bring back the nil-safe accessor as a no-arg method. Callers that need the ARM64 guard apply it at their call site. This gives clean reuse across labels, bootstrap, and instance type filter without embedding arch concerns in the struct method. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Put the ARM64 guard back into IsEnabled(arch) since it's a universal constraint of the feature, not a caller-specific concern. The instance type filter inlines the field check directly since it needs an arch-independent "is artifact streaming requested?" check that would conflict with the ARM64 guard. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…itlyEnabled, move ARM64 validation gap comment, remove redundant comments - Remove godoc on IsEnabled (self-descriptive) - Remove bootstrap comments (knowledge shouldn't be duplicated) - Restore IsArtifactStreamingExplicitlyEnabled for arch-independent checks in instance type filter and cache key - Move ARM64 validation gap comment to IsArtifactStreamingEnabled in aksnodeclass.go (fact-checked: CEL cannot cross-reference NodePool since it only has access to self) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Restore godoc on IsEnabled (keep first two lines, remove only the third line about nil-safety which was self-descriptive) - Move ARM64 validation gap NOTE to IsEnabled on ArtifactStreaming struct (where the ARM64 guard lives), not IsArtifactStreamingEnabled - Restore original comments on isInstanceTypeSupportedByArtifactStreaming Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

After PR #1611 changed the default from true to false, the nil case now means disabled. Update the Machine API test to expect nil ArtifactStreamingProfile when not specified. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>