Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

Commit

Permalink
Security update golang.org/x/net and Tidy code (#192)
Browse files Browse the repository at this point in the history
* Improve unit test.

* Fix ratelimit counter metric for global.

* Remove unused code.

* Simplify rate limit counter.

* Remove unused context deadline logging.

* Bump golang.org/x/net from 0.10.0 to 0.17.0

Bumps [golang.org/x/net](https://github.com/golang/net) from 0.10.0 to 0.17.0.
- [Commits](golang/net@v0.10.0...v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
  • Loading branch information
Baliedge and dependabot[bot] authored Oct 13, 2023
1 parent e547a49 commit 965892e
Show file tree
Hide file tree
Showing 12 changed files with 123 additions and 136 deletions.
3 changes: 1 addition & 2 deletions cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

gubernator "github.com/mailgun/gubernator/v2"
"github.com/mailgun/holster/v4/clock"
"github.com/mailgun/holster/v4/ctxutil"
"github.com/mailgun/holster/v4/errors"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -110,7 +109,7 @@ func Restart(ctx context.Context) error {
// StartWith a local cluster with specific addresses
func StartWith(localPeers []gubernator.PeerInfo) error {
for _, peer := range localPeers {
ctx, cancel := ctxutil.WithTimeout(context.Background(), clock.Second*10)
ctx, cancel := context.WithTimeout(context.Background(), clock.Second*10)
d, err := gubernator.SpawnDaemon(ctx, gubernator.DaemonConfig{
Logger: logrus.WithField("instance", peer.GRPCAddress),
GRPCListenAddress: peer.GRPCAddress,
Expand Down
3 changes: 1 addition & 2 deletions cmd/gubernator-cli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/davecgh/go-spew/spew"
guber "github.com/mailgun/gubernator/v2"
"github.com/mailgun/holster/v4/clock"
"github.com/mailgun/holster/v4/ctxutil"
"github.com/mailgun/holster/v4/errors"
"github.com/mailgun/holster/v4/setter"
"github.com/mailgun/holster/v4/syncutil"
Expand Down Expand Up @@ -179,7 +178,7 @@ func sendRequest(ctx context.Context, client guber.V1Client, req *guber.GetRateL
ctx = tracing.StartScope(ctx)
defer tracing.EndScope(ctx, nil)

ctx, cancel := ctxutil.WithTimeout(ctx, timeout)
ctx, cancel := context.WithTimeout(ctx, timeout)

// Now hit our cluster with the rate limits
resp, err := client.GetRateLimits(ctx, req)
Expand Down
3 changes: 1 addition & 2 deletions cmd/gubernator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (

gubernator "github.com/mailgun/gubernator/v2"
"github.com/mailgun/holster/v4/clock"
"github.com/mailgun/holster/v4/ctxutil"
"github.com/mailgun/holster/v4/tracing"
"github.com/sirupsen/logrus"
"go.opentelemetry.io/otel/sdk/resource"
Expand Down Expand Up @@ -78,7 +77,7 @@ func main() {
conf, err := gubernator.SetupDaemonConfig(logrus.StandardLogger(), configFile)
checkErr(err, "while getting config")

ctx, cancel := ctxutil.WithTimeout(ctx, clock.Second*10)
ctx, cancel := context.WithTimeout(ctx, clock.Second*10)

// Start the daemon
daemon, err := gubernator.SpawnDaemon(ctx, conf)
Expand Down
2 changes: 1 addition & 1 deletion config.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (c *Config) SetDefaults() error {

setter.SetDefault(&c.Behaviors.GlobalTimeout, time.Millisecond*500)
setter.SetDefault(&c.Behaviors.GlobalBatchLimit, maxBatchSize)
setter.SetDefault(&c.Behaviors.GlobalSyncWait, time.Millisecond*500)
setter.SetDefault(&c.Behaviors.GlobalSyncWait, time.Millisecond*100)

setter.SetDefault(&c.LocalPicker, NewReplicatedConsistentHash(nil, defaultReplicas))
setter.SetDefault(&c.RegionPicker, NewRegionPicker(nil))
Expand Down
7 changes: 3 additions & 4 deletions etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"encoding/json"

"github.com/mailgun/holster/v4/clock"
"github.com/mailgun/holster/v4/ctxutil"
"github.com/mailgun/holster/v4/errors"
"github.com/mailgun/holster/v4/setter"
"github.com/mailgun/holster/v4/syncutil"
Expand Down Expand Up @@ -139,7 +138,7 @@ func (e *EtcdPool) watchPeers() error {
}

func (e *EtcdPool) collectPeers(revision *int64) error {
ctx, cancel := ctxutil.WithTimeout(e.ctx, etcdTimeout)
ctx, cancel := context.WithTimeout(e.ctx, etcdTimeout)
defer cancel()

resp, err := e.conf.Client.Get(ctx, e.conf.KeyPrefix, etcd.WithPrefix())
Expand Down Expand Up @@ -232,7 +231,7 @@ func (e *EtcdPool) register(peer PeerInfo) error {
var lease *etcd.LeaseGrantResponse

register := func() error {
ctx, cancel := ctxutil.WithTimeout(e.ctx, etcdTimeout)
ctx, cancel := context.WithTimeout(e.ctx, etcdTimeout)
defer cancel()
var err error

Expand Down Expand Up @@ -296,7 +295,7 @@ func (e *EtcdPool) register(peer PeerInfo) error {
}
lastKeepAlive = clock.Now()
case <-done:
ctx, cancel := ctxutil.WithTimeout(context.Background(), etcdTimeout)
ctx, cancel := context.WithTimeout(context.Background(), etcdTimeout)
if _, err := e.conf.Client.Delete(ctx, instanceKey); err != nil {
e.log.WithError(err).
Warn("during etcd delete")
Expand Down
38 changes: 19 additions & 19 deletions functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -842,30 +842,30 @@ func TestGlobalRateLimits(t *testing.T) {
testutil.UntilPass(t, 20, clock.Millisecond*200, func(t testutil.TestingT) {
// Inspect our metrics, ensure they collected the counts we expected during this test
d := cluster.DaemonAt(0)
config := d.Config()
resp, err := http.Get(fmt.Sprintf("http://%s/metrics", config.HTTPListenAddress))
if !assert.NoError(t, err) {
return
}
defer resp.Body.Close()

m := getMetric(t, resp.Body, "gubernator_async_durations_count")
assert.NotEqual(t, 0, int(m.Value))

// V1Instance 2 should be the owner of our global rate limit
d = cluster.DaemonAt(2)
config = d.Config()
resp, err = http.Get(fmt.Sprintf("http://%s/metrics", config.HTTPListenAddress))
if !assert.NoError(t, err) {
return
metricsURL := fmt.Sprintf("http://%s/metrics", d.Config().HTTPListenAddress)
m := getMetricRequest(t, metricsURL, "gubernator_global_send_duration_count")
assert.Equal(t, 1, int(m.Value))

// Expect one peer (the owning peer) to indicate a broadcast.
var broadcastCount int
for i := 0; i < cluster.NumOfDaemons(); i++ {
d := cluster.DaemonAt(i)
metricsURL := fmt.Sprintf("http://%s/metrics", d.Config().HTTPListenAddress)
m := getMetricRequest(t, metricsURL, "gubernator_broadcast_duration_count")
broadcastCount += int(m.Value)
}
defer resp.Body.Close()

m = getMetric(t, resp.Body, "gubernator_broadcast_durations_count")
assert.NotEqual(t, 0, int(m.Value))
assert.Equal(t, 1, broadcastCount)
})
}

func getMetricRequest(t testutil.TestingT, url string, name string) *model.Sample {
resp, err := http.Get(url)
require.NoError(t, err)
defer resp.Body.Close()
return getMetric(t, resp.Body, name)
}

func TestChangeLimit(t *testing.T) {
client, errs := guber.DialV1Server(cluster.GetRandomPeer(cluster.DataCenterNone).GRPCAddress, nil)
require.Nil(t, errs)
Expand Down
51 changes: 36 additions & 15 deletions global.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package gubernator
import (
"context"

"github.com/mailgun/holster/v4/ctxutil"
"github.com/mailgun/holster/v4/syncutil"
"github.com/prometheus/client_golang/prometheus"
"google.golang.org/protobuf/proto"
Expand All @@ -28,12 +27,16 @@ import (
// globalManager manages async hit queue and updates peers in
// the cluster periodically when a global rate limit we own updates.
type globalManager struct {
asyncQueue chan *RateLimitReq
broadcastQueue chan *RateLimitReq
wg syncutil.WaitGroup
conf BehaviorConfig
log FieldLogger
instance *V1Instance
asyncQueue chan *RateLimitReq
broadcastQueue chan *RateLimitReq
wg syncutil.WaitGroup
conf BehaviorConfig
log FieldLogger
instance *V1Instance
metricGlobalSendDuration prometheus.Summary
metricBroadcastDuration prometheus.Summary
metricBroadcastCounter *prometheus.CounterVec
metricGlobalQueueLength prometheus.Gauge
}

func newGlobalManager(conf BehaviorConfig, instance *V1Instance) *globalManager {
Expand All @@ -43,6 +46,24 @@ func newGlobalManager(conf BehaviorConfig, instance *V1Instance) *globalManager
broadcastQueue: make(chan *RateLimitReq, conf.GlobalBatchLimit),
instance: instance,
conf: conf,
metricGlobalSendDuration: prometheus.NewSummary(prometheus.SummaryOpts{
Name: "gubernator_global_send_duration",
Help: "The duration of GLOBAL async sends in seconds.",
Objectives: map[float64]float64{0.5: 0.05, 0.99: 0.001},
}),
metricBroadcastDuration: prometheus.NewSummary(prometheus.SummaryOpts{
Name: "gubernator_broadcast_duration",
Help: "The duration of GLOBAL broadcasts to peers in seconds.",
Objectives: map[float64]float64{0.5: 0.05, 0.99: 0.001},
}),
metricBroadcastCounter: prometheus.NewCounterVec(prometheus.CounterOpts{
Name: "gubernator_broadcast_counter",
Help: "The count of broadcasts.",
}, []string{"condition"}),
metricGlobalQueueLength: prometheus.NewGauge(prometheus.GaugeOpts{
Name: "gubernator_global_queue_length",
Help: "The count of requests queued up for global broadcast. This is only used for GetRateLimit requests using global behavior.",
}),
}
gm.runAsyncHits()
gm.runBroadcasts()
Expand Down Expand Up @@ -108,7 +129,7 @@ func (gm *globalManager) sendHits(hits map[string]*RateLimitReq) {
client *PeerClient
req GetPeerRateLimitsReq
}
defer prometheus.NewTimer(metricGlobalSendDuration).ObserveDuration()
defer prometheus.NewTimer(gm.metricGlobalSendDuration).ObserveDuration()
peerRequests := make(map[string]*pair)

// Assign each request to a peer
Expand All @@ -132,7 +153,7 @@ func (gm *globalManager) sendHits(hits map[string]*RateLimitReq) {

// Send the rate limit requests to their respective owning peers.
for _, p := range peerRequests {
ctx, cancel := ctxutil.WithTimeout(context.Background(), gm.conf.GlobalTimeout)
ctx, cancel := context.WithTimeout(context.Background(), gm.conf.GlobalTimeout)
_, err := p.client.GetPeerRateLimits(ctx, &p.req)
cancel()

Expand All @@ -156,7 +177,7 @@ func (gm *globalManager) runBroadcasts() {

// Send the hits if we reached our batch limit
if len(updates) >= gm.conf.GlobalBatchLimit {
metricBroadcastCounter.WithLabelValues("queue_full").Inc()
gm.metricBroadcastCounter.WithLabelValues("queue_full").Inc()
gm.broadcastPeers(context.Background(), updates)
updates = make(map[string]*RateLimitReq)
return true
Expand All @@ -170,11 +191,11 @@ func (gm *globalManager) runBroadcasts() {

case <-interval.C:
if len(updates) != 0 {
metricBroadcastCounter.WithLabelValues("timer").Inc()
gm.metricBroadcastCounter.WithLabelValues("timer").Inc()
gm.broadcastPeers(context.Background(), updates)
updates = make(map[string]*RateLimitReq)
} else {
metricGlobalQueueLength.Set(0)
gm.metricGlobalQueueLength.Set(0)
}
case <-done:
return false
Expand All @@ -185,10 +206,10 @@ func (gm *globalManager) runBroadcasts() {

// broadcastPeers broadcasts global rate limit statuses to all other peers
func (gm *globalManager) broadcastPeers(ctx context.Context, updates map[string]*RateLimitReq) {
defer prometheus.NewTimer(metricBroadcastDuration).ObserveDuration()
defer prometheus.NewTimer(gm.metricBroadcastDuration).ObserveDuration()
var req UpdatePeerGlobalsReq

metricGlobalQueueLength.Set(float64(len(updates)))
gm.metricGlobalQueueLength.Set(float64(len(updates)))

for _, r := range updates {
// Copy the original since we are removing the GLOBAL behavior
Expand Down Expand Up @@ -217,7 +238,7 @@ func (gm *globalManager) broadcastPeers(ctx context.Context, updates map[string]
continue
}

ctx, cancel := ctxutil.WithTimeout(ctx, gm.conf.GlobalTimeout)
ctx, cancel := context.WithTimeout(ctx, gm.conf.GlobalTimeout)
_, err := peer.UpdatePeerGlobals(ctx, &req)
cancel()

Expand Down
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ require (
go.opentelemetry.io/otel v1.16.0
go.opentelemetry.io/otel/sdk v1.16.0
go.opentelemetry.io/otel/trace v1.16.0
golang.org/x/net v0.10.0
golang.org/x/net v0.17.0
golang.org/x/time v0.3.0
google.golang.org/api v0.108.0
google.golang.org/genproto v0.0.0-20230306155012-7f2fa6fef1f4
Expand Down Expand Up @@ -90,9 +90,9 @@ require (
go.uber.org/zap v1.21.0 // indirect
golang.org/x/mod v0.8.0 // indirect
golang.org/x/oauth2 v0.6.0 // indirect
golang.org/x/sys v0.8.0 // indirect
golang.org/x/term v0.8.0 // indirect
golang.org/x/text v0.9.0 // indirect
golang.org/x/sys v0.13.0 // indirect
golang.org/x/term v0.13.0 // indirect
golang.org/x/text v0.13.0 // indirect
golang.org/x/tools v0.6.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
Expand Down
16 changes: 8 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@ golang.org/x/net v0.0.0-20210726213435-c6fcb2dbf985/go.mod h1:9nx3DQGgdP8bBQD5qx
golang.org/x/net v0.0.0-20211209124913-491a49abca63/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
golang.org/x/net v0.10.0 h1:X2//UzNDwYmtCLn7To6G58Wr6f5ahEAQgKNzv9Y951M=
golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM=
golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
Expand Down Expand Up @@ -664,13 +664,13 @@ golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU=
golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE=
golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210615171337-6886f2dfbf5b/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.8.0 h1:n5xxQn2i3PC0yLAbjTpNT85q/Kgzcr2gIoX9OrJUols=
golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo=
golang.org/x/term v0.13.0 h1:bb+I9cTfFazGW51MZqBVmZy7+JEJMouUHTUSKVQLBek=
golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand All @@ -680,8 +680,8 @@ golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.9.0 h1:2sjJmO8cDvYveuX97RDLsxlyUxLl+GHoLxBiRdHllBE=
golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k=
golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
Expand Down
Loading

0 comments on commit 965892e

Please sign in to comment.