Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added new grpc sync config option to allow setting max receive message size. #1358

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/pkg/sync/builder/syncbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ func (sb *SyncBuilder) newGRPC(config sync.SourceConfig, logger *logger.Logger)
ProviderID: config.ProviderID,
Secure: config.TLS,
Selector: config.Selector,
MaxMsgSize: config.MaxMsgSize,
}
}

Expand Down
20 changes: 20 additions & 0 deletions core/pkg/sync/builder/syncbuilder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,26 @@ func Test_SyncsFromFromConfig(t *testing.T) {
},
wantErr: false,
},
{
name: "grpc-with-msg-size",
args: args{
logger: lg,
sources: []sync.SourceConfig{
{
URI: "grpc://host:port",
Provider: syncProviderGrpc,
ProviderID: "myapp",
CertPath: "/tmp/ca.cert",
Selector: "source=database",
MaxMsgSize: 10,
},
},
},
wantSyncs: []sync.ISync{
&grpc.Sync{},
},
wantErr: false,
},
{
name: "combined",
injectFunc: func(builder *SyncBuilder) {
Expand Down
13 changes: 12 additions & 1 deletion core/pkg/sync/grpc/grpc_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type Sync struct {
Secure bool
Selector string
URI string
MaxMsgSize int

client FlagSyncServiceClient
ready bool
Expand All @@ -62,7 +63,17 @@ func (g *Sync) Init(_ context.Context) error {
}

// Derive reusable client connection
rpcCon, err := grpc.NewClient(g.URI, grpc.WithTransportCredentials(tCredentials))
// Set MaxMsgSize if passed
var rpcCon *grpc.ClientConn

if g.MaxMsgSize > 0 {
pradeepbbl marked this conversation as resolved.
Show resolved Hide resolved
g.Logger.Info(fmt.Sprintf("setting max receive message size %d bytes default 4MB", g.MaxMsgSize))
dialOptions := grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(g.MaxMsgSize))
rpcCon, err = grpc.NewClient(g.URI, grpc.WithTransportCredentials(tCredentials), dialOptions)
} else {
rpcCon, err = grpc.NewClient(g.URI, grpc.WithTransportCredentials(tCredentials))
}

if err != nil {
err := fmt.Errorf("error initiating grpc client connection: %w", err)
g.Logger.Error(err.Error())
Expand Down
26 changes: 26 additions & 0 deletions core/pkg/sync/grpc/grpc_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
grpcmock "github.com/open-feature/flagd/core/pkg/sync/grpc/mock"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"go.uber.org/zap"
"go.uber.org/zap/zaptest/observer"
"golang.org/x/sync/errgroup"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
Expand Down Expand Up @@ -78,6 +80,30 @@ func Test_InitWithMockCredentialBuilder(t *testing.T) {
}
}

func Test_InitWithSizeOverride(t *testing.T) {
observedZapCore, observedLogs := observer.New(zap.InfoLevel)
observedLogger := zap.New(observedZapCore)

mockCtrl := gomock.NewController(t)
mockCredentialBulder := credendialsmock.NewMockBuilder(mockCtrl)

mockCredentialBulder.EXPECT().
Build(gomock.Any(), gomock.Any()).
Return(insecure.NewCredentials(), nil)

grpcSync := Sync{
URI: "grpc-target",
Logger: logger.NewLogger(observedLogger, false),
CredentialBuilder: mockCredentialBulder,
MaxMsgSize: 10,
}

err := grpcSync.Init(context.Background())

require.Nilf(t, err, "%s: expected no error, but got non nil error", t.Name())
require.Equal(t, "setting max receive message size 10 bytes default 4MB", observedLogs.All()[0].Message)
}

func Test_ReSyncTests(t *testing.T) {
const target = "localBufCon"

Expand Down
1 change: 1 addition & 0 deletions core/pkg/sync/isync.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,5 @@ type SourceConfig struct {
ProviderID string `json:"providerID,omitempty"`
Selector string `json:"selector,omitempty"`
Interval uint32 `json:"interval,omitempty"`
MaxMsgSize int `json:"maxMsgSize,omitempty"`
}
5 changes: 5 additions & 0 deletions docs/reference/sync-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Alternatively, these configurations can be passed to flagd via config file, spec
| providerID | optional `string` | Value binds to grpc connection's providerID field. gRPC server implementations may use this to identify connecting flagd instance |
| selector | optional `string` | Value binds to grpc connection's selector field. gRPC server implementations may use this to filter flag configurations |
| certPath | optional `string` | Used for grpcs sync when TLS certificate is needed. If not provided, system certificates will be used for TLS connection |
| maxMsgSize | optional `int` | Used for gRPC sync to set max receive message size (in bytes) e.g. 5242880 for 5MB. If not provided, the default is [4MB](https://pkg.go.dev/google.golang.org#grpc#MaxCallRecvMsgSize) |

The `uri` field values **do not** follow the [URI patterns](#uri-patterns). The provider type is instead derived
from the `provider` field. Only exception is the remote provider where `http(s)://` is expected by default. Incorrect
Expand All @@ -64,6 +65,7 @@ Startup command:
{"uri":"https://secure-remote/basic-auth","provider":"http","authHeader":"Basic dXNlcjpwYXNz"},
{"uri":"default/my-flag-config","provider":"kubernetes"},
{"uri":"grpc-source:8080","provider":"grpc"},
{"uri":"my-flag-source:8080","provider":"grpc", "maxMsgSize": 5242880},
{"uri":"my-flag-source:8080","provider":"grpc", "certPath": "/certs/ca.cert", "tls": true, "providerID": "flagd-weatherapp-sidecar", "selector": "source=database,app=weatherapp"}]'
```

Expand All @@ -80,6 +82,9 @@ sources:
provider: kubernetes
- uri: my-flag-source:8080
provider: grpc
- uri: my-flag-source:8080
provider: grpc
maxMsgSize: 5242880
- uri: my-flag-source:8080
provider: grpc
certPath: /certs/ca.cert
Expand Down
Loading