From 416a9bf9e67516646257deb2a9108698b3de439e Mon Sep 17 00:00:00 2001 From: sugaf1204 Date: Sun, 19 Apr 2026 17:29:01 +0900 Subject: [PATCH] fix(fluentd): sort plugin @id numerically to preserve CR definition order Lexicographic comparison placed index 10 before index 2 (-10 < -2 as strings). Use a numeric-aware comparator when both @ids share a "-" shape; fall back to lexicographic otherwise. Adds regression tests for inputs, filters, and outputs with 11 plugins per CR. Signed-off-by: sugaf1204 --- apis/fluentd/v1alpha1/plugins/params/model.go | 31 ++- ...entd-cluster-cfg-filter-order-by-index.cfg | 102 ++++++++++ ...uentd-cluster-cfg-input-order-by-index.cfg | 91 +++++++++ ...entd-cluster-cfg-output-order-by-index.cfg | 105 +++++++++++ apis/fluentd/v1alpha1/tests/helper_test.go | 41 ++++ apis/fluentd/v1alpha1/tests/tools.go | 177 ++++++++++++++++++ 6 files changed, 546 insertions(+), 1 deletion(-) create mode 100644 apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-filter-order-by-index.cfg create mode 100644 apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-input-order-by-index.cfg create mode 100644 apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-output-order-by-index.cfg diff --git a/apis/fluentd/v1alpha1/plugins/params/model.go b/apis/fluentd/v1alpha1/plugins/params/model.go index 6ac01f237..98d743aef 100644 --- a/apis/fluentd/v1alpha1/plugins/params/model.go +++ b/apis/fluentd/v1alpha1/plugins/params/model.go @@ -5,6 +5,7 @@ import ( "fmt" "sort" "strconv" + "strings" "github.com/fluent/fluent-operator/v3/pkg/utils" ) @@ -253,8 +254,36 @@ func (a PluginStoreByNameById) Less(i, j int) bool { if a[i].GetTag() != "**" && a[j].GetTag() == "**" { return true } - return a[i].GetId() < a[j].GetId() + return lessId(a[i].GetId(), a[j].GetId()) } else { return a[i].Name < a[j].Name } } + +// lessId orders @id strings so that numeric suffixes compare numerically once +// both ids share a "-" shape. A plain lexicographic compare +// would place "

-10" before "

