chore: add validation for TransparentHugePage config by mxj220 · Pull Request #8015 · Azure/AgentBaker

mxj220 · 2026-03-04T21:57:54Z

What this PR does / why we need it:
Added validation for TransparentHugePage config to ensure that legal values are used.
Which issue(s) this PR fixes:

Fixes #

Copilot

Pull request overview

Adds validation for CustomLinuxOSConfig Transparent Huge Pages (THP) settings to ensure only kernel-supported values are accepted before generating Linux node bootstrapping artifacts.

Changes:

Introduces constants for valid THP enabled and defrag values and adds (*CustomLinuxOSConfig).ValidateTHPConfig() validation.
Wires THP validation into the Linux path of GetNodeBootstrapping.
Adds unit tests for ValidateTHPConfig().

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
pkg/agent/datamodel/types.go	Adds THP allowed-value constants and `ValidateTHPConfig()` to validate `transparentHugePageEnabled/Defrag`.
pkg/agent/datamodel/types_test.go	Adds table-driven tests covering valid/invalid THP configurations.
pkg/agent/bakerapi.go	Calls THP validation before Linux bootstrapping generation.

pkg/agent/bakerapi.go

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

pkg/agent/datamodel/types.go

pkg/agent/bakerapi.go

github-actions · 2026-03-04T23:07:45Z

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
`✅ passed`	`✅ passed`	`✅ passed`	`✅ passed`	Mar 6, 2026, 11:05 PM

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

cameronmeissner · 2026-03-06T22:04:20Z

aks-node-controller/pkg/nodeconfigutils/utils.go

