From bee6483d88d7381ba778f50c557667f4eb1543eb Mon Sep 17 00:00:00 2001 From: Alfredo Beaumont Date: Thu, 3 Feb 2022 23:45:30 +0100 Subject: [PATCH] feat: add some basic flamebearer validation. (#785) * feat: add some basic flamebearer validation. * fix: remove empty line at the end of block. --- pkg/adhoc/server/convert.go | 3 + pkg/structs/flamebearer/flamebearer.go | 45 +++++++ pkg/structs/flamebearer/flamebearer_test.go | 142 ++++++++++++++++++++ 3 files changed, 190 insertions(+) diff --git a/pkg/adhoc/server/convert.go b/pkg/adhoc/server/convert.go index a6bf35194a..3155cd9346 100644 --- a/pkg/adhoc/server/convert.go +++ b/pkg/adhoc/server/convert.go @@ -21,6 +21,9 @@ func JSONToProfileV1(b []byte, _ string, _ int) (*flamebearer.FlamebearerProfile if err := json.Unmarshal(b, &profile); err != nil { return nil, fmt.Errorf("unable to unmarshall JSON: %w", err) } + if err := profile.Validate(); err != nil { + return nil, fmt.Errorf("invalid profile: %w", err) + } return &profile, nil } diff --git a/pkg/structs/flamebearer/flamebearer.go b/pkg/structs/flamebearer/flamebearer.go index 3237ee6edf..0d6308ff8a 100644 --- a/pkg/structs/flamebearer/flamebearer.go +++ b/pkg/structs/flamebearer/flamebearer.go @@ -1,6 +1,8 @@ package flamebearer import ( + "fmt" + "github.com/pyroscope-io/pyroscope/pkg/storage" "github.com/pyroscope-io/pyroscope/pkg/storage/segment" "github.com/pyroscope-io/pyroscope/pkg/storage/tree" @@ -100,3 +102,46 @@ func newTimeline(timeline *segment.Timeline) *FlamebearerTimelineV1 { Watermarks: timeline.Watermarks, } } + +func (fb FlamebearerProfile) Validate() error { + if fb.Version > 1 { + return fmt.Errorf("unsupported version %d", fb.Version) + } + return fb.FlamebearerProfileV1.Validate() +} + +// Validate the V1 profile. +// A custom validation is used as the constraints are hard to define in a generic way +// (e.g. using https://github.com/go-playground/validator) +func (fb FlamebearerProfileV1) Validate() error { + format := tree.Format(fb.Metadata.Format) + if format != tree.FormatSingle && format != tree.FormatDouble { + return fmt.Errorf("unsupported format %s", format) + } + if len(fb.Flamebearer.Names) == 0 { + return fmt.Errorf("a profile must have at least one symbol name") + } + if len(fb.Flamebearer.Levels) == 0 { + return fmt.Errorf("a profile must have at least one profiling level") + } + var mod int + switch format { + case tree.FormatSingle: + mod = 4 + case tree.FormatDouble: + mod = 7 + default: // This shouldn't happen at this point. + return fmt.Errorf("unsupported format %s", format) + } + for _, l := range fb.Flamebearer.Levels { + if len(l)%mod != 0 { + return fmt.Errorf("a profile level should have a multiple of %d values, but there's a level with %d values", mod, len(l)) + } + for i := mod - 1; i < len(l); i += mod { + if l[i] >= len(fb.Flamebearer.Names) { + return fmt.Errorf("invalid name index %d, it should be smaller than %d", l[i], len(fb.Flamebearer.Levels)) + } + } + } + return nil +} diff --git a/pkg/structs/flamebearer/flamebearer_test.go b/pkg/structs/flamebearer/flamebearer_test.go index 4fe02b510b..1a0698997e 100644 --- a/pkg/structs/flamebearer/flamebearer_test.go +++ b/pkg/structs/flamebearer/flamebearer_test.go @@ -69,6 +69,9 @@ var _ = Describe("FlamebearerProfile", func() { // Ticks Expect(p.LeftTicks).To(BeZero()) Expect(p.RightTicks).To(BeZero()) + + // Validate + Expect(p.Validate()).To(BeNil()) }) }) @@ -124,6 +127,145 @@ var _ = Describe("FlamebearerProfile", func() { // Ticks Expect(p.LeftTicks).To(Equal(uint64(3))) Expect(p.RightTicks).To(Equal(uint64(12))) + + // Validate + Expect(p.Validate()).To(BeNil()) + }) + }) +}) + +var _ = Describe("Checking profile validation", func() { + When("the version is invalid", func() { + var fb FlamebearerProfile + BeforeEach(func() { + fb.Version = 2 + }) + + Context("and we validate the profile", func() { + It("returns an error", func() { + Expect(fb.Validate()).To(MatchError("unsupported version 2")) + }) + }) + }) + + When("the format is unsupported", func() { + var fb FlamebearerProfile + + Context("and we validate the profile", func() { + It("returns an error", func() { + Expect(fb.Validate()).To(MatchError("unsupported format ")) + }) + }) + }) + + When("there are no names", func() { + var fb FlamebearerProfile + BeforeEach(func() { + fb.Metadata.Format = "single" + }) + + Context("and we validate the profile", func() { + It("returns an error", func() { + Expect(fb.Validate()).To(MatchError("a profile must have at least one symbol name")) + }) + }) + }) + + When("there are no levels", func() { + var fb FlamebearerProfile + BeforeEach(func() { + fb.Metadata.Format = "single" + fb.Flamebearer.Names = []string{"name"} + }) + + Context("and we validate the profile", func() { + It("returns an error", func() { + Expect(fb.Validate()).To(MatchError("a profile must have at least one profiling level")) + }) + }) + }) + + When("the level size is invalid for the profile format", func() { + Context("and we validate a single profile", func() { + var fb FlamebearerProfile + BeforeEach(func() { + fb.Metadata.Format = "single" + fb.Flamebearer.Names = []string{"name"} + fb.Flamebearer.Levels = [][]int{{0, 0, 0, 0, 0, 0, 0}} + }) + + It("returns an error", func() { + Expect(fb.Validate()).To(MatchError("a profile level should have a multiple of 4 values, but there's a level with 7 values")) + }) + }) + + Context("and we validate a double profile", func() { + var fb FlamebearerProfile + BeforeEach(func() { + fb.Metadata.Format = "double" + fb.Flamebearer.Names = []string{"name"} + fb.Flamebearer.Levels = [][]int{{0, 0, 0, 0}} + }) + + It("returns an error", func() { + Expect(fb.Validate()).To(MatchError("a profile level should have a multiple of 7 values, but there's a level with 4 values")) + }) + }) + }) + + When("the name index is invalid", func() { + Context("and we validate a single profile", func() { + var fb FlamebearerProfile + BeforeEach(func() { + fb.Metadata.Format = "single" + fb.Flamebearer.Names = []string{"name"} + fb.Flamebearer.Levels = [][]int{{0, 0, 0, 1}} + }) + + It("returns an error", func() { + Expect(fb.Validate()).To(MatchError("invalid name index 1, it should be smaller than 1")) + }) + }) + + Context("and we validate a double profile", func() { + var fb FlamebearerProfile + BeforeEach(func() { + fb.Metadata.Format = "double" + fb.Flamebearer.Names = []string{"name"} + fb.Flamebearer.Levels = [][]int{{0, 0, 0, 0, 0, 0, 1}} + }) + + It("returns an error", func() { + Expect(fb.Validate()).To(MatchError("invalid name index 1, it should be smaller than 1")) + }) + }) + }) + + When("everything is valid", func() { + Context("and we validate a single profile", func() { + var fb FlamebearerProfile + BeforeEach(func() { + fb.Metadata.Format = "single" + fb.Flamebearer.Names = []string{"name"} + fb.Flamebearer.Levels = [][]int{{0, 0, 0, 0}} + }) + + It("returns no error", func() { + Expect(fb.Validate()).To(BeNil()) + }) + }) + + Context("and we validate a double profile", func() { + var fb FlamebearerProfile + BeforeEach(func() { + fb.Metadata.Format = "double" + fb.Flamebearer.Names = []string{"name"} + fb.Flamebearer.Levels = [][]int{{0, 0, 0, 0, 0, 0, 0}} + }) + + It("returns an error", func() { + Expect(fb.Validate()).To(BeNil()) + }) }) }) })