From 079bc100e438221fe1fee7b32726228f6301ec40 Mon Sep 17 00:00:00 2001 From: Matt Siwiec Date: Mon, 23 Oct 2023 23:19:29 +0000 Subject: [PATCH 1/8] additional hooks unit tests Signed-off-by: Matt Siwiec --- go.mod | 3 +- go.sum | 14 +++-- internal/manualhooks/hooks_test.go | 87 ++++++++++++++++++++++++++++++ internal/testutils/db_setup.go | 7 +++ 4 files changed, 105 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 7a1e1623b..88cd1e21a 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.21 require ( entgo.io/contrib v0.4.5 entgo.io/ent v0.12.4 - github.com/99designs/gqlgen v0.17.39 + github.com/99designs/gqlgen v0.17.38 github.com/Yamashou/gqlgenc v0.15.1 github.com/brianvoe/gofakeit/v6 v6.23.2 github.com/docker/go-connections v0.4.0 @@ -120,7 +120,6 @@ require ( github.com/shirou/gopsutil/v3 v3.23.8 // indirect github.com/shoenig/go-m1cpu v0.1.6 // indirect github.com/sirupsen/logrus v1.9.3 // indirect - github.com/sosodev/duration v1.1.0 // indirect github.com/sourcegraph/conc v0.3.0 // indirect github.com/spf13/afero v1.10.0 // indirect github.com/spf13/cast v1.5.1 // indirect diff --git a/go.sum b/go.sum index de2c1e259..e81488002 100644 --- a/go.sum +++ b/go.sum @@ -46,8 +46,8 @@ entgo.io/ent v0.12.4 h1:LddPnAyxls/O7DTXZvUGDj0NZIdGSu317+aoNLJWbD8= entgo.io/ent v0.12.4/go.mod h1:Y3JVAjtlIk8xVZYSn3t3mf8xlZIn5SAOXZQxD6kKI+Q= filippo.io/edwards25519 v1.0.0 h1:0wAIcmJUqRdI8IJ/3eGi5/HwXZWPujYXXlkrQogz0Ek= filippo.io/edwards25519 v1.0.0/go.mod h1:N1IkdkCkiLB6tki+MYJoSx2JTY9NUlxZE7eHn5EwJns= -github.com/99designs/gqlgen v0.17.39 h1:wPTAyc2fqVjAWT5DsJ21k/lLudgnXzURwbsjVNegFpU= -github.com/99designs/gqlgen v0.17.39/go.mod h1:b62q1USk82GYIVjC60h02YguAZLqYZtvWml8KkhJps4= +github.com/99designs/gqlgen v0.17.38 h1:3r7G7i8UAdY0iYreNiBAA55auVsrowO0+ZhMl5g4GYU= +github.com/99designs/gqlgen v0.17.38/go.mod h1:2v+dKtpI8mIzYeW9dYN8mO69tMmjszW2xKLNcWR/5wQ= github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 h1:bvDV9vkmnHYOMsOr4WLk+Vo07yKIzd94sVoIqshQ4bU= github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24/go.mod h1:8o94RPi1/7XTJvwPpRSzSUedZrtlirdB3r9Z20bi2f8= github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 h1:L/gRVlceqvL25UVaW/CKtUDjefjrs0SPonmDGUVOYP0= @@ -108,6 +108,7 @@ github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7 github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/cpuguy83/dockercfg v0.3.1 h1:/FpZ+JaygUR/lZP2NlFI2DVfrOEMAIKP5wWEJdoYe9E= github.com/cpuguy83/dockercfg v0.3.1/go.mod h1:sugsbF4//dDlL/i+S+rtpIWp+5h0BHJHfjj5/jFyUJc= +github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY= github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= @@ -476,6 +477,8 @@ github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncj github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ= github.com/rs/zerolog v1.13.0/go.mod h1:YbFCdg8HfsridGWAh22vktObvhZbQsZXe4/zB0OKkWU= github.com/rs/zerolog v1.15.0/go.mod h1:xYTKnLHcpfU2225ny5qZjxnj9NvkumZYjJHlAThCjNc= +github.com/russross/blackfriday v1.6.0 h1:KqfZb0pUVN2lYqZUYRddxF4OR8ZMURnJIG5Y3VRLtww= +github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/sagikazarmark/locafero v0.3.0 h1:zT7VEGWC2DTflmccN/5T1etyKvxSxpHsjb9cJvm4SvQ= github.com/sagikazarmark/locafero v0.3.0/go.mod h1:w+v7UsPNFwzF1cHuOajOOzoq4U7v/ig1mpRjqV+Bu1U= @@ -502,8 +505,6 @@ github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/smallstep/assert v0.0.0-20200723003110-82e2b9b3b262 h1:unQFBIznI+VYD1/1fApl1A+9VcBk+9dcqGfnePY87LY= github.com/smallstep/assert v0.0.0-20200723003110-82e2b9b3b262/go.mod h1:MyOHs9Po2fbM1LHej6sBUT8ozbxmMOFG+E+rx/GSGuc= -github.com/sosodev/duration v1.1.0 h1:kQcaiGbJaIsRqgQy7VGlZrVw1giWO+lDoX3MCPnpVO4= -github.com/sosodev/duration v1.1.0/go.mod h1:RQIBBX0+fMLc/D9+Jb/fwvVmo0eZvDDEERAikUR6SDg= github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo= github.com/sourcegraph/conc v0.3.0/go.mod h1:Sdozi7LEKbFPqYX2/J+iBAM6HpqSLTASQIKqDmF7Mt0= github.com/spf13/afero v1.10.0 h1:EaGW2JJh15aKOejeuJ+wpFSHnbd7GE6Wvp3TsNhb6LY= @@ -550,6 +551,9 @@ github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVM github.com/ugorji/go/codec v1.1.7/go.mod h1:Ax+UKWsSmolVDwsd+7N3ZtXu+yMGCf907BLYF3GoBXY= github.com/ugorji/go/codec v1.2.11 h1:BMaWp1Bb6fHwEtbplGBGJ498wD+LKlNSl25MjdZY4dU= github.com/ugorji/go/codec v1.2.11/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg= +github.com/urfave/cli v1.22.12 h1:igJgVw1JdKH+trcLWLeLwZjU9fEfPesQ+9/e4MQ44S8= +github.com/urfave/cli/v2 v2.25.7 h1:VAzn5oq403l5pHjc4OhD54+XGO9cdKVL/7lDjF+iKUs= +github.com/urfave/cli/v2 v2.25.7/go.mod h1:8qnjx1vcq5s2/wpsqoZFndg2CE5tNFyrTvS6SinrnYQ= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/valyala/fasttemplate v1.2.1/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ= @@ -565,6 +569,8 @@ github.com/vmihailenco/tagparser v0.1.2 h1:gnjoVuB/kljJ5wICEEOpx98oXMWPLj22G67Vb github.com/vmihailenco/tagparser v0.1.2/go.mod h1:OeAg3pn3UbLjkWt+rN9oFYB6u/cQgqMEUPoW2WPyhdI= github.com/wundergraph/graphql-go-tools v1.66.4 h1:yRvXYi0jjTghi5zimTluqHXAmyS7JVlGzTlxY6aL0sI= github.com/wundergraph/graphql-go-tools v1.66.4/go.mod h1:obaEJWub7088qodhKbSGHyhRVnHlBP5M9HigN/oalLE= +github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 h1:bAn7/zixMGCfxrRTfdpNzjtPYqr8smhKouy9mxVdGPU= +github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/internal/manualhooks/hooks_test.go b/internal/manualhooks/hooks_test.go index bd334291c..150d7b4d0 100644 --- a/internal/manualhooks/hooks_test.go +++ b/internal/manualhooks/hooks_test.go @@ -354,3 +354,90 @@ func Test_PortDeleteHook(t *testing.T) { assert.Equal(t, port.ID, msg.Message().SubjectID) assert.Equal(t, deleteEventType, msg.Message().EventType) } + +func Test_MultipleLoadbalancersSharedPoolAddOrigin(t *testing.T) { + // Scenario: 2 loadbalancers in different locations, with the same owner, share a pool. + // An origin is added to the shared pool. + // Assert the owner, loadbalancers, pool, and locations are all included in the additionalSubject ID list + + // Arrange + ctx := testutils.MockPermissions(context.Background()) + + // create 2 loadbalancers with a shared pool of origins + prov := (&testutils.ProviderBuilder{}).MustNew(ctx) + lb1 := (&testutils.LoadBalancerBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) + lb2 := (&testutils.LoadBalancerBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) + pool := (&testutils.PoolBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) + _ = (&testutils.PortBuilder{PoolIDs: []gidx.PrefixedID{pool.ID}, LoadBalancerID: lb1.ID}).MustNew(ctx) + _ = (&testutils.PortBuilder{PoolIDs: []gidx.PrefixedID{pool.ID}, LoadBalancerID: lb2.ID}).MustNew(ctx) + _ = (&testutils.OriginBuilder{PoolID: pool.ID}).MustNew(ctx) + + testutils.EntClient.Origin.Use(manualhooks.OriginHooks()...) + + changesChannel, err := testutils.EventsConn.SubscribeChanges(ctx, "create.load-balancer-origin") + require.NoError(t, err, "failed to subscribe to changes") + + // Act - add another origin to the pool + ogn2 := (&testutils.OriginBuilder{PoolID: pool.ID}).MustNew(ctx) + + msg := testutils.ChannelReceiveWithTimeout[events.Message[events.ChangeMessage]](t, changesChannel, defaultTimeout) + + // Assert + expectedAdditionalSubjectIDs := []gidx.PrefixedID{ + prov.OwnerID, + lb1.ID, + lb2.ID, + lb1.LocationID, + lb2.LocationID, + pool.ID, + } + actualAdditionalSubjectIDs := msg.Message().AdditionalSubjectIDs + + assert.ElementsMatch(t, expectedAdditionalSubjectIDs, actualAdditionalSubjectIDs) + assert.Equal(t, ogn2.ID, msg.Message().SubjectID) + assert.Equal(t, createEventType, msg.Message().EventType) +} + +func Test_MultipleLoadbalancersSharedPoolDeleteOrigin(t *testing.T) { + // Scenario: 2 loadbalancers in different locations, with the same owner, share a pool. + // An origin is removed from the shared pool. + // Assert the owner, loadbalancers, pool, and locations are all included in the additionalSubject ID list + + // Arrange + ctx := testutils.MockPermissions(context.Background()) + + // create 2 loadbalancers with a shared pool of origins + prov := (&testutils.ProviderBuilder{}).MustNew(ctx) + lb1 := (&testutils.LoadBalancerBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) + lb2 := (&testutils.LoadBalancerBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) + pool := (&testutils.PoolBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) + _ = (&testutils.PortBuilder{PoolIDs: []gidx.PrefixedID{pool.ID}, LoadBalancerID: lb1.ID}).MustNew(ctx) + _ = (&testutils.PortBuilder{PoolIDs: []gidx.PrefixedID{pool.ID}, LoadBalancerID: lb2.ID}).MustNew(ctx) + _ = (&testutils.OriginBuilder{PoolID: pool.ID}).MustNew(ctx) + ogn2 := (&testutils.OriginBuilder{PoolID: pool.ID}).MustNew(ctx) + + testutils.EntClient.Origin.Use(manualhooks.OriginHooks()...) + + changesChannel, err := testutils.EventsConn.SubscribeChanges(ctx, "delete.load-balancer-origin") + require.NoError(t, err, "failed to subscribe to changes") + + // Act - update the pool to remove an origin + testutils.EntClient.Origin.DeleteOne(ogn2).ExecX(ctx) + + msg := testutils.ChannelReceiveWithTimeout[events.Message[events.ChangeMessage]](t, changesChannel, defaultTimeout) + + // Assert + expectedAdditionalSubjectIDs := []gidx.PrefixedID{ + prov.OwnerID, + lb1.ID, + lb2.ID, + lb1.LocationID, + lb2.LocationID, + pool.ID, + } + actualAdditionalSubjectIDs := msg.Message().AdditionalSubjectIDs + + assert.ElementsMatch(t, expectedAdditionalSubjectIDs, actualAdditionalSubjectIDs) + assert.Equal(t, ogn2.ID, msg.Message().SubjectID) + assert.Equal(t, deleteEventType, msg.Message().EventType) +} diff --git a/internal/testutils/db_setup.go b/internal/testutils/db_setup.go index 64631d039..feeb9cffb 100644 --- a/internal/testutils/db_setup.go +++ b/internal/testutils/db_setup.go @@ -23,6 +23,7 @@ import ( var ( testDBURI = os.Getenv("LOADBALANCERAPI_TESTDB_URI") + NATSConn *eventtools.TestNats // NATSConn exported if needed for subscribers EventsConn events.Connection // EventsConn exported if needed for subscribers EntClient *ent.Client // EntClient to use as ent client DBContainer *testcontainersx.DBContainer // DBContainer to use through entire test suite @@ -37,6 +38,7 @@ func SetupDB() { IfErrPanic("failed to start nats server", err) conn, err := events.NewConnection(nats.Config) + IfErrPanic("failed to create events connection", err) // DB and EntClient setup @@ -61,6 +63,7 @@ func SetupDB() { EventsConn = conn EntClient = c DBContainer = cntr + NATSConn = nats } // TeardownDB used for clean up test setup @@ -74,6 +77,10 @@ func TeardownDB() { if DBContainer != nil && DBContainer.Container.IsRunning() { IfErrPanic("teardown failed to terminate test db container", DBContainer.Container.Terminate(ctx)) } + + _ = EventsConn.Shutdown(ctx) + + NATSConn.Close() } // ParseDBURI parses the kind of query language from TESTDB_URI env var and initializes DBContainer as required From 52374a23e38627c05bbf5398536dacc89d890cfd Mon Sep 17 00:00:00 2001 From: Matt Siwiec Date: Tue, 24 Oct 2023 17:59:11 +0000 Subject: [PATCH 2/8] audit loadbalancer manual hook db hits and ensure providerID included in additionalSubjects Signed-off-by: Matt Siwiec --- internal/manualhooks/hooks.go | 43 +++++++++++------------------- internal/manualhooks/hooks_test.go | 6 ++--- 2 files changed, 18 insertions(+), 31 deletions(-) diff --git a/internal/manualhooks/hooks.go b/internal/manualhooks/hooks.go index 5af82ff50..89b21b72d 100644 --- a/internal/manualhooks/hooks.go +++ b/internal/manualhooks/hooks.go @@ -196,25 +196,22 @@ func LoadBalancerHooks() []ent.Hook { return retValue, err } - addSubjPortIDs, err := m.Client().Port.Query().Where(port.HasLoadBalancerWith(loadbalancer.IDEQ(objID))).IDs(ctx) - if err == nil { - for _, portID := range addSubjPortIDs { - if !slices.Contains(msg.AdditionalSubjectIDs, portID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, portID) - } - } + // Ensure we have additional relevant subjects in the msg + lb, err := m.Client().LoadBalancer.Query().WithPorts().Where(loadbalancer.IDEQ(objID)).Only(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get loadbalancer to lookup additional subject ids %s: %w", lb, err) } - lbs := getLoadBalancerIDs(ctx, objID, msg.AdditionalSubjectIDs) - for _, lb := range lbs { - lb, err := m.Client().LoadBalancer.Get(ctx, lb) - if err != nil { - return nil, fmt.Errorf("failed to get loadbalancer to lookup location %s", lb) - } + if !slices.Contains(msg.AdditionalSubjectIDs, lb.LocationID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.LocationID) + } - if !slices.Contains(msg.AdditionalSubjectIDs, lb.LocationID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.LocationID) - } + if !slices.Contains(msg.AdditionalSubjectIDs, lb.ProviderID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.ProviderID) + } + + for _, p := range lb.Edges.Ports { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, p.ID) } if len(relationships) != 0 { @@ -251,18 +248,8 @@ func LoadBalancerHooks() []ent.Hook { } additionalSubjects = append(additionalSubjects, dbObj.OwnerID) - - lbs := getLoadBalancerIDs(ctx, objID, additionalSubjects) - for _, lb := range lbs { - lb, err := m.Client().LoadBalancer.Get(ctx, lb) - if err != nil { - return nil, fmt.Errorf("failed to get loadbalancer to lookup location %s", lb) - } - - if !slices.Contains(additionalSubjects, lb.LocationID) { - additionalSubjects = append(additionalSubjects, lb.LocationID) - } - } + additionalSubjects = append(additionalSubjects, dbObj.LocationID) + additionalSubjects = append(additionalSubjects, dbObj.ProviderID) // we have all the info we need, now complete the mutation before we process the event retValue, err := next.Mutate(ctx, m) diff --git a/internal/manualhooks/hooks_test.go b/internal/manualhooks/hooks_test.go index 150d7b4d0..e21819cea 100644 --- a/internal/manualhooks/hooks_test.go +++ b/internal/manualhooks/hooks_test.go @@ -56,7 +56,7 @@ func Test_LoadbalancerCreateHook(t *testing.T) { msg := testutils.ChannelReceiveWithTimeout[events.Message[events.ChangeMessage]](t, changesChannel, defaultTimeout) // Assert - expectedAdditionalSubjectIDs := []gidx.PrefixedID{lb.ID, lb.OwnerID, lb.LocationID} + expectedAdditionalSubjectIDs := []gidx.PrefixedID{lb.ID, lb.OwnerID, lb.LocationID, lb.ProviderID} actualAdditionalSubjectIDs := msg.Message().AdditionalSubjectIDs assert.ElementsMatch(t, expectedAdditionalSubjectIDs, actualAdditionalSubjectIDs) @@ -81,7 +81,7 @@ func Test_LoadbalancerUpdateHook(t *testing.T) { msg := testutils.ChannelReceiveWithTimeout[events.Message[events.ChangeMessage]](t, changesChannel, defaultTimeout) // Assert - expectedAdditionalSubjectIDs := []gidx.PrefixedID{lb.ID, lb.OwnerID, lb.LocationID} + expectedAdditionalSubjectIDs := []gidx.PrefixedID{lb.ID, lb.OwnerID, lb.LocationID, lb.ProviderID} actualAdditionalSubjectIDs := msg.Message().AdditionalSubjectIDs assert.ElementsMatch(t, expectedAdditionalSubjectIDs, actualAdditionalSubjectIDs) @@ -106,7 +106,7 @@ func Test_LoadbalancerDeleteHook(t *testing.T) { msg := testutils.ChannelReceiveWithTimeout[events.Message[events.ChangeMessage]](t, changesChannel, defaultTimeout) // Assert - expectedAdditionalSubjectIDs := []gidx.PrefixedID{lb.OwnerID, lb.LocationID} + expectedAdditionalSubjectIDs := []gidx.PrefixedID{lb.OwnerID, lb.LocationID, lb.ProviderID} actualAdditionalSubjectIDs := msg.Message().AdditionalSubjectIDs assert.ElementsMatch(t, expectedAdditionalSubjectIDs, actualAdditionalSubjectIDs) From 511314ce4cd8919b66884452dc42aaa59415515b Mon Sep 17 00:00:00 2001 From: Matt Siwiec Date: Tue, 24 Oct 2023 18:45:36 +0000 Subject: [PATCH 3/8] audit origin manual hook db hits Signed-off-by: Matt Siwiec --- internal/manualhooks/hooks.go | 76 +++++++++++++---------------------- 1 file changed, 28 insertions(+), 48 deletions(-) diff --git a/internal/manualhooks/hooks.go b/internal/manualhooks/hooks.go index 89b21b72d..6cd9d8b39 100644 --- a/internal/manualhooks/hooks.go +++ b/internal/manualhooks/hooks.go @@ -478,37 +478,27 @@ func OriginHooks() []ent.Hook { return retValue, err } - addSubjPools, err := m.Client().Pool.Query().Where(pool.HasOriginsWith(origin.IDEQ(objID))).All(ctx) + // Ensure we have additional relevant subjects in the msg + addSubjPorts, err := m.Client().Port.Query().WithPools().WithLoadBalancer().Where(port.HasPoolsWith(pool.HasOriginsWith(origin.IDEQ(objID)))).All(ctx) if err == nil { - for _, pool := range addSubjPools { - if !slices.Contains(msg.AdditionalSubjectIDs, pool.ID) && objID != pool.ID { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, pool.ID) - } - - if !slices.Contains(msg.AdditionalSubjectIDs, pool.OwnerID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, pool.OwnerID) + for _, port := range addSubjPorts { + if !slices.Contains(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.LocationID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.LocationID) } - } - } - addSubjPorts, err := m.Client().Port.Query().Where(port.HasPoolsWith(pool.HasOriginsWith(origin.IDEQ(objID)))).All(ctx) - if err == nil { - for _, port := range addSubjPorts { if !slices.Contains(msg.AdditionalSubjectIDs, port.LoadBalancerID) { msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.LoadBalancerID) } - } - } - lbs := getLoadBalancerIDs(ctx, objID, msg.AdditionalSubjectIDs) - for _, lb := range lbs { - lb, err := m.Client().LoadBalancer.Get(ctx, lb) - if err != nil { - return nil, fmt.Errorf("failed to get loadbalancer to lookup location %s", lb) - } + for _, pool := range port.Edges.Pools { + if !slices.Contains(msg.AdditionalSubjectIDs, pool.ID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, pool.ID) + } - if !slices.Contains(msg.AdditionalSubjectIDs, lb.LocationID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.LocationID) + if !slices.Contains(msg.AdditionalSubjectIDs, pool.OwnerID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, pool.OwnerID) + } + } } } @@ -547,25 +537,27 @@ func OriginHooks() []ent.Hook { additionalSubjects = append(additionalSubjects, dbObj.PoolID) - addSubjPools, err := m.Client().Pool.Query().Where(pool.HasOriginsWith(origin.IDEQ(objID))).All(ctx) + // Ensure we have additional relevant subjects in the msg + addSubjPorts, err := m.Client().Port.Query().WithPools().WithLoadBalancer().Where(port.HasPoolsWith(pool.HasOriginsWith(origin.IDEQ(objID)))).All(ctx) if err == nil { - for _, pool := range addSubjPools { - if !slices.Contains(additionalSubjects, pool.ID) && objID != pool.ID { - additionalSubjects = append(additionalSubjects, pool.ID) - } - - if !slices.Contains(additionalSubjects, pool.OwnerID) { - additionalSubjects = append(additionalSubjects, pool.OwnerID) + for _, port := range addSubjPorts { + for _, pool := range port.Edges.Pools { + if !slices.Contains(additionalSubjects, pool.ID) { + additionalSubjects = append(additionalSubjects, pool.ID) + } + + if !slices.Contains(additionalSubjects, pool.OwnerID) { + additionalSubjects = append(additionalSubjects, pool.OwnerID) + } } - } - } - addSubjPorts, err := m.Client().Port.Query().Where(port.HasPoolsWith(pool.HasOriginsWith(origin.IDEQ(objID)))).All(ctx) - if err == nil { - for _, port := range addSubjPorts { if !slices.Contains(additionalSubjects, port.LoadBalancerID) { additionalSubjects = append(additionalSubjects, port.LoadBalancerID) } + + if !slices.Contains(additionalSubjects, port.Edges.LoadBalancer.LocationID) { + additionalSubjects = append(additionalSubjects, port.Edges.LoadBalancer.LocationID) + } } } @@ -586,18 +578,6 @@ func OriginHooks() []ent.Hook { } } - lbs := getLoadBalancerIDs(ctx, objID, additionalSubjects) - for _, lb := range lbs { - lb, err := m.Client().LoadBalancer.Get(ctx, lb) - if err != nil { - return nil, fmt.Errorf("failed to get loadbalancer to lookup location %s", lb) - } - - if !slices.Contains(additionalSubjects, lb.LocationID) { - additionalSubjects = append(additionalSubjects, lb.LocationID) - } - } - msg := events.ChangeMessage{ EventType: eventType(m.Op()), SubjectID: objID, From 7e364de028d3838dbc977012fbe6915120a777e1 Mon Sep 17 00:00:00 2001 From: Matt Siwiec Date: Tue, 24 Oct 2023 21:07:03 +0000 Subject: [PATCH 4/8] audit pool manual hook db hits Signed-off-by: Matt Siwiec --- internal/manualhooks/hooks.go | 67 ++++++++++++++---------------- internal/manualhooks/hooks_test.go | 10 ++--- 2 files changed, 37 insertions(+), 40 deletions(-) diff --git a/internal/manualhooks/hooks.go b/internal/manualhooks/hooks.go index 6cd9d8b39..8274304b0 100644 --- a/internal/manualhooks/hooks.go +++ b/internal/manualhooks/hooks.go @@ -486,6 +486,10 @@ func OriginHooks() []ent.Hook { msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.LocationID) } + if !slices.Contains(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.ProviderID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.ProviderID) + } + if !slices.Contains(msg.AdditionalSubjectIDs, port.LoadBalancerID) { msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.LoadBalancerID) } @@ -558,6 +562,10 @@ func OriginHooks() []ent.Hook { if !slices.Contains(additionalSubjects, port.Edges.LoadBalancer.LocationID) { additionalSubjects = append(additionalSubjects, port.Edges.LoadBalancer.LocationID) } + + if !slices.Contains(additionalSubjects, port.Edges.LoadBalancer.ProviderID) { + additionalSubjects = append(additionalSubjects, port.Edges.LoadBalancer.ProviderID) + } } } @@ -749,9 +757,19 @@ func PoolHooks() []ent.Hook { return retValue, err } - addSubjPorts, err := m.Client().Port.Query().Where(port.HasPoolsWith(pool.IDEQ(objID))).All(ctx) + addSubjPorts, err := m.Client().Port.Query().WithLoadBalancer().WithPools(func(q *generated.PoolQuery) { + q.WithOrigins() + }).Where(port.HasPoolsWith(pool.IDEQ(objID))).All(ctx) if err == nil { for _, port := range addSubjPorts { + for _, pool := range port.Edges.Pools { + for _, origin := range pool.Edges.Origins { + if !slices.Contains(msg.AdditionalSubjectIDs, origin.ID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, origin.ID) + } + } + } + if !slices.Contains(msg.AdditionalSubjectIDs, port.ID) && objID != port.ID { msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.ID) } @@ -759,34 +777,17 @@ func PoolHooks() []ent.Hook { if !slices.Contains(msg.AdditionalSubjectIDs, port.LoadBalancerID) { msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.LoadBalancerID) } - } - } - addSubjOrigins, err := m.Client().Origin.Query().Where(origin.HasPoolWith(pool.IDEQ(objID))).All(ctx) - if err == nil { - for _, origin := range addSubjOrigins { - if !slices.Contains(msg.AdditionalSubjectIDs, origin.ID) && objID != origin.ID { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, origin.ID) + if !slices.Contains(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.LocationID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.LocationID) } - if !slices.Contains(msg.AdditionalSubjectIDs, origin.PoolID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, origin.PoolID) + if !slices.Contains(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.ProviderID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.ProviderID) } } } - lbs := getLoadBalancerIDs(ctx, objID, msg.AdditionalSubjectIDs) - for _, lb := range lbs { - lb, err := m.Client().LoadBalancer.Get(ctx, lb) - if err != nil { - return nil, fmt.Errorf("failed to get loadbalancer to lookup location %s", lb) - } - - if !slices.Contains(msg.AdditionalSubjectIDs, lb.LocationID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.LocationID) - } - } - if len(relationships) != 0 { if err := permissions.CreateAuthRelationships(ctx, "load-balancer-pool", objID, relationships...); err != nil { return nil, fmt.Errorf("relationship request failed with error: %w", err) @@ -822,9 +823,17 @@ func PoolHooks() []ent.Hook { additionalSubjects = append(additionalSubjects, dbObj.OwnerID) - addSubjPorts, err := m.Client().Port.Query().Where(port.HasPoolsWith(pool.IDEQ(objID))).All(ctx) + addSubjPorts, err := m.Client().Port.Query().WithLoadBalancer().Where(port.HasPoolsWith(pool.IDEQ(objID))).All(ctx) if err == nil { for _, port := range addSubjPorts { + if !slices.Contains(additionalSubjects, port.Edges.LoadBalancer.LocationID) { + additionalSubjects = append(additionalSubjects, port.Edges.LoadBalancer.LocationID) + } + + if !slices.Contains(additionalSubjects, port.Edges.LoadBalancer.ProviderID) { + additionalSubjects = append(additionalSubjects, port.Edges.LoadBalancer.ProviderID) + } + if !slices.Contains(additionalSubjects, port.LoadBalancerID) { additionalSubjects = append(additionalSubjects, port.LoadBalancerID) } @@ -836,18 +845,6 @@ func PoolHooks() []ent.Hook { SubjectID: dbObj.OwnerID, }) - lbs := getLoadBalancerIDs(ctx, objID, additionalSubjects) - for _, lb := range lbs { - lb, err := m.Client().LoadBalancer.Get(ctx, lb) - if err != nil { - return nil, fmt.Errorf("failed to get loadbalancer to lookup location %s", lb) - } - - if !slices.Contains(additionalSubjects, lb.LocationID) { - additionalSubjects = append(additionalSubjects, lb.LocationID) - } - } - // we have all the info we need, now complete the mutation before we process the event retValue, err := next.Mutate(ctx, m) if err != nil { diff --git a/internal/manualhooks/hooks_test.go b/internal/manualhooks/hooks_test.go index e21819cea..be8ca1e3b 100644 --- a/internal/manualhooks/hooks_test.go +++ b/internal/manualhooks/hooks_test.go @@ -133,7 +133,7 @@ func Test_OriginCreateHook(t *testing.T) { msg := testutils.ChannelReceiveWithTimeout[events.Message[events.ChangeMessage]](t, changesChannel, defaultTimeout) // Assert - expectedAdditionalSubjectIDs := []gidx.PrefixedID{pool.ID, pool.OwnerID, lb.ID, lb.LocationID} + expectedAdditionalSubjectIDs := []gidx.PrefixedID{pool.ID, pool.OwnerID, lb.ID, lb.LocationID, lb.ProviderID} actualAdditionalSubjectIDs := msg.Message().AdditionalSubjectIDs assert.ElementsMatch(t, expectedAdditionalSubjectIDs, actualAdditionalSubjectIDs) @@ -161,7 +161,7 @@ func Test_OriginUpdateHook(t *testing.T) { msg := testutils.ChannelReceiveWithTimeout[events.Message[events.ChangeMessage]](t, changesChannel, defaultTimeout) // Assert - expectedAdditionalSubjectIDs := []gidx.PrefixedID{pool.ID, pool.OwnerID, lb.ID, lb.LocationID} + expectedAdditionalSubjectIDs := []gidx.PrefixedID{pool.ID, pool.OwnerID, lb.ID, lb.LocationID, lb.ProviderID} actualAdditionalSubjectIDs := msg.Message().AdditionalSubjectIDs assert.ElementsMatch(t, expectedAdditionalSubjectIDs, actualAdditionalSubjectIDs) @@ -189,7 +189,7 @@ func Test_OriginDeleteHook(t *testing.T) { msg := testutils.ChannelReceiveWithTimeout[events.Message[events.ChangeMessage]](t, changesChannel, defaultTimeout) // Assert - expectedAdditionalSubjectIDs := []gidx.PrefixedID{pool.ID, pool.OwnerID, lb.ID, lb.LocationID} + expectedAdditionalSubjectIDs := []gidx.PrefixedID{pool.ID, pool.OwnerID, lb.ID, lb.LocationID, lb.ProviderID} actualAdditionalSubjectIDs := msg.Message().AdditionalSubjectIDs assert.ElementsMatch(t, expectedAdditionalSubjectIDs, actualAdditionalSubjectIDs) @@ -240,7 +240,7 @@ func Test_PoolUpdateHook(t *testing.T) { msg := testutils.ChannelReceiveWithTimeout[events.Message[events.ChangeMessage]](t, changesChannel, defaultTimeout) // Assert - expectedAdditionalSubjectIDs := []gidx.PrefixedID{pool.ID, pool.OwnerID, lb.ID, lb.LocationID, origin.ID, port.ID} + expectedAdditionalSubjectIDs := []gidx.PrefixedID{pool.OwnerID, lb.ID, lb.LocationID, lb.ProviderID, origin.ID, port.ID} actualAdditionalSubjectIDs := msg.Message().AdditionalSubjectIDs assert.ElementsMatch(t, expectedAdditionalSubjectIDs, actualAdditionalSubjectIDs) @@ -267,7 +267,7 @@ func Test_PoolDeleteHook(t *testing.T) { msg := testutils.ChannelReceiveWithTimeout[events.Message[events.ChangeMessage]](t, changesChannel, defaultTimeout) // Assert - expectedAdditionalSubjectIDs := []gidx.PrefixedID{pool.OwnerID, lb.ID, lb.LocationID} + expectedAdditionalSubjectIDs := []gidx.PrefixedID{pool.OwnerID, lb.ID, lb.LocationID, lb.ProviderID} actualAdditionalSubjectIDs := msg.Message().AdditionalSubjectIDs assert.ElementsMatch(t, expectedAdditionalSubjectIDs, actualAdditionalSubjectIDs) From 7749151b49bc6182c7a114e49fb65c0ffb216ecc Mon Sep 17 00:00:00 2001 From: Matt Siwiec Date: Tue, 24 Oct 2023 22:55:12 +0000 Subject: [PATCH 5/8] audit port manual hook db hits Signed-off-by: Matt Siwiec --- internal/manualhooks/hooks.go | 92 +++++++++--------------------- internal/manualhooks/hooks_test.go | 18 +++--- 2 files changed, 37 insertions(+), 73 deletions(-) diff --git a/internal/manualhooks/hooks.go b/internal/manualhooks/hooks.go index 8274304b0..289115627 100644 --- a/internal/manualhooks/hooks.go +++ b/internal/manualhooks/hooks.go @@ -1014,48 +1014,37 @@ func PortHooks() []ent.Hook { return retValue, err } - addSubjPools, err := m.Client().Pool.Query().Where(pool.HasPortsWith(port.IDEQ(objID))).All(ctx) + // Ensure we have additional relevant subjects in the event msg + addSubjPools, err := m.Client().Pool.Query().WithPorts(func(q *generated.PortQuery) { + q.WithLoadBalancer() + }).Where(pool.HasPortsWith(port.IDEQ(objID))).All(ctx) if err == nil { for _, pool := range addSubjPools { - if !slices.Contains(msg.AdditionalSubjectIDs, pool.ID) && objID != pool.ID { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, pool.ID) - } + for _, port := range pool.Edges.Ports { + if !slices.Contains(msg.AdditionalSubjectIDs, port.LoadBalancerID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.LoadBalancerID) + } - if !slices.Contains(msg.AdditionalSubjectIDs, pool.OwnerID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, pool.OwnerID) - } - } - } - addSubjLoadBalancers, err := m.Client().LoadBalancer.Query().Where(loadbalancer.HasPortsWith(port.IDEQ(objID))).All(ctx) - if err == nil { - for _, lb := range addSubjLoadBalancers { - if !slices.Contains(msg.AdditionalSubjectIDs, lb.ID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.ID) - } + if !slices.Contains(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.LocationID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.LocationID) + } - if !slices.Contains(msg.AdditionalSubjectIDs, lb.LocationID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.LocationID) - } + if !slices.Contains(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.ProviderID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.ProviderID) + } - if !slices.Contains(msg.AdditionalSubjectIDs, lb.OwnerID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.OwnerID) + if !slices.Contains(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.OwnerID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, port.Edges.LoadBalancer.OwnerID) + } } - if !slices.Contains(msg.AdditionalSubjectIDs, lb.ProviderID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.ProviderID) + if !slices.Contains(msg.AdditionalSubjectIDs, pool.ID) && objID != pool.ID { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, pool.ID) } - } - } - lbs := getLoadBalancerIDs(ctx, objID, msg.AdditionalSubjectIDs) - for _, lb := range lbs { - lb, err := m.Client().LoadBalancer.Get(ctx, lb) - if err != nil { - return nil, fmt.Errorf("failed to get loadbalancer to lookup location %s", lb) - } - - if !slices.Contains(msg.AdditionalSubjectIDs, lb.LocationID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.LocationID) + if !slices.Contains(msg.AdditionalSubjectIDs, pool.OwnerID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, pool.OwnerID) + } } } @@ -1087,12 +1076,16 @@ func PortHooks() []ent.Hook { return nil, fmt.Errorf("object doesn't have an id %s", objID) } - dbObj, err := m.Client().Port.Get(ctx, objID) + dbObj, err := m.Client().Port.Query().WithLoadBalancer().Where(port.IDEQ(objID)).Only(ctx) if err != nil { return nil, fmt.Errorf("failed to load object to get values for event, err %w", err) } + // Ensure we have additional relevant subjects in the event msg additionalSubjects = append(additionalSubjects, dbObj.LoadBalancerID) + additionalSubjects = append(additionalSubjects, dbObj.Edges.LoadBalancer.LocationID) + additionalSubjects = append(additionalSubjects, dbObj.Edges.LoadBalancer.OwnerID) + additionalSubjects = append(additionalSubjects, dbObj.Edges.LoadBalancer.ProviderID) // we have all the info we need, now complete the mutation before we process the event retValue, err := next.Mutate(ctx, m) @@ -1106,37 +1099,6 @@ func PortHooks() []ent.Hook { } } - addSubjLoadBalancer, err := m.Client().LoadBalancer.Get(ctx, dbObj.LoadBalancerID) - if err == nil { - if !slices.Contains(additionalSubjects, addSubjLoadBalancer.ID) { - additionalSubjects = append(additionalSubjects, addSubjLoadBalancer.ID) - } - - if !slices.Contains(additionalSubjects, addSubjLoadBalancer.LocationID) { - additionalSubjects = append(additionalSubjects, addSubjLoadBalancer.LocationID) - } - - if !slices.Contains(additionalSubjects, addSubjLoadBalancer.OwnerID) { - additionalSubjects = append(additionalSubjects, addSubjLoadBalancer.OwnerID) - } - - if !slices.Contains(additionalSubjects, addSubjLoadBalancer.ProviderID) { - additionalSubjects = append(additionalSubjects, addSubjLoadBalancer.ProviderID) - } - } - - lbs := getLoadBalancerIDs(ctx, objID, additionalSubjects) - for _, lb := range lbs { - lb, err := m.Client().LoadBalancer.Get(ctx, lb) - if err != nil { - return nil, fmt.Errorf("failed to get loadbalancer to lookup location %s", lb) - } - - if !slices.Contains(additionalSubjects, lb.LocationID) { - additionalSubjects = append(additionalSubjects, lb.LocationID) - } - } - msg := events.ChangeMessage{ EventType: eventType(m.Op()), SubjectID: objID, diff --git a/internal/manualhooks/hooks_test.go b/internal/manualhooks/hooks_test.go index be8ca1e3b..4f3ae162f 100644 --- a/internal/manualhooks/hooks_test.go +++ b/internal/manualhooks/hooks_test.go @@ -365,9 +365,9 @@ func Test_MultipleLoadbalancersSharedPoolAddOrigin(t *testing.T) { // create 2 loadbalancers with a shared pool of origins prov := (&testutils.ProviderBuilder{}).MustNew(ctx) - lb1 := (&testutils.LoadBalancerBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) - lb2 := (&testutils.LoadBalancerBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) - pool := (&testutils.PoolBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) + lb1 := (&testutils.LoadBalancerBuilder{OwnerID: "tnttent-testing", Provider: prov}).MustNew(ctx) + lb2 := (&testutils.LoadBalancerBuilder{OwnerID: "tnttent-testing", Provider: prov}).MustNew(ctx) + pool := (&testutils.PoolBuilder{OwnerID: "tnttent-testing"}).MustNew(ctx) _ = (&testutils.PortBuilder{PoolIDs: []gidx.PrefixedID{pool.ID}, LoadBalancerID: lb1.ID}).MustNew(ctx) _ = (&testutils.PortBuilder{PoolIDs: []gidx.PrefixedID{pool.ID}, LoadBalancerID: lb2.ID}).MustNew(ctx) _ = (&testutils.OriginBuilder{PoolID: pool.ID}).MustNew(ctx) @@ -384,7 +384,8 @@ func Test_MultipleLoadbalancersSharedPoolAddOrigin(t *testing.T) { // Assert expectedAdditionalSubjectIDs := []gidx.PrefixedID{ - prov.OwnerID, + prov.ID, + lb1.OwnerID, lb1.ID, lb2.ID, lb1.LocationID, @@ -408,9 +409,9 @@ func Test_MultipleLoadbalancersSharedPoolDeleteOrigin(t *testing.T) { // create 2 loadbalancers with a shared pool of origins prov := (&testutils.ProviderBuilder{}).MustNew(ctx) - lb1 := (&testutils.LoadBalancerBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) - lb2 := (&testutils.LoadBalancerBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) - pool := (&testutils.PoolBuilder{OwnerID: prov.OwnerID}).MustNew(ctx) + lb1 := (&testutils.LoadBalancerBuilder{OwnerID: "tnttent-testing", Provider: prov}).MustNew(ctx) + lb2 := (&testutils.LoadBalancerBuilder{OwnerID: "tnttent-testing", Provider: prov}).MustNew(ctx) + pool := (&testutils.PoolBuilder{OwnerID: "tnttent-testing"}).MustNew(ctx) _ = (&testutils.PortBuilder{PoolIDs: []gidx.PrefixedID{pool.ID}, LoadBalancerID: lb1.ID}).MustNew(ctx) _ = (&testutils.PortBuilder{PoolIDs: []gidx.PrefixedID{pool.ID}, LoadBalancerID: lb2.ID}).MustNew(ctx) _ = (&testutils.OriginBuilder{PoolID: pool.ID}).MustNew(ctx) @@ -428,7 +429,8 @@ func Test_MultipleLoadbalancersSharedPoolDeleteOrigin(t *testing.T) { // Assert expectedAdditionalSubjectIDs := []gidx.PrefixedID{ - prov.OwnerID, + prov.ID, + lb1.OwnerID, lb1.ID, lb2.ID, lb1.LocationID, From d44500eb92a3366bdd794034730d837a129b8e6d Mon Sep 17 00:00:00 2001 From: Matt Siwiec Date: Tue, 24 Oct 2023 23:16:28 +0000 Subject: [PATCH 6/8] bump test event msg channel timeout to 5s; align Signed-off-by: Matt Siwiec --- internal/manualhooks/hooks.go | 26 ++++++++++++++------------ internal/manualhooks/hooks_test.go | 2 +- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/internal/manualhooks/hooks.go b/internal/manualhooks/hooks.go index 289115627..e770b094f 100644 --- a/internal/manualhooks/hooks.go +++ b/internal/manualhooks/hooks.go @@ -198,20 +198,20 @@ func LoadBalancerHooks() []ent.Hook { // Ensure we have additional relevant subjects in the msg lb, err := m.Client().LoadBalancer.Query().WithPorts().Where(loadbalancer.IDEQ(objID)).Only(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get loadbalancer to lookup additional subject ids %s: %w", lb, err) - } - - if !slices.Contains(msg.AdditionalSubjectIDs, lb.LocationID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.LocationID) - } + if err == nil { + if !slices.Contains(msg.AdditionalSubjectIDs, lb.LocationID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.LocationID) + } - if !slices.Contains(msg.AdditionalSubjectIDs, lb.ProviderID) { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.ProviderID) - } + if !slices.Contains(msg.AdditionalSubjectIDs, lb.ProviderID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, lb.ProviderID) + } - for _, p := range lb.Edges.Ports { - msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, p.ID) + for _, p := range lb.Edges.Ports { + if !slices.Contains(msg.AdditionalSubjectIDs, p.ID) { + msg.AdditionalSubjectIDs = append(msg.AdditionalSubjectIDs, p.ID) + } + } } if len(relationships) != 0 { @@ -757,6 +757,7 @@ func PoolHooks() []ent.Hook { return retValue, err } + // Ensure we have additional relevant subjects in the msg addSubjPorts, err := m.Client().Port.Query().WithLoadBalancer().WithPools(func(q *generated.PoolQuery) { q.WithOrigins() }).Where(port.HasPoolsWith(pool.IDEQ(objID))).All(ctx) @@ -823,6 +824,7 @@ func PoolHooks() []ent.Hook { additionalSubjects = append(additionalSubjects, dbObj.OwnerID) + // Ensure we have additional relevant subjects in the msg addSubjPorts, err := m.Client().Port.Query().WithLoadBalancer().Where(port.HasPoolsWith(pool.IDEQ(objID))).All(ctx) if err == nil { for _, port := range addSubjPorts { diff --git a/internal/manualhooks/hooks_test.go b/internal/manualhooks/hooks_test.go index 4f3ae162f..803851245 100644 --- a/internal/manualhooks/hooks_test.go +++ b/internal/manualhooks/hooks_test.go @@ -18,7 +18,7 @@ import ( const ( ownerPrefix = "testown" locationPrefix = "testloc" - defaultTimeout = 2 * time.Second + defaultTimeout = 5 * time.Second ) var ( From a48a3ed2361b4eb5f5bced00875c81870a4b9f8c Mon Sep 17 00:00:00 2001 From: Matt Siwiec Date: Thu, 26 Oct 2023 17:48:10 +0000 Subject: [PATCH 7/8] test with one retry Signed-off-by: Matt Siwiec --- .github/workflows/test-go.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-go.yml b/.github/workflows/test-go.yml index 2dbedfc45..731d74fad 100644 --- a/.github/workflows/test-go.yml +++ b/.github/workflows/test-go.yml @@ -40,5 +40,8 @@ jobs: - name: Install atlas for db migrations on ${{ matrix.ci-database }} run: go install ariga.io/atlas/cmd/atlas@latest + # with one retry - name: Run go tests for ${{ matrix.ci-database }} - run: LOADBALANCERAPI_TESTDB_URI="${{ matrix.env-database-uri }}" go test -race -coverprofile=coverage.txt -covermode=atomic -tags testtools ./... + run: | + LOADBALANCERAPI_TESTDB_URI="${{ matrix.env-database-uri }}" go test -race -coverprofile=coverage.txt -covermode=atomic -tags testtools ./... || \ + LOADBALANCERAPI_TESTDB_URI="${{ matrix.env-database-uri }}" go test -race -coverprofile=coverage.txt -covermode=atomic -tags testtools ./... From 7afd8701e35c43f1b2b92a78e8e29daac2a0473e Mon Sep 17 00:00:00 2001 From: Matt Siwiec Date: Thu, 26 Oct 2023 19:09:56 +0000 Subject: [PATCH 8/8] tweek flake Signed-off-by: Matt Siwiec --- internal/manualhooks/hooks_test.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/internal/manualhooks/hooks_test.go b/internal/manualhooks/hooks_test.go index 803851245..4b6a9020b 100644 --- a/internal/manualhooks/hooks_test.go +++ b/internal/manualhooks/hooks_test.go @@ -363,6 +363,9 @@ func Test_MultipleLoadbalancersSharedPoolAddOrigin(t *testing.T) { // Arrange ctx := testutils.MockPermissions(context.Background()) + changesChannel, err := testutils.EventsConn.SubscribeChanges(ctx, "create.load-balancer-origin") + require.NoError(t, err, "failed to subscribe to changes") + // create 2 loadbalancers with a shared pool of origins prov := (&testutils.ProviderBuilder{}).MustNew(ctx) lb1 := (&testutils.LoadBalancerBuilder{OwnerID: "tnttent-testing", Provider: prov}).MustNew(ctx) @@ -370,15 +373,11 @@ func Test_MultipleLoadbalancersSharedPoolAddOrigin(t *testing.T) { pool := (&testutils.PoolBuilder{OwnerID: "tnttent-testing"}).MustNew(ctx) _ = (&testutils.PortBuilder{PoolIDs: []gidx.PrefixedID{pool.ID}, LoadBalancerID: lb1.ID}).MustNew(ctx) _ = (&testutils.PortBuilder{PoolIDs: []gidx.PrefixedID{pool.ID}, LoadBalancerID: lb2.ID}).MustNew(ctx) - _ = (&testutils.OriginBuilder{PoolID: pool.ID}).MustNew(ctx) testutils.EntClient.Origin.Use(manualhooks.OriginHooks()...) - changesChannel, err := testutils.EventsConn.SubscribeChanges(ctx, "create.load-balancer-origin") - require.NoError(t, err, "failed to subscribe to changes") - // Act - add another origin to the pool - ogn2 := (&testutils.OriginBuilder{PoolID: pool.ID}).MustNew(ctx) + ogn := (&testutils.OriginBuilder{PoolID: pool.ID}).MustNew(ctx) msg := testutils.ChannelReceiveWithTimeout[events.Message[events.ChangeMessage]](t, changesChannel, defaultTimeout) @@ -395,7 +394,7 @@ func Test_MultipleLoadbalancersSharedPoolAddOrigin(t *testing.T) { actualAdditionalSubjectIDs := msg.Message().AdditionalSubjectIDs assert.ElementsMatch(t, expectedAdditionalSubjectIDs, actualAdditionalSubjectIDs) - assert.Equal(t, ogn2.ID, msg.Message().SubjectID) + assert.Equal(t, ogn.ID, msg.Message().SubjectID) assert.Equal(t, createEventType, msg.Message().EventType) } @@ -407,6 +406,9 @@ func Test_MultipleLoadbalancersSharedPoolDeleteOrigin(t *testing.T) { // Arrange ctx := testutils.MockPermissions(context.Background()) + changesChannel, err := testutils.EventsConn.SubscribeChanges(ctx, "delete.load-balancer-origin") + require.NoError(t, err, "failed to subscribe to changes") + // create 2 loadbalancers with a shared pool of origins prov := (&testutils.ProviderBuilder{}).MustNew(ctx) lb1 := (&testutils.LoadBalancerBuilder{OwnerID: "tnttent-testing", Provider: prov}).MustNew(ctx) @@ -419,9 +421,6 @@ func Test_MultipleLoadbalancersSharedPoolDeleteOrigin(t *testing.T) { testutils.EntClient.Origin.Use(manualhooks.OriginHooks()...) - changesChannel, err := testutils.EventsConn.SubscribeChanges(ctx, "delete.load-balancer-origin") - require.NoError(t, err, "failed to subscribe to changes") - // Act - update the pool to remove an origin testutils.EntClient.Origin.DeleteOne(ogn2).ExecX(ctx)