Skip to content

Commit efda06a

Browse files
authored
refactor: remove deprecated bearerToken option (#1816)
## This PR Removes deprecated `bearerToken` option from all references in code and in documentation. It also updates some tests, to use authHeader with `Bearer` type instead. ### Related Issues Fixes #1687 ### Notes This may be breaking change, as it removes field from Sync struct. I updated all local tests and resources, but let me know if there are any other resources that are maintained by flagd community, that might create Sync struct with that field. Signed-off-by: Marcin Olko <molko@google.com> Signed-off-by: m-olko <molko@google.com>
1 parent 8396f0d commit efda06a

File tree

7 files changed

+17
-125
lines changed

7 files changed

+17
-125
lines changed

core/pkg/sync/builder/syncbuilder_test.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -216,11 +216,6 @@ func Test_SyncsFromFromConfig(t *testing.T) {
216216
CertPath: "/tmp/ca.cert",
217217
Selector: "source=database",
218218
},
219-
{
220-
URI: "https://host:port",
221-
Provider: syncProviderHTTP,
222-
BearerToken: "token",
223-
},
224219
{
225220
URI: "https://host:port",
226221
Provider: syncProviderHTTP,
@@ -251,7 +246,6 @@ func Test_SyncsFromFromConfig(t *testing.T) {
251246
wantSyncs: []sync.ISync{
252247
&grpc.Sync{},
253248
&http.Sync{},
254-
&http.Sync{},
255249
&file.Sync{},
256250
&kubernetes.Sync{},
257251
&blob.Sync{},

core/pkg/sync/builder/utils.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,6 @@ func ParseSources(sourcesFlag string) ([]sync.SourceConfig, error) {
2222
if sp.Provider == "" {
2323
return syncProvidersParsed, errors.New("sync provider argument parse: provider is a required field")
2424
}
25-
if sp.AuthHeader != "" && sp.BearerToken != "" {
26-
return syncProvidersParsed, errors.New(
27-
"sync provider argument parse: both authHeader and bearerToken are defined, only one is allowed at a time",
28-
)
29-
}
3025
}
3126
return syncProvidersParsed, nil
3227
}

core/pkg/sync/builder/utils_test.go

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func TestParseSource(t *testing.T) {
2626
"multiple-syncs": {
2727
in: `[
2828
{"uri":"config/samples/example_flags.json","provider":"file"},
29-
{"uri":"http://test.com","provider":"http","bearerToken":":)"},
29+
{"uri":"http://test.com","provider":"http","authHeader":"Bearer :)"},
3030
{"uri":"host:port","provider":"grpc"},
3131
{"uri":"default/my-crd","provider":"kubernetes"},
3232
{"uri":"gs://bucket-name/path/to/file","provider":"gcs"},
@@ -42,7 +42,7 @@ func TestParseSource(t *testing.T) {
4242
{
4343
URI: "http://test.com",
4444
Provider: syncProviderHTTP,
45-
BearerToken: ":)",
45+
AuthHeader: "Bearer :)",
4646
},
4747
{
4848
URI: "host:port",
@@ -69,8 +69,6 @@ func TestParseSource(t *testing.T) {
6969
"multiple-syncs-with-options": {
7070
in: `[
7171
{"uri":"config/samples/example_flags.json","provider":"file"},
72-
{"uri":"http://my-flag-source.json","provider":"http","bearerToken":"bearer-dji34ld2l"},
73-
{"uri":"https://secure-remote","provider":"http","bearerToken":"bearer-dji34ld2l"},
7472
{"uri":"https://secure-remote","provider":"http","authHeader":"Bearer bearer-dji34ld2l"},
7573
{"uri":"https://secure-remote","provider":"http","authHeader":"Basic dXNlcjpwYXNz"},
7674
{"uri":"http://site.com","provider":"http","interval":77 },
@@ -84,16 +82,6 @@ func TestParseSource(t *testing.T) {
8482
URI: "config/samples/example_flags.json",
8583
Provider: syncProviderFile,
8684
},
87-
{
88-
URI: "http://my-flag-source.json",
89-
Provider: syncProviderHTTP,
90-
BearerToken: "bearer-dji34ld2l",
91-
},
92-
{
93-
URI: "https://secure-remote",
94-
Provider: syncProviderHTTP,
95-
BearerToken: "bearer-dji34ld2l",
96-
},
9785
{
9886
URI: "https://secure-remote",
9987
Provider: syncProviderHTTP,
@@ -127,22 +115,6 @@ func TestParseSource(t *testing.T) {
127115
},
128116
},
129117
},
130-
"multiple-auth-options": {
131-
in: `[
132-
{"uri":"https://secure-remote","provider":"http","authHeader":"Bearer bearer-dji34ld2l","bearerToken":"bearer-dji34ld2l"}
133-
]`,
134-
expectErr: true,
135-
out: []sync.SourceConfig{
136-
{
137-
URI: "https://secure-remote",
138-
Provider: syncProviderHTTP,
139-
AuthHeader: "Bearer bearer-dji34ld2l",
140-
BearerToken: "bearer-dji34ld2l",
141-
TLS: false,
142-
Interval: 0,
143-
},
144-
},
145-
},
146118
"empty": {
147119
in: `[]`,
148120
expectErr: false,

core/pkg/sync/http/http_sync.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ type Sync struct {
2929
cron Cron
3030
lastBodySHA string
3131
logger *logger.Logger
32-
bearerToken string
3332
authHeader string
3433
interval uint32
3534
ready bool
@@ -107,9 +106,6 @@ func (hs *Sync) ReSync(ctx context.Context, dataSync chan<- sync.DataSync) error
107106
}
108107

109108
func (hs *Sync) Init(_ context.Context) error {
110-
if hs.bearerToken != "" {
111-
hs.logger.Warn("Deprecation Alert: bearerToken option is deprecated, please use authHeader instead")
112-
}
113109
return nil
114110
}
115111

@@ -176,9 +172,6 @@ func (hs *Sync) fetchBody(ctx context.Context, fetchAll bool) (string, bool, err
176172

177173
if hs.authHeader != "" {
178174
req.Header.Set("Authorization", hs.authHeader)
179-
} else if hs.bearerToken != "" {
180-
bearer := fmt.Sprintf("Bearer %s", hs.bearerToken)
181-
req.Header.Set("Authorization", bearer)
182175
}
183176

184177
if hs.eTag != "" && !fetchAll {
@@ -299,7 +292,6 @@ func NewHTTP(config sync.SourceConfig, logger *logger.Logger) *Sync {
299292
zap.String("component", "sync"),
300293
zap.String("sync", "http"),
301294
),
302-
bearerToken: config.BearerToken,
303295
authHeader: config.AuthHeader,
304296
interval: interval,
305297
cron: cron.New(),

core/pkg/sync/http/http_sync_test.go

Lines changed: 2 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,6 @@ func TestHTTPSync_Fetch(t *testing.T) {
125125
tests := map[string]struct {
126126
setup func(t *testing.T, client *syncmock.MockClient)
127127
uri string
128-
bearerToken string
129128
authHeader string
130129
eTagHeader string
131130
lastBodySHA string
@@ -181,37 +180,6 @@ func TestHTTPSync_Fetch(t *testing.T) {
181180
}
182181
},
183182
},
184-
"authorization with bearerToken": {
185-
setup: func(t *testing.T, client *syncmock.MockClient) {
186-
expectedToken := "bearer-1234"
187-
client.EXPECT().Do(gomock.Any()).DoAndReturn(func(req *http.Request) (*http.Response, error) {
188-
actualAuthHeader := req.Header.Get("Authorization")
189-
if actualAuthHeader != "Bearer "+expectedToken {
190-
t.Fatalf("expected Authorization header to be 'Bearer %s', got %s", expectedToken, actualAuthHeader)
191-
}
192-
return &http.Response{
193-
Header: buildHeaders(map[string][]string{"Content-Type": {"application/json"}}),
194-
Body: io.NopCloser(strings.NewReader("test response")),
195-
StatusCode: http.StatusOK,
196-
}, nil
197-
})
198-
},
199-
uri: "http://localhost",
200-
bearerToken: "bearer-1234",
201-
lastBodySHA: "",
202-
handleResponse: func(t *testing.T, httpSync Sync, _ string, err error) {
203-
if err != nil {
204-
t.Fatalf("fetch: %v", err)
205-
}
206-
207-
expectedLastBodySHA := "UjeJHtCU_wb7OHK-tbPoHycw0TqlHzkWJmH4y6cqg50="
208-
if httpSync.lastBodySHA != expectedLastBodySHA {
209-
t.Errorf(
210-
"expected last body sha to be: '%s', got: '%s'", expectedLastBodySHA, httpSync.lastBodySHA,
211-
)
212-
}
213-
},
214-
},
215183
"authorization with authHeader": {
216184
setup: func(t *testing.T, client *syncmock.MockClient) {
217185
expectedHeader := "Basic dXNlcjpwYXNz"
@@ -348,7 +316,6 @@ func TestHTTPSync_Fetch(t *testing.T) {
348316
httpSync := Sync{
349317
uri: tt.uri,
350318
client: mockClient,
351-
bearerToken: tt.bearerToken,
352319
authHeader: tt.authHeader,
353320
lastBodySHA: tt.lastBodySHA,
354321
logger: logger.NewLogger(nil, false),
@@ -361,30 +328,6 @@ func TestHTTPSync_Fetch(t *testing.T) {
361328
}
362329
}
363330

364-
func TestSync_Init(t *testing.T) {
365-
tests := []struct {
366-
name string
367-
bearerToken string
368-
}{
369-
{"with bearerToken", "bearer-1234"},
370-
{"without bearerToken", ""},
371-
}
372-
373-
for _, tt := range tests {
374-
t.Run(tt.name, func(t *testing.T) {
375-
httpSync := Sync{
376-
bearerToken: tt.bearerToken,
377-
logger: logger.NewLogger(nil, false),
378-
}
379-
380-
if err := httpSync.Init(context.Background()); err != nil {
381-
t.Errorf("Init() error = %v", err)
382-
}
383-
})
384-
}
385-
386-
}
387-
388331
func TestHTTPSync_Resync(t *testing.T) {
389332
ctrl := gomock.NewController(t)
390333
source := "http://localhost"
@@ -393,7 +336,6 @@ func TestHTTPSync_Resync(t *testing.T) {
393336
tests := map[string]struct {
394337
setup func(t *testing.T, client *syncmock.MockClient)
395338
uri string
396-
bearerToken string
397339
lastBodySHA string
398340
handleResponse func(*testing.T, Sync, string, error)
399341
wantErr bool
@@ -448,7 +390,6 @@ func TestHTTPSync_Resync(t *testing.T) {
448390
httpSync := Sync{
449391
uri: tt.uri,
450392
client: mockClient,
451-
bearerToken: tt.bearerToken,
452393
lastBodySHA: tt.lastBodySHA,
453394
logger: logger.NewLogger(nil, false),
454395
}
@@ -617,7 +558,7 @@ func TestHTTPSync_OAuth(t *testing.T) {
617558
l := logger.NewLogger(nil, false)
618559
s := NewHTTP(sync.SourceConfig{
619560
URI: ts.URL,
620-
BearerToken: "it_should_be_replaced_by_oauth",
561+
AuthHeader: "Bearer it_should_be_replaced_by_oauth",
621562
OAuth: &sync.OAuthCredentialHandler{
622563
ClientID: clientID,
623564
ClientSecret: clientSecret,
@@ -710,7 +651,7 @@ func TestHTTPSync_OAuthFolderSecrets(t *testing.T) {
710651
l := logger.NewLogger(nil, false)
711652
s := NewHTTP(sync.SourceConfig{
712653
URI: ts.URL + flagsPath,
713-
BearerToken: "it_should_be_replaced_by_oauth",
654+
AuthHeader: "Bearer it_should_be_replaced_by_oauth",
714655
OAuth: &sync.OAuthCredentialHandler{
715656
ClientID: clientID,
716657
ClientSecret: clientSecret,

core/pkg/sync/isync.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ type SourceConfig struct {
3939
URI string `json:"uri"`
4040
Provider string `json:"provider"`
4141

42-
BearerToken string `json:"bearerToken,omitempty"`
4342
AuthHeader string `json:"authHeader,omitempty"`
4443
CertPath string `json:"certPath,omitempty"`
4544
TLS bool `json:"tls,omitempty"`

docs/reference/sync-configuration.md

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,17 @@ The flagd accepts a string argument, which should be a JSON representation of an
4747

4848
Alternatively, these configurations can be passed to flagd via config file, specified using the `--config` flag.
4949

50-
| Field | Type | Note |
51-
| ----------- | ------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
52-
| uri | required `string` | Flag configuration source of the sync |
53-
| provider | required `string` | Provider type - `file`, `fsnotify`, `fileinfo`, `kubernetes`, `http`, `grpc`, `gcs` or `azblob` |
54-
| authHeader | optional `string` | Used for http sync; set this to include the complete `Authorization` header value for any authentication scheme (e.g., "Bearer token_here", "Basic base64_credentials", etc.). Cannot be used with `bearerToken` |
55-
| bearerToken | optional `string` | (Deprecated) Used for http sync; token gets appended to `Authorization` header with [bearer schema](https://www.rfc-editor.org/rfc/rfc6750#section-2.1). Cannot be used with `authHeader` |
56-
| interval | optional `uint32` | Used for http, gcs and azblob syncs; requests will be made at this interval. Defaults to 5 seconds. |
57-
| tls | optional `boolean` | Enable/Disable secure TLS connectivity. Currently used only by gRPC sync. Default (ex: if unset) is false, which will use an insecure connection |
58-
| providerID | optional `string` | Value binds to grpc connection's providerID field. gRPC server implementations may use this to identify connecting flagd instance |
59-
| selector | optional `string` | Value binds to grpc connection's selector field. gRPC server implementations may use this to filter flag configurations |
60-
| certPath | optional `string` | Used for grpcs sync when TLS certificate is needed. If not provided, system certificates will be used for TLS connection |
61-
| 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) |
50+
| Field | Type | Note |
51+
| ----------- | ------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
52+
| uri | required `string` | Flag configuration source of the sync |
53+
| provider | required `string` | Provider type - `file`, `fsnotify`, `fileinfo`, `kubernetes`, `http`, `grpc`, `gcs` or `azblob` |
54+
| authHeader | optional `string` | Used for http sync; set this to include the complete `Authorization` header value for any authentication scheme (e.g., "Bearer token_here", "Basic base64_credentials", etc.). |
55+
| interval | optional `uint32` | Used for http, gcs and azblob syncs; requests will be made at this interval. Defaults to 5 seconds. |
56+
| tls | optional `boolean` | Enable/Disable secure TLS connectivity. Currently used only by gRPC sync. Default (ex: if unset) is false, which will use an insecure connection |
57+
| providerID | optional `string` | Value binds to grpc connection's providerID field. gRPC server implementations may use this to identify connecting flagd instance |
58+
| selector | optional `string` | Value binds to grpc connection's selector field. gRPC server implementations may use this to filter flag configurations |
59+
| certPath | optional `string` | Used for grpcs sync when TLS certificate is needed. If not provided, system certificates will be used for TLS connection |
60+
| 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) |
6261

6362
The `uri` field values **do not** follow the [URI patterns](#uri-patterns). The provider type is instead derived
6463
from the `provider` field. Only exception is the remote provider where `http(s)://` is expected by default. Incorrect
@@ -94,7 +93,7 @@ Startup command:
9493
--sources='[{"uri":"config/samples/example_flags.json","provider":"file"},
9594
{"uri":"config/samples/example_flags.json","provider":"fsnotify"},
9695
{"uri":"config/samples/example_flags.json","provider":"fileinfo"},
97-
{"uri":"http://my-flag-source/flags.json","provider":"http","bearerToken":"bearer-dji34ld2l"},
96+
{"uri":"http://my-flag-source/flags.json","provider":"http","authHeader":"Bearer bearer-dji34ld2l"},
9897
{"uri":"https://secure-remote/bearer-auth/flags.json","provider":"http","authHeader":"Bearer bearer-dji34ld2l"},
9998
{"uri":"https://secure-remote/basic-auth/flags.json","provider":"http","authHeader":"Basic dXNlcjpwYXNz"},
10099
{"uri":"default/my-flag-config","provider":"kubernetes"},
@@ -118,7 +117,7 @@ sources:
118117
provider: fileinfo
119118
- uri: http://my-flag-source/flags.json
120119
provider: http
121-
bearerToken: bearer-dji34ld2l
120+
authHeader: "Bearer bearer-dji34ld2l"
122121
- uri: default/my-flag-config
123122
provider: kubernetes
124123
- uri: my-flag-source:8080

0 commit comments

Comments
 (0)