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

kustomize/advanced: kustomize v5 compatibility #6

Merged
merged 2 commits into from
May 8, 2023
Merged

kustomize/advanced: kustomize v5 compatibility #6

merged 2 commits into from
May 8, 2023

Conversation

KellerFuchs
Copy link
Contributor

The bare patches attribute was apparently a legacy thing, which v5 attributed new meaning to; see kubernetes-sigs/kustomize#4911.

patchesStrategicMerge should work on both v4 and v5.

@KellerFuchs
Copy link
Contributor Author

@hashbang/administrators That's blocking for (re)deploying wkd on #!, since the cluster was updated to kustomize v5.

@KellerFuchs
kustomize/advanced: kustomize v5 compatibility
591cb78 
The bare `patches` attribute was apparently a legacy thing,
which v5 attributed new meaning to; see kustomize#4911.

patchesStrategicMerge should work on both v4 and v5.

Closes: drGrove#5
daurnimator
daurnimator reviewed May 4, 2023
View reviewed changes
Copy link
Contributor

@daurnimator daurnimator left a comment

Choose a reason for hiding this comment

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

patchesStrategicMerge is actually the deprecated one.

What's also deprecated is passing strings directly in patches.
Rather it should be a dictionary.

kustomize/advanced/kustomization.yaml Outdated Show resolved Hide resolved
@KellerFuchs
Copy link
Contributor Author

patchesStrategicMerge is actually the deprecated one.

What's also deprecated is passing strings directly in patches. Rather it should be a dictionary.

Yeah, but doing this will break for kustomize v4.
I guess that means you both think it's unimportant so I'll go ahead and apply that ;3

@KellerFuchs @daurnimator
kustomize/advanced: Use kustomize v5's new syntax instead
c47ec97 
By request of daurnimator & dgrove.

Co-authored-by: daurnimator <quae@daurnimator.com>
@KellerFuchs
Copy link
Contributor Author

PS: I forgot to mention but I did test it and kustomize build succeeds.

@daurnimator
Copy link
Contributor

Yeah, but doing this will break for kustomize v4.

Does it? I thought the form I suggested should work back to kustomize v3.

I guess that means you both think it's unimportant so I'll go ahead and apply that ;3

But yeah: the only 2 versions of kustomize to worry about are latest release and whatever is included with latest kubectl.

@KellerFuchs
Copy link
Contributor Author

Yeah, but doing this will break for kustomize v4.

Does it? I thought the form I suggested should work back to kustomize v3.

Sorry, I think I misread the description in the linked kustomize PR at first 😅

I guess that means you both think it's unimportant so I'll go ahead and apply that ;3

But yeah: the only 2 versions of kustomize to worry about are latest release and whatever is included with latest kubectl.

OK, good to know.

@KellerFuchs
Copy link
Contributor Author

@drGrove Ping for merge?

@drGrove drGrove merged commit 66be667 into drGrove:master May 8, 2023
@drGrove drGrove deleted the kellerfuchs/kustomize5 branch May 8, 2023 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daurnimator daurnimator daurnimator left review comments

@drGrove drGrove drGrove approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@KellerFuchs @daurnimator @drGrove