Prepare v1.12.3 release (#8217)

Signed-off-by: Johan Fylling <johan.dev@fylling.se>

CHANGELOG.md

@@ -3,6 +3,23 @@
 All notable changes to this project will be documented in this file. This
 project adheres to [Semantic Versioning](http://semver.org/).
 
+## 1.12.3
+
+This is a bug fix release addressing two issues:
+
+###  Bundle polling is being misconfigured when discovery bundle is updated ([#8215](https://github.com/open-policy-agent/opa/issues/8215))
+
+This is an issue where the polling interval for discovery (`discovery.polling.min_delay_seconds` and `discovery.polling.max_delay_seconds`) were misinterpreted on reconfiguration, causing extremely long update intervals.
+
+Reported by @loganmiller-chime, authored by @sspaink
+
+### Decision log `size` buffer `buffer_size_limit_bytes` misconfigured during reconfiguration ([#8213](https://github.com/open-policy-agent/opa/pull/8213))
+
+This is a regression in the decision log, where the `decision_logs.reporting.buffer_size_limit_bytes` was mistakenly assigned the value of `decision_logs.reporting.upload_size_limit_bytes` during reconfiguration.
+This issue is only present when `decision_logs.reporting.buffer_type` is set to `size`, which is the default value.
+
+Authored by @sspaink
+
 ## 1.12.2
 
 This bug fix release address issues found in the new string interpolation feature

v1/download/config.go

@@ -25,8 +25,10 @@ const (
 
 // PollingConfig represents polling configuration for the downloader.
 type PollingConfig struct {
-	MinDelaySeconds           *int64 `json:"min_delay_seconds,omitempty"`            // min amount of time to wait between successful poll attempts
+	MinDelaySeconds           *int64 `json:"min_delay_seconds,omitempty"` // min amount of time to wait between successful poll attempts, represents the user provided value
-	MaxDelaySeconds           *int64 `json:"max_delay_seconds,omitempty"`            // max amount of time to wait between poll attempts
+	MaxDelaySeconds           *int64 `json:"max_delay_seconds,omitempty"` // max amount of time to wait between poll attempts, represents the user provided value
+	parsedMinDelaySeconds     *int64 // nanosecond resolution of MinDelaySeconds
+	parsedMaxDelaySeconds     *int64 // nanosecond resolution of MaxDelaySeconds
 	LongPollingTimeoutSeconds *int64 `json:"long_polling_timeout_seconds,omitempty"` // max amount of time the server should wait before issuing a timeout if there's no update available
 }
 
@@ -70,10 +72,10 @@ func (c *Config) ValidateAndInjectDefaults() error {
 
 	// scale to seconds
 	minSeconds := int64(time.Duration(min) * time.Second)
-	c.Polling.MinDelaySeconds = &minSeconds
+	c.Polling.parsedMinDelaySeconds = &minSeconds
 
 	maxSeconds := int64(time.Duration(max) * time.Second)
-	c.Polling.MaxDelaySeconds = &maxSeconds
+	c.Polling.parsedMaxDelaySeconds = &maxSeconds
 
 	if c.Polling.LongPollingTimeoutSeconds != nil {
 		if *c.Polling.LongPollingTimeoutSeconds < 1 {

v1/download/config_test.go

@@ -6,6 +6,7 @@ package download
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 	"time"
 )
@@ -13,11 +14,13 @@ import (
 func TestConfigValidation(t *testing.T) {
 
 	tests := []struct {
-		note    string
+		note         string
-		input   string
+		input        string
-		wantErr bool
+		wantErr      bool
-		expMin  time.Duration
+		expMin       *int64
-		expMax  time.Duration
+		expMax       *int64
+		expParsedMin time.Duration
+		expParsedMax time.Duration
 	}{
 		{
 			note: "min > max",
@@ -30,10 +33,12 @@ func TestConfigValidation(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			note:   "empty",
+			note:         "empty",
-			input:  `{}`,
+			input:        `{}`,
-			expMin: time.Second * time.Duration(defaultMinDelaySeconds),
+			expMin:       nil,
-			expMax: time.Second * time.Duration(defaultMaxDelaySeconds),
+			expMax:       nil,
+			expParsedMin: time.Second * time.Duration(defaultMinDelaySeconds),
+			expParsedMax: time.Second * time.Duration(defaultMaxDelaySeconds),
 		},
 		{
 			note: "min missing",
@@ -61,8 +66,16 @@ func TestConfigValidation(t *testing.T) {
 					"max_delay_seconds": 30
 				}
 			}`,
-			expMin: time.Second * 10,
+			expMin: func() *int64 {
-			expMax: time.Second * 30,
+				min := int64(10)
+				return &min
+			}(),
+			expMax: func() *int64 {
+				max := int64(30)
+				return &max
+			}(),
+			expParsedMin: time.Second * 10,
+			expParsedMax: time.Second * 30,
 		},
 		{
 			note: "long polling timeout < 1",
@@ -76,25 +89,107 @@ func TestConfigValidation(t *testing.T) {
 	}
 
 	for _, test := range tests {
+		t.Run(test.note, func(t *testing.T) {
+			var config Config
 
-		var config Config
+			if err := json.Unmarshal([]byte(test.input), &config); err != nil {
-
+				t.Fatal(err)
-		if err := json.Unmarshal([]byte(test.input), &config); err != nil {
-			t.Fatal(err)
-		}
-
-		err := config.ValidateAndInjectDefaults()
-		if err != nil && !test.wantErr {
-			t.Errorf("Unexpected error on: %v, err: %v", test.input, err)
-		}
-
-		if err == nil {
-			if time.Duration(*config.Polling.MinDelaySeconds) != test.expMin {
-				t.Errorf("For %q expected min %v but got %v", test.note, test.expMin, time.Duration(*config.Polling.MinDelaySeconds))
 			}
-			if time.Duration(*config.Polling.MaxDelaySeconds) != test.expMax {
+
-				t.Errorf("For %q expected min %v but got %v", test.note, test.expMax, time.Duration(*config.Polling.MaxDelaySeconds))
+			// no matter how many calls to ValidateAndInjectDefaults, the values should stay the same
+			for range 3 {
+				err := config.ValidateAndInjectDefaults()
+				if err != nil && !test.wantErr {
+					t.Errorf("Unexpected error on: %v, err: %v", test.input, err)
+				} else if err != nil && test.wantErr {
+					return
+				}
+
+				if config.Polling.MinDelaySeconds == nil && test.expMin != nil {
+					t.Fatal("Expected min delay seconds to be set")
+				}
+				if config.Polling.MinDelaySeconds != nil && *config.Polling.MinDelaySeconds != *test.expMin {
+					t.Errorf("For %q expected min %v but got %v", test.note, test.expMin, time.Duration(*config.Polling.MinDelaySeconds))
+				}
+				if config.Polling.MaxDelaySeconds == nil && test.expMax != nil {
+					t.Fatal("Expected min delay seconds to be set")
+				}
+				if config.Polling.MaxDelaySeconds != nil && *config.Polling.MaxDelaySeconds != *test.expMax {
+					t.Errorf("For %q expected max %v but got %v", test.note, test.expMax, time.Duration(*config.Polling.MaxDelaySeconds))
+				}
+
+				if time.Duration(*config.Polling.parsedMinDelaySeconds) != test.expParsedMin {
+					t.Errorf("For %q expected min %v but got %v", test.note, test.expParsedMin, time.Duration(*config.Polling.MinDelaySeconds))
+				}
+				if time.Duration(*config.Polling.parsedMaxDelaySeconds) != test.expParsedMax {
+					t.Errorf("For %q expected max %v but got %v", test.note, test.expParsedMax, time.Duration(*config.Polling.MaxDelaySeconds))
+				}
 			}
-		}
+		})
+	}
+}
+
+func TestConfigValidationUpdate(t *testing.T) {
+	expMin := int64(10)
+	expMax := int64(30)
+	expParsedMin := time.Second * time.Duration(expMin)
+	expParsedMax := time.Second * time.Duration(expMax)
+	var config Config
+
+	if err := json.Unmarshal([]byte(fmt.Sprintf(`{
+				"polling": {
+					"min_delay_seconds": %d,
+					"max_delay_seconds": %d
+				}
+			}`, expMin, expMax)), &config); err != nil {
+		t.Fatal(err)
+	}
+
+	err := config.ValidateAndInjectDefaults()
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	if *config.Polling.MinDelaySeconds != expMin {
+		t.Errorf("expected min %v but got %v", expMin, time.Duration(*config.Polling.MinDelaySeconds))
+	}
+
+	if config.Polling.MaxDelaySeconds != nil && *config.Polling.MaxDelaySeconds != expMax {
+		t.Errorf("expected max %v but got %v", expMax, time.Duration(*config.Polling.MaxDelaySeconds))
+	}
+
+	if time.Duration(*config.Polling.parsedMinDelaySeconds) != expParsedMin {
+		t.Errorf("expected min %v but got %v", expParsedMin, time.Duration(*config.Polling.MinDelaySeconds))
+	}
+	if time.Duration(*config.Polling.parsedMaxDelaySeconds) != expParsedMax {
+		t.Errorf("expected max %v but got %v", expParsedMax, time.Duration(*config.Polling.MaxDelaySeconds))
+	}
+
+	expMin = int64(50)
+	expMax = int64(100)
+	expParsedMin = time.Second * time.Duration(expMin)
+	expParsedMax = time.Second * time.Duration(expMax)
+
+	config.Polling.MinDelaySeconds = &expMin
+	config.Polling.MaxDelaySeconds = &expMax
+
+	err = config.ValidateAndInjectDefaults()
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	if *config.Polling.MinDelaySeconds != expMin {
+		t.Errorf("expected min %v but got %v", expMin, time.Duration(*config.Polling.MinDelaySeconds))
+	}
+
+	if config.Polling.MaxDelaySeconds != nil && *config.Polling.MaxDelaySeconds != expMax {
+		t.Errorf("expected max %v but got %v", expMax, time.Duration(*config.Polling.MaxDelaySeconds))
+	}
+
+	if time.Duration(*config.Polling.parsedMinDelaySeconds) != expParsedMin {
+		t.Errorf("expected min %v but got %v", expParsedMin, time.Duration(*config.Polling.MinDelaySeconds))
+	}
+	if time.Duration(*config.Polling.parsedMaxDelaySeconds) != expParsedMax {
+		t.Errorf("expected max %v but got %v", expParsedMax, time.Duration(*config.Polling.MaxDelaySeconds))
 	}
 }

v1/download/download.go

@@ -231,7 +231,7 @@ func (d *Downloader) loop(ctx context.Context) {
 		// are scaled from int seconds to ns in ValidateAndInjectDefaults.
 		if err != nil {
 			// when there was an error, use a delay that's based on the retry count
-			delay = util.DefaultBackoff(float64(minRetryDelay), float64(*d.config.Polling.MaxDelaySeconds), retry)
+			delay = util.DefaultBackoff(float64(minRetryDelay), float64(*d.config.Polling.parsedMaxDelaySeconds), retry)
 		} else if !d.longPollingEnabled || d.config.Polling.LongPollingTimeoutSeconds == nil {
 			// revert the response header timeout value on the http client's transport
 			if *d.client.Config().ResponseHeaderTimeoutSeconds == 0 {
@@ -239,8 +239,8 @@ func (d *Downloader) loop(ctx context.Context) {
 			}
 
 			// when polling, use a jittered delay based on min and max delay config
-			min := float64(*d.config.Polling.MinDelaySeconds)
+			min := float64(*d.config.Polling.parsedMinDelaySeconds)
-			max := float64(*d.config.Polling.MaxDelaySeconds)
+			max := float64(*d.config.Polling.parsedMaxDelaySeconds)
 			delay = time.Duration(((max - min) * rand.Float64()) + min)
 		}
 

v1/download/oci_download.go

@@ -177,11 +177,11 @@ func (d *OCIDownloader) loop(ctx context.Context) {
 		}
 
 		if err != nil {
-			delay = util.DefaultBackoff(float64(minRetryDelay), float64(*d.config.Polling.MaxDelaySeconds), retry)
+			delay = util.DefaultBackoff(float64(minRetryDelay), float64(*d.config.Polling.parsedMaxDelaySeconds), retry)
 		} else {
 			// revert the response header timeout value on the http client's transport
-			min := float64(*d.config.Polling.MinDelaySeconds)
+			min := float64(*d.config.Polling.parsedMinDelaySeconds)
-			max := float64(*d.config.Polling.MaxDelaySeconds)
+			max := float64(*d.config.Polling.parsedMaxDelaySeconds)
 			delay = time.Duration(((max - min) * rand.Float64()) + min)
 		}
 

v1/plugins/bundle/plugin.go

@@ -147,7 +147,7 @@ func (p *Plugin) Stop(ctx context.Context) {
 
 // Reconfigure notifies the plugin that it's configuration has changed.
 // Any bundle configs that have changed or been added/removed will take
-// affect.
+// effect.
 func (p *Plugin) Reconfigure(ctx context.Context, config any) {
 	// Reconfiguring should not occur in parallel, lock to ensure
 	// nothing swaps underneath us with the current p.config and the updated one.

v1/plugins/bundle/plugin_test.go

@@ -7573,6 +7573,41 @@ result := true`,
 	}
 }
 
+func TestMultipleCallsToReconfigure(t *testing.T) {
+	minDelaySeconds := int64(60)
+	maxDelaySeconds := int64(120)
+
+	store := inmem.NewWithOpts(inmem.OptRoundTripOnWrite(false), inmem.OptReturnASTValuesOnRead(true))
+	manager := getTestManagerWithOpts(nil, store)
+
+	cfg := Config{
+		Bundles: map[string]*Source{
+			"b1": {
+				Config: download.Config{
+					Polling: download.PollingConfig{
+						MinDelaySeconds: &minDelaySeconds,
+						MaxDelaySeconds: &maxDelaySeconds,
+					},
+				},
+				Resource:       "/b1",
+				SizeLimitBytes: int64(bundle.DefaultSizeLimitBytes),
+			},
+		},
+	}
+	plugin := New(&cfg, manager)
+
+	plugin.Reconfigure(t.Context(), &plugin.config)
+	plugin.Reconfigure(t.Context(), &plugin.config)
+
+	// after multiple calls to reconfigure with the same plugin config, the values should stay the same
+	if *plugin.config.Bundles["b1"].Polling.MaxDelaySeconds != maxDelaySeconds {
+		t.Fatalf("expected MaxDelaySeconds to be %d but got %d", maxDelaySeconds, *plugin.config.Bundles["b1"].Polling.MaxDelaySeconds)
+	}
+	if *plugin.config.Bundles["b1"].Polling.MinDelaySeconds != minDelaySeconds {
+		t.Fatalf("expected MaxDelaySeconds to be %d but got %d", maxDelaySeconds, *plugin.config.Bundles["b1"].Polling.MaxDelaySeconds)
+	}
+}
+
 type testModule struct {
 	Path string
 	Data string

v1/plugins/logs/plugin.go

@@ -951,7 +951,7 @@ func (p *Plugin) reconfigure(ctx context.Context, config any) {
 	case eventBufferType:
 		limit = *p.config.Reporting.BufferSizeLimitEvents
 	case sizeBufferType:
-		limit = *p.config.Reporting.UploadSizeLimitBytes
+		limit = *p.config.Reporting.BufferSizeLimitBytes
 	}
 	p.b.Reconfigure(
 		limit,

v1/version/version.go

@@ -10,7 +10,7 @@ import (
 	"runtime/debug"
 )
 
-var Version = "1.12.2"
+var Version = "1.12.3"
 
 // GoVersion is the version of Go this was built with
 var GoVersion = runtime.Version()