Skip to content

fix: merge LaunchPlan security context per field on registration#3442

Open
1fanwang wants to merge 1 commit into
flyteorg:masterfrom
1fanwang:fix-lp-security-context-merge
Open

fix: merge LaunchPlan security context per field on registration#3442
1fanwang wants to merge 1 commit into
flyteorg:masterfrom
1fanwang:fix-lp-security-context-merge

Conversation

@1fanwang

@1fanwang 1fanwang commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Why are the changes needed?

Registering a launch plan that declares secrets (or tokens) while passing registration Options that only set a service account silently drops the secrets. get_serializable_launch_plan does options.security_context or entity.security_context — a wholesale replace — so any non-empty options context wipes whatever the launch plan authored.

What changes were proposed in this pull request?

Merge the two security contexts per field instead of replacing wholesale: options win field-by-field (run_as, secrets, tokens), so a service-account-only override stops clobbering authored secrets.

How was this patch tested?

test_get_serializable_launch_plan drives the same path. Also end-to-end against a real flyteadmin (flyte demo): register the secrets launch plan with service-account-only options and read it back — before, secrets returns None (dropped); after, it's preserved alongside the overridden service account.

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Signed-off-by: 1fanwang <1fannnw@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant