Skip to content
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

Jump to bottom

fix: default overrides not respected for empty fields. Fixes #11130 #11222

Closed
wants to merge 1 commit into from
Closed

fix: default overrides not respected for empty fields. Fixes #11130 #11222

wants to merge 1 commit into from

Conversation

rohankmr414
Copy link
Member

Fixes #11130

Motivation

json.Unmarshal ignores a field if it is set to an empty object and doesn't override in main container settings if set in workflow-controller-configmap

Modifications

Used jsonpatch to create a merge patch for empty field overrides

Verification

Tested locally

@rohankmr414
fix: default overrides not respected for empty fields
6e4617f 
Signed-off-by: Rohan Kumar <rohan@pipekit.io>
@rohankmr414 rohankmr414 requested a review from isubasinghe June 15, 2023 23:42
@rohankmr414 rohankmr414 marked this pull request as draft June 15, 2023 23:47
isubasinghe
isubasinghe approved these changes Jun 16, 2023
View reviewed changes
Copy link
Member

@isubasinghe isubasinghe left a comment

Choose a reason for hiding this comment

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

LGTM.

Your changes look good to me, the names they use originally are quite bad: a, b and c, I would recommend renaming to something a bit more descriptive.

@stale
Copy link

stale bot commented Aug 12, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is a mentoring request, please provide an update here. Thank you for your contributions.

@stale stale bot added the problem/stale This has not had a response in some time label Aug 12, 2023
@stale
Copy link

stale bot commented Sep 17, 2023

This issue has been closed due to inactivity. Feel free to re-open if you still encounter this issue.

@stale stale bot closed this Sep 17, 2023
@agilgur5 agilgur5 added type/security Security related area/spec Changes to the workflow specification. labels Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isubasinghe isubasinghe isubasinghe approved these changes

Assignees
No one assigned
Labels
area/spec Changes to the workflow specification. problem/stale This has not had a response in some time type/security Security related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

securityContext default override not as expected
3 participants
@rohankmr414 @isubasinghe @agilgur5