Skip to content

feat: add ServiceAccountImagePullProfile for identity-binding-based image pull #7596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
qweeah wants to merge 2 commits into
base: main
Choose a base branch
from
+1,576 −403
Open

feat: add ServiceAccountImagePullProfile for identity-binding-based image pull #7596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb23b9c
feat: setup kubelet config to support ib-based image pull
qweeah Jan 15, 2026
e5b74fb
test: data
qweeah Jan 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions aks-node-controller/parser/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,3 +810,26 @@ func getLocalDnsMemoryLimitInMb(aksnodeconfig *aksnodeconfigv1.Configuration) st
}

// ---------------------- End of localdns related helper code ----------------------//
// ---------------------- Service Account Image Pull helper functions ----------------------//

// getServiceAccountImagePullEnabled returns whether service account based image pull is enabled.
func getServiceAccountImagePullEnabled(config *aksnodeconfigv1.Configuration) bool {
return config.GetServiceAccountImagePullProfile().GetEnabled()
Copy link
Contributor

@cameronmeissner cameronmeissner Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need the helpers if they're simply dereferencing config without any pre/post-processing

Copy link
Contributor Author

@qweeah qweeah Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ServiceAccountImagePullProfile can be nil. Just to avoid dereferencing nil pointer errors as is mentioned in #7596 (comment)

Copy link
Contributor Author

@qweeah qweeah Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolving this. @cameronmeissner Feel free to reopen if you have any further concerns.

Copy link
Contributor

@cameronmeissner cameronmeissner Jan 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant getServiceAccountImagePullEnabled - that can be replaced simply by config.GetServiceAccountImagePullProfile().GetEnabled() where we actually assign the environment variables

Copy link
Contributor

@cameronmeissner cameronmeissner Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qweeah - also @Devinwong since this is the same feedback you gave me as well on one of my older PRs

Copy link
Collaborator

@Devinwong Devinwong Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cameron is correct. You can use that instead of creating your custom functions (although it's also no harm, just not necessary). Also for the other functions you have below. The default getters are nil safe.

}

// getServiceAccountImagePullDefaultClientID returns the default client ID for service account image pull.
func getServiceAccountImagePullDefaultClientID(config *aksnodeconfigv1.Configuration) string {
return config.GetServiceAccountImagePullProfile().GetDefaultClientId()
}

// getServiceAccountImagePullDefaultTenantID returns the default tenant ID for service account image pull.
func getServiceAccountImagePullDefaultTenantID(config *aksnodeconfigv1.Configuration) string {
return config.GetServiceAccountImagePullProfile().GetDefaultTenantId()
}

// getServiceAccountImagePullLocalAuthoritySNI returns the local authority SNI for service account image pull.
func getServiceAccountImagePullLocalAuthoritySNI(config *aksnodeconfigv1.Configuration) string {
return config.GetServiceAccountImagePullProfile().GetLocalAuthoritySni()
}

// ---------------------- End of Service Account Image Pull helper functions ----------------------//
184 changes: 184 additions & 0 deletions aks-node-controller/parser/helper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1811,3 +1811,187 @@ func Test_getLocalDnsMemoryLimitInMb(t *testing.T) {
})
}
}

func Test_getServiceAccountImagePullEnabled(t *testing.T) {
tests := []struct {
name string
config *aksnodeconfigv1.Configuration
want bool
}{
{
name: "nil Configuration",
config: nil,
want: false,
},
{
name: "nil ServiceAccountImagePullProfile",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: nil,
},
want: false,
},
{
name: "enabled true",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: &aksnodeconfigv1.ServiceAccountImagePullProfile{
Enabled: true,
},
},
want: true,
},
{
name: "enabled false",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: &aksnodeconfigv1.ServiceAccountImagePullProfile{
Enabled: false,
},
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := getServiceAccountImagePullEnabled(tt.config); got != tt.want {
t.Errorf("getServiceAccountImagePullEnabled() = %v, want %v", got, tt.want)
}
})
}
}

func Test_getServiceAccountImagePullDefaultClientID(t *testing.T) {
tests := []struct {
name string
config *aksnodeconfigv1.Configuration
want string
}{
{
name: "nil Configuration",
config: nil,
want: "",
},
{
name: "nil ServiceAccountImagePullProfile",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: nil,
},
want: "",
},
{
name: "with client ID",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: &aksnodeconfigv1.ServiceAccountImagePullProfile{
DefaultClientId: "test-client-id",
},
},
want: "test-client-id",
},
{
name: "empty client ID",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: &aksnodeconfigv1.ServiceAccountImagePullProfile{
DefaultClientId: "",
},
},
want: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := getServiceAccountImagePullDefaultClientID(tt.config); got != tt.want {
t.Errorf("getServiceAccountImagePullDefaultClientID() = %v, want %v", got, tt.want)
}
})
}
}

func Test_getServiceAccountImagePullDefaultTenantID(t *testing.T) {
tests := []struct {
name string
config *aksnodeconfigv1.Configuration
want string
}{
{
name: "nil Configuration",
config: nil,
want: "",
},
{
name: "nil ServiceAccountImagePullProfile",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: nil,
},
want: "",
},
{
name: "with tenant ID",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: &aksnodeconfigv1.ServiceAccountImagePullProfile{
DefaultTenantId: "test-tenant-id",
},
},
want: "test-tenant-id",
},
{
name: "empty tenant ID",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: &aksnodeconfigv1.ServiceAccountImagePullProfile{
DefaultTenantId: "",
},
},
want: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := getServiceAccountImagePullDefaultTenantID(tt.config); got != tt.want {
t.Errorf("getServiceAccountImagePullDefaultTenantID() = %v, want %v", got, tt.want)
}
})
}
}

func Test_getServiceAccountImagePullLocalAuthoritySNI(t *testing.T) {
tests := []struct {
name string
config *aksnodeconfigv1.Configuration
want string
}{
{
name: "nil Configuration",
config: nil,
want: "",
},
{
name: "nil ServiceAccountImagePullProfile",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: nil,
},
want: "",
},
{
name: "with local authority SNI",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: &aksnodeconfigv1.ServiceAccountImagePullProfile{
LocalAuthoritySni: "test-sni.local",
},
},
want: "test-sni.local",
},
{
name: "empty local authority SNI",
config: &aksnodeconfigv1.Configuration{
ServiceAccountImagePullProfile: &aksnodeconfigv1.ServiceAccountImagePullProfile{
LocalAuthoritySni: "",
},
},
want: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := getServiceAccountImagePullLocalAuthoritySNI(tt.config); got != tt.want {
t.Errorf("getServiceAccountImagePullLocalAuthoritySNI() = %v, want %v", got, tt.want)
}
})
}
}
4 changes: 4 additions & 0 deletions aks-node-controller/parser/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ func getCSEEnv(config *aksnodeconfigv1.Configuration) map[string]string {
"LOCALDNS_MEMORY_LIMIT": getLocalDnsMemoryLimitInMb(config),
"LOCALDNS_GENERATED_COREFILE": getLocalDnsCorefileBase64(config),
"DISABLE_PUBKEY_AUTH": fmt.Sprintf("%v", config.GetDisablePubkeyAuth()),
"SERVICE_ACCOUNT_IMAGE_PULL_ENABLED": fmt.Sprintf("%v", getServiceAccountImagePullEnabled(config)),
"SERVICE_ACCOUNT_IMAGE_PULL_DEFAULT_CLIENT_ID": getServiceAccountImagePullDefaultClientID(config),
"SERVICE_ACCOUNT_IMAGE_PULL_DEFAULT_TENANT_ID": getServiceAccountImagePullDefaultTenantID(config),
"IDENTITY_BINDINGS_LOCAL_AUTHORITY_SNI": getServiceAccountImagePullLocalAuthoritySNI(config),
}

for i, cert := range config.CustomCaCerts {
Expand Down
Loading
Loading