Sitelet https://github.com/kubernetes/kops/pull/18600
Skip to content

nodeup: don't mutate the cluster spec when clearing authenticator config - #18600

Merged
kubernetes-prow[bot] merged 1 commit into
kubernetes:masterfrom
hakman:fix-aws-iam-authenticator-addon
Jul 16, 2026
Merged

kubernetes-prow[bot] merged 1 commit into
kubernetes:masterfrom
hakman:fix-aws-iam-authenticator-addon

Conversation

@hakman

@hakman hakman commented Jul 16, 2026

Copy link
Copy Markdown
Member

BuildNodeUpConfig assigned the shared Authentication pointer into the nodeup config and then replaced its AWS field with an empty struct, wiping spec.authentication.aws on the live cluster object. Since #18215 moved addon rendering to task run time, the aws-iam-authenticator manifest rendered after this mutation, losing backendMode, clusterID and identityMappings.

Copy the struct before clearing the field, and extend the complex integration test to cover backendMode: CRD with identity mappings.

@kubernetes-prow
kubernetes-prow Bot requested a review from olemarkus July 16, 2026 10:47
@kubernetes-prow
kubernetes-prow Bot requested a review from zetaab July 16, 2026 10:47
@kubernetes-prow kubernetes-prow Bot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 16, 2026
@hakman

hakman commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

/cc @rifelpet @ameukam

@kubernetes-prow
kubernetes-prow Bot requested review from ameukam and rifelpet July 16, 2026 10:47
@hakman

hakman commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

/retest

1 similar comment
@hakman

hakman commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

/retest

@rifelpet

Copy link
Copy Markdown
Member

Can we check for other api fields that could be in this same situation?

@hakman

hakman commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

Can we check for other api fields that could be in this same situation?

I did a quick check, nothing popped up...

@hakman
hakman force-pushed the fix-aws-iam-authenticator-addon branch from 0155a74 to 01bd103 Compare July 16, 2026 11:55
BuildNodeUpConfig assigned the shared Authentication pointer into the
nodeup config and then replaced its AWS field with an empty struct,
wiping spec.authentication.aws on the live cluster object. Since kubernetes#18215
moved addon rendering to task run time, the aws-iam-authenticator
manifest rendered after this mutation, losing backendMode, clusterID
and identityMappings.

Copy the struct before clearing the field, and extend the complex
integration test to cover backendMode: CRD with identity mappings.
@hakman
hakman force-pushed the fix-aws-iam-authenticator-addon branch from 01bd103 to 26dcc14 Compare July 16, 2026 11:56
@kubernetes-prow kubernetes-prow Bot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 16, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rifelpet

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow kubernetes-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 16, 2026
@kubernetes-prow
kubernetes-prow Bot merged commit 2057287 into kubernetes:master Jul 16, 2026
27 checks passed
kubernetes-prow Bot added a commit that referenced this pull request Jul 16, 2026
…00-origin-release-1.36

Automated cherry pick of #18600: nodeup: don't mutate the cluster spec when clearing authenticator config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/api cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants