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

Make ordering configurable #4019

Merged
merged 9 commits into from
Dec 2, 2022
Merged

Make ordering configurable #4019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
76ee60d
api: Add new types for customizeable resource ordering
yanniszark Oct 2, 2021
1b2fd3c
plugins: Implement SortOrderTransformer plugin
yanniszark Oct 2, 2021
8e0fc12
plugins: Add boilerplate and generate code for new SortOrderTransformer
yanniszark Oct 2, 2021
88f80cd
build: Add option to denote if the reorder flag was set by the user
yanniszark Dec 31, 2021
6143039
api/krusty: Ensure sort ordering works with CLI flag and kustomization
yanniszark Oct 2, 2021
202768f
krusty: Add e2e test for SortOrderTransformer
yanniszark Oct 2, 2021
a05b8fb
plugins: Purge LegacyOrderTransformer
yanniszark Oct 17, 2022
4e0cb79
Update go.work.sum
yanniszark Nov 21, 2022
e581778
review: Make review changes
yanniszark Nov 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile-plugins.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ _builtinplugins = \
HashTransformer.go \
ImageTagTransformer.go \
LabelTransformer.go \
LegacyOrderTransformer.go \
SortOrderTransformer.go \
NamespaceTransformer.go \
PatchJson6902Transformer.go \
PatchStrategicMergeTransformer.go \
Expand Down Expand Up @@ -63,7 +63,7 @@ $(pGen)/GkeSaGenerator.go: $(pSrc)/gkesagenerator/GkeSaGenerator.go
$(pGen)/HashTransformer.go: $(pSrc)/hashtransformer/HashTransformer.go
$(pGen)/ImageTagTransformer.go: $(pSrc)/imagetagtransformer/ImageTagTransformer.go
$(pGen)/LabelTransformer.go: $(pSrc)/labeltransformer/LabelTransformer.go
$(pGen)/LegacyOrderTransformer.go: $(pSrc)/legacyordertransformer/LegacyOrderTransformer.go
$(pGen)/SortOrderTransformer.go: $(pSrc)/sortordertransformer/SortOrderTransformer.go
$(pGen)/NamespaceTransformer.go: $(pSrc)/namespacetransformer/NamespaceTransformer.go
$(pGen)/PatchJson6902Transformer.go: $(pSrc)/patchjson6902transformer/PatchJson6902Transformer.go
$(pGen)/PatchStrategicMergeTransformer.go: $(pSrc)/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go
Expand Down
2 changes: 0 additions & 2 deletions api/builtins/builtins.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ type (
IAMPolicyGeneratorPlugin = internal.IAMPolicyGeneratorPlugin
ImageTagTransformerPlugin = internal.ImageTagTransformerPlugin
LabelTransformerPlugin = internal.LabelTransformerPlugin
LegacyOrderTransformerPlugin = internal.LegacyOrderTransformerPlugin
NamespaceTransformerPlugin = internal.NamespaceTransformerPlugin
PatchJson6902TransformerPlugin = internal.PatchJson6902TransformerPlugin
PatchStrategicMergeTransformerPlugin = internal.PatchStrategicMergeTransformerPlugin
Expand All @@ -37,7 +36,6 @@ var (
NewIAMPolicyGeneratorPlugin = internal.NewIAMPolicyGeneratorPlugin
NewImageTagTransformerPlugin = internal.NewImageTagTransformerPlugin
NewLabelTransformerPlugin = internal.NewLabelTransformerPlugin
NewLegacyOrderTransformerPlugin = internal.NewLegacyOrderTransformerPlugin
NewNamespaceTransformerPlugin = internal.NewNamespaceTransformerPlugin
NewPatchJson6902TransformerPlugin = internal.NewPatchJson6902TransformerPlugin
NewPatchStrategicMergeTransformerPlugin = internal.NewPatchStrategicMergeTransformerPlugin
Expand Down
46 changes: 0 additions & 46 deletions api/internal/builtins/LegacyOrderTransformer.go
View file
Open in desktop

This file was deleted.

244 changes: 244 additions & 0 deletions api/internal/builtins/SortOrderTransformer.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 14 additions & 15 deletions api/internal/plugins/builtinhelpers/builtinplugintype_string.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions api/internal/plugins/builtinhelpers/builtins.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const (
HashTransformer
ImageTagTransformer
LabelTransformer
LegacyOrderTransformer
NamespaceTransformer
PatchJson6902Transformer
PatchStrategicMergeTransformer
Expand Down Expand Up @@ -100,7 +99,6 @@ var TransformerFactories = map[BuiltinPluginType]func() resmap.TransformerPlugin
HashTransformer: builtins.NewHashTransformerPlugin,
ImageTagTransformer: builtins.NewImageTagTransformerPlugin,
LabelTransformer: builtins.NewLabelTransformerPlugin,
LegacyOrderTransformer: builtins.NewLegacyOrderTransformerPlugin,
Copy link
Contributor

Choose a reason for hiding this comment

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

Lines like this one suggest that the old transformer was exposed in the transformers field, and I confirmed this experimentally. Here's the setup:

# kustomization.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
  - resources.yaml # any resources that will show the difference btw fifo and legacy will do

transformers:
  - transformer.yaml
# transformer.yaml
apiVersion: builtin
kind: LegacyOrderTransformer # the presence of this alone causes legacy order
metadata:
  name: reorder
kustomize build testdir/ --reorder=none # without the transformer, this will be respected. With the transformer, order will be legacy.

@natasha41575 do you think we need to preserve this? It was not documented, I don't see any usage of it at all in a cross-Github search, and I don't think it is very useful. So I'm inclined to think it is fine to get rid of as this PR currently does.

Copy link
Contributor

Choose a reason for hiding this comment

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

Happy to get rid of it, I really doubt anyone uses it. If we can get this PR in the next couple weeks, we can also include this as part of the v5 breaking changes.

NamespaceTransformer: builtins.NewNamespaceTransformerPlugin,
PatchJson6902Transformer: builtins.NewPatchJson6902TransformerPlugin,
PatchStrategicMergeTransformer: builtins.NewPatchStrategicMergeTransformerPlugin,
Expand All @@ -111,4 +109,7 @@ var TransformerFactories = map[BuiltinPluginType]func() resmap.TransformerPlugin
ReplacementTransformer: builtins.NewReplacementTransformerPlugin,
ReplicaCountTransformer: builtins.NewReplicaCountTransformerPlugin,
ValueAddTransformer: builtins.NewValueAddTransformerPlugin,
// Do not wired SortOrderTransformer as a builtin plugin.
// We only want it to be available in the top-level kustomization.
// See: https://github.com/kubernetes-sigs/kustomize/issues/3913
}
Loading