From 7623ffc22582ca61a7179f3310ef32e66be2ea82 Mon Sep 17 00:00:00 2001 From: Christian Schaible Date: Tue, 12 Nov 2024 14:29:20 +0000 Subject: [PATCH] Merged PR 678247: fix: Adjust SequenceNumberGenerator to revert specific sequence numbers Concurrent invocations on the SequenceNumberGenerator can lead to gaps and duplicate sequence numbers. To fix the issue an internal "backlog" is added as a buffer. If a new number is requested the "backlog" is checked first before creating a new number. The "host" HTTP-Header is filtered out hide internal information. The following build dependencies have been updated: - Go has been updated to version 1.23.3. - Golanglint-ci has been updated to version v1.62.0. - Buf has been updated to version v1.46.0 The following library dependencies have been updated: - go.opentelemetry.io/otel has been updated to version v1.32.0 - go.opentelemetry.io/otel/trace has been updated to version v1.32.0 Related work items: #696569 --- .azuredevops/build-pipeline.yml | 6 ++-- audit/api/builder.go | 4 +-- audit/api/builder_test.go | 7 +++-- audit/api/model.go | 4 +-- audit/api/model_test.go | 4 +++ audit/utils/sequence_generator.go | 29 +++++++++++++++----- audit/utils/sequence_generator_test.go | 38 +++++++++++++++++++++++++- go.mod | 8 +++--- go.sum | 12 ++++---- 9 files changed, 85 insertions(+), 27 deletions(-) diff --git a/.azuredevops/build-pipeline.yml b/.azuredevops/build-pipeline.yml index 965bdb7..08b8ef4 100644 --- a/.azuredevops/build-pipeline.yml +++ b/.azuredevops/build-pipeline.yml @@ -3,11 +3,11 @@ pool: variables: - name: bufVersion - value: v1.45.0 + value: v1.46.0 - name: golangCiLintVersion - value: v1.61.0 + value: v1.62.0 - name: goVersion - value: 1.23.2 + value: 1.23.3 - name: protobufValidateVersion value: v1.1.0 - name: protobufVersion diff --git a/audit/api/builder.go b/audit/api/builder.go index f34c702..a94f3eb 100644 --- a/audit/api/builder.go +++ b/audit/api/builder.go @@ -433,8 +433,8 @@ func (builder *AuditEventBuilder) NextSequenceNumber() SequenceNumber { } // RevertSequenceNumber can be called to decrease the sequence number on the utils.SequenceNumberGenerator in case of an error -func (builder *AuditEventBuilder) RevertSequenceNumber() { - (*builder.sequenceNumberGenerator).Revert() +func (builder *AuditEventBuilder) RevertSequenceNumber(number SequenceNumber) { + (*builder.sequenceNumberGenerator).Revert(uint64(number)) } func (builder *AuditEventBuilder) AsSystemEvent() *AuditEventBuilder { diff --git a/audit/api/builder_test.go b/audit/api/builder_test.go index 7fdfb35..1bda6ba 100644 --- a/audit/api/builder_test.go +++ b/audit/api/builder_test.go @@ -1136,7 +1136,10 @@ func Test_AuditEventBuilder(t *testing.T) { builder := NewAuditEventBuilder(api, sequenceNumberGenerator, "demo-service", "worker-id", "eu01") assert.Equal(t, SequenceNumber(0), builder.NextSequenceNumber()) - builder.RevertSequenceNumber() - assert.Equal(t, SequenceNumber(0), builder.NextSequenceNumber()) + assert.Equal(t, SequenceNumber(1), builder.NextSequenceNumber()) + assert.Equal(t, SequenceNumber(2), builder.NextSequenceNumber()) + builder.RevertSequenceNumber(SequenceNumber(1)) + assert.Equal(t, SequenceNumber(1), builder.NextSequenceNumber()) + assert.Equal(t, SequenceNumber(3), builder.NextSequenceNumber()) }) } diff --git a/audit/api/model.go b/audit/api/model.go index 547f3ef..2b2bd5a 100644 --- a/audit/api/model.go +++ b/audit/api/model.go @@ -576,12 +576,12 @@ func byteArrayToPbStruct(bytes []byte) (*structpb.Struct, error) { return structpb.NewStruct(bodyMap) } -// FilterAndMergeHeaders filters ":authority", "Authorization" and "B3" headers as well as +// FilterAndMergeHeaders filters ":authority", "Authorization", "B3" and "Host" headers as well as // all headers starting with the prefixes "X-" and "STACKIT-". // Headers are merged if there is more than one value for a given name. func FilterAndMergeHeaders(headers map[string][]string) map[string]string { var resultMap = make(map[string]string) - skipHeaders := []string{":authority", "authorization", "b3"} + skipHeaders := []string{":authority", "authorization", "b3", "host"} skipPrefixHeaders := []string{"x-", "stackit-"} if len(headers) == 0 { diff --git a/audit/api/model_test.go b/audit/api/model_test.go index a1724c5..b75f9be 100644 --- a/audit/api/model_test.go +++ b/audit/api/model_test.go @@ -316,7 +316,11 @@ func Test_FilterAndMergeRequestHeaders(t *testing.T) { t.Run("skip headers", func(t *testing.T) { headers := make(map[string][]string) headers["Authorization"] = []string{"ey..."} + headers["authorization"] = []string{"ey..."} headers["B3"] = []string{"80f198ee56343ba864fe8b2a57d3eff7-e457b5a2e4d86bd1-1-05e3ac9a4f6e3b90"} + headers["b3"] = []string{"80f198ee56343ba864fe8b2a57d3eff7-e457b5a2e4d86bd1-1-05e3ac9a4f6e3b90"} + headers["Host"] = []string{"localhost:9090"} + headers["host"] = []string{"localhost:9090"} headers[":authority"] = []string{"localhost:9090"} filteredHeaders := FilterAndMergeHeaders(headers) diff --git a/audit/utils/sequence_generator.go b/audit/utils/sequence_generator.go index ebf16f2..89333b6 100644 --- a/audit/utils/sequence_generator.go +++ b/audit/utils/sequence_generator.go @@ -1,6 +1,9 @@ package utils -import "sync" +import ( + "slices" + "sync" +) // SequenceNumberGenerator can be used to generate increasing numbers. type SequenceNumberGenerator interface { @@ -8,12 +11,13 @@ type SequenceNumberGenerator interface { // Next returns the next number Next() uint64 - // Revert can be used to decrease the number (e.g. in case of an error) - Revert() + // Revert can be used to revert a specific number (e.g. in case of an error) + Revert(uint64) } // DefaultSequenceNumberGenerator is a mutex protected implementation of SequenceNumberGenerator type DefaultSequenceNumberGenerator struct { + backlog []uint64 sequenceNumber uint64 sequenceNumberLock sync.Mutex } @@ -22,6 +26,7 @@ type DefaultSequenceNumberGenerator struct { // of SequenceNumberGenerator. func NewDefaultSequenceNumberGenerator() *SequenceNumberGenerator { var generator SequenceNumberGenerator = &DefaultSequenceNumberGenerator{ + backlog: make([]uint64, 0), sequenceNumber: 0, sequenceNumberLock: sync.Mutex{}, } @@ -32,14 +37,24 @@ func NewDefaultSequenceNumberGenerator() *SequenceNumberGenerator { func (g *DefaultSequenceNumberGenerator) Next() uint64 { g.sequenceNumberLock.Lock() defer g.sequenceNumberLock.Unlock() - next := g.sequenceNumber - g.sequenceNumber += 1 + var next uint64 + if len(g.backlog) == 0 { + next = g.sequenceNumber + g.sequenceNumber += 1 + } else { + next = g.backlog[0] + g.backlog = g.backlog[1:] + } return next } // Revert implements SequenceNumberGenerator.Revert -func (g *DefaultSequenceNumberGenerator) Revert() { +func (g *DefaultSequenceNumberGenerator) Revert(value uint64) { g.sequenceNumberLock.Lock() defer g.sequenceNumberLock.Unlock() - g.sequenceNumber -= 1 + if value == g.sequenceNumber-1 { + g.sequenceNumber -= 1 + } else if !slices.Contains(g.backlog, value) { + g.backlog = append(g.backlog, value) + } } diff --git a/audit/utils/sequence_generator_test.go b/audit/utils/sequence_generator_test.go index 08fa08c..fe4a232 100644 --- a/audit/utils/sequence_generator_test.go +++ b/audit/utils/sequence_generator_test.go @@ -16,7 +16,43 @@ func Test_DefaultSequenceNumberGenerator(t *testing.T) { var sequenceGenerator = NewDefaultSequenceNumberGenerator() assert.Equal(t, uint64(0), (*sequenceGenerator).Next()) assert.Equal(t, uint64(1), (*sequenceGenerator).Next()) - (*sequenceGenerator).Revert() + (*sequenceGenerator).Revert(uint64(1)) assert.Equal(t, uint64(1), (*sequenceGenerator).Next()) }) + + t.Run("revert first", func(t *testing.T) { + var sequenceGenerator = NewDefaultSequenceNumberGenerator() + assert.Equal(t, uint64(0), (*sequenceGenerator).Next()) + assert.Equal(t, uint64(1), (*sequenceGenerator).Next()) + (*sequenceGenerator).Revert(uint64(0)) + assert.Equal(t, uint64(0), (*sequenceGenerator).Next()) + }) + + t.Run("revert same value multiple times", func(t *testing.T) { + var sequenceGenerator = NewDefaultSequenceNumberGenerator() + assert.Equal(t, uint64(0), (*sequenceGenerator).Next()) + assert.Equal(t, uint64(1), (*sequenceGenerator).Next()) + assert.Equal(t, uint64(2), (*sequenceGenerator).Next()) + (*sequenceGenerator).Revert(uint64(1)) + (*sequenceGenerator).Revert(uint64(1)) + assert.Equal(t, uint64(1), (*sequenceGenerator).Next()) + assert.Equal(t, uint64(3), (*sequenceGenerator).Next()) + }) + + t.Run("get and revert multiple", func(t *testing.T) { + var sequenceGenerator = NewDefaultSequenceNumberGenerator() + assert.Equal(t, uint64(0), (*sequenceGenerator).Next()) + assert.Equal(t, uint64(1), (*sequenceGenerator).Next()) + (*sequenceGenerator).Revert(uint64(1)) + assert.Equal(t, uint64(1), (*sequenceGenerator).Next()) + assert.Equal(t, uint64(2), (*sequenceGenerator).Next()) + assert.Equal(t, uint64(3), (*sequenceGenerator).Next()) + assert.Equal(t, uint64(4), (*sequenceGenerator).Next()) + (*sequenceGenerator).Revert(uint64(2)) + (*sequenceGenerator).Revert(uint64(3)) + assert.Equal(t, uint64(2), (*sequenceGenerator).Next()) + assert.Equal(t, uint64(3), (*sequenceGenerator).Next()) + assert.Equal(t, uint64(5), (*sequenceGenerator).Next()) + assert.Equal(t, uint64(6), (*sequenceGenerator).Next()) + }) } diff --git a/go.mod b/go.mod index b3d6dd8..725675d 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module dev.azure.com/schwarzit/schwarzit.stackit-public/audit-go.git -go 1.23.2 +go 1.23.3 require ( buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.35.1-20240920164238-5a7b106cbb87.1 @@ -10,8 +10,8 @@ require ( github.com/rs/zerolog v1.33.0 github.com/stretchr/testify v1.9.0 github.com/testcontainers/testcontainers-go v0.34.0 - go.opentelemetry.io/otel v1.31.0 - go.opentelemetry.io/otel/trace v1.31.0 + go.opentelemetry.io/otel v1.32.0 + go.opentelemetry.io/otel/trace v1.32.0 google.golang.org/protobuf v1.35.1 ) @@ -61,7 +61,7 @@ require ( github.com/tklauser/numcpus v0.6.1 // indirect github.com/yusufpapurcu/wmi v1.2.3 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect - go.opentelemetry.io/otel/metric v1.31.0 // indirect + go.opentelemetry.io/otel/metric v1.32.0 // indirect golang.org/x/crypto v0.24.0 // indirect golang.org/x/exp v0.0.0-20240325151524-a685a6edb6d8 // indirect golang.org/x/net v0.26.0 // indirect diff --git a/go.sum b/go.sum index c451dc1..54e9e44 100644 --- a/go.sum +++ b/go.sum @@ -142,18 +142,18 @@ github.com/yusufpapurcu/wmi v1.2.3 h1:E1ctvB7uKFMOJw3fdOW32DwGE9I7t++CRUEMKvFoFi github.com/yusufpapurcu/wmi v1.2.3/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 h1:jq9TW8u3so/bN+JPT166wjOI6/vQPF6Xe7nMNIltagk= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0/go.mod h1:p8pYQP+m5XfbZm9fxtSKAbM6oIllS7s2AfxrChvc7iw= -go.opentelemetry.io/otel v1.31.0 h1:NsJcKPIW0D0H3NgzPDHmo0WW6SptzPdqg/L1zsIm2hY= -go.opentelemetry.io/otel v1.31.0/go.mod h1:O0C14Yl9FgkjqcCZAsE053C13OaddMYr/hz6clDkEJE= +go.opentelemetry.io/otel v1.32.0 h1:WnBN+Xjcteh0zdk01SVqV55d/m62NJLJdIyb4y/WO5U= +go.opentelemetry.io/otel v1.32.0/go.mod h1:00DCVSB0RQcnzlwyTfqtxSm+DRr9hpYrHjNGiBHVQIg= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.19.0 h1:Mne5On7VWdx7omSrSSZvM4Kw7cS7NQkOOmLcgscI51U= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.19.0/go.mod h1:IPtUMKL4O3tH5y+iXVyAXqpAwMuzC1IrxVS81rummfE= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0 h1:IeMeyr1aBvBiPVYihXIaeIZba6b8E1bYp7lbdxK8CQg= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0/go.mod h1:oVdCUtjq9MK9BlS7TtucsQwUcXcymNiEDjgDD2jMtZU= -go.opentelemetry.io/otel/metric v1.31.0 h1:FSErL0ATQAmYHUIzSezZibnyVlft1ybhy4ozRPcF2fE= -go.opentelemetry.io/otel/metric v1.31.0/go.mod h1:C3dEloVbLuYoX41KpmAhOqNriGbA+qqH6PQ5E5mUfnY= +go.opentelemetry.io/otel/metric v1.32.0 h1:xV2umtmNcThh2/a/aCP+h64Xx5wsj8qqnkYZktzNa0M= +go.opentelemetry.io/otel/metric v1.32.0/go.mod h1:jH7CIbbK6SH2V2wE16W05BHCtIDzauciCRLoc/SyMv8= go.opentelemetry.io/otel/sdk v1.19.0 h1:6USY6zH+L8uMH8L3t1enZPR3WFEmSTADlqldyHtJi3o= go.opentelemetry.io/otel/sdk v1.19.0/go.mod h1:NedEbbS4w3C6zElbLdPJKOpJQOrGUJ+GfzpjUvI0v1A= -go.opentelemetry.io/otel/trace v1.31.0 h1:ffjsj1aRouKewfr85U2aGagJ46+MvodynlQ1HYdmJys= -go.opentelemetry.io/otel/trace v1.31.0/go.mod h1:TXZkRk7SM2ZQLtR6eoAWQFIHPvzQ06FJAsO1tJg480A= +go.opentelemetry.io/otel/trace v1.32.0 h1:WIC9mYrXf8TmY/EXuULKc8hR17vE+Hjv2cssQDe03fM= +go.opentelemetry.io/otel/trace v1.32.0/go.mod h1:+i4rkvCraA+tG6AzwloGaCtkx53Fa+L+V8e9a7YvhT8= go.opentelemetry.io/proto/otlp v1.0.0 h1:T0TX0tmXU8a3CbNXzEKGeU5mIVOdf0oykP+u2lIVU/I= go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v80hjKIs5JXpM= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=