-2" because '1' < '2'. Falls back to +// lexicographic order when either side lacks a numeric trailing segment or +// when the parsed prefixes differ. +func lessId(id1, id2 string) bool { + p1, n1, ok1 := splitIdIndex(id1) + p2, n2, ok2 := splitIdIndex(id2) + if ok1 && ok2 && p1 == p2 { + return n1 < n2 + } + return id1 < id2 +} + +// splitIdIndex splits an @id at the last '-'. It returns the prefix, the +// parsed numeric index, and true when the trailing segment is an integer. +func splitIdIndex(id string) (string, int, bool) { + idx := strings.LastIndex(id, "-") + if idx < 0 { + return "", 0, false + } + n, err := strconv.Atoi(id[idx+1:]) + if err != nil { + return "", 0, false + } + return id[:idx], n, true +} diff --git a/apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-filter-order-by-index.cfg b/apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-filter-order-by-index.cfg new file mode 100644 index 000000000..3e8824a3a --- /dev/null +++ b/apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-filter-order-by-index.cfg @@ -0,0 +1,102 @@ + + @type forward + bind 0.0.0.0 + port 24224 + + + @id main + @type label_router + + @label @a2170d34e9940ec56d328100e375c43e + + namespaces default,kube-system + + + + diff --git a/apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-input-order-by-index.cfg b/apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-input-order-by-index.cfg new file mode 100644 index 000000000..90badbf0d --- /dev/null +++ b/apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-input-order-by-index.cfg @@ -0,0 +1,91 @@ + + @type forward + bind 0.0.0.0 + port 24224 + + + @id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-0 + @type sample + rate 1 + tag input-order-00 + + + @id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-1 + @type sample + rate 2 + tag input-order-01 + + + @id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-2 + @type sample + rate 3 + tag input-order-02 + + + @id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-3 + @type sample + rate 4 + tag input-order-03 + + + @id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-4 + @type sample + rate 5 + tag input-order-04 + + + @id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-5 + @type sample + rate 6 + tag input-order-05 + + + @id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-6 + @type sample + rate 7 + tag input-order-06 + + + @id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-7 + @type sample + rate 8 + tag input-order-07 + + + @id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-8 + @type sample + rate 9 + tag input-order-08 + + + @id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-9 + @type sample + rate 10 + tag input-order-09 + + + @id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-10 + @type sample + rate 11 + tag input-order-10 + + + @id main + @type label_router + + @label @a2170d34e9940ec56d328100e375c43e + + namespaces default,kube-system + + + + diff --git a/apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-output-order-by-index.cfg b/apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-output-order-by-index.cfg new file mode 100644 index 000000000..b5df9e4f2 --- /dev/null +++ b/apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-output-order-by-index.cfg @@ -0,0 +1,105 @@ + + @type forward + bind 0.0.0.0 + port 24224 + + + @id main + @type label_router + + @label @a2170d34e9940ec56d328100e375c43e + + namespaces default,kube-system + + + + diff --git a/apis/fluentd/v1alpha1/tests/helper_test.go b/apis/fluentd/v1alpha1/tests/helper_test.go index 9eb42de9f..b73072eea 100644 --- a/apis/fluentd/v1alpha1/tests/helper_test.go +++ b/apis/fluentd/v1alpha1/tests/helper_test.go @@ -498,6 +498,47 @@ func Test_DuplicateRemovalCRSpecs(t *testing.T) { } } +// Test_ClusterCfgOutputOrderByIndex guards against a previously-seen string-sort +// regression: outputs with @id suffixes "-0..-10" must render in numeric order, +// not "-0, -1, -10, -2, -3, ...". +func Test_ClusterCfgOutputOrderByIndex(t *testing.T) { + sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{}) + testClusterConfigWithFiltersAndOutputs(t, sl, Fluentd, &FluentdClusterFluentdConfig1, []fluentdv1alpha1.ClusterFilter{}, []fluentdv1alpha1.ClusterOutput{FluentdClusterOutputOrderByIndex}, "./expected/fluentd-cluster-cfg-output-order-by-index.cfg", false) +} + +// Test_ClusterCfgFilterOrderByIndex guards against the same string-sort regression +// for filters attached as children of a label plugin. +func Test_ClusterCfgFilterOrderByIndex(t *testing.T) { + sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{}) + testClusterConfigWithFiltersAndOutputs(t, sl, Fluentd, &FluentdClusterFluentdConfig1, []fluentdv1alpha1.ClusterFilter{FluentdClusterFilterOrderByIndex}, []fluentdv1alpha1.ClusterOutput{FluentdclusterOutput2ES}, "./expected/fluentd-cluster-cfg-filter-order-by-index.cfg", false) +} + +// Test_ClusterCfgInputOrderByIndex asserts that inputs declared in a single CR +// render in CR definition order. Inputs are not re-sorted by @id in +// RenderMainConfig, so this acts as a boundary guard for any future change that +// would accidentally introduce such a sort on the input rendering path. +func Test_ClusterCfgInputOrderByIndex(t *testing.T) { + g := NewGomegaWithT(t) + sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{}) + + psr := fluentdv1alpha1.NewGlobalPluginResources("main") + psr.CombineGlobalInputsPlugins(sl, Fluentd.Spec.GlobalInputs) + + clustercfgRouter, err := psr.BuildCfgRouter(&FluentdClusterFluentdConfig1) + g.Expect(err).NotTo(HaveOccurred()) + clusterInputs := []fluentdv1alpha1.ClusterInput{FluentdClusterInputOrderByIndex} + clusterOutputs := []fluentdv1alpha1.ClusterOutput{FluentdclusterOutput2ES} + clustercfgResources, _ := psr.PatchAndFilterClusterLevelResources(sl, FluentdClusterFluentdConfig1.GetCfgId(), clusterInputs, []fluentdv1alpha1.ClusterFilter{}, clusterOutputs) + err = psr.WithCfgResources(*clustercfgRouter.Label, clustercfgResources) + g.Expect(err).NotTo(HaveOccurred()) + + for i := 0; i < maxRuntimes; i++ { + config, errs := psr.RenderMainConfig(false) + g.Expect(errs).NotTo(HaveOccurred()) + g.Expect(string(getExpectedCfg("./expected/fluentd-cluster-cfg-input-order-by-index.cfg"))).To(Equal(config)) + } +} + func Test_RecordTransformer(t *testing.T) { sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{}) testClusterConfigWithFiltersAndOutputs(t, sl, Fluentd, &FluentdClusterFluentdConfig1, []fluentdv1alpha1.ClusterFilter{FluentdClusterRecordTransformerFilter}, []fluentdv1alpha1.ClusterOutput{FluentdClusterOutputCluster}, "./expected/fluentd-cluster-cfg-filter-recordTransformer.cfg", false) diff --git a/apis/fluentd/v1alpha1/tests/tools.go b/apis/fluentd/v1alpha1/tests/tools.go index ac4691795..75b9a24ff 100644 --- a/apis/fluentd/v1alpha1/tests/tools.go +++ b/apis/fluentd/v1alpha1/tests/tools.go @@ -906,6 +906,180 @@ spec: insecure: true ` + // FluentdClusterFilterOrderByIndex contains 11 recordTransformer filters (indexes 0..10). + // Once the index reaches 10, the string-based comparison on @id surfaces a mis-order bug. + FluentdClusterFilterOrderByIndex fluentdv1alpha1.ClusterFilter + FluentdClusterFilterOrderByIndexRaw = ` +apiVersion: fluentd.fluent.io/v1alpha1 +kind: ClusterFilter +metadata: + name: fluentd-filter-order + labels: + filter.fluentd.fluent.io/enabled: "true" +spec: + filters: + - recordTransformer: + records: + - key: k00 + value: v00 + - recordTransformer: + records: + - key: k01 + value: v01 + - recordTransformer: + records: + - key: k02 + value: v02 + - recordTransformer: + records: + - key: k03 + value: v03 + - recordTransformer: + records: + - key: k04 + value: v04 + - recordTransformer: + records: + - key: k05 + value: v05 + - recordTransformer: + records: + - key: k06 + value: v06 + - recordTransformer: + records: + - key: k07 + value: v07 + - recordTransformer: + records: + - key: k08 + value: v08 + - recordTransformer: + records: + - key: k09 + value: v09 + - recordTransformer: + records: + - key: k10 + value: v10 +` + + // FluentdClusterInputOrderByIndex contains 11 sample inputs (indexes 0..10) inside a single CR. + // Used to assert that the input rendering path preserves CR definition order. + FluentdClusterInputOrderByIndex fluentdv1alpha1.ClusterInput + FluentdClusterInputOrderByIndexRaw = ` +apiVersion: fluentd.fluent.io/v1alpha1 +kind: ClusterInput +metadata: + name: fluentd-input-order + labels: + input.fluentd.fluent.io/enabled: "true" +spec: + inputs: + - sample: + tag: input-order-00 + rate: 1 + - sample: + tag: input-order-01 + rate: 2 + - sample: + tag: input-order-02 + rate: 3 + - sample: + tag: input-order-03 + rate: 4 + - sample: + tag: input-order-04 + rate: 5 + - sample: + tag: input-order-05 + rate: 6 + - sample: + tag: input-order-06 + rate: 7 + - sample: + tag: input-order-07 + rate: 8 + - sample: + tag: input-order-08 + rate: 9 + - sample: + tag: input-order-09 + rate: 10 + - sample: + tag: input-order-10 + rate: 11 +` + + // FluentdClusterOutputOrderByIndex contains 11 elasticsearch outputs (indexes 0..10). + // Once the index reaches 10, the string-based comparison on @id surfaces a mis-order bug. + FluentdClusterOutputOrderByIndex fluentdv1alpha1.ClusterOutput + FluentdClusterOutputOrderByIndexRaw = ` +apiVersion: fluentd.fluent.io/v1alpha1 +kind: ClusterOutput +metadata: + name: fluentd-output-order + labels: + output.fluentd.fluent.io/enabled: "true" +spec: + outputs: + - elasticsearch: + host: elasticsearch-logging-data.kubesphere-logging-system.svc + port: 9200 + logstashFormat: true + logstashPrefix: ks-logstash-log-00 + - elasticsearch: + host: elasticsearch-logging-data.kubesphere-logging-system.svc + port: 9200 + logstashFormat: true + logstashPrefix: ks-logstash-log-01 + - elasticsearch: + host: elasticsearch-logging-data.kubesphere-logging-system.svc + port: 9200 + logstashFormat: true + logstashPrefix: ks-logstash-log-02 + - elasticsearch: + host: elasticsearch-logging-data.kubesphere-logging-system.svc + port: 9200 + logstashFormat: true + logstashPrefix: ks-logstash-log-03 + - elasticsearch: + host: elasticsearch-logging-data.kubesphere-logging-system.svc + port: 9200 + logstashFormat: true + logstashPrefix: ks-logstash-log-04 + - elasticsearch: + host: elasticsearch-logging-data.kubesphere-logging-system.svc + port: 9200 + logstashFormat: true + logstashPrefix: ks-logstash-log-05 + - elasticsearch: + host: elasticsearch-logging-data.kubesphere-logging-system.svc + port: 9200 + logstashFormat: true + logstashPrefix: ks-logstash-log-06 + - elasticsearch: + host: elasticsearch-logging-data.kubesphere-logging-system.svc + port: 9200 + logstashFormat: true + logstashPrefix: ks-logstash-log-07 + - elasticsearch: + host: elasticsearch-logging-data.kubesphere-logging-system.svc + port: 9200 + logstashFormat: true + logstashPrefix: ks-logstash-log-08 + - elasticsearch: + host: elasticsearch-logging-data.kubesphere-logging-system.svc + port: 9200 + logstashFormat: true + logstashPrefix: ks-logstash-log-09 + - elasticsearch: + host: elasticsearch-logging-data.kubesphere-logging-system.svc + port: 9200 + logstashFormat: true + logstashPrefix: ks-logstash-log-10 +` + esCredentials corev1.Secret esCredentialsRaw = ` apiVersion: v1 @@ -1071,6 +1245,9 @@ func init() { MustParseIntoObject(FluentdOutputMixedCopy1Raw, &FluentdOutputMixedCopy1) MustParseIntoObject(FluentdOutputMixedCopy2Raw, &FluentdOutputMixedCopy2) MustParseIntoObject(FluentdOutputMixedCopy3Raw, &FluentdOutputMixedCopy3) + MustParseIntoObject(FluentdClusterFilterOrderByIndexRaw, &FluentdClusterFilterOrderByIndex) + MustParseIntoObject(FluentdClusterInputOrderByIndexRaw, &FluentdClusterInputOrderByIndex) + MustParseIntoObject(FluentdClusterOutputOrderByIndexRaw, &FluentdClusterOutputOrderByIndex) MustParseIntoObject(esCredentialsRaw, &esCredentials) MustParseIntoObject(lokiHttpCredentialsRaw, &lokiHttpCredentials) MustParseIntoObject(lokiTenantNameRaw, &lokiTenantName)