+		"never":         true,
+	}
+
+	if v := osConfig.GetTransparentHugepageSupport(); v != "" && !validEnabled[strings.ToLower(v)] {


empty string check is redundant since validEnabled[strings.ToLower(v)] would resolve to false regardless if v is empty

technically, if I didn't skip on empty string, then an empty string would throw the warning instead of proceeding silently. May be unnecessarily concerning to a user who intentionally left the config blank

Devinwong

lgtm

cameronmeissner · 2026-03-06T22:07:19Z

pkg/agent/bakerapi.go

 	if config.AgentPoolProfile.IsWindows() {
 		validateAndSetWindowsNodeBootstrappingConfiguration(config)
 	} else {
+		config.AgentPoolProfile.CustomLinuxOSConfig.ValidateTHPConfig()


I might rename this to ValidateAndDefaultTHPConfig(), since this defaults the respective settings to empty strings if the input is invalid

(this applies here and in aks-node-controller)

I'm open to this rename, but the empty strings are also just for best practices. The original code leaves the invalid string in the config but it doesn't do anything, which causes the default to happen anyway.

aks-node-controller/pkg/nodeconfigutils/utils.go

cameronmeissner · 2026-03-06T22:09:34Z

aks-node-controller/pkg/nodeconfigutils/utils.go

+		osConfig.TransparentHugepageSupport = ""
+	}
+	if v := osConfig.GetTransparentDefrag(); v != "" && !validDefrag[strings.ToLower(v)] {
+		log.Printf("WARNING: invalid transparent_defrag value %q, must be one of: always, defer, defer+madvise, madvise, never; resetting to default", v)


I'm not sure log is the correct mechanism by which we should be logging these messages out - @r2k1 @Devinwong can clarify maybe

cameronmeissner · 2026-03-06T22:12:31Z

aks-node-controller/pkg/nodeconfigutils/utils.go

+// ValidateTHPConfig checks that TransparentHugepageSupport and TransparentDefrag
+// contain valid kernel values when set. Invalid values are logged as warnings
+// and reset to empty (kernel default).
+func ValidateTHPConfig(cfg *aksnodeconfigv1.Configuration) {


what's the actual intended behavior? if this is a customer-exposed setting, wouldn't we want RP to return a 4xx if they specify an invalid value, rather than falling back to empty for TransparentHugepageSupport and GetTransparentDefrag? for that to happen, this validation would need to happen within the RP rather than here

From Tim:

"I'm in two minds about this. If we abort with an error then any user doing something strange in prod right now will suddenly have failing nodepool scale operations. Which could cause a sev2 or more depending on the Cx. But if we silently ignore the value, then customer's clusters might regress."

In its current state, our code writes the invalid user string to the file, i.e. "false." The user intended "false" to disable THP, but since it's the wrong key word, it does nothing, which causes the default behavior ("always") to occur.

By clearing the user string, I'm not changing the code's existing behavior, I'm just cleaning it up a bit.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

pkg/agent/datamodel/types.go

pkg/agent/datamodel/types_test.go

aks-node-controller/pkg/nodeconfigutils/utils.go

aks-node-controller/pkg/nodeconfigutils/utils_test.go

add validation for TransparentHugePage config

578a37e

Copilot AI review requested due to automatic review settings March 4, 2026 21:57

mxj220 requested review from AbelHu, Devinwong, awesomenix, calvin197, cameronmeissner, djsly, ganeshkumarashok, juan-lee, junjiezhang1997, lilypan26, pdamianov-dev, phealy, r2k1, sulixu, surajssd, timmy-wright and zachary-bailey as code owners March 4, 2026 21:57

mxj220 temporarily deployed to test March 4, 2026 21:58 — with GitHub Actions Inactive

Copilot started reviewing on behalf of mxj220 March 4, 2026 21:58 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

pkg/agent/bakerapi.go Outdated Show resolved Hide resolved

remove globals

bea4deb

mxj220 temporarily deployed to test March 4, 2026 22:02 — with GitHub Actions Inactive

ginkgo tests

b8e8b98

Copilot AI review requested due to automatic review settings March 4, 2026 22:09

mxj220 temporarily deployed to test March 4, 2026 22:09 — with GitHub Actions Inactive

Copilot started reviewing on behalf of mxj220 March 4, 2026 22:10 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

timmy-wright reviewed Mar 4, 2026

View reviewed changes

pkg/agent/datamodel/types.go Outdated Show resolved Hide resolved

timmy-wright reviewed Mar 4, 2026

View reviewed changes

pkg/agent/bakerapi.go Outdated Show resolved Hide resolved

add scriptless

b09ec9a

mxj220 temporarily deployed to test March 4, 2026 23:07 — with GitHub Actions Inactive

case insensitive, log warning instead of error

0142718

Copilot AI review requested due to automatic review settings March 5, 2026 17:18

mxj220 temporarily deployed to test March 5, 2026 17:33 — with GitHub Actions Inactive

Copilot AI reviewed Mar 5, 2026

View reviewed changes

fix tests

40dc468

mxj220 temporarily deployed to test March 5, 2026 18:57 — with GitHub Actions Inactive

awesomenix approved these changes Mar 6, 2026

View reviewed changes

cameronmeissner reviewed Mar 6, 2026

View reviewed changes

Devinwong approved these changes Mar 6, 2026

View reviewed changes

cameronmeissner reviewed Mar 6, 2026

View reviewed changes

aks-node-controller/pkg/nodeconfigutils/utils.go Show resolved Hide resolved

cameronmeissner reviewed Mar 6, 2026

View reviewed changes

connect in scriptless

12595a5

Copilot AI review requested due to automatic review settings March 6, 2026 22:50

mxj220 temporarily deployed to test March 6, 2026 22:50 — with GitHub Actions Inactive

Copilot started reviewing on behalf of mxj220 March 6, 2026 22:51 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

force lowercase for valid strings

dd2ff72

mxj220 temporarily deployed to test March 6, 2026 23:04 — with GitHub Actions Inactive

Conversation

mxj220 commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

cameronmeissner Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Devinwong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

github-actions bot commented Mar 4, 2026 •

edited

Loading

cameronmeissner Mar 6, 2026 •

edited

Loading